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

rustls requires hostname for server #206

Closed
ayende opened this issue Jan 1, 2019 · 8 comments
Closed

rustls requires hostname for server #206

ayende opened this issue Jan 1, 2019 · 8 comments

Comments

@ayende
Copy link

ayende commented Jan 1, 2019

I was trying to write a small test server with rustls and I run into this issue: briansmith/webpki#54

I was trying to use: openssl s_client -connect 127.0.0.1:4888 -tls1_2 to test my server and it went up and died on me.
The problem was that it gave this error: Error: Io { source: Custom { kind: InvalidData, error: CorruptMessagePayload(Handshake) } }

This is a really bad experience. I understand that the dependency needs to be fixed, but in the mean time ( and in general ), reporting a better error in this case can make a lot of difference for figuring out what is wrong.

Changing ServerNamePayload::read_hostname to return a Result with a clear indication of the failure, would be great.

@briansmith
Copy link
Contributor

One option would be, when failing to parse the DNS name using webpki's DNS name parser, to try to parse it as a IP address. If the IP address parsing succeeds after DNS name parsing fails, then report a more specific error.

@ayende
Copy link
Author

ayende commented Jan 2, 2019

I think that it is far more common to have an IP there than an actual invalid hostname.
Just having an error that talks about IPs are not allowed would be a huge help.

"Failed DNS name parsing, are you using an IP address, that isn't currently supported."

@hannesdejager
Copy link

Would be nice if this one can be adressed somehow.

@briansmith
Copy link
Contributor

I suggest we close this as a dupe of #184, as it seems to be the same issue.

@djc
Copy link
Member

djc commented Jan 17, 2022

I've been interpreting this as an issue to provide a better error for this, so I do think this is meaningfully distinct from #184. Whether it's worth keeping this open probably depends on how close #184 is. It's still a common pitfall for users.

zancas added a commit to zingolabs/zingolib_original that referenced this issue Apr 26, 2022
AloeareV pushed a commit to zingolabs/zingolib_original that referenced this issue May 2, 2022
AloeareV pushed a commit to zingolabs/zingolib_original that referenced this issue May 3, 2022
AloeareV pushed a commit to zingolabs/zingolib_original that referenced this issue May 12, 2022
AloeareV pushed a commit to zingolabs/zingolib_original that referenced this issue May 18, 2022
@jbr
Copy link
Contributor

jbr commented Jan 12, 2023

It seems like one hurdle to implementing this is the fact that Codec is implemented in terms of Option instead of Result. Has there been thought put into making Codec return Result in order to propagate failure types?

@djc
Copy link
Member

djc commented Jan 12, 2023

Yeah, I think it would make sense to change Codec to yield Result types.

@cpu
Copy link
Member

cpu commented Mar 31, 2023

The issues with the Codec return were addressed, and Rustls 0.21.0 has support for IP address subjects in certificates. I'm going to close this ticket since I believe the associated work is completed. Please feel free to comment or file a new issue if there are remaining gaps to consider.

Thanks!

@cpu cpu closed this as completed Mar 31, 2023
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

No branches or pull requests

6 participants