empty root CA certificate causes C++ server to incorrectly accept clients

[cauthu]

hi,

i think there's a bug somewhere that is causing the c++ server to interpret an empty pem_root_certs to mean "don't verify clients" even though it's told to GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY.

here are the three scenarios: the server uses the wrong root cert, the right root cert, and an empty root cert. the client does the same thing in all cases.

(i will attach the example and an optional patch for some logging)

---

when server uses an unrelated ca cert...

expected outcome: server rejects client
actual outcome: correct

server:

ssl-helloworld$ GRPC_VERBOSITY=INFO ./greeter_server unrelated-cert.pem server.key.pem server-self-signed.cert.pem 
pem_root_certs length: 1919
I0810 09:46:01.689992524    1090 ev_epollsig_linux.c:71]     epoll engine will be using signal: 40
I0810 09:46:01.690109983    1090 server_builder.cc:269]      Synchronous server. Num CQs: 1, Min pollers: 1, Max Pollers: 2, CQ timeout (msec): 10000
I0810 09:46:01.691597364    1090 ssl_transport_security.c:573] ____ ca name [/C=AU/ST=Some-State/O=Internet Widgits Pty Ltd].
I0810 09:46:01.691607584    1090 ssl_transport_security.c:584] ____ num_roots 1.
I0810 09:46:01.691624128    1090 ssl_transport_security.c:1459] TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY!!!
Server listening on 127.0.0.1:50051
E0810 09:46:03.419989514    1093 ssl_transport_security.c:1365] verify error:num=18:self signed certificate:depth=0:/C=AU/ST=Some-State/O=Internet Widgits Pty Ltd/CN=client
E0810 09:46:03.420031706    1093 ssl_transport_security.c:925] Handshake failed with fatal error SSL_ERROR_SSL: error:14089086:SSL routines:ssl3_get_client_certificate:certificate verify failed.

client:

ssl-helloworld$ ./greeter_client server-self-signed.cert.pem client-key.pem client-self-signed-cert.pem 
14: Connect Failed
Greeter received: RPC failed
ssl-helloworld$ 

---

when server uses the correct ca cert...

expected outcome: server accepts client, and client prints hello world
actual outcome: correct

server:

ssl-helloworld$ GRPC_VERBOSITY=INFO ./greeter_server client-self-signed-cert.pem server.key.pem server-self-signed.cert.pem 
pem_root_certs length: 1968
I0810 09:47:14.952839284    1124 ev_epollsig_linux.c:71]     epoll engine will be using signal: 40
I0810 09:47:14.953103981    1124 server_builder.cc:269]      Synchronous server. Num CQs: 1, Min pollers: 1, Max Pollers: 2, CQ timeout (msec): 10000
I0810 09:47:14.957798359    1124 ssl_transport_security.c:573] ____ ca name [/C=AU/ST=Some-State/O=Internet Widgits Pty Ltd/CN=client].
I0810 09:47:14.957848062    1124 ssl_transport_security.c:584] ____ num_roots 1.
I0810 09:47:14.957885994    1124 ssl_transport_security.c:1459] TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY!!!
Server listening on 127.0.0.1:50051
I0810 09:47:18.699935934    1131 ssl_transport_security.c:1368] depth=0:/C=AU/ST=Some-State/O=Internet Widgits Pty Ltd/CN=client

client:

ssl-helloworld$ ./greeter_client server-self-signed.cert.pem client-key.pem client-self-signed-cert.pem 
Greeter received: Hello world
ssl-helloworld$ 

---

when server uses an EMPTY ca cert...

expected outcome: server rejects client
actual outcome: INCORRECT: client is accepted and prints hello world

server:

ssl-helloworld$ GRPC_VERBOSITY=INFO ./greeter_server empty.pem server.key.pem server-self-signed.cert.pem 
pem_root_certs length: 0
I0810 09:50:16.470812718    1192 ev_epollsig_linux.c:71]     epoll engine will be using signal: 40
I0810 09:50:16.471159187    1192 server_builder.cc:269]      Synchronous server. Num CQs: 1, Min pollers: 1, Max Pollers: 2, CQ timeout (msec): 10000
Server listening on 127.0.0.1:50051

client:

ssl-helloworld$ ./greeter_client server-self-signed.cert.pem client-key.pem client-self-signed-cert.pem 
Greeter received: Hello world
ssl-helloworld$ 

---

What version of gRPC and what language are you using?

latest master at

commit faef8bcc6533ac5fbc6a8efa4ff964fe897f2b5c
Merge: f20bf78 e1418e4
Author: Vijay Pai <vpai@google.com>
Date:   Thu Aug 10 07:05:19 2017 -0700

    Merge pull request #12140 from vjpai/slicer
    
    Switch QPS generic RPC to idiomatic Slice API

What operating system (Linux, Windows, …) and version?

linux, ubuntu 16.04

Linux laptop 4.10.0-30-generic #34~16.04.1-Ubuntu SMP Wed Aug 2 02:13:56 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

What runtime / compiler are you using (e.g. python version or version of gcc)

Python 2.7.12 (default, Nov 19 2016, 06:48:10) 
[GCC 5.4.0 20160609] on linux2

[cauthu]

1. the cpp/ssl-helloworld example, including the keys/certs (client and server both use self-signed certs)
   ssl-helloworld.zip

2. the patch to ssl_transport_security.c just for some logging
   0001-some-logging-in-cert-verification.patch.txt

[cauthu]

for convenience, here's the relevant code in the server

  auto client_ca_pem = get_file_contents(argv[1]); // for verifying clients
  auto my_key_pem = get_file_contents(argv[2]);
  auto my_cert_pem = get_file_contents(argv[3]);

  grpc::SslServerCredentialsOptions::PemKeyCertPair pkcp = {
    my_key_pem.c_str(), my_cert_pem.c_str()
  };

  grpc::SslServerCredentialsOptions ssl_opts(GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY);
  ssl_opts.pem_root_certs = client_ca_pem;
  ssl_opts.pem_key_cert_pairs.push_back(pkcp);
  std::cout << "pem_root_certs length: " << ssl_opts.pem_root_certs.length() << std::endl;

  std::shared_ptr<grpc::ServerCredentials> creds = grpc::SslServerCredentials(ssl_opts);

  ServerBuilder builder;
  // Listen on the given address without any authentication mechanism.
  builder.AddListeningPort(server_address, creds);

and the client

  auto server_ca_pem = get_file_contents(argv[1]);
  auto my_key_pem = get_file_contents(argv[2]);
  auto my_cert_pem = get_file_contents(argv[3]);

  grpc::SslCredentialsOptions ssl_opts;
  ssl_opts.pem_root_certs = server_ca_pem;
  ssl_opts.pem_private_key = my_key_pem;
  ssl_opts.pem_cert_chain = my_cert_pem;

  std::shared_ptr<grpc::ChannelCredentials> creds = grpc::SslCredentials(ssl_opts);

  GreeterClient greeter(grpc::CreateChannel(
      "localhost:50051", creds));
  std::string user("world");
  std::string reply = greeter.SayHello(user);
  std::cout << "Greeter received: " << reply << std::endl;

[cauthu]

the python server is behaving correctly when given the empty ca certificate: it rejects the client.
in particular, it is being clear reporting the error when loading the empty ca certificate.

server:

ssl-helloworld$ GRPC_VERBOSITY=INFO python ./greeter_server.py empty.pem server.key.pem server-self-signed.cert.pem 
I0810 11:13:46.816352080    6010 ev_epollsig_linux.c:71]     epoll engine will be using signal: 40
client_ca_pem length: 0
E0810 11:13:46.824827204    6010 ssl_transport_security.c:588] Could not load any root certificate.
E0810 11:13:46.824862054    6010 ssl_transport_security.c:1437] Invalid verification certs.
E0810 11:13:46.824878103    6010 security_connector.c:902]   Handshaker factory creation failed with TSI_INVALID_ARGUMENT.
E0810 11:13:46.824903644    6010 server_secure_chttp2.c:81]  {"created":"@1502388826.824890226","description":"Unable to create secure server with credentials of type Ssl.","file":"src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c","file_line":60,"security_status":1}

client:

ssl-helloworld$ python ./greeter_client.py server-self-signed.cert.pem client-key.pem client-self-signed-cert.pem 
Traceback (most recent call last):
  File "./greeter_client.py", line 43, in <module>
    run()
  File "./greeter_client.py", line 38, in run
    response = stub.SayHello(helloworld_pb2.HelloRequest(name='you'))
  File "/home/cauthu/.local/lib/python2.7/site-packages/grpc/_channel.py", line 492, in __call__
    return _end_unary_response_blocking(state, call, False, deadline)
  File "/home/cauthu/.local/lib/python2.7/site-packages/grpc/_channel.py", line 440, in _end_unary_response_blocking
    raise _Rendezvous(state, None, None, deadline)
grpc._channel._Rendezvous: <_Rendezvous of RPC that terminated with (StatusCode.UNAVAILABLE, Connect Failed)>
ssl-helloworld$ 

---

server code

from concurrent import futures
import time

import grpc

import helloworld_pb2
import helloworld_pb2_grpc

_ONE_DAY_IN_SECONDS = 60 * 60 * 24


class Greeter(helloworld_pb2_grpc.GreeterServicer):

  def SayHello(self, request, context):
    return helloworld_pb2.HelloReply(message='Hello, %s!' % request.name)


import sys

client_ca_pem = open(sys.argv[1]).read() # for verifying clients
my_key_pem = open(sys.argv[2]).read()
my_cert_pem = open(sys.argv[3]).read()

def serve():
  server_credentials = grpc.ssl_server_credentials(
      [(my_key_pem, my_cert_pem)],
      require_client_auth=True,
      root_certificates=client_ca_pem, # for verifying clients
  )

  print("client_ca_pem length: {}".format(len(client_ca_pem)))

  server = grpc.server(futures.ThreadPoolExecutor(max_workers=10))
  helloworld_pb2_grpc.add_GreeterServicer_to_server(Greeter(), server)
  server.add_secure_port('[::]:50051', server_credentials)
  server.start()
  try:
    while True:
      time.sleep(_ONE_DAY_IN_SECONDS)
  except KeyboardInterrupt:
    server.stop(0)

if __name__ == '__main__':
  serve()

client code

from __future__ import print_function

import grpc

import helloworld_pb2
import helloworld_pb2_grpc

import sys

server_ca_pem = open(sys.argv[1]).read()
my_key_pem = open(sys.argv[2]).read()
my_cert_pem = open(sys.argv[3]).read()

def run():
  credentials = grpc.ssl_channel_credentials(
    private_key=my_key_pem, certificate_chain=my_cert_pem,
    root_certificates=server_ca_pem,
  )
  
  channel = grpc.secure_channel('localhost:50051', credentials)
  stub = helloworld_pb2_grpc.GreeterStub(channel)
  response = stub.SayHello(helloworld_pb2.HelloRequest(name='you'))
  print("Greeter client received: " + response.message)


if __name__ == '__main__':
  run()

[jboeuf]

@deepaklukose would you mind having a look at the issue? Thanks!

[deepaklukose]

@jboeuf https://github.com/grpc/grpc/blob/master/src/core/tsi/ssl_transport_security.h#L144 does indeed claim that the pem_client_root_certs needs to be non-null for the server to validate the client certs. Let me update the code to get rid of this since rejecting the client seems like the safer option.

[jboeuf]

Thanks much Deepak!

…

On Thu, Aug 10, 2017 at 10:21 PM, deepaklukose ***@***.***> wrote: @jboeuf <https://github.com/jboeuf> https://github.com/grpc/grpc/ blob/master/src/core/tsi/ssl_transport_security.h#L144 does indeed claim that the pem_client_root_certs needs to be non-null for the server to validate the client certs. Let me update the code to get rid of this since rejecting the client seems like the safer option. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12146 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF-P7Znx9AnmI5JaG0RFyOm__mn7vnnBks5sW-TTgaJpZM4Ozw3g> .

[cauthu]

That'd be great! Thanks, Deepak! (sent from phone) On Aug 10, 2017 10:24 PM, "jboeuf" <notifications@github.com> wrote: Thanks much Deepak!

On Thu, Aug 10, 2017 at 10:21 PM, deepaklukose ***@***.***> wrote: @jboeuf <https://github.com/jboeuf> https://github.com/grpc/grpc/ blob/master/src/core/tsi/ssl_transport_security.h#L144 does indeed claim that the pem_client_root_certs needs to be non-null for the server to validate the client certs. Let me update the code to get rid of this since rejecting the client seems like the safer option. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12146 (comment)>, or

mute

the thread <https://github.com/notifications/unsubscribe-

auth/AF-P7Znx9AnmI5JaG0RFyOm__mn7vnnBks5sW-TTgaJpZM4Ozw3g>

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#12146 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABHdpKgnC_38OfavEdWU2B20UZlhhgVOks5sW-WEgaJpZM4Ozw3g> .

[kboyapa1]

What is the status of this issue? is this closed? if so, Can please share the commit ID in which the issue is fixed?

We are also facing similar issue in python when used grpc.dynamic_ssl_server_credentials() function to update the server credentials dynamically for hitless certificate rotation. The scenario is,

1. The server is started with valid certificate and client can connect with the server.
2. We deleted the roots.pem from the server.
3. Now attempted a new client connection with the same crt details as used in step-1. The client connection is accepted eventhough roots.pem is deleted.

grpc.dynamic_ssl_server_credentials() api notes says that
certificate_configuration_fetcher (callable) – A callable that takes no arguments and should return a ServerCertificateConfiguration to replace the server’s current certificate, or None for no change (i.e., the server will continue its current certificate config). The library will call this callback on every new client connection before starting the TLS handshake with the client, thus allowing the user application to optionally return a new ServerCertificateConfiguration that the server will then use for the handshake.

[deepaklukose]

The fix has not been committed. The proposed fix (#12193) needs a lot more work to rebase for which I unfortunately don't have the time right now.

[cauthu]

1. The server is started with valid certificate and client can connect with the server.
2. We deleted the roots.pem from the server.
3. Now attempted a new client connection with the same crt details as used in step-1

@kboyapa1 so i assume you want the server to reject all new clients after you delete the roots.pem? while i understand that technically, that is the behavior promised by the grpc.dynamic_ssl_server_credentials() api, i dont know why that would be useful in real life? is it a scenario where you want to essentially temporarily "take a server offline" but instead of shutting down the service/cutting off network, you want to keep the server running, so that it can continue serving existing clients and also avoid the subsequent server startup cost, and in the mean time just rejects new clients? then later you "bring the server back up" by simply restoring the roots.pem?

[kboyapa1]

@cauthu Your understanding is correct. I am expecting server should reject all new client connections when roots.pem is deleted.

We are looking for hitless server certificate rotation. The existing client connections should not interrupted due to new certificates installation. All the new client connections should use new certificates to connect to the server.

using grpc.dynamic_ssl_server_credentials() api,

Scenario-1: (Working)

1. Server started with cert-1(private_key_certificate_chain_pairs + roots.pem) and client session-1 with cert-1 establisged.
2. Installed new certificate cert-2 (with different private_key_certificate_chain_pairs + roots.pem). The client session-2 with cert-1 should not connect. Its not connecting. The client session-1 should not terminate. (Working)
3. Client session-2 with cert-2 will connect to Server. Client session-1 also continues to work.

Scenario-2 (Not Working):

1. Server started with cert-1(private_key_certificate_chain_pairs + roots.pem) and client session-1 with cert-1 establisged.
2. Remove the roots.pem while server is running. The client session-2 with cert-1 should not connect because roots.pem is deleted. But client session-2 with cert-1 is connecting. (Not Working).
   The reason for this is, grpc.dynamic_ssl_server_credentials() returns None and gRPC server continues to use previously loaded certificate. So clients with old certificates still connect to server.

Scenario-3 (Not Working):

1. Server started with cert-1(private_key_certificate_chain_pairs + roots.pem) and client session-1 with cert-1 establisged.
2. Install new certificate cert-2 but invalid private_key_certificate_chain_pairs. The client session-2 with cert-1 should not connect because of invalid chain_pairs. But client session-2 with cert-1 is connecting. (Not Working).
   Seeing error message "security_connector.cc:692] Failed fetching new server credentials, continuing to use previously-loaded credentials."

[jiangtaoli2016]

@cauthu This should be fixed. Let me know if problem still exists. Thanks for patience.

[cauthu]

Ah cool, thanks Jiangtao.

…

On Fri, Dec 14, 2018, 8:37 AM Jiangtao Li ***@***.*** wrote: @cauthu <https://github.com/cauthu> This should be fixed. Let me know if problem still exists. Thanks for patience. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12146 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABHdpPOqH0UipDwvfSkbSec18_rQwcj_ks5u49PlgaJpZM4Ozw3g> .