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: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0 #6

Closed
atc0005 opened this issue Aug 31, 2020 · 15 comments
Assignees
Labels
bug Something isn't working question Further information is requested
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Aug 31, 2020

While testing a preview build of the check_cert binary, the above error message was thrown when our Nagios instance used the Go 1.15-generated binary against a system without any SANs entries, but with the CommonName field set.

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

This problem was not surfaced when performing initial testing with the go1.15beta1 release as noted on #10.

@atc0005 atc0005 added the bug Something isn't working label Aug 31, 2020
@atc0005 atc0005 added this to the Next Release milestone Aug 31, 2020
@atc0005 atc0005 self-assigned this Aug 31, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

This problem was not surfaced when performing initial testing with the go1.15beta1 release as noted on GH-24.

I tested again just now using the lscert-v0.1.1-4-gcaf56ac-dirty-linux-amd64 binary I generated on June 11th and it did have the same result as noted above. I'm guessing that I did not test the Go 1.15 changes against all servers in our fleet and only against the most common configurations.

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

https://golang.org/doc/go1.15#commonname

X.509 CommonName deprecation

The deprecated, legacy behavior of treating the CommonName field on X.509 certificates as a host name when no Subject Alternative Names are present is now disabled by default. It can be temporarily re-enabled by adding the value x509ignoreCN=0 to the GODEBUG environment variable.

Note that if the CommonName is an invalid host name, it's always ignored, regardless of GODEBUG settings. Invalid names include those with any characters other than letters, digits, hyphens and underscores, and those with empty labels or trailing dots.

It's probably worth toggling this setting on for both binaries at application startup.

The bigger question:

Is a certificate valid if no Subject Alternative Name is present?

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

https://support.dnsimple.com/articles/what-is-ssl-san/

From a technical standpoint, every certificate issued today is effectively a SAN certificate, as the CA/B forum requires the certification authority to add the content of the common name to the SAN as well. Even if the certificate covers a single name, it will still use the SAN extension and include that single name.

Since certificates have a limited lifetime, I expect the Go developer's assumption is that only very old, likely expired certificates will be missing a SANs entry.

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

https://medium.com/@sajithekanayaka/solved-java-security-cert-certificateexception-no-subject-alternative-names-present-eec1669faf0d

When the server certificate is having Subject Alternative Names (SAN), the requesting home name must match with one of the SANs. If the server’s SSL certificate does not have SANs, then the requesting home name must match with the Common Name (CN) of the certificate.

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

If a subjectAltName extension of type dNSName is present, that MUST
be used as the identity. Otherwise, the (most specific) Common Name
field in the Subject field of the certificate MUST be used. Although
the use of the Common Name is existing practice, it is deprecated and
Certification Authorities are encouraged to use the dNSName instead.

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

It's probably worth toggling this setting on for both binaries at application startup.

I think this is what I'll end up doing. Not sure yet if by adding an additional flag (e.g., --force-common-name or --ignore-sans or ...).

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

It's probably worth toggling this setting on for both binaries at application startup.

This is worth noting:

All Go 1.15 changes landed. Moving to Go 1.16 to drop the opt-out.

golang/go#24151 (comment)

If I read that correctly, then the environment variable approach to disable the behavioral change would not make it into later Go versions.

Ultimately the certs that we're working with (locally generated by campus CA) would need to be replaced with certificates containing SANs entries (at least the single entry reflecting the DNS name).

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

Relevant bits from the stdlib (crypto/x509/verify.go):

// ignoreCN disables interpreting Common Name as a hostname. See issue 24151.
var ignoreCN = !strings.Contains(os.Getenv("GODEBUG"), "x509ignoreCN=0")

// ...

const (

// ...

	// NameConstraintsWithoutSANs results when a leaf certificate doesn't
	// contain a Subject Alternative Name extension, but a CA certificate
	// contains name constraints, and the Common Name can be interpreted as
	// a hostname.
	//
	// This error is only returned when legacy Common Name matching is enabled
	// by setting the GODEBUG environment variable to "x509ignoreCN=1". This
	// setting might be removed in the future.
	NameConstraintsWithoutSANs

// ...

)

// ...

// VerifyHostname returns nil if c is a valid certificate for the named host.
// Otherwise it returns an error describing the mismatch.
//
// IP addresses can be optionally enclosed in square brackets and are checked
// against the IPAddresses field. Other names are checked case insensitively
// against the DNSNames field. If the names are valid hostnames, the certificate
// fields can have a wildcard as the left-most label.
//
// The legacy Common Name field is ignored unless it's a valid hostname, the
// certificate doesn't have any Subject Alternative Names, and the GODEBUG
// environment variable is set to "x509ignoreCN=0". Support for Common Name is
// deprecated will be entirely removed in the future.
func (c *Certificate) VerifyHostname(h string) error {
// ...
}

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

Probably worth evaluating our fleet to determine just how many of the certs failing this check we're actually dealing with. If just one, then we can likely consider that a bug with the certificate and work towards getting it replaced.

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

Also relevant:

and:

// check SANS entries if provided via command-line
if len(config.SANsEntries) > 0 {
// Check for special keyword, skip SANs entry checks if provided
firstSANsEntry := strings.ToLower(strings.TrimSpace(config.SANsEntries[0]))
if firstSANsEntry != strings.ToLower(strings.TrimSpace(SkipSANSCheckKeyword)) {
if mismatched, err := certs.CheckSANsEntries(certChain[0], config.SANsEntries); err != nil {

@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

Basically it's worth reconsidering support for ignoring SANs entries as part of resolving this. The two items would almost need to be considered together since they're related to the same overall issue.

@atc0005 atc0005 pinned this issue Aug 31, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Aug 31, 2020

Basically it's worth reconsidering support for ignoring SANs entries as part of resolving this.

While we are using the support for skipping SANs checks (on our Nagios system) for systems that didn't have them at all, we were also using the SKIPSANSCHECKS keyword for monitoring certs such as used by outlook.office365.com with 20+ SANs entries.

The current version of this tool assumes that if you list any SANs entries, then you want exactly those SANs entries to be present on the certificate. More or less than specified is considered an error condition.

All of that said, keeping the SKIPSANSCHECKS keyword support for now is probably the correct way to go for now.

We're replacing local certs without SANs entries, so I suspect we'll be fine with the Go 1.15 default behavior and going forward should be fine with Go 1.16 changes. Once the first beta release of 1.16 drops we can test further.

@atc0005 atc0005 modified the milestones: Next Release, Future Sep 1, 2020
@atc0005 atc0005 added the question Further information is requested label Sep 1, 2020
@atc0005 atc0005 unpinned this issue Sep 1, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Oct 14, 2020

Probably worth evaluating our fleet to determine just how many of the certs failing this check we're actually dealing with. If just one, then we can likely consider that a bug with the certificate and work towards getting it replaced.

This is what we ended up doing.

We're replacing local certs without SANs entries, so I suspect we'll be fine with the Go 1.15 default behavior and going forward should be fine with Go 1.16 changes. Once the first beta release of 1.16 drops we can test further.

Still the mindset here.

@atc0005
Copy link
Owner Author

atc0005 commented Feb 26, 2023

From doc comments in the current codebase:

// Go 1.17 removed support for the legacy behavior of treating the
// CommonName field on X.509 certificates as a host name when no
// Subject Alternative Names are present. Go 1.17 also removed
// support for re-enabling the behavior by way of adding the value
// x509ignoreCN=0 to the GODEBUG environment variable.

Not sure what else to do here. Perhaps sufficient support is present to consider this worked around/resolved for now:

  • the SKIPSANSCHECKS keyword is still supported
  • explicit functionality is present to allow skipping SANs checks
  • support is present to allow skipping hostname validation if SANs entries are not present on a certificate

@atc0005 atc0005 closed this as completed Feb 26, 2023
@atc0005
Copy link
Owner Author

atc0005 commented Feb 26, 2023

@atc0005 atc0005 removed this from the Future milestone Feb 26, 2023
@atc0005 atc0005 added this to the v0.8.0 milestone Feb 26, 2023
@atc0005 atc0005 transferred this issue from another repository Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant