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

Open upstream Request For Enhancement to export a sentinel error (or constant) for x509: certificate relies on legacy Common Name field, use SANs instead #520

Open
atc0005 opened this issue Feb 26, 2023 · 0 comments
Assignees
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Feb 26, 2023

Overview

GH-276 added initial support for matching the x509: certificate relies on legacy Common Name field, use SANs instead x509.HostnameError error text and offering sysadmins advice for how to resolve it.

Later work in this project shifted how this detection/advice was handled, but the core implementation remains the same:

  1. detect this error condition using string matching
  2. surface the error with advice to the sysadmin
Show implementation

// ErrX509CertReliesOnCommonName mirrors the unexported error string
// emitted by the HostnameError.Error() method from the x509 package.
//
// This error string is emitted when a certificate is missing Subject
// Alternate Names (SANs) AND a specified hostname matches the Common Name
// field.
ErrX509CertReliesOnCommonName = errors.New("x509: certificate relies on legacy Common Name field, use SANs instead")

// 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.
//
// We attempt to detect this situation in order to supply additional
// troubleshooting information and guidance to resolve the issue. We
// accomplish this by setting a very specific error and looking for this
// error later when deciding which feedback to provide.
//
// We intentionally do not mark this validation check result as ignored as
// the sysadmin did not opt to explicitly do so.
case verifyErr != nil &&
(verifyErr.Error() == ErrX509CertReliesOnCommonName.Error() ||
len(certChain[0].DNSNames) == 0):
return HostnameValidationResult{
certChain: certChain,
leafCert: leafCert,
hostnameValue: hostnameValue,
err: ErrX509CertReliesOnCommonName,
ignored: false,
ignoreWhenEmptySANsList: ignoreIfSANsEmpty,
ignoreIfSANsEmptyFlagName: ignoreIfSANsEmptyFlagName,
// Medium priority bump since this is an issue that the sysadmin
// has a workaround available for.
priorityModifier: priorityModifierMedium,
}

Ideally we would not have to perform string matching to detect the error, but due to how the error is implemented upstream I am not aware of any other option:

// ...
func (h HostnameError) Error() string {
	c := h.Certificate

	if !c.hasSANExtension() && matchHostnames(c.Subject.CommonName, h.Host) {
		return "x509: certificate relies on legacy Common Name field, use SANs instead"
	}
// ...

I don't know what is required to open a proposal or Request for Enhancement (RFE) in the upstream Go project, but that appears to be the next step to have the error surfaced as an value that we can reliably match against. While string matching works now, I don't know how long it will continue to be provided with that exact pattern.

References

@atc0005 atc0005 added this to the Future milestone Feb 26, 2023
@atc0005 atc0005 self-assigned this Feb 26, 2023
atc0005 added a commit that referenced this issue Feb 26, 2023
The `ErrX509CertReliesOnCommonName` sentinel error is used
by this project to aid in detecting this specific error
scenario. While performing a string match for this error
works at present, it feels brittle and subject to break
in the future if the error text should change.

By opening a RFE/proposal in the upstream Go project, we may
gain a supported/stable approach for matching the error in
the future.

refs GH-520
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

1 participant