[PATCH 6 of 6] SSL: fix order of checks during SSL certificate verification

[Piotr Sikora]

# HG changeset patch
# User Piotr Sikora <piotrsikora at google.com>
# Date 1471429000 25200
#      Wed Aug 17 03:16:40 2016 -0700
# Node ID 7bc55832b01ad62ac85f7fe5c72cbc4a7f212c3b
# Parent  5550dfc1414afcd5471b7fc8ca4482f7e18ba865
SSL: fix order of checks during SSL certificate verification.

SSL_get_verify_result() should be called only if certificate was presented
by the peer, otherwise returned value is the default one, which happens to
be X509_V_OK, but it doesn't indicate success and it's considered a bug:
https://www.openssl.org/docs/manmaster/ssl/SSL_get_verify_result.html

Signed-off-by: Piotr Sikora <piotrsikora at google.com>

diff -r 5550dfc1414a -r 7bc55832b01a src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -3080,6 +3080,21 @@ ngx_ssl_verify_client(ngx_connection_t *
     long   rc;
     X509  *cert;
 
+    cert = SSL_get_peer_certificate(c->ssl->connection);
+
+    if (cert == NULL) {
+
+        if (verify != NGX_SSL_VERIFY_REQUIRED) {
+            return NGX_OK;
+        }
+
+        ngx_ssl_remove_cached_session(ssl->ctx,
+                                      SSL_get0_session(c->ssl->connection));
+        return NGX_DECLINED;
+    }
+
+    X509_free(cert);
+
     rc = SSL_get_verify_result(c->ssl->connection);
 
     if (rc != X509_V_OK
@@ -3091,18 +3106,6 @@ ngx_ssl_verify_client(ngx_connection_t *
         return (ngx_int_t) rc;
     }
 
-    if (verify == NGX_SSL_VERIFY_REQUIRED) {
-        cert = SSL_get_peer_certificate(c->ssl->connection);
-
-        if (cert == NULL) {
-            ngx_ssl_remove_cached_session(ssl->ctx,
-                                          SSL_get0_session(c->ssl->connection));
-            return NGX_DECLINED;
-        }
-
-        X509_free(cert);
-    }
-
     return NGX_OK;
 }
 
@@ -3110,7 +3113,15 @@ ngx_ssl_verify_client(ngx_connection_t *
 ngx_int_t
 ngx_ssl_verify_host(ngx_connection_t *c, ngx_str_t *name)
 {
-    long  rc;
+    long   rc;
+    X509  *cert;
+
+    cert = SSL_get_peer_certificate(c->ssl->connection);
+    if (cert == NULL) {
+        return NGX_ERROR;
+    }
+
+    X509_free(cert);
 
     rc = SSL_get_verify_result(c->ssl->connection);
     if (rc != X509_V_OK) {
@@ -3638,22 +3649,20 @@ ngx_ssl_get_client_verify(ngx_connection
 {
     X509  *cert;
 
+    cert = SSL_get_peer_certificate(c->ssl->connection);
+    if (cert == NULL) {
+        ngx_str_set(s, "NONE");
+        return NGX_OK;
+    }
+
+    X509_free(cert);
+
     if (SSL_get_verify_result(c->ssl->connection) != X509_V_OK) {
         ngx_str_set(s, "FAILED");
         return NGX_OK;
     }
 
-    cert = SSL_get_peer_certificate(c->ssl->connection);
-
-    if (cert) {
-        ngx_str_set(s, "SUCCESS");
-
-    } else {
-        ngx_str_set(s, "NONE");
-    }
-
-    X509_free(cert);
-
+    ngx_str_set(s, "SUCCESS");
     return NGX_OK;
 }