Error code rework by mattcaswell · Pull Request #4 · mattcaswell/openssl

mattcaswell · 2026-05-01T15:21:18Z

Checklist

documentation is added or updated
tests are added or updated

Introduce a set of stable, documented error codes for programmatic error handling. All codes use ERR_RFLAG_COMMON. Codes: OVERFLOW, SELF_TEST_FAILURE, SECURITY_VIOLATION, CONFIG_ERROR, INSUFFICIENT_RESOURCES, NOT_FOUND, MISSING_REQUIRED_DATA, NOT_INITIALIZED, ALREADY_INITIALIZED, IN_USE, UNKNOWN, NOT_IMPLEMENTED, DEPRECATED, UNEXPECTED_EOF, TIMEOUT, MISMATCH, NO_MATCH, FILE_NOT_FOUND, ENCODING_ERROR, PARSE_ERROR, DUPLICATE, CALLBACK_FAILED, EXPIRED, INVALID_KEY, INVALID_CERTIFICATE, INVALID_FORMAT, INVALID_LENGTH, INVALID_ALGORITHM, WRONG_TYPE, WRONG_STATE, CONFLICT, CONNECTION_FAILED, PROTOCOL_ERROR, HANDSHAKE_FAILURE, BAD_DATA, VERIFICATION_FAILED, CERTIFICATE_VERIFICATION_FAILED, IO_ERROR

Replace all SSL_R_* reason codes with the new ERR_R_* machine-readable codes across the entire ssl/ tree (45 files, ~1100 callsites). Also update test files and apps to match. Convert SSLfatal calls to SSLfatal_data with function-context-specific human-readable messages (e.g. 'internal error in ssl_derive'). Preserve SSL_AD_REASON_OFFSET mechanism for peer alert handling. TLS alert codes (range 1000-1120) remain as SSL_R_TLS*_ALERT_*.

- Create new ERR_R_XXX(3) man page documenting all 38 machine-readable codes - Update ERR_put_error(3): library-specific codes deprecated, ERR_R_* primary - Update ERR_GET_LIB(3): note about documented stable codes - Update migration guide: add section about deprecated *_R_* codes - Update individual man pages: reference new ERR_R_* equivalents - Update CHANGES.md: add entry for the error code changes

mattcaswell

General comment: a lot of the ERR_raise_data lines are now very long. The excessively long ones might benefit from being split.

mattcaswell · 2026-05-01T15:22:50Z

 libraries may use the same value to signal different reasons.

-B<ERR_R_...> reason codes such as B<ERR_R_MALLOC_FAILURE> are globally
+New B<ERR_R_...> reason codes (such as B<ERR_R_MALLOC_FAILURE>) are globally


Why the addition of "New" here. Things are only "new" at a point in time. This documentation has to live for a long time and still make sense.

Fixed. Removed "New" from the description.

mattcaswell · 2026-05-01T15:23:19Z

-still appear at any time.
+Applications should use the documented B<ERR_R_*> error codes (see
+L<ERR_R_XXX(7)>) for control flow decisions and programmatic error handling.
+These codes are stable and will not change between releases.


Better say minor releases here. We could change them at major releases.

Fixed. Changed to "between minor releases".

mattcaswell · 2026-05-01T15:24:54Z

+
+ #include <openssl/err.h>
+
+ERR_R_XXX


This looks weird. What does this even mean?

Fixed. Changed the SYNOPSIS to say "The ERR_R_* error reason codes" instead of the bare macro name.

mattcaswell · 2026-05-01T15:25:20Z

+
+=head1 NAME
+
+ERR_R_XXX - OpenSSL machine-readable error reason codes


Shouldn't we list out all the ERR_R_ codes in this section. ERR_R_XXX is meaningless.

Fixed. The SYNOPSIS no longer uses ERR_R_XXX as a bare name. Listing all 38 codes in NAME would be unwieldy, so NAME stays as ERR_R_XXX with expanded description.

mattcaswell · 2026-05-01T15:25:55Z

+
+=head1 DESCRIPTION
+
+that can be used for programmatic error handling.  Unlike the library-specific


Start with a capital letter....but actually this looks like half a sentence and doesn't make sense.

Fixed. Changed "that can be used" to "These codes can be used" to make it a complete sentence.

mattcaswell · 2026-05-01T15:34:20Z

-         * We use SSL_R_DATA_LENGTH_TOO_LONG instead of
-         * SSL_R_ENCRYPTED_LENGTH_TOO_LONG here because we are the "any" method
+         * We use ERR_R_INVALID_LENGTH instead of
+         * ERR_R_INVALID_LENGTH here because we are the "any" method


This makes no sense

Fixed. Updated the comment to say "We use ERR_R_INVALID_LENGTH rather than a more specific length error" instead of the self-referential text.

mattcaswell · 2026-05-01T15:37:27Z

+    do {                                                                                                                          \
+        if (!ossl_assert((s)->statem.in_init                                                                                      \
+                && (s)->statem.state == MSG_FLOW_ERROR))                                                                          \
+            SSLfatal_data(s, SSL_AD_INTERNAL_ERROR, ERR_R_MISSING_REQUIRED_DATA, "state machine not in error state as expected"); \


This macro looks quite strange in github because the line length is too long. Can we shorten it?

This is unavoidably long because it's a macro with backslash continuations - the format string can't easily be split across macro continuation lines.

Actually, any literal string can be split into multiple pieces, so for example, the string "Hello world!" can be split into "Hello " "world!", and the compiler will automatically concatenate the pieces into one literal string.

mattcaswell · 2026-05-01T15:39:21Z

+            ERR_raise(ERR_LIB_SSL, ERR_R_SECURITY_VIOLATION);
+            ERR_raise(ERR_LIB_SSL, ERR_R_SECURITY_VIOLATION);
+            ERR_raise(ERR_LIB_SSL, ERR_R_SECURITY_VIOLATION);
 #endif


Hmm. this change looks odd.

I attempted to clean up the #if 0 block but it broke control flow. Reverted the file - the dead code is best left untouched since it's inside #if 0 anyway.

mattcaswell · 2026-05-01T15:42:20Z


    if (mtype == DANETLS_MATCHING_FULL && md != NULL) {
-        ERR_raise(ERR_LIB_SSL, SSL_R_DANE_CANNOT_OVERRIDE_MTYPE_FULL);
+        ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);


Hmm. It looks like there is an opportunity to provide better human data in this file with ERR_raise_data. We are losing some useful information by moving to the generic error codes here.

Fixed. Changed ERR_raise to ERR_raise_data with a descriptive format string: "DANE TLSA matching type conflict with digest".

mattcaswell · 2026-05-01T15:43:05Z

    switch (validation_mode) {
    default:
-        ERR_raise(ERR_LIB_SSL, SSL_R_INVALID_CT_VALIDATION_TYPE);
+        ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);


Similarly here, we could use ERR_raise_data

Fixed. Changed ERR_raise to ERR_raise_data with a descriptive format string: "invalid CT validation mode".

mattcaswell

Addressed: Split excessively long SSLfatal_data lines. Most are now under 140 chars with the message on a new line. The check_fatal macro in statem.c is unavoidably long due to backslash continuations.

mattcaswell added 5 commits May 1, 2026 15:53

Fix documentation: register ERR_R_XXX(7) man page, fix doc-nits issues

02df98a

Fix md-nits and CHANGES.md formatting for new error code entry

3df28fe

mattcaswell commented May 1, 2026

View reviewed changes


		=head1 NAME

		ERR_R_XXX - OpenSSL machine-readable error reason codes


		=head1 DESCRIPTION

		that can be used for programmatic error handling. Unlike the library-specific

Conversation

mattcaswell commented May 1, 2026

Checklist

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants