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

x509 hostname errors on Go HEAD #194

Closed
nhooyr opened this issue Jul 27, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@nhooyr
Copy link

commented Jul 27, 2018

If you try to dial an instance say project-dev:us-central1:main, pinging will throw an error x509: Common Name is not a valid hostname: project-dev:main

Looks like the Go x509 package was updated to throw errors on invalid hostnames.

@hfwang

This comment has been minimized.

Copy link
Member

commented Jul 31, 2018

I wonder if this is related: golang/go#24151

Cloud SQL uses CNs but not SANs in its certificates, from my reading of that thread, it seems like its ok to not use a hostname in the CN?

@FiloSottile

This comment has been minimized.

Copy link

commented Aug 2, 2018

_o/ hi! This is https://tip.golang.org/doc/go1.11#crypto/x509. You need to either move that name to a SAN, or (not recommended) not verify the name with Verify/VerifyHostname, and write your own CN check.

Is this going to affect everyone using CloudSQL? Should I open a Google internal issue to switch to SANs?

@hfwang

This comment has been minimized.

Copy link
Member

commented Aug 3, 2018

Thanks FiloSottile for the pointer.

We are testing a fix for this internally and will try to update this repository hopefully this evening or tomorrow.

@hfwang hfwang closed this in #196 Aug 5, 2018

@forrestdix forrestdix referenced this issue Aug 29, 2018

Closed

X509 error #203

zombiezen added a commit to zombiezen/go-cloud that referenced this issue Oct 9, 2018

zombiezen added a commit to google/go-cloud that referenced this issue Oct 11, 2018

@nicknovitski nicknovitski referenced this issue Oct 23, 2018

Merged

cloud-sql-proxy: 1.11 -> 1.13 #48920

3 of 7 tasks complete

agl added a commit to google/boringssl that referenced this issue Apr 22, 2019

Require certificates under name constraints use SANs.
The common name fallback does not interact well with name constraints.
Until we remove this fallback, we must resolve this conflict.

Blindly applying name constraints to the common name will reject
"decorative" common names that aren't intended to be hostnames (e.g.
[0]). We need to guess based on format whether the common name is a DNS
name. It is important this same check is applied to *both* name
constraints and name matching, which means the OpenSSL version (see
5bd5dcd49605ca2aa7931599894302a3ac4b0b04,
d02d80b2e80adfdde49f76cf7c7af4e013f45005, and
55a6250f1e7336e8a7d89fb609eb23398715ff6f) is unsuitable as a
compatibility data point.

In theory we could limit this to chains with name constraints, which are
uncommon, but X509_check_host sees only the leaf. We must apply it
uniformly. That means a strict check risks problems with malformed
non-WebPKI setups like [1].

For a first pass, mirror Go's behavior. Like Go, rather than run
SAN-less DNS-like common names through name constraints, we simply
reject all such certificates. Name constraints now exclude all leaf
certificates that can trigger the common name fallback. They are rare
enough that we can hopefully hold them to a higher standard.

Note this does not make misclassified decorative common names any worse,
compared to the checking the name constraint. Such names would not have
matched the constraint anyway.

Update-Note: This can may cause two kinds of errors:

1. Leaf certificates whose chain contains a name constraint and lack
   SANs may be rejected with X509_V_ERR_NAME_CONSTRAINTS_WITHOUT_SANS.

2. Leaf certificates which use the common name fallback and verify
   against an insufficiently DNS-looking hostname may fail with
   X509_V_ERR_HOSTNAME_MISMATCH.

In both cases, the fix is to include the subjectAltName in the
certificate, rather than rely on the common name fallback. (Refining the
heuristic is also an option, but the two failure modes pull it in
opposite directions, so this is tricky.)

[0] golang/go#24151
[1] GoogleCloudPlatform/cloudsql-proxy#194

Change-Id: If25557de428768292a14ba3bdeeffbd74e3a3bf8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35665
Reviewed-by: Adam Langley <agl@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.