Merge pull request #5958 from deepaklukose/skip_client_cert_verification

Add various options to verify ssl/tls client cert including letting the application handle the authentication.

6 files changed, 129 insertions, 14 deletions

diff --git a/src/core/lib/security/credentials.c b/src/core/lib/security/credentials.c
index 2c7d31519c..fd5ad3589b 100644
--- a/src/core/lib/security/credentials.c
+++ b/src/core/lib/security/credentials.c
@@ -338,10 +338,11 @@ static void ssl_build_config(const char *pem_root_certs,
 
 static void ssl_build_server_config(
     const char *pem_root_certs, grpc_ssl_pem_key_cert_pair *pem_key_cert_pairs,
-    size_t num_key_cert_pairs, int force_client_auth,
+    size_t num_key_cert_pairs,
+    grpc_ssl_client_certificate_request_type client_certificate_request,
     grpc_ssl_server_config *config) {
   size_t i;
-  config->force_client_auth = force_client_auth;
+  config->client_certificate_request = client_certificate_request;
   if (pem_root_certs != NULL) {
     ssl_copy_key_material(pem_root_certs, &config->pem_root_certs,
                           &config->pem_root_certs_size);
@@ -391,21 +392,35 @@ grpc_channel_credentials *grpc_ssl_credentials_create(
 grpc_server_credentials *grpc_ssl_server_credentials_create(
     const char *pem_root_certs, grpc_ssl_pem_key_cert_pair *pem_key_cert_pairs,
     size_t num_key_cert_pairs, int force_client_auth, void *reserved) {
+  return grpc_ssl_server_credentials_create_ex(
+      pem_root_certs, pem_key_cert_pairs, num_key_cert_pairs,
+      force_client_auth
+          ? GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY
+          : GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE,
+      reserved);
+}
+
+grpc_server_credentials *grpc_ssl_server_credentials_create_ex(
+    const char *pem_root_certs, grpc_ssl_pem_key_cert_pair *pem_key_cert_pairs,
+    size_t num_key_cert_pairs,
+    grpc_ssl_client_certificate_request_type client_certificate_request,
+    void *reserved) {
   grpc_ssl_server_credentials *c =
       gpr_malloc(sizeof(grpc_ssl_server_credentials));
   GRPC_API_TRACE(
-      "grpc_ssl_server_credentials_create("
+      "grpc_ssl_server_credentials_create_ex("
       "pem_root_certs=%s, pem_key_cert_pairs=%p, num_key_cert_pairs=%lu, "
-      "force_client_auth=%d, reserved=%p)",
+      "client_certificate_request=%d, reserved=%p)",
       5, (pem_root_certs, pem_key_cert_pairs, (unsigned long)num_key_cert_pairs,
-          force_client_auth, reserved));
+          client_certificate_request, reserved));
   GPR_ASSERT(reserved == NULL);
   memset(c, 0, sizeof(grpc_ssl_server_credentials));
   c->base.type = GRPC_CHANNEL_CREDENTIALS_TYPE_SSL;
   gpr_ref_init(&c->base.refcount, 1);
   c->base.vtable = &ssl_server_vtable;
   ssl_build_server_config(pem_root_certs, pem_key_cert_pairs,
-                          num_key_cert_pairs, force_client_auth, &c->config);
+                          num_key_cert_pairs, client_certificate_request,
+                          &c->config);
   return &c->base;
 }
 
diff --git a/src/core/lib/security/security_connector.c b/src/core/lib/security/security_connector.c
index 59863ba064..2d2023bdf5 100644
--- a/src/core/lib/security/security_connector.c
+++ b/src/core/lib/security/security_connector.c
@@ -668,6 +668,31 @@ gpr_slice grpc_get_default_ssl_roots_for_testing(void) {
   return compute_default_pem_root_certs_once();
 }
 
+static tsi_client_certificate_request_type
+get_tsi_client_certificate_request_type(
+    grpc_ssl_client_certificate_request_type grpc_request_type) {
+  switch (grpc_request_type) {
+    case GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE:
+      return TSI_DONT_REQUEST_CLIENT_CERTIFICATE;
+
+    case GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_BUT_DONT_VERIFY:
+      return TSI_REQUEST_CLIENT_CERTIFICATE_BUT_DONT_VERIFY;
+
+    case GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY:
+      return TSI_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY;
+
+    case GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_BUT_DONT_VERIFY:
+      return TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_BUT_DONT_VERIFY;
+
+    case GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY:
+      return TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY;
+
+    default:
+      // Is this a sane default
+      return TSI_DONT_REQUEST_CLIENT_CERTIFICATE;
+  }
+}
+
 size_t grpc_get_default_ssl_roots(const unsigned char **pem_root_certs) {
   /* TODO(jboeuf@google.com): Maybe revisit the approach which consists in
      loading all the roots once for the lifetime of the process. */
@@ -782,15 +807,16 @@ grpc_security_status grpc_ssl_server_security_connector_create(
   gpr_ref_init(&c->base.base.refcount, 1);
   c->base.base.url_scheme = GRPC_SSL_URL_SCHEME;
   c->base.base.vtable = &ssl_server_vtable;
-  result = tsi_create_ssl_server_handshaker_factory(
+  result = tsi_create_ssl_server_handshaker_factory_ex(
       (const unsigned char **)config->pem_private_keys,
       config->pem_private_keys_sizes,
       (const unsigned char **)config->pem_cert_chains,
       config->pem_cert_chains_sizes, config->num_key_cert_pairs,
       config->pem_root_certs, config->pem_root_certs_size,
-      config->force_client_auth, ssl_cipher_suites(), alpn_protocol_strings,
-      alpn_protocol_string_lengths, (uint16_t)num_alpn_protocols,
-      &c->handshaker_factory);
+      get_tsi_client_certificate_request_type(
+          config->client_certificate_request),
+      ssl_cipher_suites(), alpn_protocol_strings, alpn_protocol_string_lengths,
+      (uint16_t)num_alpn_protocols, &c->handshaker_factory);
   if (result != TSI_OK) {
     gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
             tsi_result_to_string(result));
diff --git a/src/core/lib/security/security_connector.h b/src/core/lib/security/security_connector.h
index c9e262b1ad..2c893cd5e9 100644
--- a/src/core/lib/security/security_connector.h
+++ b/src/core/lib/security/security_connector.h
@@ -241,7 +241,7 @@ typedef struct {
   size_t num_key_cert_pairs;
   unsigned char *pem_root_certs;
   size_t pem_root_certs_size;
-  int force_client_auth;
+  grpc_ssl_client_certificate_request_type client_certificate_request;
 } grpc_ssl_server_config;
 
 /* Creates an SSL server_security_connector.
diff --git a/src/core/lib/tsi/ssl_transport_security.c b/src/core/lib/tsi/ssl_transport_security.c
index 045901cc72..e91c6316e7 100644
--- a/src/core/lib/tsi/ssl_transport_security.c
+++ b/src/core/lib/tsi/ssl_transport_security.c
@@ -718,6 +718,14 @@ static tsi_result build_alpn_protocol_name_list(
   return TSI_OK;
 }
 
+// The verification callback is used for clients that don't really care about
+// the server's certificate, but we need to pull it anyway, in case a higher
+// layer wants to look at it. In this case the verification may fail, but
+// we don't really care.
+static int NullVerifyCallback(int preverify_ok, X509_STORE_CTX *ctx) {
+  return 1;
+}
+
 /* --- tsi_frame_protector methods implementation. ---*/
 
 static tsi_result ssl_protector_protect(tsi_frame_protector *self,
@@ -1390,6 +1398,26 @@ tsi_result tsi_create_ssl_server_handshaker_factory(
     const char *cipher_list, const unsigned char **alpn_protocols,
     const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols,
     tsi_ssl_handshaker_factory **factory) {
+  return tsi_create_ssl_server_handshaker_factory_ex(
+      pem_private_keys, pem_private_keys_sizes, pem_cert_chains,
+      pem_cert_chains_sizes, key_cert_pair_count, pem_client_root_certs,
+      pem_client_root_certs_size,
+      force_client_auth ? TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY
+                        : TSI_DONT_REQUEST_CLIENT_CERTIFICATE,
+      cipher_list, alpn_protocols, alpn_protocols_lengths, num_alpn_protocols,
+      factory);
+}
+
+tsi_result tsi_create_ssl_server_handshaker_factory_ex(
+    const unsigned char **pem_private_keys,
+    const size_t *pem_private_keys_sizes, const unsigned char **pem_cert_chains,
+    const size_t *pem_cert_chains_sizes, size_t key_cert_pair_count,
+    const unsigned char *pem_client_root_certs,
+    size_t pem_client_root_certs_size,
+    tsi_client_certificate_request_type client_certificate_request,
+    const char *cipher_list, const unsigned char **alpn_protocols,
+    const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols,
+    tsi_ssl_handshaker_factory **factory) {
   tsi_ssl_server_handshaker_factory *impl = NULL;
   tsi_result result = TSI_OK;
   size_t i = 0;
@@ -1445,7 +1473,6 @@ tsi_result tsi_create_ssl_server_handshaker_factory(
       if (result != TSI_OK) break;
 
       if (pem_client_root_certs != NULL) {
-        int flags = SSL_VERIFY_PEER;
         STACK_OF(X509_NAME) *root_names = NULL;
         result = ssl_ctx_load_verification_certs(
             impl->ssl_contexts[i], pem_client_root_certs,
@@ -1455,8 +1482,29 @@ tsi_result tsi_create_ssl_server_handshaker_factory(
           break;
         }
         SSL_CTX_set_client_CA_list(impl->ssl_contexts[i], root_names);
-        if (force_client_auth) flags |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
-        SSL_CTX_set_verify(impl->ssl_contexts[i], flags, NULL);
+        switch (client_certificate_request) {
+          case TSI_DONT_REQUEST_CLIENT_CERTIFICATE:
+            SSL_CTX_set_verify(impl->ssl_contexts[i], SSL_VERIFY_NONE, NULL);
+            break;
+          case TSI_REQUEST_CLIENT_CERTIFICATE_BUT_DONT_VERIFY:
+            SSL_CTX_set_verify(impl->ssl_contexts[i], SSL_VERIFY_PEER,
+                               NullVerifyCallback);
+            break;
+          case TSI_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY:
+            SSL_CTX_set_verify(impl->ssl_contexts[i], SSL_VERIFY_PEER, NULL);
+            break;
+          case TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_BUT_DONT_VERIFY:
+            SSL_CTX_set_verify(
+                impl->ssl_contexts[i],
+                SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
+                NullVerifyCallback);
+            break;
+          case TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY:
+            SSL_CTX_set_verify(
+                impl->ssl_contexts[i],
+                SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL);
+            break;
+        }
         /* TODO(jboeuf): Add revocation verification. */
       }
 
diff --git a/src/core/lib/tsi/ssl_transport_security.h b/src/core/lib/tsi/ssl_transport_security.h
index 211c8f9656..7407246118 100644
--- a/src/core/lib/tsi/ssl_transport_security.h
+++ b/src/core/lib/tsi/ssl_transport_security.h
@@ -142,6 +142,23 @@ tsi_result tsi_create_ssl_server_handshaker_factory(
     const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols,
     tsi_ssl_handshaker_factory **factory);
 
+/* Same as tsi_create_ssl_server_handshaker_factory method except uses
+   tsi_client_certificate_request_type to support more ways to handle client
+   certificate authentication.
+   - client_certificate_request, if set to non-zero will force the client to
+     authenticate with an SSL cert. Note that this option is ignored if
+     pem_client_root_certs is NULL or pem_client_roots_certs_size is 0 */
+tsi_result tsi_create_ssl_server_handshaker_factory_ex(
+    const unsigned char **pem_private_keys,
+    const size_t *pem_private_keys_sizes, const unsigned char **pem_cert_chains,
+    const size_t *pem_cert_chains_sizes, size_t key_cert_pair_count,
+    const unsigned char *pem_client_root_certs,
+    size_t pem_client_root_certs_size,
+    tsi_client_certificate_request_type client_certificate_request,
+    const char *cipher_suites, const unsigned char **alpn_protocols,
+    const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols,
+    tsi_ssl_handshaker_factory **factory);
+
 /* Creates a handshaker.
   - self is the factory from which the handshaker will be created.
   - server_name_indication indicates the name of the server the client is
diff --git a/src/core/lib/tsi/transport_security_interface.h b/src/core/lib/tsi/transport_security_interface.h
index d81ec0963a..3e8c9d7ffe 100644
--- a/src/core/lib/tsi/transport_security_interface.h
+++ b/src/core/lib/tsi/transport_security_interface.h
@@ -59,6 +59,15 @@ typedef enum {
   TSI_OUT_OF_RESOURCES = 12
 } tsi_result;
 
+typedef enum {
+  // Default option
+  TSI_DONT_REQUEST_CLIENT_CERTIFICATE,
+  TSI_REQUEST_CLIENT_CERTIFICATE_BUT_DONT_VERIFY,
+  TSI_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY,
+  TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_BUT_DONT_VERIFY,
+  TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY,
+} tsi_client_certificate_request_type;
+
 const char *tsi_result_to_string(tsi_result result);
 
 /* --- tsi tracing --- */