From 545b2b63048ee07e7fd983e5551ba5812e662786 Mon Sep 17 00:00:00 2001 From: jsing Date: Mon, 30 Aug 2021 19:25:43 +0000 Subject: [PATCH] Clean up and simplify info and msg callbacks. The info and msg callbacks result in duplication - both for code that refers to the function pointers and for the call sites. Avoid this by providing typedefs for the function pointers and pulling the calling sequences into their own functions. ok inoguchi@ tb@ --- lib/libssl/d1_pkt.c | 29 ++++++++------------------- lib/libssl/ssl_both.c | 22 ++++++++------------- lib/libssl/ssl_clnt.c | 24 ++++++++-------------- lib/libssl/ssl_lib.c | 30 +++++++++++++++++++++------- lib/libssl/ssl_locl.h | 27 +++++++++++++++---------- lib/libssl/ssl_pkt.c | 45 +++++++++++------------------------------- lib/libssl/ssl_srvr.c | 24 ++++++++-------------- lib/libssl/tls13_lib.c | 14 ++++--------- 8 files changed, 88 insertions(+), 127 deletions(-) diff --git a/lib/libssl/d1_pkt.c b/lib/libssl/d1_pkt.c index 4f0678f0b89..f99b8ff3712 100644 --- a/lib/libssl/d1_pkt.c +++ b/lib/libssl/d1_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_pkt.c,v 1.106 2021/08/30 19:12:25 jsing Exp $ */ +/* $OpenBSD: d1_pkt.c,v 1.107 2021/08/30 19:25:43 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -515,10 +515,9 @@ dtls1_get_record(SSL *s) int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) { - int al, i, j, ret; + int al, i, ret; unsigned int n; SSL3_RECORD_INTERNAL *rr; - void (*cb)(const SSL *ssl, int type2, int val) = NULL; if (S3I(s)->rbuf.buf == NULL) /* Not initialized yet */ if (!ssl3_setup_buffers(s)) @@ -727,9 +726,8 @@ dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) /* no need to check sequence number on HELLO REQUEST messages */ - if (s->internal->msg_callback) - s->internal->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, - D1I(s)->handshake_fragment, 4, s, s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, SSL3_RT_HANDSHAKE, + D1I(s)->handshake_fragment, 4); if (SSL_is_init_finished(s) && !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) && @@ -774,19 +772,10 @@ dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) D1I(s)->alert_fragment_len = 0; - if (s->internal->msg_callback) - s->internal->msg_callback(0, s->version, SSL3_RT_ALERT, - D1I(s)->alert_fragment, 2, s, s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, SSL3_RT_ALERT, D1I(s)->alert_fragment, 2); - if (s->internal->info_callback != NULL) - cb = s->internal->info_callback; - else if (s->ctx->internal->info_callback != NULL) - cb = s->ctx->internal->info_callback; - - if (cb != NULL) { - j = (alert_level << 8) | alert_descr; - cb(s, SSL_CB_READ_ALERT, j); - } + ssl_info_callback(s, SSL_CB_READ_ALERT, + (alert_level << 8) | alert_descr); if (alert_level == SSL3_AL_WARNING) { S3I(s)->warn_alert = alert_descr; @@ -832,9 +821,7 @@ dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) rr->length = 0; - if (s->internal->msg_callback) - s->internal->msg_callback(0, s->version, SSL3_RT_CHANGE_CIPHER_SPEC, - rr->data, 1, s, s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, SSL3_RT_CHANGE_CIPHER_SPEC, rr->data, 1); /* We can't process a CCS now, because previous handshake * messages are still missing, so just drop it. diff --git a/lib/libssl/ssl_both.c b/lib/libssl/ssl_both.c index 03c5a2f1e95..e4834f23dc2 100644 --- a/lib/libssl/ssl_both.c +++ b/lib/libssl/ssl_both.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_both.c,v 1.33 2021/07/01 17:53:39 jsing Exp $ */ +/* $OpenBSD: ssl_both.c,v 1.34 2021/08/30 19:25:43 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -150,10 +150,8 @@ ssl3_do_write(SSL *s, int type) (unsigned char *)&s->internal->init_buf->data[s->internal->init_off], ret); if (ret == s->internal->init_num) { - if (s->internal->msg_callback) - s->internal->msg_callback(1, s->version, type, s->internal->init_buf->data, - (size_t)(s->internal->init_off + s->internal->init_num), s, - s->internal->msg_callback_arg); + ssl_msg_callback(s, 1, type, s->internal->init_buf->data, + (size_t)(s->internal->init_off + s->internal->init_num)); return (1); } @@ -456,10 +454,8 @@ ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok) s->internal->init_num = 0; skip_message = 1; - if (s->internal->msg_callback) - s->internal->msg_callback(0, s->version, - SSL3_RT_HANDSHAKE, p, 4, s, - s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, + SSL3_RT_HANDSHAKE, p, 4); } } } while (skip_message); @@ -516,11 +512,9 @@ ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok) tls1_transcript_record(s, (unsigned char *)s->internal->init_buf->data, s->internal->init_num + 4); - if (s->internal->msg_callback) - s->internal->msg_callback(0, s->version, - SSL3_RT_HANDSHAKE, s->internal->init_buf->data, - (size_t)s->internal->init_num + 4, s, - s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, SSL3_RT_HANDSHAKE, + s->internal->init_buf->data, + (size_t)s->internal->init_num + 4); } *ok = 1; diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c index 8864909c9e6..519e823354f 100644 --- a/lib/libssl/ssl_clnt.c +++ b/lib/libssl/ssl_clnt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_clnt.c,v 1.107 2021/06/30 09:59:07 jsing Exp $ */ +/* $OpenBSD: ssl_clnt.c,v 1.108 2021/08/30 19:25:43 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -179,18 +179,12 @@ static int ca_dn_cmp(const X509_NAME * const *a, const X509_NAME * const *b); int ssl3_connect(SSL *s) { - void (*cb)(const SSL *ssl, int type, int val) = NULL; - int ret = -1; int new_state, state, skip = 0; + int ret = -1; ERR_clear_error(); errno = 0; - if (s->internal->info_callback != NULL) - cb = s->internal->info_callback; - else if (s->ctx->internal->info_callback != NULL) - cb = s->ctx->internal->info_callback; - s->internal->in_handshake++; if (!SSL_in_init(s) || SSL_in_before(s)) SSL_clear(s); @@ -210,8 +204,8 @@ ssl3_connect(SSL *s) case SSL_ST_OK|SSL_ST_CONNECT: s->server = 0; - if (cb != NULL) - cb(s, SSL_CB_HANDSHAKE_START, 1); + + ssl_info_callback(s, SSL_CB_HANDSHAKE_START, 1); if (!ssl_legacy_stack_version(s, s->version)) { SSLerror(s, ERR_R_INTERNAL_ERROR); @@ -597,8 +591,7 @@ ssl3_connect(SSL *s) s->internal->handshake_func = ssl3_connect; s->ctx->internal->stats.sess_connect_good++; - if (cb != NULL) - cb(s, SSL_CB_HANDSHAKE_DONE, 1); + ssl_info_callback(s, SSL_CB_HANDSHAKE_DONE, 1); if (SSL_is_dtls(s)) { /* done with handshaking */ @@ -623,10 +616,10 @@ ssl3_connect(SSL *s) goto end; } - if ((cb != NULL) && (S3I(s)->hs.state != state)) { + if (S3I(s)->hs.state != state) { new_state = S3I(s)->hs.state; S3I(s)->hs.state = state; - cb(s, SSL_CB_CONNECT_LOOP, 1); + ssl_info_callback(s, SSL_CB_CONNECT_LOOP, 1); S3I(s)->hs.state = new_state; } } @@ -635,8 +628,7 @@ ssl3_connect(SSL *s) end: s->internal->in_handshake--; - if (cb != NULL) - cb(s, SSL_CB_CONNECT_EXIT, ret); + ssl_info_callback(s, SSL_CB_CONNECT_EXIT, ret); return (ret); } diff --git a/lib/libssl/ssl_lib.c b/lib/libssl/ssl_lib.c index bb4b700e0b9..c5cc6d05faf 100644 --- a/lib/libssl/ssl_lib.c +++ b/lib/libssl/ssl_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_lib.c,v 1.262 2021/07/01 17:53:39 jsing Exp $ */ +/* $OpenBSD: ssl_lib.c,v 1.263 2021/08/30 19:25:43 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1184,9 +1184,7 @@ SSL_callback_ctrl(SSL *s, int cmd, void (*fp)(void)) { switch (cmd) { case SSL_CTRL_SET_MSG_CALLBACK: - s->internal->msg_callback = (void (*)(int write_p, int version, - int content_type, const void *buf, size_t len, - SSL *ssl, void *arg))(fp); + s->internal->msg_callback = (ssl_msg_callback_fn *)(fp); return (1); default: @@ -1284,9 +1282,7 @@ SSL_CTX_callback_ctrl(SSL_CTX *ctx, int cmd, void (*fp)(void)) { switch (cmd) { case SSL_CTRL_SET_MSG_CALLBACK: - ctx->internal->msg_callback = (void (*)(int write_p, int version, - int content_type, const void *buf, size_t len, SSL *ssl, - void *arg))(fp); + ctx->internal->msg_callback = (ssl_msg_callback_fn *)fp; return (1); default: @@ -2622,6 +2618,26 @@ ssl_clear_cipher_write_state(SSL *s) tls12_record_layer_clear_write_state(s->internal->rl); } +void +ssl_info_callback(const SSL *s, int type, int value) +{ + ssl_info_callback_fn *cb; + + if ((cb = s->internal->info_callback) == NULL) + cb = s->ctx->internal->info_callback; + if (cb != NULL) + cb(s, type, value); +} + +void +ssl_msg_callback(SSL *s, int is_write, int content_type, + const void *msg_buf, size_t msg_len) +{ + if (s->internal->msg_callback != NULL) + s->internal->msg_callback(is_write, s->version, content_type, + msg_buf, msg_len, s, s->internal->msg_callback_arg); +} + /* Fix this function so that it takes an optional type parameter */ X509 * SSL_get_certificate(const SSL *s) diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h index d25ac1a1a60..7ff3e0713dc 100644 --- a/lib/libssl/ssl_locl.h +++ b/lib/libssl/ssl_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_locl.h,v 1.357 2021/08/30 19:12:25 jsing Exp $ */ +/* $OpenBSD: ssl_locl.h,v 1.358 2021/08/30 19:25:43 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -662,6 +662,10 @@ int tls12_record_layer_seal_record(struct tls12_record_layer *rl, uint8_t content_type, const uint8_t *content, size_t content_len, CBB *out); +typedef void (ssl_info_callback_fn)(const SSL *s, int type, int val); +typedef void (ssl_msg_callback_fn)(int is_write, int version, int content_type, + const void *buf, size_t len, SSL *ssl, void *arg); + typedef struct ssl_ctx_internal_st { uint16_t min_tls_version; uint16_t max_tls_version; @@ -704,11 +708,10 @@ typedef struct ssl_ctx_internal_st { int (*app_verify_cookie_cb)(SSL *ssl, const unsigned char *cookie, unsigned int cookie_len); - void (*info_callback)(const SSL *ssl,int type,int val); /* used if SSL's info_callback is NULL */ + ssl_info_callback_fn *info_callback; /* callback that allows applications to peek at protocol messages */ - void (*msg_callback)(int write_p, int version, int content_type, - const void *buf, size_t len, SSL *ssl, void *arg); + ssl_msg_callback_fn *msg_callback; void *msg_callback_arg; int (*default_verify_callback)(int ok,X509_STORE_CTX *ctx); /* called 'verify_callback' in the SSL */ @@ -854,17 +857,17 @@ typedef struct ssl_internal_st { /* true when we are actually in SSL_accept() or SSL_connect() */ int in_handshake; int (*handshake_func)(SSL *); + + ssl_info_callback_fn *info_callback; + /* callback that allows applications to peek at protocol messages */ - void (*msg_callback)(int write_p, int version, int content_type, - const void *buf, size_t len, SSL *ssl, void *arg); + ssl_msg_callback_fn *msg_callback; void *msg_callback_arg; - /* Default generate session ID callback. */ - GEN_SESSION_CB generate_session_id; - int (*verify_callback)(int ok,X509_STORE_CTX *ctx); /* fail if callback returns 0 */ - void (*info_callback)(const SSL *ssl,int type,int val); /* optional informational callback */ + /* Default generate session ID callback. */ + GEN_SESSION_CB generate_session_id; /* TLS extension debug callback */ void (*tlsext_debug_cb)(SSL *s, int client_server, int type, @@ -1177,6 +1180,10 @@ void ssl_clear_cipher_read_state(SSL *s); void ssl_clear_cipher_write_state(SSL *s); int ssl_clear_bad_session(SSL *s); +void ssl_info_callback(const SSL *s, int type, int value); +void ssl_msg_callback(SSL *s, int is_write, int content_type, + const void *msg_buf, size_t msg_len); + CERT *ssl_cert_new(void); CERT *ssl_cert_dup(CERT *cert); void ssl_cert_free(CERT *c); diff --git a/lib/libssl/ssl_pkt.c b/lib/libssl/ssl_pkt.c index 9aa71f7d4fb..049a7df3c34 100644 --- a/lib/libssl/ssl_pkt.c +++ b/lib/libssl/ssl_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_pkt.c,v 1.49 2021/08/28 15:20:58 jsing Exp $ */ +/* $OpenBSD: ssl_pkt.c,v 1.50 2021/08/30 19:25:43 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -714,8 +714,7 @@ ssl3_write_pending(SSL *s, int type, const unsigned char *buf, unsigned int len) int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) { - void (*cb)(const SSL *ssl, int type2, int val) = NULL; - int al, i, j, ret, rrcount = 0; + int al, i, ret, rrcount = 0; unsigned int n; SSL3_RECORD_INTERNAL *rr; @@ -914,10 +913,8 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) goto fatal_err; } - if (s->internal->msg_callback) - s->internal->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, - S3I(s)->handshake_fragment, 4, s, - s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, SSL3_RT_HANDSHAKE, + S3I(s)->handshake_fragment, 4); if (SSL_is_init_finished(s) && !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) && @@ -978,19 +975,11 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) S3I(s)->alert_fragment_len = 0; - if (s->internal->msg_callback) - s->internal->msg_callback(0, s->version, SSL3_RT_ALERT, - S3I(s)->alert_fragment, 2, s, s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, SSL3_RT_ALERT, + S3I(s)->alert_fragment, 2); - if (s->internal->info_callback != NULL) - cb = s->internal->info_callback; - else if (s->ctx->internal->info_callback != NULL) - cb = s->ctx->internal->info_callback; - - if (cb != NULL) { - j = (alert_level << 8) | alert_descr; - cb(s, SSL_CB_READ_ALERT, j); - } + ssl_info_callback(s, SSL_CB_READ_ALERT, + (alert_level << 8) | alert_descr); if (alert_level == SSL3_AL_WARNING) { S3I(s)->warn_alert = alert_descr; @@ -1064,11 +1053,7 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) rr->length = 0; - if (s->internal->msg_callback) { - s->internal->msg_callback(0, s->version, - SSL3_RT_CHANGE_CIPHER_SPEC, rr->data, 1, s, - s->internal->msg_callback_arg); - } + ssl_msg_callback(s, 0, SSL3_RT_CHANGE_CIPHER_SPEC, rr->data, 1); S3I(s)->change_cipher_spec = 1; if (!ssl3_do_change_cipher_spec(s)) @@ -1224,7 +1209,6 @@ ssl3_send_alert(SSL *s, int level, int desc) int ssl3_dispatch_alert(SSL *s) { - void (*cb)(const SSL *ssl, int type, int val); int ret; S3I(s)->alert_dispatch = 0; @@ -1241,15 +1225,10 @@ ssl3_dispatch_alert(SSL *s) if (S3I(s)->send_alert[0] == SSL3_AL_FATAL) (void)BIO_flush(s->wbio); - if (s->internal->msg_callback) - s->internal->msg_callback(1, s->version, SSL3_RT_ALERT, - S3I(s)->send_alert, 2, s, s->internal->msg_callback_arg); + ssl_msg_callback(s, 1, SSL3_RT_ALERT, S3I(s)->send_alert, 2); - if ((cb = s->internal->info_callback) == NULL) - cb = s->ctx->internal->info_callback; - if (cb != NULL) - cb(s, SSL_CB_WRITE_ALERT, (S3I(s)->send_alert[0] << 8) | - S3I(s)->send_alert[1]); + ssl_info_callback(s, SSL_CB_WRITE_ALERT, + (S3I(s)->send_alert[0] << 8) | S3I(s)->send_alert[1]); return ret; } diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index 2097ccdebfb..a473d5af053 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.117 2021/06/29 19:43:15 jsing Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.118 2021/08/30 19:25:43 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -174,20 +174,14 @@ int ssl3_accept(SSL *s) { - void (*cb)(const SSL *ssl, int type, int val) = NULL; unsigned long alg_k; - int ret = -1; int new_state, state, skip = 0; int listen = 0; + int ret = -1; ERR_clear_error(); errno = 0; - if (s->internal->info_callback != NULL) - cb = s->internal->info_callback; - else if (s->ctx->internal->info_callback != NULL) - cb = s->ctx->internal->info_callback; - if (SSL_is_dtls(s)) listen = D1I(s)->listen; @@ -212,8 +206,8 @@ ssl3_accept(SSL *s) case SSL_ST_BEFORE|SSL_ST_ACCEPT: case SSL_ST_OK|SSL_ST_ACCEPT: s->server = 1; - if (cb != NULL) - cb(s, SSL_CB_HANDSHAKE_START, 1); + + ssl_info_callback(s, SSL_CB_HANDSHAKE_START, 1); if (!ssl_legacy_stack_version(s, s->version)) { SSLerror(s, ERR_R_INTERNAL_ERROR); @@ -705,8 +699,7 @@ ssl3_accept(SSL *s) /* s->server=1; */ s->internal->handshake_func = ssl3_accept; - if (cb != NULL) - cb(s, SSL_CB_HANDSHAKE_DONE, 1); + ssl_info_callback(s, SSL_CB_HANDSHAKE_DONE, 1); } ret = 1; @@ -735,10 +728,10 @@ ssl3_accept(SSL *s) } - if ((cb != NULL) && (S3I(s)->hs.state != state)) { + if (S3I(s)->hs.state != state) { new_state = S3I(s)->hs.state; S3I(s)->hs.state = state; - cb(s, SSL_CB_ACCEPT_LOOP, 1); + ssl_info_callback(s, SSL_CB_ACCEPT_LOOP, 1); S3I(s)->hs.state = new_state; } } @@ -747,8 +740,7 @@ ssl3_accept(SSL *s) end: /* BIO_flush(s->wbio); */ s->internal->in_handshake--; - if (cb != NULL) - cb(s, SSL_CB_ACCEPT_EXIT, ret); + ssl_info_callback(s, SSL_CB_ACCEPT_EXIT, ret); return (ret); } diff --git a/lib/libssl/tls13_lib.c b/lib/libssl/tls13_lib.c index f064521c8b5..77b4364f566 100644 --- a/lib/libssl/tls13_lib.c +++ b/lib/libssl/tls13_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_lib.c,v 1.59 2021/04/07 21:48:23 tb Exp $ */ +/* $OpenBSD: tls13_lib.c,v 1.60 2021/08/30 19:25:43 jsing Exp $ */ /* * Copyright (c) 2018, 2019 Joel Sing * Copyright (c) 2019 Bob Beck @@ -162,8 +162,7 @@ tls13_legacy_handshake_message_recv_cb(void *arg) return; tls13_handshake_msg_data(ctx->hs_msg, &cbs); - s->internal->msg_callback(0, TLS1_3_VERSION, SSL3_RT_HANDSHAKE, - CBS_data(&cbs), CBS_len(&cbs), s, s->internal->msg_callback_arg); + ssl_msg_callback(s, 0, SSL3_RT_HANDSHAKE, CBS_data(&cbs), CBS_len(&cbs)); } static void @@ -177,8 +176,7 @@ tls13_legacy_handshake_message_sent_cb(void *arg) return; tls13_handshake_msg_data(ctx->hs_msg, &cbs); - s->internal->msg_callback(1, TLS1_3_VERSION, SSL3_RT_HANDSHAKE, - CBS_data(&cbs), CBS_len(&cbs), s, s->internal->msg_callback_arg); + ssl_msg_callback(s, 1, SSL3_RT_HANDSHAKE, CBS_data(&cbs), CBS_len(&cbs)); } static void @@ -186,12 +184,8 @@ tls13_legacy_info_cb(void *arg, int state, int ret) { struct tls13_ctx *ctx = arg; SSL *s = ctx->ssl; - void (*cb)(const SSL *, int, int); - if ((cb = s->internal->info_callback) == NULL) - cb = s->ctx->internal->info_callback; - if (cb != NULL) - cb(s, state, ret); + ssl_info_callback(s, state, ret); } static int -- 2.20.1