Verifying the peer name on the X509 Certs correctly.

- The SANs take precedence over the CN.
- The CN is only checked if there are no SANs.
- Fixing the tests as the test cert did not list *.test.google.com in
  the SANs. Will fix the test cert another time...

9 files changed, 28 insertions, 20 deletions

diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c
index 92fcb96dd2..f1a52746d2 100644
--- a/src/core/tsi/ssl_transport_security.c
+++ b/src/core/tsi/ssl_transport_security.c
@@ -1282,25 +1282,18 @@ tsi_result tsi_create_ssl_server_handshaker_factory(
 
 int tsi_ssl_peer_matches_name(const tsi_peer* peer, const char* name) {
   size_t i = 0;
-  const tsi_peer_property* property = tsi_peer_get_property_by_name(
-      peer, TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY);
-  if (property == NULL || property->type != TSI_PEER_PROPERTY_TYPE_STRING) {
-    gpr_log(GPR_ERROR, "Invalid x509 subject common name property.");
-    return 0;
-  }
-  if (does_entry_match_name(property->value.string.data,
-                            property->value.string.length, name)) {
-    return 1;
-  }
+  size_t san_count = 0;
 
-  property = tsi_peer_get_property_by_name(
+  /* Check the SAN first. */
+  const tsi_peer_property* property = tsi_peer_get_property_by_name(
       peer, TSI_X509_SUBJECT_ALTERNATIVE_NAMES_PEER_PROPERTY);
   if (property == NULL || property->type != TSI_PEER_PROPERTY_TYPE_LIST) {
     gpr_log(GPR_ERROR, "Invalid x509 subject alternative names property.");
     return 0;
   }
 
-  for (i = 0; i < property->value.list.child_count; i++) {
+  san_count = property->value.list.child_count;
+  for (i = 0; i < san_count; i++) {
     const tsi_peer_property* alt_name_property =
         &property->value.list.children[i];
     if (alt_name_property->type != TSI_PEER_PROPERTY_TYPE_STRING) {
@@ -1312,5 +1305,20 @@ int tsi_ssl_peer_matches_name(const tsi_peer* peer, const char* name) {
       return 1;
     }
   }
+
+  /* If there's no SAN, try the CN. */
+  if (san_count == 0) {
+    property = tsi_peer_get_property_by_name(
+        peer, TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY);
+    if (property == NULL || property->type != TSI_PEER_PROPERTY_TYPE_STRING) {
+      gpr_log(GPR_ERROR, "Invalid x509 subject common name property.");
+      return 0;
+    }
+    if (does_entry_match_name(property->value.string.data,
+                              property->value.string.length, name)) {
+      return 1;
+    }
+  }
+
   return 0; /* Not found. */
 }
diff --git a/src/node/test/interop_sanity_test.js b/src/node/test/interop_sanity_test.js
index 8dc933eac5..6b3aa3dd84 100644
--- a/src/node/test/interop_sanity_test.js
+++ b/src/node/test/interop_sanity_test.js
@@ -40,7 +40,7 @@ var server;
 
 var port;
 
-var name_override = 'foo.test.google.com';
+var name_override = 'foo.test.google.fr';
 
 describe('Interop tests', function() {
   before(function(done) {
diff --git a/src/php/tests/interop/interop_client.php b/src/php/tests/interop/interop_client.php
index 5a09fc7d78..82ca438169 100755
--- a/src/php/tests/interop/interop_client.php
+++ b/src/php/tests/interop/interop_client.php
@@ -215,7 +215,7 @@ $stub = new grpc\testing\TestServiceClient(
     new Grpc\BaseStub(
         $server_address,
         [
-            'grpc.ssl_target_name_override' => 'foo.test.google.com',
+            'grpc.ssl_target_name_override' => 'foo.test.google.fr',
             'credentials' => $credentials
          ]));
 
diff --git a/src/php/tests/unit_tests/SecureEndToEndTest.php b/src/php/tests/unit_tests/SecureEndToEndTest.php
index b19ac80ddd..c23dd791ac 100755
--- a/src/php/tests/unit_tests/SecureEndToEndTest.php
+++ b/src/php/tests/unit_tests/SecureEndToEndTest.php
@@ -47,7 +47,7 @@ class SecureEndToEndTest extends PHPUnit_Framework_TestCase{
     $this->channel = new Grpc\Channel(
         'localhost:' . $port,
         [
-            'grpc.ssl_target_name_override' => 'foo.test.google.com',
+            'grpc.ssl_target_name_override' => 'foo.test.google.fr',
             'credentials' => $credentials
          ]);
   }
diff --git a/src/ruby/bin/interop/interop_client.rb b/src/ruby/bin/interop/interop_client.rb
index 76402b7c89..380ceb11df 100755
--- a/src/ruby/bin/interop/interop_client.rb
+++ b/src/ruby/bin/interop/interop_client.rb
@@ -291,7 +291,7 @@ Args = Struct.new(:default_service_account, :host, :host_override,
 # validates the the command line options, returning them as a Hash.
 def parse_args
   args = Args.new
-  args.host_override = 'foo.test.google.com'
+  args.host_override = 'foo.test.google.fr'
   OptionParser.new do |opts|
     opts.on('--oauth_scope scope',
             'Scope for OAuth tokens') { |v| args['oauth_scope'] = v }
diff --git a/src/ruby/bin/math_client.rb b/src/ruby/bin/math_client.rb
index cb085d4d42..db254efb00 100755
--- a/src/ruby/bin/math_client.rb
+++ b/src/ruby/bin/math_client.rb
@@ -127,7 +127,7 @@ def main
   if options['secure']
     stub_opts = {
       :creds => test_creds,
-      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com'
+      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.fr'
     }
     p stub_opts
     p options['host']
diff --git a/src/ruby/bin/noproto_client.rb b/src/ruby/bin/noproto_client.rb
index 44710520d2..f3fd110347 100755
--- a/src/ruby/bin/noproto_client.rb
+++ b/src/ruby/bin/noproto_client.rb
@@ -89,7 +89,7 @@ def main
   if options['secure']
     stub_opts = {
       :creds => test_creds,
-      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com'
+      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.fr'
     }
     p stub_opts
     p options['host']
diff --git a/src/ruby/spec/client_server_spec.rb b/src/ruby/spec/client_server_spec.rb
index 52c985786a..030ff328f2 100644
--- a/src/ruby/spec/client_server_spec.rb
+++ b/src/ruby/spec/client_server_spec.rb
@@ -353,7 +353,7 @@ describe 'the secure http client/server' do
     @server = GRPC::Core::Server.new(@server_queue, nil, server_creds)
     server_port = @server.add_http2_port(server_host, true)
     @server.start
-    args = { Channel::SSL_TARGET => 'foo.test.google.com' }
+    args = { Channel::SSL_TARGET => 'foo.test.google.fr' }
     @ch = Channel.new("0.0.0.0:#{server_port}", args,
                       GRPC::Core::Credentials.new(certs[0], nil, nil))
   end
diff --git a/src/ruby/spec/generic/client_stub_spec.rb b/src/ruby/spec/generic/client_stub_spec.rb
index 297a133831..adf354f4ee 100644
--- a/src/ruby/spec/generic/client_stub_spec.rb
+++ b/src/ruby/spec/generic/client_stub_spec.rb
@@ -116,7 +116,7 @@ describe 'ClientStub' do
       host = FAKE_HOST
       blk = proc do
         opts = {
-          GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com',
+          GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.fr',
           a_channel_arg: 'an_arg',
           creds: GRPC::Core::Credentials.new(certs[0], nil, nil)
         }