Inconsistent hostname verification applied when filename flag is used

[atc0005]

For the lscert and check_cert tools, using both the filename and server flags together is blocked by config validation. The dns-name flag is optional for both tools.

For lscert, the VerifyHostname() method call is performed only if the server flag is set and only if there are certs present in the chain:

check-cert/cmd/lscert/main.go

Lines 206 to 226 in eb5eb94

	if cfg.Server != "" {
	if len(certChain) > 0 {
	hostnameValueToUse := cfg.Server
	// Allow user to explicitly specify which hostname should be used
	// for comparison against the leaf certificate.
	if cfg.DNSName != "" {
	hostnameValueToUse = cfg.DNSName
	}
	// verify leaf certificate is valid for the provided server FQDN
	if err := certChain[0].VerifyHostname(hostnameValueToUse); err != nil {
	fmt.Printf("- WARNING: Provided hostname %q does not match server certificate: %v\n", hostnameValueToUse, err)
	} else {
	fmt.Println("- OK: Provided hostname matches discovered certificate")
	}
	}
	}

If the optional dns-name flag is provided it is used for hostname verification, but the current implementation requires that the server flag is set; this implementation disables hostname verification if examining a certificate file. It is likely that this was an intentional decision; this behavior was already present in the initial prototype release, so I don't have a GH issue noting the intention behind the choice.

For check_cert, the logic to determine whether hostname verification is applied is more complex.

check-cert/cmd/check_cert/main.go

Lines 313 to 321 in eb5eb94

	if certsSummary.TotalCertsCount > 0 {
	hostnameValue := cfg.Server
	// Allow user to explicitly specify which hostname should be used
	// for comparison against the leaf certificate.
	if cfg.DNSName != "" {
	hostnameValue = cfg.DNSName
	}

1. We first assert that there is at least one certificate in the chain
2. We start with using the server flag value for later hostname verification (may be empty)
3. If the dns-name flag is used, we pick that value for hostname verification over the server flag value

check-cert/cmd/check_cert/main.go

Lines 331 to 339 in eb5eb94

	switch {
	case len(certChain[0].DNSNames) == 0 &&
	cfg.DisableHostnameVerificationIfEmptySANsList:
	log.Warn().
	Str("hostname", hostnameValue).
	Str("cert_cn", certChain[0].Subject.CommonName).
	Str("sans_entries", fmt.Sprintf("%s", certChain[0].DNSNames)).
	Msg("disabling hostname verification as requested for empty SANs list")

4. If the leaf cert has an empty SANs list and the user opted to skip hostname verification (when SANs list is empty) we emit a warning and then skip hostname verification

check-cert/cmd/check_cert/main.go

Line 354 in eb5eb94

verifyErr := certChain[0].VerifyHostname(hostnameValue)

5. Regardless of whether the SANS list is populated, if the user didn't opt to skip hostname verification we do so here

check-cert/cmd/check_cert/main.go

Lines 367 to 370 in eb5eb94

	case verifyErr != nil &&
	// TODO: Is there value in matching the specific error string?
	(verifyErr.Error() == certs.X509CertReliesOnCommonName ||
	len(certChain[0].DNSNames) == 0):

6. If there is a verification error we check if it was because the SANs list was empty and fail with verbose feedback to assist sysadmin with resolving the issue

check-cert/cmd/check_cert/main.go

Line 404 in eb5eb94

case verifyErr != nil:

7. If there was a verification error for another reason we fail with a recommendation to specify the correct site FQDN instead of a host FQDN

For check_cert, the first "gate" is whether a certificate is present in the chain (this is a little ironic as the block just above it fails the plugin if zero certs were found). Then, hostname verification is performed regardless of whether the filename or server flag was used. Instead of requiring that the dns-name flag is used if the filename flag was specified, hostname verification is performed anyway and presumably the user is guided to provide the dns-name flag based on the hostname verification error encountered.

[atc0005]

For lscert, the VerifyHostname() method call is performed only if the server flag is set and only if there are certs present in the chain

Question: Should lscert attempt to perform hostname verification for certificate files?

• If dns-name is specified, then the user is presumably looking to apply verification to the certificate.
• If it's not, then the user is likely just wanting to view the cert metadata and be told whether it is expired.

[atc0005]

For check_cert, we probably need to assert that dns-name is specified if server is not set and if we want to enforce hostname verification.

That said, it's probably a good idea to step back and decide which validations are optional, which are required and what triggers each of them.

All of this is surfaced with work picking back up on #29. See also #64.

[atc0005]

For lscert, the VerifyHostname() method call is performed only if the server flag is set and only if there are certs present in the chain

Question: Should lscert attempt to perform hostname verification for certificate files?

• If dns-name is specified, then the user is presumably looking to apply verification to the certificate.
• If it's not, then the user is likely just wanting to view the cert metadata and be told whether it is expired.

This is worth going ahead and applying as a standalone PR.

Larger changes to cleanup logic in check_cert is likely best postponed until at least partial support for #64 is implemented.