ERR_reason_error_string() returns NULL for no_application_protocol alert

[jchampio]

Hello all,

OpenSSL 3.0 doesn't correctly report a reason string when handling the no_application_protocol alert; instead, ERR_reason_error_string() returns NULL. ERR_print_errors() and friends are similarly unhelpful:

E01FFFF7FFFF0000:error:0A000460:SSL routines:ssl3_read_bytes:reason(1120):ssl/record/rec_layer_s3.c:1590:SSL alert number 120

From inspection, later versions of OpenSSL 3.x also have this issue, but I haven't tested with all of them.

---

To my eyes, it looks like the lookup machinery expects the reason code to be 1120 (that is, SSL_AD_REASON_OFFSET + TLS1_AD_NO_APPLICATION_PROTOCOL), but SSL_R_NO_APPLICATION_PROTOCOL is defined to be 235.

I'm suspicious that the unknown_psk_identity alert may have the same issue, since SSL_R_PSK_IDENTITY_NOT_FOUND is defined as 223 and not 1115.

Here is a sample patch. If this looks close to right, I can open a pull request, but I'm unsure what to do with the existing SSL_R_NO_APPLICATION_PROTOCOL code.

diff --git a/crypto/err/openssl.ec b/crypto/err/openssl.ec
index 3612c195f0..3d395a92a4 100644
--- a/crypto/err/openssl.ec
+++ b/crypto/err/openssl.ec
@@ -78,4 +78,4 @@ R SSL_R_TLSV1_BAD_CERTIFICATE_STATUS_RESPONSE   1113
 R SSL_R_TLSV1_BAD_CERTIFICATE_HASH_VALUE        1114
 R TLS1_AD_UNKNOWN_PSK_IDENTITY                  1115
 R SSL_R_TLSV13_ALERT_CERTIFICATE_REQUIRED       1116
-R TLS1_AD_NO_APPLICATION_PROTOCOL               1120
+R SSL_R_TLSV1_ALERT_NO_APPLICATION_PROTOCOL     1120
diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
index b156fc2ffd..7e37ef4f16 100644
--- a/include/openssl/sslerr.h
+++ b/include/openssl/sslerr.h
@@ -283,6 +283,7 @@
 # define SSL_R_TLSV1_ALERT_INAPPROPRIATE_FALLBACK         1086
 # define SSL_R_TLSV1_ALERT_INSUFFICIENT_SECURITY          1071
 # define SSL_R_TLSV1_ALERT_INTERNAL_ERROR                 1080
+# define SSL_R_TLSV1_ALERT_NO_APPLICATION_PROTOCOL        1120
 # define SSL_R_TLSV1_ALERT_NO_RENEGOTIATION               1100
 # define SSL_R_TLSV1_ALERT_PROTOCOL_VERSION               1070
 # define SSL_R_TLSV1_ALERT_RECORD_OVERFLOW                1022
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 41898844ff..48ff9f0142 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -457,6 +457,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
     "tlsv1 alert insufficient security"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_TLSV1_ALERT_INTERNAL_ERROR),
     "tlsv1 alert internal error"},
+    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_TLSV1_ALERT_NO_APPLICATION_PROTOCOL),
+    "no application protocol"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_TLSV1_ALERT_NO_RENEGOTIATION),
     "tlsv1 alert no renegotiation"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_TLSV1_ALERT_PROTOCOL_VERSION),

Thanks!

[nhorman]

The alert message seems to be correct to me. From the TLS 1.3 RFC

The alert message is one of the values (1,2) (representing warning, fatal), concatenated with the actual message value, in this case no application protocol (defined as 120, both in the rfc, and in the code as TLS1_AD_NO_APPLICATION_PROTOCOL). this matches the value reported in the error you see in your report

The 1 in the 1120 values you are referencing I believe are pre-concatenated values showing both the severity value and the alert value

I believe SSL_R_NO_APPLICATION_PROTOCOL is a library internal error encoding value.

As to the reason this got reported, there is only one reason: The peer on your connection sent an alert message with that value in the alert field. Its a bit odd that you would receive that message while doing an ssl_read, as nominally the ALPN is exchanged as part of the client hello message during the handshake. I suppose its feasible that a peer would allow for no ALPN to be negotiated during the handshake and then error out when none is established during data exchange, but again, odd.

Edit: As for the lack of reason string with the error, it appears there is not number->string mapping that exists inlibssl. there is one in one of the test libraries, but generally speaking tls alerts appear to be reported in the error stack as their numerical values.

[jchampio]

Edit: As for the lack of reason string with the error,

Right, this is the focus of my report.

it appears there is not number->string mapping that exists inlibssl.

As far as I can tell, many other alerts get correctly mapped to their reason strings. My patch appears to fix that for this case. (But whether the fix is the correct approach, I don't know.)

[nhorman]

Ok, fair enough, can you open a PR with the changes reference for review please?

[jchampio]

@nhorman Done; I've moved my open questions into that PR. Thanks!

[jchampio]

Thanks everybody!