]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Replace handling of negative verification result with SSL_set_retry_verify()
authorTomas Mraz <tomas@openssl.org>
Mon, 7 Mar 2022 14:46:58 +0000 (15:46 +0100)
committerTomas Mraz <tomas@openssl.org>
Mon, 14 Mar 2022 08:39:03 +0000 (09:39 +0100)
Provide a different mechanism to indicate that the application wants
to retry the verification. The negative result of the callback function
now indicates an error again.

Instead the SSL_set_retry_verify() can be called from the callback
to indicate that the handshake should be suspended.

Fixes #17568

Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17825)

13 files changed:
CHANGES.md
doc/build.info
doc/man3/SSL_CTX_set_cert_verify_callback.pod
doc/man3/SSL_CTX_set_verify.pod
doc/man3/SSL_set_retry_verify.pod [new file with mode: 0644]
doc/man3/SSL_want.pod
doc/man3/X509_verify_cert.pod
include/openssl/ssl.h.in
ssl/ssl_lib.c
ssl/statem/statem_clnt.c
test/helpers/handshake.c
test/sslapitest.c
util/other.syms

index c7e3391d4b58e11832d4c5d9a2d55a1e8cd61a54..eb6174966fe3d03fcea380caa010b14f7f35640f 100644 (file)
@@ -123,6 +123,12 @@ breaking changes, and mappings for the large list of deprecated functions.
 
    *Paul Dale*
 
+ * The negative return value handling of the certificate verification callback
+   was reverted. The replacement is to set the verification retry state with
+   the SSL_set_retry_verify() function.
+
+   *Tomáš Mráz*
+
 ### Changes between 3.0.0 and 3.0.1 [14 dec 2021]
 
  * Fixed invalid handling of X509_verify_cert() internal errors in libssl
index 97e6bd3b51932edb652b34cc21a5752bc0e0a9b0..4e1db237d78d6113e04e3912b40d063e1622d50b 100644 (file)
@@ -2547,6 +2547,10 @@ DEPEND[html/man3/SSL_set_fd.html]=man3/SSL_set_fd.pod
 GENERATE[html/man3/SSL_set_fd.html]=man3/SSL_set_fd.pod
 DEPEND[man/man3/SSL_set_fd.3]=man3/SSL_set_fd.pod
 GENERATE[man/man3/SSL_set_fd.3]=man3/SSL_set_fd.pod
+DEPEND[html/man3/SSL_set_retry_verify.html]=man3/SSL_set_retry_verify.pod
+GENERATE[html/man3/SSL_set_retry_verify.html]=man3/SSL_set_retry_verify.pod
+DEPEND[man/man3/SSL_set_retry_verify.3]=man3/SSL_set_retry_verify.pod
+GENERATE[man/man3/SSL_set_retry_verify.3]=man3/SSL_set_retry_verify.pod
 DEPEND[html/man3/SSL_set_session.html]=man3/SSL_set_session.pod
 GENERATE[html/man3/SSL_set_session.html]=man3/SSL_set_session.pod
 DEPEND[man/man3/SSL_set_session.3]=man3/SSL_set_session.pod
@@ -3356,6 +3360,7 @@ html/man3/SSL_set_async_callback.html \
 html/man3/SSL_set_bio.html \
 html/man3/SSL_set_connect_state.html \
 html/man3/SSL_set_fd.html \
+html/man3/SSL_set_retry_verify.html \
 html/man3/SSL_set_session.html \
 html/man3/SSL_set_shutdown.html \
 html/man3/SSL_set_verify_result.html \
@@ -3948,6 +3953,7 @@ man/man3/SSL_set_async_callback.3 \
 man/man3/SSL_set_bio.3 \
 man/man3/SSL_set_connect_state.3 \
 man/man3/SSL_set_fd.3 \
+man/man3/SSL_set_retry_verify.3 \
 man/man3/SSL_set_session.3 \
 man/man3/SSL_set_shutdown.3 \
 man/man3/SSL_set_verify_result.3 \
index fdeeaee6d75589dc4a063272cd046a691f957d82..ecd312c350e10c4d92859f6047d94a0c74c04675 100644 (file)
@@ -36,16 +36,18 @@ In server mode, a return value of 0 leads to handshake failure.
 In client mode, the behaviour is as follows.
 All values, including 0, are ignored
 if the verification mode is B<SSL_VERIFY_NONE>.
-Otherwise, when the return value is 0, the handshake will fail.
-
-In client mode I<callback> may also return -1,
-typically on failure verifying the server certificate.
-This makes the handshake suspend and return control to the calling application
-with B<SSL_ERROR_WANT_RETRY_VERIFY>.
-The app can for instance fetch further certificates or cert status information
-needed for the verification.
-Calling L<SSL_connect(3)> again resumes the connection attempt
-by retrying the server certificate verification step.
+Otherwise, when the return value is less than or equal to 0, the handshake will
+fail.
+
+In client mode I<callback> may also call the L<SSL_set_retry_verify(3)>
+function on the B<SSL> object set in the I<x509_store_ctx> ex data (see
+L<SSL_get_ex_data_X509_STORE_CTX_idx(3)>) and return 1. This would be
+typically done in case the certificate verification was not yet able
+to succeed. This makes the handshake suspend and return control to the
+calling application with B<SSL_ERROR_WANT_RETRY_VERIFY>. The app can for
+instance fetch further certificates or cert status information needed for
+the verification. Calling L<SSL_connect(3)> again resumes the connection
+attempt by retrying the server certificate verification step.
 This process may even be repeated if need be.
 
 In any case a viable verification result value must be reflected
@@ -89,6 +91,7 @@ SSL_CTX_set_cert_verify_callback() does not provide diagnostic information.
 L<ssl(7)>, L<SSL_CTX_set_verify(3)>,
 L<X509_STORE_CTX_set_error(3)>,
 L<SSL_get_verify_result(3)>,
+L<SSL_set_retry_verify(3)>,
 L<SSL_CTX_load_verify_locations(3)>
 
 =head1 COPYRIGHT
index e3271aff01a844132e691e64c4f85adb7480438c..c085097617b2c68fceee1aca6b2e3cb7edadefbe 100644 (file)
@@ -44,6 +44,21 @@ L<SSL_new(3)>. Within the callback function,
 B<SSL_get_ex_data_X509_STORE_CTX_idx> can be called to get the data index
 of the current SSL object that is doing the verification.
 
+In client mode B<verify_callback> may also call the L<SSL_set_retry_verify(3)>
+function on the B<SSL> object set in the I<x509_store_ctx> ex data (see
+L<SSL_get_ex_data_X509_STORE_CTX_idx(3)>) and return 1.
+This would be typically done in case the certificate verification was not yet
+able to succeed.
+This makes the handshake suspend and return control to the calling application
+with B<SSL_ERROR_WANT_RETRY_VERIFY>.
+The application can for instance fetch further certificates or cert status
+information needed for the verification.
+Note that the handshake may still be aborted if a subsequent invocation of the
+callback (e.g. at a lower depth, or for a separate error condition) returns 0.
+Calling L<SSL_connect(3)> again resumes the connection attempt by retrying the
+server certificate verification step.
+This process may even be repeated if need be.
+
 SSL_CTX_set_verify_depth() sets the maximum B<depth> for the certificate chain
 verification that shall be allowed for B<ctx>.
 
diff --git a/doc/man3/SSL_set_retry_verify.pod b/doc/man3/SSL_set_retry_verify.pod
new file mode 100644 (file)
index 0000000..4eb7f44
--- /dev/null
@@ -0,0 +1,70 @@
+=pod
+
+=head1 NAME
+
+SSL_set_retry_verify - indicate that certificate verification should be retried
+
+=head1 SYNOPSIS
+
+ #include <openssl/ssl.h>
+
+ int SSL_set_retry_verify(SSL *ssl);
+
+=head1 DESCRIPTION
+
+SSL_set_retry_verify() should be called from the certificate verification
+callback on a client when the application wants to indicate that the handshake
+should be suspended and the control should be returned to the application.
+L<SSL_want_retry_verify(3)> will return 1 as a consequence until the handshake
+is resumed again by the application, retrying the verification step.
+
+Please refer to L<SSL_CTX_set_cert_verify_callback(3)> for further details.
+
+=head1 NOTES
+
+The effect of calling SSL_set_retry_verify() outside of the certificate
+verification callback on the client side is undefined.
+
+=head1 RETURN VALUES
+
+SSL_set_retry verify() returns 1 on success, 0 otherwise.
+
+=head1 EXAMPLES
+
+The following code snippet shows how to obtain the B<SSL> object associated
+with the B<X509_STORE_CTX> to call the SSL_set_retry_verify() function:
+
+    int idx = SSL_get_ex_data_X509_STORE_CTX_idx();
+    SSL *ssl;
+
+    /* this should not happen but check anyway */
+    if (idx < 0
+        || (ssl = X509_STORE_CTX_get_ex_data(ctx, idx)) == NULL) 
+        return 0;
+
+    if (/* we need to retry verification callback */)
+        return SSL_set_retry_verify(ssl);
+
+    /* do normal processing of the verification callback */
+
+=head1 SEE ALSO
+
+L<ssl(7)>, L<SSL_connect(3)>, L<SSL_CTX_set_cert_verify_callback(3)>,
+L<SSL_want_retry_verify(3)>
+
+=head1 HISTORY
+
+SSL_set_retry_verify() was added in OpenSSL 3.0.2 to replace backwards
+incompatible handling of a negative return value from the verification
+callback.
+
+=head1 COPYRIGHT
+
+Copyright 2022 The OpenSSL Project Authors. All Rights Reserved.
+
+Licensed under the Apache License 2.0 (the "License").  You may not use
+this file except in compliance with the License.  You can obtain a copy
+in the file LICENSE in the source distribution or at
+L<https://www.openssl.org/source/license.html>.
+
+=cut
index 831094ae0a69871af5465c115c1600c50b02264e..22920135b9ef6bb48594de4abb047483b78b1207 100644 (file)
@@ -70,8 +70,8 @@ A call to L<SSL_get_error(3)> should return B<SSL_ERROR_WANT_X509_LOOKUP>.
 
 =item SSL_RETRY_VERIFY
 
-The operation did not complete because an application callback set by
-SSL_CTX_set_cert_verify_callback() has asked to be called again.
+The operation did not complete because a certificate verification callback
+has asked to be called again via L<SSL_set_retry_verify(3)>.
 A call to L<SSL_get_error(3)> should return B<SSL_ERROR_WANT_RETRY_VERIFY>.
 
 =item SSL_ASYNC_PAUSED
index a14a0b25c4885fb2435d6e59dee8cb6febf4b6cc..6172ea5b1fad467f16b51919143828b4e3bc6d09 100644 (file)
@@ -52,9 +52,7 @@ A negative return value from X509_verify_cert() can occur if it is invoked
 incorrectly, such as with no certificate set in I<ctx>, or when it is called
 twice in succession without reinitialising I<ctx> for the second call.
 A negative return value can also happen due to internal resource problems
-or because an internal inconsistency has been detected
-or if a retry operation is requested during internal lookups
-(which never happens with standard lookup methods).
+or because an internal inconsistency has been detected.
 Applications must interpret any return value <= 0 as an error.
 
 The X509_STORE_CTX_verify() behaves like X509_verify_cert() except that its
index 47f277969cb63ad47954a1a443204231ce1c3c7f..b496dbccf38c65c06f99e74af138a0f4273a8710 100644 (file)
@@ -1309,6 +1309,7 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION)
 # define SSL_CTRL_GET_TMP_KEY                    133
 # define SSL_CTRL_GET_NEGOTIATED_GROUP           134
 # define SSL_CTRL_GET_IANA_GROUPS                135
+# define SSL_CTRL_SET_RETRY_VERIFY               136
 # define SSL_CERT_SET_FIRST                      1
 # define SSL_CERT_SET_NEXT                       2
 # define SSL_CERT_SET_SERVER                     3
@@ -2135,6 +2136,8 @@ __owur int SSL_get_ex_data_X509_STORE_CTX_idx(void);
         SSL_CTX_ctrl(ctx,SSL_CTRL_SET_MAX_PIPELINES,m,NULL)
 # define SSL_set_max_pipelines(ssl,m) \
         SSL_ctrl(ssl,SSL_CTRL_SET_MAX_PIPELINES,m,NULL)
+# define SSL_set_retry_verify(ssl) \
+        (SSL_ctrl(ssl,SSL_CTRL_SET_RETRY_VERIFY,0,NULL) > 0)
 
 void SSL_CTX_set_default_read_buffer_len(SSL_CTX *ctx, size_t len);
 void SSL_set_default_read_buffer_len(SSL *s, size_t len);
index 9138cd659bd9d80aef9fd733ff55d8bbb3c6777e..ab56e66e3a14efea9ffd1c41261f01424e432453 100644 (file)
@@ -2393,6 +2393,9 @@ long SSL_ctrl(SSL *s, int cmd, long larg, void *parg)
         return 1;
     case SSL_CTRL_GET_RI_SUPPORT:
         return s->s3.send_connection_binding;
+    case SSL_CTRL_SET_RETRY_VERIFY:
+        s->rwstate = SSL_RETRY_VERIFY;
+        return 1;
     case SSL_CTRL_CERT_FLAGS:
         return (s->cert->cert_flags |= larg);
     case SSL_CTRL_CLEAR_CERT_FLAGS:
index 2b0bfc7285de8136bb665585f330d8ad110bfdb9..1c4889431aa529af9b29cf610f68a1b980c342a9 100644 (file)
@@ -1859,9 +1859,10 @@ WORK_STATE tls_post_process_server_certificate(SSL *s, WORK_STATE wst)
     size_t certidx;
     int i;
 
+    if (s->rwstate == SSL_RETRY_VERIFY)
+        s->rwstate = SSL_NOTHING;
     i = ssl_verify_cert_chain(s, s->session->peer_chain);
-    if (i == -1) {
-        s->rwstate = SSL_RETRY_VERIFY;
+    if (i > 0 && s->rwstate == SSL_RETRY_VERIFY) {
         return WORK_MORE_A;
     }
     /*
@@ -1878,7 +1879,7 @@ WORK_STATE tls_post_process_server_certificate(SSL *s, WORK_STATE wst)
      * (less clean) historic behaviour of performing validation if any flag is
      * set. The *documented* interface remains the same.
      */
-    if (s->verify_mode != SSL_VERIFY_NONE && i == 0) {
+    if (s->verify_mode != SSL_VERIFY_NONE && i <= 0) {
         SSLfatal(s, ssl_x509err2alert(s->verify_result),
                  SSL_R_CERTIFICATE_VERIFY_FAILED);
         return WORK_ERROR;
index cee11bbc18df8df82e3ea161953af46a72bbf935..0e503017aacf573c9e897a6936794a53a9c3b7b2 100644 (file)
@@ -305,10 +305,18 @@ static int verify_reject_cb(X509_STORE_CTX *ctx, void *arg) {
 
 static int n_retries = 0;
 static int verify_retry_cb(X509_STORE_CTX *ctx, void *arg) {
+    int idx = SSL_get_ex_data_X509_STORE_CTX_idx();
+    SSL *ssl;
+
+    /* this should not happen but check anyway */
+    if (idx < 0
+        || (ssl = X509_STORE_CTX_get_ex_data(ctx, idx)) == NULL)
+        return 0;
+
     if (--n_retries < 0)
         return 1;
-    X509_STORE_CTX_set_error(ctx, X509_V_ERR_APPLICATION_VERIFICATION);
-    return -1;
+
+    return SSL_set_retry_verify(ssl);
 }
 
 static int verify_accept_cb(X509_STORE_CTX *ctx, void *arg) {
index 0c6a5e14c1d20697cd307f6281e07483e9e366dc..437a9c8f2538df21541f63da59298d98c547c599 100644 (file)
@@ -557,10 +557,19 @@ end:
 static int verify_retry_cb(X509_STORE_CTX *ctx, void *arg)
 {
     int res = X509_verify_cert(ctx);
+    int idx = SSL_get_ex_data_X509_STORE_CTX_idx();
+    SSL *ssl;
+
+    /* this should not happen but check anyway */
+    if (idx < 0
+        || (ssl = X509_STORE_CTX_get_ex_data(ctx, idx)) == NULL)
+        return 0;
 
     if (res == 0 && X509_STORE_CTX_get_error(ctx) ==
         X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY)
-        return -1; /* indicate SSL_ERROR_WANT_RETRY_VERIFY */
+        /* indicate SSL_ERROR_WANT_RETRY_VERIFY */
+        return SSL_set_retry_verify(ssl);
+
     return res;
 }
 
index 82766810f11b783c3df4768eb974e84726ea0484..3eaafee3a88fb56f9368818ce62c78d93a99b5e2 100644 (file)
@@ -677,3 +677,4 @@ EVP_PKEY_security_bits                  define
 EVP_PKEY_size                           define
 EVP_PKEY_id                             define
 EVP_PKEY_base_id                        define
+SSL_set_retry_verify                    define