Skip to content

TLS Diagnose Formatting Fixes #11342

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.

Sign up for GitHub

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

Merged
HridoyRoy merged 2 commits into from
Apr 12, 2021
Merged

TLS Diagnose Formatting Fixes #11342

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ef39e6
diagnose formatting fixes
Apr 12, 2021
3804715
diagnose formatting fixes
Apr 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions vault/diagnose/tls_verification.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ func ListenerChecks(listeners []listenerutil.Listener) error {
return fmt.Errorf(maxVersionError, l.TLSMaxVersion)
}

var err error
// Perform checks on the TLS Cryptographic Information.
if err = TLSFileChecks(l.TLSCertFile, l.TLSKeyFile); err != nil {
if err := TLSFileChecks(l.TLSCertFile, l.TLSKeyFile); err != nil {
return err
}
}
Expand Down Expand Up @@ -117,15 +116,10 @@ func TLSFileChecks(certFilePath, keyFilePath string) error {
// After verify passes, we need to check the values on the certificate itself.
// This is a separate check beyond the certificate expiry and chain checks.

cert, err := tls.LoadX509KeyPair(certFilePath, keyFilePath)
_, err = tls.LoadX509KeyPair(certFilePath, keyFilePath)
if err != nil {
return err
}
x509Cert, err := x509.ParseCertificate(cert.Certificate[0])
if err != nil {
return err
}
cert.Leaf = x509Cert

return nil
}
Expand Down
44 changes: 22 additions & 22 deletions vault/diagnose/tls_verification_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestTLSValidCert(t *testing.T) {
}
err := ListenerChecks(listeners)
if err != nil {
t.Error(err.Error())
t.Fatalf(err.Error())
}
}

Expand All @@ -50,10 +50,10 @@ func TestTLSFakeCert(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if !strings.Contains(err.Error(), "could not decode cert") {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -78,10 +78,10 @@ func TestTLSTrailingData(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if !strings.Contains(err.Error(), "asn1: syntax error: trailing data") {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -104,10 +104,10 @@ func TestTLSExpiredCert(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if !strings.Contains(err.Error(), "certificate has expired or is not yet valid") {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -130,10 +130,10 @@ func TestTLSMismatchedCryptographicInfo(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if err.Error() != "tls: private key type does not match public key type" {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}

listeners = []listenerutil.Listener{
Expand All @@ -153,10 +153,10 @@ func TestTLSMismatchedCryptographicInfo(t *testing.T) {
}
err = ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if err.Error() != "tls: private key type does not match public key type" {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -179,10 +179,10 @@ func TestTLSMultiKeys(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if !strings.Contains(err.Error(), "pem block does not parse to a certificate") {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -204,10 +204,10 @@ func TestTLSMultiCerts(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if !strings.Contains(err.Error(), "found a certificate rather than a key in the PEM for the private key") {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -231,10 +231,10 @@ func TestTLSInvalidRoot(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if err.Error() != "failed to verify certificate: x509: certificate signed by unknown authority" {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -258,7 +258,7 @@ func TestTLSNoRoot(t *testing.T) {
}
err := ListenerChecks(listeners)
if err != nil {
t.Error("Server certificate without root certificate is insecure, but still valid.")
t.Fatalf("Server certificate without root certificate is insecure, but still valid.")
}
}

Expand All @@ -282,10 +282,10 @@ func TestTLSInvalidMinVersion(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if err.Error() != fmt.Errorf(minVersionError, "0").Error() {
t.Errorf("Bad error message: %w", err)
t.Fatalf("Bad error message: %s", err)
}
}

Expand All @@ -309,9 +309,9 @@ func TestTLSInvalidMaxVersion(t *testing.T) {
}
err := ListenerChecks(listeners)
if err == nil {
t.Error("TLS Config check on fake certificate should fail")
t.Fatalf("TLS Config check on fake certificate should fail")
}
if err.Error() != fmt.Errorf(maxVersionError, "0").Error() {
t.Errorf("Bad error message: %w", err)
Copy link
Contributor

@swayne275 swayne275 Apr 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think error here was fine since nothing after this depended on the results of that check, but fatal is good too!

t.Fatalf("Bad error message: %s", err)
}
}