search

akka / akka-grpc

A platform to build and run apps that are elastic, agile, and resilient. SDK, libraries, and hosted environments.
https://doc.akka.io/libraries/akka-grpc/current/
Other
431 stars 124 forks source link

Improve client TLS configuration API #397

Open ignasi35 opened 6 years ago

ignasi35 commented 6 years ago

It is possible to build a (arguably) malformed GrpcSettings if useTls is set to true but no SSLContext is provided:

https://github.com/akka/akka-grpc/blob/83965cfe9c32644e174a9e4eecfcc60ad66bf24d/runtime/src/main/scala/akka/grpc/internal/NettyClientUtils.scala#L45-L53

The user expects TLS to be used and a fallback to PLAINTEXT happens instead.

raboof commented 6 years ago

I think the rationale behind having a useTls flag and an Option[SSLContext] parameter was that it might be sensible to have useTls without a specific Option[SSLContext] use the system-wide SSL configuration.

This would indeed suggest the fallback to PLAINTEXT here is wrong.

This still allows having useTls set to false but the Option[SSLContext] filled. It might make things more safe and readable to roll those into 1 field allowing either 'no tls', 'tls with default settings' and 'tls with specific settings'. Another option would be to only have the Option[SSLContext] and provide an easy way to construct an SSLContext for the system-wide defaults. We'd have to experiment if that leads to any rough edges, though.

ignasi35 commented 6 years ago

I just noticed the override-authority may only make sense when TLS is desired, in which case we could also consider that field into the mix.