Use a record layer specific message callback

Don't use the message callback from the SSL object. Instead we use a wrapper callback so that the record layer does not need to be aware of the SSL object. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #18132)
openssl · Aug 18, 2022 · 3c7b9ef · 3c7b9ef
1 parent 9dd9023
commit 3c7b9ef
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 16 deletions.
diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
@@ -170,7 +170,8 @@ struct ossl_record_layer_st
 
     /* Callbacks */
     void *cbarg;
-    OSSL_FUNC_rlayer_skip_early_data_fn *rlayer_skip_early_data;
+    OSSL_FUNC_rlayer_skip_early_data_fn *skip_early_data;
+    OSSL_FUNC_rlayer_msg_callback_fn *msg_callback;
 
     /* Function pointers for version specific functions */
     struct record_functions_st *funcs;

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
@@ -446,7 +446,6 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
     PACKET pkt, sslv2pkt;
     SSL_MAC_BUF *macbufs = NULL;
     int ret = OSSL_RECORD_RETURN_FATAL;
-    SSL *ssl = SSL_CONNECTION_GET_SSL(s);
 
     rr = rl->rrec;
     rbuf = &rl->rbuf;
@@ -524,9 +523,7 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
                 if (!PACKET_get_1(&pkt, &type)
                         || !PACKET_get_net_2(&pkt, &version)
                         || !PACKET_get_net_2_len(&pkt, &thisrr->length)) {
-                    if (s->msg_callback)
-                        s->msg_callback(0, 0, SSL3_RT_HEADER, p, 5, ssl,
-                                        s->msg_callback_arg);
+                    rl->msg_callback(0, 0, SSL3_RT_HEADER, p, 5, rl->cbarg);
                     RLAYERfatal(rl, SSL_AD_DECODE_ERROR, ERR_R_INTERNAL_ERROR);
                     return OSSL_RECORD_RETURN_FATAL;
                 }
@@ -545,9 +542,7 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
                     return OSSL_RECORD_RETURN_FATAL;
                 }
 
-                if (s->msg_callback)
-                    s->msg_callback(0, version, SSL3_RT_HEADER, p, 5, ssl,
-                                    s->msg_callback_arg);
+                rl->msg_callback(0, version, SSL3_RT_HEADER, p, 5, rl->cbarg);
 
                 if (thisrr->length >
                     SSL3_BUFFER_get_len(rbuf) - SSL3_RT_HEADER_LENGTH) {
@@ -729,7 +724,7 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
             /* RLAYERfatal() already got called */
             goto end;
         }
-        if (num_recs == 1 && rl->rlayer_skip_early_data(rl->cbarg)) {
+        if (num_recs == 1 && rl->skip_early_data(rl->cbarg)) {
             /*
              * Valid early_data that we cannot decrypt will fail here. We treat
              * it like an empty record.
@@ -941,18 +936,15 @@ int tls_default_post_process_record(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, SSL
 int tls13_common_post_process_record(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec,
                                      SSL_CONNECTION *s)
 {
-    SSL *ssl = SSL_CONNECTION_GET_SSL(s);
-
     if (rec->type != SSL3_RT_APPLICATION_DATA
             && rec->type != SSL3_RT_ALERT
             && rec->type != SSL3_RT_HANDSHAKE) {
         RLAYERfatal(rl, SSL_AD_UNEXPECTED_MESSAGE, SSL_R_BAD_RECORD_TYPE);
         return 0;
     }
 
-    if (s->msg_callback)
-        s->msg_callback(0, rl->version, SSL3_RT_INNER_CONTENT_TYPE,
-                        &rec->type, 1, ssl, s->msg_callback_arg);
+    rl->msg_callback(0, rl->version, SSL3_RT_INNER_CONTENT_TYPE, &rec->type,
+                        1, rl->cbarg);
 
     /*
      * TLSv1.3 alert and handshake records are required to be non-zero in
@@ -1135,7 +1127,10 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
     for (; fns->function_id != 0; fns++) {
         switch (fns->function_id) {
         case OSSL_FUNC_RLAYER_SKIP_EARLY_DATA:
-            rl->rlayer_skip_early_data = OSSL_FUNC_rlayer_skip_early_data(fns);
+            rl->skip_early_data = OSSL_FUNC_rlayer_skip_early_data(fns);
+            break;
+        case OSSL_FUNC_RLAYER_MSG_CALLBACK:
+            rl->msg_callback = OSSL_FUNC_rlayer_msg_callback(fns);
             break;
         default:
             /* Just ignore anything we don't understand */

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
@@ -1749,8 +1749,21 @@ size_t RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl)
     return SSL3_RECORD_get_length(&rl->rrec[0]);
 }
 
+static void rlayer_msg_callback_wrapper(int write_p, int version,
+                                        int content_type, const void *buf,
+                                        size_t len, void *cbarg)
+{
+    SSL_CONNECTION *s = cbarg;
+    SSL *ssl = SSL_CONNECTION_GET_SSL(s);
+
+    if (s->msg_callback != NULL)
+        s->msg_callback(write_p, version, content_type, buf, len, ssl,
+                        s->msg_callback_arg);
+}
+
 static const OSSL_DISPATCH rlayer_dispatch[] = {
     { OSSL_FUNC_RLAYER_SKIP_EARLY_DATA, (void (*)(void))ossl_statem_skip_early_data },
+    { OSSL_FUNC_RLAYER_MSG_CALLBACK, (void (*)(void))rlayer_msg_callback_wrapper },
     { 0, NULL }
 };
 

diff --git a/ssl/record/record.h b/ssl/record/record.h
@@ -293,4 +293,9 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, int direction,
                              const SSL_COMP *comp);
 
 # define OSSL_FUNC_RLAYER_SKIP_EARLY_DATA        1
-OSSL_CORE_MAKE_FUNC(const OSSL_PARAM *, rlayer_skip_early_data,(void *cbarg))
+OSSL_CORE_MAKE_FUNC(const OSSL_PARAM *, rlayer_skip_early_data, (void *cbarg))
+# define OSSL_FUNC_RLAYER_MSG_CALLBACK           2
+OSSL_CORE_MAKE_FUNC(void, rlayer_msg_callback, (int write_p, int version,
+                                                int content_type,
+                                                const void *buf, size_t len,
+                                                void *cbarg))