When strict SCT fails record verification failure

Since with SSL_VERIFY_NONE, the connection may continue and the
session may even be cached, we should save some evidence that the
chain was not sufficiently verified and would have been rejected
with SSL_VERIFY_PEER.  To that end when a CT callback returs failure
we set the verify result to X509_V_ERR_NO_VALID_SCTS.

Note: We only run the CT callback in the first place if the verify
result is still X509_V_OK prior to start of the callback.

RT #4502

Reviewed-by: Tim Hudson <tjh@openssl.org>

4 files changed, 41 insertions, 12 deletions

diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index 5341e79..ae54de1 100644
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -165,6 +165,8 @@ const char *X509_verify_cert_error_string(long n)
         return ("Invalid certificate verification context");
     case X509_V_ERR_STORE_LOOKUP:
         return ("Issuer certificate lookup error");
+    case X509_V_ERR_NO_VALID_SCTS:
+        return ("Certificate Transparency required, but no valid SCTs found");
 
     default:
         /* Printing an error number into a static buffer is not thread-safe */
diff --git a/doc/ssl/SSL_CTX_set_ct_validation_callback.pod b/doc/ssl/SSL_CTX_set_ct_validation_callback.pod
index ec51c75..bcd68d3 100644
--- a/doc/ssl/SSL_CTX_set_ct_validation_callback.pod
+++ b/doc/ssl/SSL_CTX_set_ct_validation_callback.pod
@@ -33,21 +33,29 @@ The behaviour of the callback is determined by the B<validation_mode> argument,
 which can be either of B<SSL_CT_VALIDATION_PERMISSIVE> or
 B<SSL_CT_VALIDATION_STRICT> as described below.
 
+If B<validation_mode> is equal to B<SSL_CT_VALIDATION_STRICT>, then in a full
+TLS handshake with the verification mode set to B<SSL_VERIFY_PEER>, if the peer
+presents no valid SCTs the handshake will be aborted.
+If the verification mode is B<SSL_VERIFY_NONE>, the handshake will continue
+despite lack of valid SCTs.
+However, in that case if the verification status before the built-in callback
+was B<X509_V_OK> it will be set to B<X509_V_ERR_NO_VALID_SCTS> after the
+callback.
+Applications can call L<SSL_get_verify_result(3)> to check the status at
+handshake completion, even after session resumption since the verification
+status is part of the saved session state.
+See L<SSL_set_verify(3)>, <SSL_get_verify_result(3)>, L<SSL_session_reused(3)>.
+
 If B<validation_mode> is equal to B<SSL_CT_VALIDATION_PERMISSIVE>, then the
-handshake continues regardless of the validation status of any SCTs.
-The application can inspect the validation status of the SCTs at handshake
-completion.
+handshake continues, and the verification status is not modified, regardless of
+the validation status of any SCTs.
+The application can still inspect the validation status of the SCTs at
+handshake completion.
 Note that with session resumption there will not be any SCTs presented during
 the handshake.
 Therefore, in applications that delay SCT policy enforcement until after
-handshake completion, SCT checks should only be performed when the session is
-not reused.
-See L<SSL_session_reused(3)>.
-
-If B<validation_mode> is equal to B<SSL_CT_VALIDATION_STRICT>, then in a full
-TLS handshake with the verification mode set to B<SSL_VERIFY_PEER>, if the peer
-presents no valid SCTs the handshake will be aborted.
-See L<SSL_set_verify(3)>.
+handshake completion, such delayed SCT checks should only be performed when the
+session is not resumed.
 
 SSL_set_ct_validation_callback() and SSL_CTX_set_ct_validation_callback()
 register a custom callback that may implement a different policy than either of
@@ -112,6 +120,7 @@ callback) is set.
 =head1 SEE ALSO
 
 L<ssl(3)>,
+<SSL_get_verify_result(3)>,
 L<SSL_session_reused(3)>,
 L<SSL_set_verify(3)>,
 L<SSL_CTX_set_verify(3)>,
diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index 44f1f16..3adfaa3 100644
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -158,11 +158,12 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth);
 # define         X509_V_ERR_EE_KEY_TOO_SMALL                     66
 # define         X509_V_ERR_CA_KEY_TOO_SMALL                     67
 # define         X509_V_ERR_CA_MD_TOO_WEAK                       68
-
 /* Caller error */
 # define         X509_V_ERR_INVALID_CALL                         69
 /* Issuer lookup error */
 # define         X509_V_ERR_STORE_LOOKUP                         70
+/* Certificate transparency */
+# define         X509_V_ERR_NO_VALID_SCTS                        71
 
 /* Certificate verify flags */
 
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 000a509..9fb6e89 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -4134,6 +4134,23 @@ int ssl_validate_ct(SSL *s)
 
 end:
     CT_POLICY_EVAL_CTX_free(ctx);
+    /*
+     * With SSL_VERIFY_NONE the session may be cached and re-used despite a
+     * failure return code here.  Also the application may wish the complete
+     * the handshake, and then disconnect cleanly at a higher layer, after
+     * checking the verification status of the completed connection.
+     *
+     * We therefore force a certificate verification failure which will be
+     * visible via SSL_get_verify_result() and cached as part of any resumed
+     * session.
+     *
+     * Note: the permissive callback is for information gathering only, always
+     * returns success, and does not affect verification status.  Only the
+     * strict callback or a custom application-specified callback can trigger
+     * connection failure or record a verification error.
+     */
+    if (ret <= 0)
+        s->verify_result = X509_V_ERR_NO_VALID_SCTS;
     return ret;
 }