-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
crypto/x509: certificate validation in Windows fails to validate IP in SAN #37176
Comments
To add - I also verified this behaviour has been present at least since |
I don't know if there is an error here - I am not a crypto expert. But I would like to try and reproduce your error here first. You said nothing about your server and how you generated your certificate. Is your server is written in Go? Can I see your code? What are the commands to generate and use your certificate? Thank you. Alex |
Had the same issue here: Modifying checkChainSSLServerPolicy in root_windows.go as follows would clear up the problem, but I'm not entirely sure, whether any security issues would arise as a result: |
@alexbrainman I'm sorry, it's been quite a while - hopefully @naoriel's duplication can help. I think there is definitely an error here - the error message alone doesn't make sense (it's valid for , but not for ).
No, not written in Go. The server is Java-based, though at the time I was able to reproduce it with a Go-based server hosted on a separate port (an echo-server that just returns your request back to you). Again, it worked on both Linux/Mac connections but failed on the Windows connection (client-side failure).
Unfortunately it's been several months since the filing of this ticket and I had not seen your notification, so I don't have this information, but it was nothing out-of-the-ordinary - I believe this would be reproducible with any certificate request and certificate authority, where an IP is left out of the CN but present in the "IP Address". @naoriel's example looks spot on. |
I revisited the code in Here is a patch file I prepared: @FiloSottile : If you've got a moment to spare, I'd be very interested in your thoughts on the matter. |
No worries. I will let the experts look at @naoriel code. Alex |
Looks like this could be a, uh, feature of CryptoAPI. Chromium ran into the same issue (https://bugs.chromium.org/p/chromium/issues/detail?id=72726) which confirmed that The Chromium approach was to ignore the invalid CN return by setting |
Well, that's... weird. I guess we should just take over name validation from the OS, after checking that it checks name constraints on all names on the certificate. Since this has been the case forever, and I want to go one cycle without breaking people on crypto/x509, targeting Go 1.17. |
@FiloSottile Are we still targeting 1.17 for this? |
I'll move to Backlog since there hasn't been activity here and it seems this can happen in a later release. If it's important to continue to target 1.17, please move it back and add an assignee who plans to get this resolved (or add a release-blocking label with a justification). |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I ran the following quick program to reproduce it:
Where
192.168.210.129
is a custom server using a self-signed certificate. The notable aspects of this cert are that I'm connecting via IP (which is not present in the CN) and it is only present in theIP Address
field of the SAN, like so:When I compile the binary to run on a Windows box, I run the following:
What did you expect to see?
I expect to see a response from the server. This server is healthy and I can access it from my browser, so I expect to see an HTML output from the last line.
What did you see instead?
I see the following output while running:
The interesting part is that it that's the same IP address in both notes.
I then ran a modified version of
go
to add apanic
near https://sourcegraph.com/github.com/golang/go@release-branch.go1.11/-/blob/src/crypto/x509/root_windows.go#L129, changing:to
and see the following stack trace to verify this is indeed where it fails, which is Windows-specific.
I also further validated I do not see such issues on Linux or Mac machines.
For additional debugging help, I should also mention I only have access to one specific Windows machine that I tested with at the time, so I'm not sure if this is apparent on other Windows versions.
My Windows version is:
Windows Server 2012 R2 Standard
The text was updated successfully, but these errors were encountered: