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

Implement hostname checking against a custom CN #141

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

golddranks
Copy link

@golddranks golddranks commented Oct 15, 2019

Overview

Implements an option expect_custom_cn for TlsConnectionBuilder.

For some use cases outside of the common HTTPS usage, the server hostname checking benefits from customizability.

My main use case is due to AWS's recent announcement[1] about phasing out "path style" URLs for the S3 storage service, favoring the "virtual host" style URLs. (For example from https://s3.amazonaws.com/bucket.name/object.name to https://bucket.name.s3.amazonaws.com/object.name) The problem with this is that S3 bucket names can contain dots, but they are certified using a wildcard certificate with CN *.s3.amazonaws.com. Because wildcards don't match multiple labels, S3 buckets with dots in their names cause TLS errors.

The immediate remedy would be to use danger_accept_invalid_hostnames, but this exposes the client to MitM attacks so it's unacceptable. The subdomain namespace is guaranteed to be used for bucket names by AWS who controls the DNS s3.amazonaws.com and the bucket names, so implementing a custom check would be an acceptable solution here.

I think that NativeTLS would benefit from this additional customizability.

TODO / Points of discussion

  • Is there need to add danger_ prefix like with the other method that disable/modify standard verification?
    • Although this method can certainly be misused, I deemed the probability for that to be smaller than with the other methods so I left the prefix out, but if you think otherwise, I'm happy to add it.
  • I looked a bit into implementing custom hostname validation as a callback, but it might provide to be hard to implement in a backend-agnostic way, so I settled with this simple strategy for user to provide an "expected CN", which I believe to cover most of the actual use cases.
  • I used impl Trait in argument position, which requires Rust 1.26. If this crate is intended to compile with older compilers than that, I'm happy to change it.
  • Anything else?

[1] https://aws.amazon.com/jp/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/

@sfackler
Copy link
Owner

Why wouldn't you just pass the custom cn in the domain argument of connect?

@golddranks
Copy link
Author

golddranks commented Oct 15, 2019

Ah, shoot. The domain parameter in TlsConnector::connect is for exactly this use case? The reason why didn't realise that is that higher level wrappers that use NativeTLS for creating TLS connections don't expose that.

And indeed, the CN is ought to be connectionhost-specific, so it doesn't make as much sense as a connector argument..

So, either there is room for implementing callback-based, more customizable validation, or exposing the functionality in the higher level libraries. Hmm...

@golddranks
Copy link
Author

golddranks commented Oct 15, 2019

I mean, code like this doesn't seem to leave leeway for customization. I wonder which part of the stack would be a "correct" place to do this.

    let tls_connector = tokio_tls::TlsConnector::from(native_tls.clone());
    let http = hyper_tls::HttpsConnector::from((http, tls_connector));
    let (io, connected) = http.connect(dst).await?;

@golddranks
Copy link
Author

It seems that the correct place to inject the CN would be on the occasion of calling connect. One could add an alternative connect_custom_cn method for hyper_tls::HttpsConnector that takes an additional CN parameter. But then Hyper::Client wouldn't call that, because it uses the Connect trait.

@golddranks
Copy link
Author

golddranks commented Oct 15, 2019

@sfackler Do you think there could be common ground between the three TLS backends to implement and provide some kind callback-based custom verification API? I know that OpenSSL provides means for this, but I know nothing about the other two, and the difference between the APIs between the three could provide hard to abstract over.

FWIW, I asked in Reqwest repo about support for setting the hostname on TLS connect. I guess with Hyper you would be able to do it if you implement the Connector type for yourself. Haven't tried yet, though.

@sfackler
Copy link
Owner

It does seem like you'd want to be able to somewhere have a mapping from domain name to expected CN. I kind of think the right place for that would be up in hyper_tls, but I'm not 100% positive that's right.

@sfackler
Copy link
Owner

It also seems absurd that there isn't some more "normal" way of dealing with the AWS change than getting every TLS client to support custom CN overrides.

@golddranks
Copy link
Author

Yes, it's kinda absurd. As far as I know, the current state of affairs is this:
"Bucket Names with Dots – It is important to note that bucket names with “.” characters are perfectly valid for website hosting and other use cases. However, there are some known issues with TLS and with SSL certificates. We are hard at work on a plan to support virtual-host requests to these buckets, and will share the details well ahead of September 30, 2020."

Since I don't know when and how they will eventually be supported, and I need to write code for even new buckets with dots (naming policies etc.), I'm investigating this. Thanks for sharing your thoughts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants