Add error checking to tls_session_secret_cb() calls

Failure of this undocumented callback was previously silently ignored.
Follow OpenSSL's behavior and throw an internal error (for lack of a
better choice) if the callback failed or if it set the master_key_length
to a negative number.

Unindent the success path and clean up some strange idioms.

ok jsing

2 files changed, 49 insertions, 32 deletions

diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index f5b8802a69..6f93b55ddc 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.142 2022/06/06 13:18:34 tb Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.143 2022/06/07 17:14:17 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -923,16 +923,26 @@ ssl3_get_server_hello(SSL *s)
          * Check if we want to resume the session based on external
          * pre-shared secret.
          */
-        if (s->internal->tls_session_secret_cb) {
+        if (s->internal->tls_session_secret_cb != NULL) {
                 SSL_CIPHER *pref_cipher = NULL;
-                s->session->master_key_length = sizeof(s->session->master_key);
-                if (s->internal->tls_session_secret_cb(s, s->session->master_key,
-                    &s->session->master_key_length, NULL, &pref_cipher,
-                    s->internal->tls_session_secret_cb_arg)) {
-                        s->session->cipher = pref_cipher ? pref_cipher :
-                            ssl3_get_cipher_by_value(cipher_suite);
-                        s->s3->flags |= SSL3_FLAGS_CCS_OK;
+                int master_key_length = sizeof(s->session->master_key);
+
+                if (!s->internal->tls_session_secret_cb(s,
+                    s->session->master_key, &master_key_length, NULL,
+                    &pref_cipher, s->internal->tls_session_secret_cb_arg)) {
+                        SSLerror(s, ERR_R_INTERNAL_ERROR);
+                        goto err;
+                }
+                if (master_key_length <= 0) {
+                        SSLerror(s, ERR_R_INTERNAL_ERROR);
+                        goto err;
                 }
+                s->session->master_key_length = master_key_length;
+
+                if ((s->session->cipher = pref_cipher) == NULL)
+                        s->session->cipher =
+                            ssl3_get_cipher_by_value(cipher_suite);
+                s->s3->flags |= SSL3_FLAGS_CCS_OK;
         }
 
         if (s->session->session_id_length != 0 &&
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index 359395051a..35f3d585ac 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.141 2022/02/05 14:54:10 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.142 2022/06/07 17:14:17 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1055,34 +1055,41 @@ ssl3_get_client_hello(SSL *s)
                 }
         }
 
-        if (!s->internal->hit && s->internal->tls_session_secret_cb) {
+        if (!s->internal->hit && s->internal->tls_session_secret_cb != NULL) {
                 SSL_CIPHER *pref_cipher = NULL;
+                int master_key_length = sizeof(s->session->master_key);
 
-                s->session->master_key_length = sizeof(s->session->master_key);
-                if (s->internal->tls_session_secret_cb(s, s->session->master_key,
-                    &s->session->master_key_length, ciphers, &pref_cipher,
-                    s->internal->tls_session_secret_cb_arg)) {
-                        s->internal->hit = 1;
-                        s->session->ciphers = ciphers;
-                        s->session->verify_result = X509_V_OK;
+                if (!s->internal->tls_session_secret_cb(s,
+                    s->session->master_key, &master_key_length, ciphers,
+                    &pref_cipher, s->internal->tls_session_secret_cb_arg)) {
+                        SSLerror(s, ERR_R_INTERNAL_ERROR);
+                        goto err;
+                }
+                if (master_key_length <= 0) {
+                        SSLerror(s, ERR_R_INTERNAL_ERROR);
+                        goto err;
+                }
+                s->session->master_key_length = master_key_length;
 
-                        ciphers = NULL;
+                s->internal->hit = 1;
+                s->session->verify_result = X509_V_OK;
 
-                        /* check if some cipher was preferred by call back */
-                        pref_cipher = pref_cipher ? pref_cipher :
-                            ssl3_choose_cipher(s, s->session->ciphers,
-                            SSL_get_ciphers(s));
-                        if (pref_cipher == NULL) {
-                                al = SSL_AD_HANDSHAKE_FAILURE;
-                                SSLerror(s, SSL_R_NO_SHARED_CIPHER);
-                                goto fatal_err;
-                        }
-
-                        s->session->cipher = pref_cipher;
+                s->session->ciphers = ciphers;
+                ciphers = NULL;
 
-                        sk_SSL_CIPHER_free(s->cipher_list);
-                        s->cipher_list = sk_SSL_CIPHER_dup(s->session->ciphers);
+                /* Check if some cipher was preferred by the callback. */
+                if (pref_cipher == NULL)
+                        pref_cipher = ssl3_choose_cipher(s, s->session->ciphers,
+                            SSL_get_ciphers(s));
+                if (pref_cipher == NULL) {
+                        al = SSL_AD_HANDSHAKE_FAILURE;
+                        SSLerror(s, SSL_R_NO_SHARED_CIPHER);
+                        goto fatal_err;
                 }
+                s->session->cipher = pref_cipher;
+
+                sk_SSL_CIPHER_free(s->cipher_list);
+                s->cipher_list = sk_SSL_CIPHER_dup(s->session->ciphers);
         }
 
         /*