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

crypto/x509: Go 1.15 will break default connections to AWS RDS #39568

Closed
kevinburke1 opened this issue Jun 12, 2020 · 31 comments
Closed

crypto/x509: Go 1.15 will break default connections to AWS RDS #39568

kevinburke1 opened this issue Jun 12, 2020 · 31 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Milestone

Comments

@kevinburke1
Copy link

kevinburke1 commented Jun 12, 2020

Amazon Relational Database Storage (RDS) is a popular database storage engine. RDS offers a method to securely connect to your database instance, described here: https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.SSL.html.

RDS databases - Postgres ones, anyway, have not tested the other SQL flavors - send back a certificate that includes a common name field. Our own databases are inside of a VPN so it's a little trickier to share access for you to test them that way, but let me know if you need to and we can work something out.

/usr/local/opt/openssl@1.1/bin/openssl s_client -starttls postgres secretdatabasename.us-west-2.rds.amazonaws.com:5432
CONNECTED(00000005)
depth=2 C = US, L = Seattle, ST = Washington, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS Root 2019 CA
verify error:num=19:self signed certificate in certificate chain
verify return:1
depth=2 C = US, L = Seattle, ST = Washington, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS Root 2019 CA
verify return:1
depth=1 C = US, ST = Washington, L = Seattle, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS us-west-2 2019 CA
verify return:1
depth=0 CN = secretdatabasename.us-west-2.rds.amazonaws.com, OU = RDS, O = Amazon.com, L = Seattle, ST = Washington, C = US
verify return:1
---
Certificate chain
 0 s:CN = secretdatabasename.us-west-2.rds.amazonaws.com, OU = RDS, O = Amazon.com, L = Seattle, ST = Washington, C = US
   i:C = US, ST = Washington, L = Seattle, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS us-west-2 2019 CA
 1 s:C = US, ST = Washington, L = Seattle, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS us-west-2 2019 CA
   i:C = US, L = Seattle, ST = Washington, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS Root 2019 CA
 2 s:C = US, L = Seattle, ST = Washington, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS Root 2019 CA
   i:C = US, L = Seattle, ST = Washington, O = "Amazon Web Services, Inc.", OU = Amazon RDS, CN = Amazon RDS Root 2019 CA

There are no problems connecting using Go up through 1.14. However, if you try to connect to a RDS database using TLS and the Go 1.15 beta, this is the message that you get:

x509: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0

I suspect that a lot of people are going to get caught out by this. (Well, maybe not, maybe people don't encrypt connections to the database.)

To be clear I think it is good that we are trying to get code (and certificates) to do the right thing. But it's not great that a bunch of code that previously worked will now not work.

I don't have the juice with AWS to ask them to update the certs to use SAN's. Perhaps someone reading this thread does have the juice, and can explain the problem and ask them to upgrade their certificates?

Could we call this out more clearly in the docs? For example, we could move the description of the change closer to the top of go1.15.html, or indicate that we expect that this change is going to break a lot of existing deployments, including RDS and possibly others.

@kevinburke1 kevinburke1 changed the title crypto/x509: Go 1.15 will break connections to AWS RDS crypto/x509: Go 1.15 will break default connections to AWS RDS Jun 12, 2020
@ianlancetaylor
Copy link
Member

CC @FiloSottile @katiehockman

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Jun 12, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.15 milestone Jun 12, 2020
@ianlancetaylor
Copy link
Member

Marking as a 1.15 release blocker to make sure that we make some sort of fix or decision.

@kevinburke1
Copy link
Author

Amazon's response may be something along the lines of "this doesn't really matter if you are using our VPC since packets in a VPC are encrypted and authenticated" but you'd still need it if you were connecting to the database from outside of AWS.

@graywolf-at-work
Copy link

Doesn't this kinda break https://golang.org/doc/go1compat? Or this this (not using common name) classify as a matter of security?

@mholt
Copy link

mholt commented Jun 13, 2020

@graywolf-at-work No, that's only a language guarantee. Old Go programs still compile and run. This is like phasing out support for SSLv3 and Windows XP, and is necessary to keep the standard library current with external changes.

@fouwels
Copy link

fouwels commented Jun 13, 2020

CN verification is still specified by RFC 6125 as a fallback if SAN isn’t present.

Breaking external compatibility to prove a point seems excessive.

https://tools.ietf.org/html/rfc6125#section-6.4.4

@ulikunitz
Copy link
Contributor

CL 231379 made "ignoreCN" the default. Reverting the first line of the diff, should bring back the old behavior.

@rolandshoemaker
Copy link
Member

It should be noted that this will only break non-publicly trusted, non-web PKI certificates since the CN is mandated in the CABF BRs to be, if present at all, one of the identities in the SAN, making this change non-breaking for conforming CAs.

CN was deprecated in 2000, and RFC 6125 made the fallback to it optional, rather a requirement (which was initially suggested in the RFC that deprecated it, RFC 2818).

@alex
Copy link
Contributor

alex commented Jun 13, 2020

It seems like the most efficient path forward here is if someone from AWS can indicate their plans around adding SANs to RDS certificates.

@kevinburke1
Copy link
Author

There's some indication on Twitter that the AWS team is looking at it but I'm not sure it will be ready by the release date. I don't know people in the right places at AWS to say otherwise. Filippo asked them for a timeline when they have one. https://twitter.com/seakoz/status/1271681101149270016

@FiloSottile
Copy link
Contributor

The ideal resolution here is for AWS to use the correct field in their certificates. Let's wait to hear from them, and then we can decide whether to back off this change to Go 1.16.

Note that this change doesn't make it impossible to (manually) use CN for verification, it simply aligns with the spec in not considering it a hostname or an IP for VerifyHostname or VerifyOptions.DNSName purposes.

CN has been deprecated for 20 years, the specs do not recommend supporting it anymore, the browsers successfully dropped it, the WebPKI CAs are prohibited from setting it to anything that is not also in the SANs, and its behavior is completely unspecified.

The last point is a security issue with Name Constraint validation (which we support), because CN is not constrained (because it's not a hostname), so if we consider it a hostname we allow NCs to be bypassed. We have workarounds but they are complex unspecified hacks (see #24151).

@kevinburke1
Copy link
Author

I'm just gonna say - I caught this because I deployed the 1.15 beta to our staging environment shortly after it was released. This gave us and AWS months of lead time between detection and the deadline for the release.

Please try to run the betas/RC's in your staging environments so that the entire community can benefit from the issues you find there, before they go live to production. Thanks!

@kse-clearhaus
Copy link

We run a 3-D Secure Server. The certificates used by the scheme HTTPS endpoints
are universally issued by internal CAs.

This change will also break our connections to several major card schemes,
since they do not include SANs in certificates.

I'll grant you, that we will be able to work around it.
My point is, there are likely many areas where this causes issues.
Since there's actually an RFC that allows for a fallback, I would appriciate
if this was allowed.

Though I applaud your attempt to enforce a 20 year old deprecation :)

On a sidenote:

These CA's universally also encode nearly everything as ASN.1
PrintableString. This causes us heaps of trouble, much more than this would.

There's been several issues regarding this:
https://github.com/golang/go/issues?q=is%3Aissue+is%3Aclosed+printablestring

@FiloSottile FiloSottile self-assigned this Jun 23, 2020
@cagedmantis
Copy link
Contributor

@FiloSottile If it's decided that this is going to be rolled back then it would be best to do it before RC1 is released.

@FiloSottile
Copy link
Contributor

@FiloSottile If it's decided that this is going to be rolled back then it would be best to do it before RC1 is released.

I don't think this needs to be rolled back based only on the AWS RDS breakage.

I do think (hopefully incorrectly) that we'll see more breakage as more people test this, but if we roll it back based on my hunch of what will come out of RC1, then we're not actually learning what breaks and we'll never get to land this, because the same argument will apply unchanged for Go 1.16rc1.

@toothrot toothrot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jul 13, 2020
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 13, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/243221 mentions this issue: doc/go1.15: surface the crypto/x509 Common Name deprecation note

@toothrot
Copy link
Contributor

As an update, we intend on releasing the RC with this change. There will be documentation in the release notes on how to get the old behavior while people work through issues. We're keeping this issue labeled as a release-blocker for Go 1.15, pending feedback from more people testing.

@luxifer
Copy link

luxifer commented Jul 24, 2020

For the record I'm experiencing the same issue as described but with Cisco ISE. Certificates generated by ISE contains CN but no SAN thus the certificate is not validated by the go tls.

gopherbot pushed a commit that referenced this issue Jul 24, 2020
Updates #39568
Updates #37419
Updates #24151

Change-Id: I44c940e09e26a039076396bbfecb2b1574197cf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/243221
Reviewed-by: Kevin Burke <kev@inburke.com>
@danp
Copy link
Contributor

danp commented Jul 29, 2020

AWS have added a note to https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.SSL-certificate-rotation.html about this issue:

If you are using a Go version 1.15 application with a DB instance that was created or updated to the rds-ca-2019 certificate prior to July 28, 2020, you must update the certificate again. Run the modify-db-instance command shown in the AWS CLI section using rds-ca-2019 as the CA certificate identifier. In this case, it isn't possible to update the certificate using the AWS Management Console. If you created your DB instance or updated its certificate after July 28, 2020, no action is required. For more information, see Go GitHub issue #39568

shahruk10 added a commit to cobaltspeech/sdk-juzu that referenced this issue Mar 25, 2021
  - Go >= 1.15 dropped support for verifying hostname/IP address in
    the Common Name field of a certificate. It now requires that the
    hostname be present in Subject Alternative Names (SAN) extension.
    See golang/go#39568 (comment)
shahruk10 added a commit to cobaltspeech/sdk-cubic that referenced this issue Mar 25, 2021
  - Go >= 1.15 dropped support for verifying hostname/IP address in
    the Common Name field of a certificate. It now requires that the
    hostname be present in Subject Alternative Names (SAN) extension.
    See golang/go#39568 (comment)
shahruk10 added a commit to cobaltspeech/sdk-juzu that referenced this issue Mar 25, 2021
  - Go >= 1.15 dropped support for verifying hostname/IP address in
    the Common Name field of a certificate. It now requires that the
    hostname be present in Subject Alternative Names (SAN) extension.
    See golang/go#39568 (comment)
shahruk10 added a commit to cobaltspeech/sdk-juzu that referenced this issue Apr 1, 2021
  - Go >= 1.15 dropped support for verifying hostname/IP address in
    the Common Name field of a certificate. It now requires that the
    hostname be present in Subject Alternative Names (SAN) extension.
    See golang/go#39568 (comment)
shahruk10 added a commit to cobaltspeech/sdk-cubic that referenced this issue Apr 1, 2021
  - Go >= 1.15 dropped support for verifying hostname/IP address in
    the Common Name field of a certificate. It now requires that the
    hostname be present in Subject Alternative Names (SAN) extension.
    See golang/go#39568 (comment)
shahruk10 added a commit to cobaltspeech/sdk-cubic that referenced this issue Apr 1, 2021
  - Go >= 1.15 dropped support for verifying hostname/IP address in
    the Common Name field of a certificate. It now requires that the
    hostname be present in Subject Alternative Names (SAN) extension.
    See golang/go#39568 (comment)
shahruk10 added a commit to cobaltspeech/sdk-juzu that referenced this issue May 13, 2021
  - Go >= 1.15 dropped support for verifying hostname/IP address in
    the Common Name field of a certificate. It now requires that the
    hostname be present in Subject Alternative Names (SAN) extension.
    See golang/go#39568 (comment)
@golang golang locked and limited conversation to collaborators Dec 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Projects
None yet
Development

No branches or pull requests