NewSessionTickets with an early_data extension must have a valid max …

…value

The max_early_data value must be 0xffffffff if the extension is present in
a NewSessionTicket message in QUIC. Otherwise it is a PROTOCOL_VIOLATION.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #21686)

include/internal/quic_tls.h

@@ -101,5 +101,6 @@ int ossl_quic_tls_get_error(QUIC_TLS *qtls,
                             ERR_STATE **error_state);
 
 int ossl_quic_tls_is_cert_request(QUIC_TLS *qtls);
+int ossl_quic_tls_has_bad_max_early_data(QUIC_TLS *qtls);
 
 #endif

ssl/quic/quic_channel.c

@@ -994,14 +994,28 @@ static int ch_on_handshake_alert(void *arg, unsigned char alert_code)
      * TLS CertificateRequest messages, and clients MUST treat receipt of such
      * messages as a connection error of type PROTOCOL_VIOLATION.
      */
-    if (alert_code == SSL3_AD_UNEXPECTED_MESSAGE
+    if (alert_code == SSL_AD_UNEXPECTED_MESSAGE
             && ch->handshake_complete
             && ossl_quic_tls_is_cert_request(ch->qtls))
         ossl_quic_channel_raise_protocol_error(ch,
                                                QUIC_ERR_PROTOCOL_VIOLATION,
                                                0,
                                                "Post-handshake TLS "
                                                "CertificateRequest received");
+    /*
+     * RFC 9001 s. 4.6.1: Servers MUST NOT send the early_data extension with a
+     * max_early_data_size field set to any value other than 0xffffffff. A
+     * client MUST treat receipt of a NewSessionTicket that contains an
+     * early_data extension with any other value as a connection error of type
+     * PROTOCOL_VIOLATION.
+     */
+    else if (alert_code == SSL_AD_ILLEGAL_PARAMETER
+             && ch->handshake_complete
+             && ossl_quic_tls_has_bad_max_early_data(ch->qtls))
+        ossl_quic_channel_raise_protocol_error(ch,
+                                               QUIC_ERR_PROTOCOL_VIOLATION,
+                                               0,
+                                               "Bad max_early_data received");
     else
         ossl_quic_channel_raise_protocol_error(ch,
                                                QUIC_ERR_CRYPTO_ERR_BEGIN

ssl/quic/quic_tls.c

@@ -853,3 +853,19 @@ int ossl_quic_tls_is_cert_request(QUIC_TLS *qtls)
 
     return sc->s3.tmp.message_type == SSL3_MT_CERTIFICATE_REQUEST;
 }
+
+/*
+ * Returns true if the last session associated with the connection has an
+ * invalid max_early_data value for QUIC.
+ */
+int ossl_quic_tls_has_bad_max_early_data(QUIC_TLS *qtls)
+{
+    uint32_t max_early_data = SSL_get0_session(qtls->args.s)->ext.max_early_data;
+
+    /*
+     * If max_early_data was present we always ensure a non-zero value is
+     * stored in the session for QUIC. Therefore if max_early_data == 0 here
+     * we can be confident that it was not present in the NewSessionTicket
+     */
+    return max_early_data != 0xffffffff && max_early_data != 0;
+}

ssl/statem/extensions_clnt.c

@@ -1934,6 +1934,22 @@ int tls_parse_stoc_early_data(SSL_CONNECTION *s, PACKET *pkt,
 
         s->session->ext.max_early_data = max_early_data;
 
+        if (SSL_IS_QUIC_HANDSHAKE(s) && max_early_data != 0xffffffff) {
+            /*
+             * QUIC allows missing max_early_data, or a max_early_data value
+             * of 0xffffffff. Missing max_early_data is stored in the session
+             * as 0. This is indistinguishable in OpenSSL from a present
+             * max_early_data value that was 0. In order that later checks for
+             * invalid max_early_data correctly treat as an error the case where
+             * max_early_data is present and it is 0, we store any invalid
+             * value in the same (non-zero) way. Otherwise we would have to
+             * introduce a new flag just for this.
+             */
+            s->session->ext.max_early_data = 1;
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_INVALID_MAX_EARLY_DATA);
+            return 0;
+        }
+
         return 1;
     }