Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide API to pass TLS / Certificate issues to C# code #16480

Closed
mkvonarx opened this issue Aug 28, 2018 · 8 comments
Closed

Provide API to pass TLS / Certificate issues to C# code #16480

mkvonarx opened this issue Aug 28, 2018 · 8 comments

Comments

@mkvonarx
Copy link

What version of gRPC and what language are you using?

gRPC version 1.14.1
C# on .NET 4.7.1 on Windows, C# on Mono 5.14.0.177 on CentOS 7

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

Windows & Linux (CentOS)

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

.NET 4.7.1
Mono 5.14.0.177

What did you do?

Test various error scenarios where gRPC calls using TLS / X.509 will fail.

What did you expect to see?

gRPC library telling my code what has failed and why.

What did you see instead?

  • Nothing at all on the gRPC server.
  • Misleading RpcException on the gRPC client.

Anything else we should know about your project / environment?

Problem:

  • When something goes wrong with TLS and/or X.509 certificates, gRPC won't notify C# code, neither on the server nor on the client side. These TLS / certificate issues are only visible via the gRPC logs (via Grpc.Core.Logging.ILogger). This lack of information in my code makes error handling, monitoring and troubleshooting very difficult.

Solution (Feature Request):

  • Add an API to client and server code through which gRPC can notify code using gRPC of TLS and certificate issues.
  • On the client side, this might be more information inside the RpcException.

Examples:

  • error case: corrupt server certificate file (e.g. flipped byte):
    • server: no indication at all
    • client: RpcException with Status(StatusCode=Unavailable, Detail="Connect Failed"), log "Handshake failed with fatal error SSL_ERROR_SSL: error:0400006b:RSA routines:OPENSSL_internal:BLOCK_TYPE_IS_NOT_01."
  • error case: server has incorrect certificate that is issued for another server (no matching CN / SubjectAlternativeName)
    • server: log "No match found for server name: aaaa."
    • client: RpcException with Status(StatusCode=Unavailable, Detail="Connect Failed").
  • error case: client & server use different root CA
    • server: no indication at all
    • client: RpcException with Status(StatusCode=Unavailable, Detail="Connect Failed"), log "Handshake failed with fatal error SSL_ERROR_SSL: error:1000007d:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED."
  • error case: insecure client (without TLS) calls secure server (with TLS)
    • server: log "Handshake failed with fatal error SSL_ERROR_SSL: error:100000f7:SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER."
    • client: RpcException with Status(StatusCode=Unknown, Detail="Stream removed")
  • error case: secure client (with TLS) calls insecure server (without TLS)
    • server: no indication at all
    • client: RpcException with Status(StatusCode=Unavailable, Detail="Connect Failed"), log "Handshake failed with fatal error SSL_ERROR_SSL: error:100000f7:SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER."
@clehene
Copy link

clehene commented Mar 7, 2019

Dealing with gRPC errors in C# is extremely confusing. This is a well documented example.

@stale
Copy link

stale bot commented Sep 4, 2019

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 180 days. It will be closed automatically if no further update occurs in 1 day. Thank you for your contributions!

@jtattermusch
Copy link
Contributor

You can enable extra logging (which will likely give you some indication of what went wrong) by following https://github.com/grpc/grpc/blob/master/TROUBLESHOOTING.md

@stale stale bot removed the disposition/stale label Sep 4, 2019
@mkvonarx
Copy link
Author

mkvonarx commented Sep 5, 2019

While extra logging is certainly interesting during development and when getting logs from production environments, logs do not help my code to tell the user (via UI, SNMP, ...) about what went wrong.

@jtattermusch
Copy link
Contributor

jtattermusch commented Sep 9, 2019

@yihuazhang any ideas how we could better surface the TLS problems to wrapped languages? I agree the current error messages are cryptic (see examples above)

@yihuazhang
Copy link
Contributor

I originally thought we could surface TLS related error messages from TSI layer all the way to gRPC users, but it turns out requiring changes to TSI API's (tsi_handshaker_next) as the current TSI API's are not designed to surface error messages to its callers (only enum tsi_result status is returned). So without changing the current API's, I prefer to add new API's returning corresponding TLS error messages as suggested by @mkvonarx.

@markdroth @jiangtaoli2016 HDYT?

@markdroth
Copy link
Member

I think it would be better to change the TSI APIs to return real errors. To be honest, I have always felt that tsi_result was far too simplistic of a mechanism. I think that we should instead use something more like grpc_error, or maybe something more like grpc::Status.

@stale
Copy link

stale bot commented Apr 12, 2020

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 180 days. It will be closed automatically if no further update occurs in 1 day. Thank you for your contributions!

@stale stale bot closed this as completed Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants