From 96f1443265cfe018db99050531191e5295692a84 Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Wed, 31 Aug 2022 16:25:14 -0400 Subject: [PATCH] Fix various trivial warnings from staticcheck in the PKI plugin (#16946) * Fix up simple warnings in production code * Address warnings from static check in the PKI test classes --- builtin/logical/pki/backend_test.go | 68 +++++++++++------------ builtin/logical/pki/cert_util.go | 27 +++------ builtin/logical/pki/chain_util.go | 8 +-- builtin/logical/pki/crl_test.go | 12 ++-- builtin/logical/pki/crl_util.go | 2 +- builtin/logical/pki/ocsp.go | 9 ++- builtin/logical/pki/ocsp_test.go | 6 +- builtin/logical/pki/path_config_crl.go | 2 +- builtin/logical/pki/path_fetch_issuers.go | 8 +-- builtin/logical/pki/path_issue_sign.go | 2 +- builtin/logical/pki/path_revoke.go | 4 +- builtin/logical/pki/path_roles.go | 2 +- builtin/logical/pki/path_tidy.go | 2 +- builtin/logical/pki/path_tidy_test.go | 2 +- builtin/logical/pki/storage.go | 6 +- builtin/logical/pki/test_helpers.go | 51 +++-------------- builtin/logical/pki/util.go | 2 +- 17 files changed, 81 insertions(+), 132 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index acc9e2a12..1028c28e7 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -128,7 +128,7 @@ func TestPKI_RequireCN(t *testing.T) { // Issue a cert with require_cn set to true and with common name supplied. // It should succeed. - resp, err = CBWrite(b, s, "issue/example", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/example", map[string]interface{}{ "common_name": "foobar.com", }) if err != nil { @@ -137,7 +137,7 @@ func TestPKI_RequireCN(t *testing.T) { // Issue a cert with require_cn set to true and with out supplying the // common name. It should error out. - resp, err = CBWrite(b, s, "issue/example", map[string]interface{}{}) + _, err = CBWrite(b, s, "issue/example", map[string]interface{}{}) if err == nil { t.Fatalf("expected an error due to missing common_name") } @@ -1079,7 +1079,7 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep { } cert := parsedCertBundle.Certificate - actualDiff := time.Now().Sub(cert.NotBefore) + actualDiff := time.Since(cert.NotBefore) certRoleDiff := (role.NotBeforeDuration - actualDiff).Truncate(time.Second) // These times get truncated, so give a 1 second buffer on each side if certRoleDiff >= -1*time.Second && certRoleDiff <= 1*time.Second { @@ -1512,8 +1512,8 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep { return fmt.Errorf("error parsing cert bundle: %s", err) } cert := parsedCertBundle.Certificate - var emptyIPs []net.IP - var expected []net.IP = append(emptyIPs, expectedIp...) + var expected []net.IP + expected = append(expected, expectedIp...) if diff := deep.Equal(cert.IPAddresses, expected); len(diff) > 0 { return fmt.Errorf("wrong SAN IPs, diff: %v", diff) } @@ -1589,8 +1589,8 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep { if err != nil { return err } - var emptyOthers []otherNameUtf8 - var expected []otherNameUtf8 = append(emptyOthers, expectedOthers...) + var expected []otherNameUtf8 + expected = append(expected, expectedOthers...) if diff := deep.Equal(foundOthers, expected); len(diff) > 0 { return fmt.Errorf("wrong SAN IPs, diff: %v", diff) } @@ -1874,11 +1874,11 @@ func TestBackend_PathFetchValidRaw(t *testing.T) { t.Fatalf("failed read ca/pem, %#v", resp) } // check the raw cert matches the response body - if bytes.Compare(resp.Data[logical.HTTPRawBody].([]byte), []byte(rootCaAsPem)) != 0 { + if !bytes.Equal(resp.Data[logical.HTTPRawBody].([]byte), []byte(rootCaAsPem)) { t.Fatalf("failed to get raw cert") } - resp, err = b.HandleRequest(context.Background(), &logical.Request{ + _, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, Path: "roles/example", Storage: storage, @@ -1927,7 +1927,7 @@ func TestBackend_PathFetchValidRaw(t *testing.T) { // check the raw cert matches the response body rawBody := resp.Data[logical.HTTPRawBody].([]byte) bodyAsPem := []byte(strings.TrimSpace(string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: rawBody})))) - if bytes.Compare(bodyAsPem, expectedCert) != 0 { + if !bytes.Equal(bodyAsPem, expectedCert) { t.Fatalf("failed to get raw cert for serial number: %s", expectedSerial) } if resp.Data[logical.HTTPContentType] != "application/pkix-cert" { @@ -1948,7 +1948,7 @@ func TestBackend_PathFetchValidRaw(t *testing.T) { } // check the pem cert matches the response body - if bytes.Compare(resp.Data[logical.HTTPRawBody].([]byte), expectedCert) != 0 { + if !bytes.Equal(resp.Data[logical.HTTPRawBody].([]byte), expectedCert) { t.Fatalf("failed to get pem cert") } if resp.Data[logical.HTTPContentType] != "application/pem-certificate-chain" { @@ -2631,7 +2631,7 @@ func TestBackend_SignSelfIssued(t *testing.T) { t.Fatalf("expected error due to different issuer; cert info is\nIssuer\n%#v\nSubject\n%#v\n", ssCert.Issuer, ssCert.Subject) } - ss, ssCert = getSelfSigned(t, template, template, key) + ss, _ = getSelfSigned(t, template, template, key) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, Path: "root/sign-self-issued", @@ -2765,7 +2765,7 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { // Test with flag present and true ss, _ = getSelfSigned(t, template, template, key) - resp, err = b.HandleRequest(context.Background(), &logical.Request{ + _, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, Path: "root/sign-self-issued", Storage: storage, @@ -2866,7 +2866,7 @@ func TestBackend_OID_SANs(t *testing.T) { } // First test some bad stuff that shouldn't work - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar.com", "ip_sans": "1.2.3.4", "alt_names": "foo.foobar.com,bar.foobar.com", @@ -2878,7 +2878,7 @@ func TestBackend_OID_SANs(t *testing.T) { t.Fatal("expected error") } - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar.com", "ip_sans": "1.2.3.4", "alt_names": "foo.foobar.com,bar.foobar.com", @@ -2890,7 +2890,7 @@ func TestBackend_OID_SANs(t *testing.T) { t.Fatal("expected error") } - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar.com", "ip_sans": "1.2.3.4", "alt_names": "foo.foobar.com,bar.foobar.com", @@ -2902,7 +2902,7 @@ func TestBackend_OID_SANs(t *testing.T) { t.Fatal("expected error") } - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar.com", "ip_sans": "1.2.3.4", "alt_names": "foo.foobar.com,bar.foobar.com", @@ -2914,7 +2914,7 @@ func TestBackend_OID_SANs(t *testing.T) { t.Fatal("expected error") } - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar.com", "ip_sans": "1.2.3.4", "alt_names": "foo.foobar.com,bar.foobar.com", @@ -3058,7 +3058,7 @@ func TestBackend_AllowedSerialNumbers(t *testing.T) { t.Fatal(err) } - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar", "ttl": "1h", }) @@ -3066,7 +3066,7 @@ func TestBackend_AllowedSerialNumbers(t *testing.T) { t.Fatal(err) } - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar", "ttl": "1h", "serial_number": "foobar", @@ -3085,7 +3085,7 @@ func TestBackend_AllowedSerialNumbers(t *testing.T) { t.Fatal(err) } - resp, err = CBWrite(b, s, "issue/test", map[string]interface{}{ + _, err = CBWrite(b, s, "issue/test", map[string]interface{}{ "common_name": "foobar", "ttl": "1h", // Not a valid serial number @@ -3669,7 +3669,7 @@ func setCerts() { if err != nil { panic(err) } - subjKeyID, err = certutil.GetSubjKeyID(rak) + _, err = certutil.GetSubjKeyID(rak) if err != nil { panic(err) } @@ -3699,7 +3699,7 @@ func setCerts() { if err != nil { panic(err) } - subjKeyID, err = certutil.GetSubjKeyID(edk) + _, err = certutil.GetSubjKeyID(edk) if err != nil { panic(err) } @@ -4014,7 +4014,7 @@ func runFullCAChainTest(t *testing.T, keyType string) { requireCertInCaChainString(t, fullChain, rootCert, "expected root cert within root cert/ca_chain") // Make sure when we issue a leaf certificate we get the full chain back. - resp, err = CBWrite(b_root, s_root, "roles/example", map[string]interface{}{ + _, err = CBWrite(b_root, s_root, "roles/example", map[string]interface{}{ "allowed_domains": "example.com", "allow_subdomains": "true", "max_ttl": "1h", @@ -4066,7 +4066,7 @@ func runFullCAChainTest(t *testing.T, keyType string) { require.Equal(t, parseCert(t, intermediateCaChain[0]), intermediaryCaCert, "intermediate signed cert should have been part of ca_chain") require.Equal(t, parseCert(t, intermediateCaChain[1]), rootCaCert, "root cert should have been part of ca_chain") - resp, err = CBWrite(b_int, s_int, "intermediate/set-signed", map[string]interface{}{ + _, err = CBWrite(b_int, s_int, "intermediate/set-signed", map[string]interface{}{ "certificate": intermediateCert + "\n" + rootCert + "\n", }) if err != nil { @@ -4092,7 +4092,7 @@ func runFullCAChainTest(t *testing.T, keyType string) { requireCertInCaChainString(t, fullChain, rootCert, "expected full chain to contain root certificate from pki-intermediate/cert/ca_chain") // Make sure when we issue a leaf certificate we get the full chain back. - resp, err = CBWrite(b_int, s_int, "roles/example", map[string]interface{}{ + _, err = CBWrite(b_int, s_int, "roles/example", map[string]interface{}{ "allowed_domains": "example.com", "allow_subdomains": "true", "max_ttl": "1h", @@ -4112,7 +4112,7 @@ func runFullCAChainTest(t *testing.T, keyType string) { // "external" CAs behave as expected. b_ext, s_ext := createBackendWithStorage(t) - resp, err = CBWrite(b_ext, s_ext, "config/ca", map[string]interface{}{ + _, err = CBWrite(b_ext, s_ext, "config/ca", map[string]interface{}{ "pem_bundle": intermediateKey + "\n" + intermediateCert + "\n" + rootCert + "\n", }) if err != nil { @@ -4137,7 +4137,7 @@ func runFullCAChainTest(t *testing.T, keyType string) { } // Now issue a short-lived certificate from our pki-external. - resp, err = CBWrite(b_ext, s_ext, "roles/example", map[string]interface{}{ + _, err = CBWrite(b_ext, s_ext, "roles/example", map[string]interface{}{ "allowed_domains": "example.com", "allow_subdomains": "true", "max_ttl": "1h", @@ -4233,7 +4233,7 @@ func RoleIssuanceRegressionHelper(t *testing.T, b *backend, s logical.Storage, i for _, AllowLocalhost := range test.AllowLocalhost.ToValues() { for _, AllowWildcardCertificates := range test.AllowWildcardCertificates.ToValues() { role := fmt.Sprintf("issuance-regression-%d-bare-%v-glob-%v-subdomains-%v-localhost-%v-wildcard-%v", index, AllowBareDomains, AllowGlobDomains, AllowSubdomains, AllowLocalhost, AllowWildcardCertificates) - resp, err := CBWrite(b, s, "roles/"+role, map[string]interface{}{ + _, err := CBWrite(b, s, "roles/"+role, map[string]interface{}{ "allowed_domains": test.AllowedDomains, "allow_bare_domains": AllowBareDomains, "allow_glob_domains": AllowGlobDomains, @@ -4254,7 +4254,7 @@ func RoleIssuanceRegressionHelper(t *testing.T, b *backend, s logical.Storage, i t.Fatal(err) } - resp, err = CBWrite(b, s, "issue/"+role, map[string]interface{}{ + resp, err := CBWrite(b, s, "issue/"+role, map[string]interface{}{ "common_name": test.CommonName, "exclude_cn_from_sans": true, }) @@ -4470,7 +4470,7 @@ func TestBackend_Roles_IssuanceRegression(t *testing.T) { tested += RoleIssuanceRegressionHelper(t, b, s, index, test) } - t.Log(fmt.Sprintf("Issuance regression expanded matrix test scenarios: %d", tested)) + t.Logf("Issuance regression expanded matrix test scenarios: %d", tested) } type KeySizeRegression struct { @@ -4520,7 +4520,7 @@ func RoleKeySizeRegressionHelper(t *testing.T, b *backend, s logical.Storage, in for _, roleKeyBits := range test.RoleKeyBits { for _, roleSignatureBits := range test.RoleSignatureBits { role := fmt.Sprintf("key-size-regression-%d-keytype-%v-keybits-%d-signature-bits-%d", index, test.RoleKeyType, roleKeyBits, roleSignatureBits) - resp, err := CBWrite(b, s, "roles/"+role, map[string]interface{}{ + _, err := CBWrite(b, s, "roles/"+role, map[string]interface{}{ "key_type": test.RoleKeyType, "key_bits": roleKeyBits, "signature_bits": roleSignatureBits, @@ -4625,7 +4625,7 @@ func TestBackend_Roles_KeySizeRegression(t *testing.T) { tested += RoleKeySizeRegressionHelper(t, b, s, index, test) } - t.Log(fmt.Sprintf("Key size regression expanded matrix test scenarios: %d", tested)) + t.Logf("Key size regression expanded matrix test scenarios: %d", tested) } func TestRootWithExistingKey(t *testing.T) { @@ -4884,7 +4884,7 @@ func TestIssuanceTTLs(t *testing.T) { // Sleep until the parent cert expires and the clock rolls over // to the next second. - time.Sleep(rootCert.NotAfter.Sub(time.Now()) + (1500 * time.Millisecond)) + time.Sleep(time.Until(rootCert.NotAfter) + (1500 * time.Millisecond)) resp, err = CBWrite(b, s, "issuer/root", map[string]interface{}{ "issuer_name": "root", diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 6cb3b6773..71c314fec 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -745,11 +745,10 @@ func signCert(b *backend, csrString := data.apiData.Get("csr").(string) if csrString == "" { - return nil, errutil.UserError{Err: fmt.Sprintf("\"csr\" is empty")} + return nil, errutil.UserError{Err: "\"csr\" is empty"} } - pemBytes := []byte(csrString) - pemBlock, pemBytes := pem.Decode(pemBytes) + pemBlock, _ := pem.Decode([]byte(csrString)) if pemBlock == nil { return nil, errutil.UserError{Err: "csr contains no data"} } @@ -1195,8 +1194,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if csr != nil && data.role.UseCSRSANs { if len(csr.IPAddresses) > 0 { if !data.role.AllowIPSANs { - return nil, errutil.UserError{Err: fmt.Sprintf( - "IP Subject Alternative Names are not allowed in this role, but was provided some via CSR")} + return nil, errutil.UserError{Err: "IP Subject Alternative Names are not allowed in this role, but was provided some via CSR"} } ipAddresses = csr.IPAddresses } @@ -1225,8 +1223,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if len(csr.URIs) > 0 { if len(data.role.AllowedURISANs) == 0 { return nil, errutil.UserError{ - Err: fmt.Sprintf( - "URI Subject Alternative Names are not allowed in this role, but were provided via CSR"), + Err: "URI Subject Alternative Names are not allowed in this role, but were provided via CSR", } } @@ -1235,8 +1232,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn valid := validateURISAN(b, data, uri.String()) if !valid { return nil, errutil.UserError{ - Err: fmt.Sprintf( - "URI Subject Alternative Names were provided via CSR which are not valid for this role"), + Err: "URI Subject Alternative Names were provided via CSR which are not valid for this role", } } @@ -1248,8 +1244,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if len(uriAlt) > 0 { if len(data.role.AllowedURISANs) == 0 { return nil, errutil.UserError{ - Err: fmt.Sprintf( - "URI Subject Alternative Names are not allowed in this role, but were provided via the API"), + Err: "URI Subject Alternative Names are not allowed in this role, but were provided via the API", } } @@ -1257,8 +1252,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn valid := validateURISAN(b, data, uri) if !valid { return nil, errutil.UserError{ - Err: fmt.Sprintf( - "URI Subject Alternative Names were provided via the API which are not valid for this role"), + Err: "URI Subject Alternative Names were provided via the API which are not valid for this role", } } @@ -1307,8 +1301,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn } if ttl > 0 && notAfterAlt != "" { return nil, errutil.UserError{ - Err: fmt.Sprintf( - "Either ttl or not_after should be provided. Both should not be provided in the same request."), + Err: "Either ttl or not_after should be provided. Both should not be provided in the same request.", } } @@ -1516,9 +1509,7 @@ func handleOtherCSRSANs(in *x509.CertificateRequest, sans map[string][]string) e return err } if len(certTemplate.ExtraExtensions) > 0 { - for _, v := range certTemplate.ExtraExtensions { - in.ExtraExtensions = append(in.ExtraExtensions, v) - } + in.ExtraExtensions = append(in.ExtraExtensions, certTemplate.ExtraExtensions...) } return nil } diff --git a/builtin/logical/pki/chain_util.go b/builtin/logical/pki/chain_util.go index 315297c51..334000a7c 100644 --- a/builtin/logical/pki/chain_util.go +++ b/builtin/logical/pki/chain_util.go @@ -267,9 +267,7 @@ func (sc *storageContext) rebuildIssuersChains(referenceCert *issuerEntry /* opt continue } - for _, child := range children { - toVisit = append(toVisit, child) - } + toVisit = append(toVisit, children...) } // Setup the toVisit queue. @@ -582,9 +580,7 @@ func processAnyCliqueOrCycle( continue } - for _, child := range children { - cliquesToProcess = append(cliquesToProcess, child) - } + cliquesToProcess = append(cliquesToProcess, children...) // While we're here, add this cycle node to the closure. closure[cycleNode] = true diff --git a/builtin/logical/pki/crl_test.go b/builtin/logical/pki/crl_test.go index e918f4eb2..b8d2c8bc2 100644 --- a/builtin/logical/pki/crl_test.go +++ b/builtin/logical/pki/crl_test.go @@ -235,7 +235,7 @@ func crlEnableDisableIntermediateTestForBackend(t *testing.T, withRoot bool) { t.Fatal("expected signed intermediate info") } intermediateSignedData := resp.Data - var certs string = intermediateSignedData["certificate"].(string) + certs := intermediateSignedData["certificate"].(string) caSerial := intermediateSignedData["serial_number"].(string) caSerials := []string{caSerial} if withRoot { @@ -244,10 +244,12 @@ func crlEnableDisableIntermediateTestForBackend(t *testing.T, withRoot bool) { caSerials = append(caSerials, rootSerial) } - resp, err = CBWrite(b_int, s_int, "intermediate/set-signed", map[string]interface{}{ + _, err = CBWrite(b_int, s_int, "intermediate/set-signed", map[string]interface{}{ "certificate": certs, }) - + if err != nil { + t.Fatal(err) + } crlEnableDisableTestForBackend(t, b_int, s_int, caSerials) } @@ -404,7 +406,7 @@ func TestCrlRebuilder(t *testing.T) { // Make sure we have ticked over to the next second for { - diff := time.Now().Sub(crl1.ThisUpdate) + diff := time.Since(crl1.ThisUpdate) if diff.Seconds() >= 1 { break } @@ -1097,8 +1099,6 @@ func TestAutoRebuild(t *testing.T) { mainCRL := getParsedCrlAtPath(t, client, "/v1/pki/crl").TBSCertList requireSerialNumberInCRL(t, mainCRL, newLeafSerial) } - - break } } diff --git a/builtin/logical/pki/crl_util.go b/builtin/logical/pki/crl_util.go index e915843e2..6a7f96f9b 100644 --- a/builtin/logical/pki/crl_util.go +++ b/builtin/logical/pki/crl_util.go @@ -1103,7 +1103,7 @@ func getRevokedCertEntries(sc *storageContext, issuerIDCertMap map[issuerID]*x50 // TODO: In this case, remove it and continue? How likely is this to // happen? Alternately, could skip it entirely, or could implement a // delete function so that there is a way to remove these - return nil, nil, errutil.InternalError{Err: fmt.Sprintf("found revoked serial but actual certificate is empty")} + return nil, nil, errutil.InternalError{Err: "found revoked serial but actual certificate is empty"} } err = revokedEntry.DecodeJSON(&revInfo) diff --git a/builtin/logical/pki/ocsp.go b/builtin/logical/pki/ocsp.go index be849cdde..a3711c01d 100644 --- a/builtin/logical/pki/ocsp.go +++ b/builtin/logical/pki/ocsp.go @@ -31,11 +31,10 @@ const ( ) type ocspRespInfo struct { - formattedSerialNumber string - serialNumber *big.Int - ocspStatus int - revocationTimeUTC *time.Time - issuerID issuerID + serialNumber *big.Int + ocspStatus int + revocationTimeUTC *time.Time + issuerID issuerID } // These response variables should not be mutated, instead treat them as constants diff --git a/builtin/logical/pki/ocsp_test.go b/builtin/logical/pki/ocsp_test.go index 6e11c8b22..d14914cf5 100644 --- a/builtin/logical/pki/ocsp_test.go +++ b/builtin/logical/pki/ocsp_test.go @@ -608,12 +608,12 @@ func requireOcspResponseSignedBy(t *testing.T, ocspResp *ocsp.Response, key cryp hasher.Write(ocspResp.TBSResponseData) hashData := hasher.Sum(nil) - switch key.(type) { + switch typedKey := key.(type) { case *rsa.PublicKey: - err := rsa.VerifyPKCS1v15(key.(*rsa.PublicKey), hashAlgo, hashData, ocspResp.Signature) + err := rsa.VerifyPKCS1v15(typedKey, hashAlgo, hashData, ocspResp.Signature) require.NoError(t, err, "the ocsp response was not signed by the expected public rsa key.") case *ecdsa.PublicKey: - verify := ecdsa.VerifyASN1(key.(*ecdsa.PublicKey), hashData, ocspResp.Signature) + verify := ecdsa.VerifyASN1(typedKey, hashData, ocspResp.Signature) require.True(t, verify, "the certificate was not signed by the expected public ecdsa key.") } } diff --git a/builtin/logical/pki/path_config_crl.go b/builtin/logical/pki/path_config_crl.go index 34ca5b549..30b8047b5 100644 --- a/builtin/logical/pki/path_config_crl.go +++ b/builtin/logical/pki/path_config_crl.go @@ -198,7 +198,7 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra } if config.EnableDelta && !config.AutoRebuild { - return logical.ErrorResponse(fmt.Sprintf("Delta CRLs cannot be enabled when auto rebuilding is disabled as the complete CRL is always regenerated!")), nil + return logical.ErrorResponse("Delta CRLs cannot be enabled when auto rebuilding is disabled as the complete CRL is always regenerated!"), nil } entry, err := logical.StorageEntryJSON("config/crl", config) diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index 6e56eb1b9..ef167c516 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -361,7 +361,7 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da if newUsage != issuer.Usage { if issuer.Revoked && newUsage.HasUsage(IssuanceUsage) { // Forbid allowing cert signing on its usage. - return logical.ErrorResponse(fmt.Sprintf("This issuer was revoked; unable to modify its usage to include certificate signing again. Reissue this certificate (preferably with a new key) and modify that entry instead.")), nil + return logical.ErrorResponse("This issuer was revoked; unable to modify its usage to include certificate signing again. Reissue this certificate (preferably with a new key) and modify that entry instead."), nil } // Ensure we deny adding CRL usage if the bits are missing from the @@ -371,7 +371,7 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da return nil, fmt.Errorf("unable to parse issuer's certificate: %v", err) } if (cert.KeyUsage&x509.KeyUsageCRLSign) == 0 && newUsage.HasUsage(CRLSigningUsage) { - return logical.ErrorResponse(fmt.Sprintf("This issuer's underlying certificate lacks the CRLSign KeyUsage value; unable to set CRLSigningUsage on this issuer as a result.")), nil + return logical.ErrorResponse("This issuer's underlying certificate lacks the CRLSign KeyUsage value; unable to set CRLSigningUsage on this issuer as a result."), nil } issuer.Usage = newUsage @@ -576,7 +576,7 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat if newUsage != issuer.Usage { if issuer.Revoked && newUsage.HasUsage(IssuanceUsage) { // Forbid allowing cert signing on its usage. - return logical.ErrorResponse(fmt.Sprintf("This issuer was revoked; unable to modify its usage to include certificate signing again. Reissue this certificate (preferably with a new key) and modify that entry instead.")), nil + return logical.ErrorResponse("This issuer was revoked; unable to modify its usage to include certificate signing again. Reissue this certificate (preferably with a new key) and modify that entry instead."), nil } cert, err := issuer.GetCertificate() @@ -584,7 +584,7 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat return nil, fmt.Errorf("unable to parse issuer's certificate: %v", err) } if (cert.KeyUsage&x509.KeyUsageCRLSign) == 0 && newUsage.HasUsage(CRLSigningUsage) { - return logical.ErrorResponse(fmt.Sprintf("This issuer's underlying certificate lacks the CRLSign KeyUsage value; unable to set CRLSigningUsage on this issuer as a result.")), nil + return logical.ErrorResponse("This issuer's underlying certificate lacks the CRLSign KeyUsage value; unable to set CRLSigningUsage on this issuer as a result."), nil } issuer.Usage = newUsage diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index c50898598..e523de074 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -385,7 +385,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d switch { case role.GenerateLease == nil: return nil, fmt.Errorf("generate lease in role is nil") - case *role.GenerateLease == false: + case !*role.GenerateLease: // If lease generation is disabled do not populate `Secret` field in // the response resp = &logical.Response{ diff --git a/builtin/logical/pki/path_revoke.go b/builtin/logical/pki/path_revoke.go index 23bdce0ed..d33c013ca 100644 --- a/builtin/logical/pki/path_revoke.go +++ b/builtin/logical/pki/path_revoke.go @@ -156,7 +156,7 @@ func (b *backend) pathRevokeWriteHandleCertificate(ctx context.Context, req *log // Ensure we have a well-formed serial number before continuing. serial := serialFromCert(certReference) if len(serial) == 0 { - return "", false, nil, errutil.UserError{Err: fmt.Sprintf("invalid serial number on presented certificate")} + return "", false, nil, errutil.UserError{Err: "invalid serial number on presented certificate"} } // We have two approaches here: we could start verifying against issuers @@ -230,7 +230,7 @@ func (b *backend) pathRevokeWriteHandleCertificate(ctx context.Context, req *log return serial, true, certReference.Raw, nil } - return serial, false, nil, errutil.UserError{Err: fmt.Sprintf("unable to verify signature on presented cert from any present issuer in this mount; certificates from previous CAs will need to have their issuing CA and key re-imported if revocation is necessary")} + return serial, false, nil, errutil.UserError{Err: "unable to verify signature on presented cert from any present issuer in this mount; certificates from previous CAs will need to have their issuing CA and key re-imported if revocation is necessary"} } func (b *backend) pathRevokeWriteHandleKey(ctx context.Context, req *logical.Request, cert []byte, keyPem string) error { diff --git a/builtin/logical/pki/path_roles.go b/builtin/logical/pki/path_roles.go index f20c79eb5..89a899a8c 100644 --- a/builtin/logical/pki/path_roles.go +++ b/builtin/logical/pki/path_roles.go @@ -933,7 +933,7 @@ func (b *backend) pathRolePatch(ctx context.Context, req *logical.Request, data // no_store implies generate_lease := false if entry.NoStore { *entry.GenerateLease = false - if ok && generateLease.(bool) || !ok && (*oldEntry.GenerateLease == true) { + if ok && generateLease.(bool) || !ok && *oldEntry.GenerateLease { warning = "mutually exclusive values no_store=true and generate_lease=true were both specified; no_store=true takes priority" } } else { diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index 5d3114444..474cff9c1 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -603,7 +603,7 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ } if config.Enabled && !(config.CertStore || config.RevokedCerts || config.IssuerAssocs) { - return logical.ErrorResponse(fmt.Sprintf("Auto-tidy enabled but no tidy operations were requested. Enable at least one tidy operation to be run (tidy_cert_store / tidy_revoked_certs / tidy_revoked_cert_issuer_associations).")), nil + return logical.ErrorResponse("Auto-tidy enabled but no tidy operations were requested. Enable at least one tidy operation to be run (tidy_cert_store / tidy_revoked_certs / tidy_revoked_cert_issuer_associations)."), nil } return nil, sc.writeAutoTidyConfig(config) diff --git a/builtin/logical/pki/path_tidy_test.go b/builtin/logical/pki/path_tidy_test.go index 661959867..af0d1199e 100644 --- a/builtin/logical/pki/path_tidy_test.go +++ b/builtin/logical/pki/path_tidy_test.go @@ -112,7 +112,7 @@ func TestAutoTidy(t *testing.T) { require.NotEmpty(t, resp.Data["certificate"]) // Wait for cert to expire and the safety buffer to elapse. - time.Sleep(leafCert.NotAfter.Sub(time.Now()) + 3*time.Second) + time.Sleep(time.Until(leafCert.NotAfter) + 3*time.Second) // Wait for auto-tidy to run afterwards. var foundTidyRunning string diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index a2a80f954..c89c9352a 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -210,7 +210,7 @@ func (sc *storageContext) listKeys() ([]keyID, error) { func (sc *storageContext) fetchKeyById(keyId keyID) (*keyEntry, error) { if len(keyId) == 0 { - return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch pki key: empty key identifier")} + return nil, errutil.InternalError{Err: "unable to fetch pki key: empty key identifier"} } entry, err := sc.Storage.Get(sc.Context, keyPrefix+keyId.String()) @@ -566,7 +566,7 @@ func (sc *storageContext) resolveKeyReference(reference string) (keyID, error) { // fetchIssuerById returns an issuerEntry based on issuerId, if none found an error is returned. func (sc *storageContext) fetchIssuerById(issuerId issuerID) (*issuerEntry, error) { if len(issuerId) == 0 { - return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch pki issuer: empty issuer identifier")} + return nil, errutil.InternalError{Err: "unable to fetch pki issuer: empty issuer identifier"} } entry, err := sc.Storage.Get(sc.Context, issuerPrefix+issuerId.String()) @@ -807,7 +807,7 @@ func (sc *storageContext) importIssuer(certValue string, issuerName string) (*is } func areCertificatesEqual(cert1 *x509.Certificate, cert2 *x509.Certificate) bool { - return bytes.Compare(cert1.Raw, cert2.Raw) == 0 + return bytes.Equal(cert1.Raw, cert2.Raw) } func (sc *storageContext) setLocalCRLConfig(mapping *localCRLConfigEntry) error { diff --git a/builtin/logical/pki/test_helpers.go b/builtin/logical/pki/test_helpers.go index 6e6ecff37..907bafe58 100644 --- a/builtin/logical/pki/test_helpers.go +++ b/builtin/logical/pki/test_helpers.go @@ -14,7 +14,7 @@ import ( "encoding/pem" "fmt" "hash" - "io/ioutil" + "io" "strings" "testing" @@ -41,8 +41,7 @@ func createBackendWithStorage(t testing.TB) (*backend, logical.Storage) { } func mountPKIEndpoint(t testing.TB, client *api.Client, path string) { - var err error - err = client.Sys().Mount(path, &api.MountInput{ + err := client.Sys().Mount(path, &api.MountInput{ Type: "pki", Config: api.MountConfigInput{ DefaultLeaseTTL: "16h", @@ -54,13 +53,13 @@ func mountPKIEndpoint(t testing.TB, client *api.Client, path string) { // Signing helpers func requireSignedBy(t *testing.T, cert *x509.Certificate, key crypto.PublicKey) { - switch key.(type) { + switch typedKey := key.(type) { case *rsa.PublicKey: - requireRSASignedBy(t, cert, key.(*rsa.PublicKey)) + requireRSASignedBy(t, cert, typedKey) case *ecdsa.PublicKey: - requireECDSASignedBy(t, cert, key.(*ecdsa.PublicKey)) + requireECDSASignedBy(t, cert, typedKey) case ed25519.PublicKey: - requireED25519SignedBy(t, cert, key.(ed25519.PublicKey)) + requireED25519SignedBy(t, cert, typedKey) default: require.Fail(t, "unknown public key type %#v", key) } @@ -181,42 +180,6 @@ func getParsedCrl(t *testing.T, client *api.Client, mountPoint string) *pkix.Cer return getParsedCrlAtPath(t, client, path) } -func getParsedCrlForIssuer(t *testing.T, client *api.Client, mountPoint string, issuer string) *pkix.CertificateList { - path := fmt.Sprintf("/v1/%v/issuer/%v/crl/der", mountPoint, issuer) - crl := getParsedCrlAtPath(t, client, path) - - // Now fetch the issuer as well and verify the certificate - path = fmt.Sprintf("/v1/%v/issuer/%v/der", mountPoint, issuer) - req := client.NewRequest("GET", path) - resp, err := client.RawRequest(req) - if err != nil { - t.Fatal(err) - } - defer resp.Body.Close() - - certBytes, err := ioutil.ReadAll(resp.Body) - if err != nil { - t.Fatalf("err: %s", err) - } - if len(certBytes) == 0 { - t.Fatalf("expected certificate in response body") - } - - cert, err := x509.ParseCertificate(certBytes) - if err != nil { - t.Fatal(err) - } - if cert == nil { - t.Fatalf("expected parsed certificate") - } - - if err := cert.CheckCRLSignature(crl); err != nil { - t.Fatalf("expected valid signature on CRL for issuer %v: %v", issuer, crl) - } - - return crl -} - func getParsedCrlAtPath(t *testing.T, client *api.Client, path string) *pkix.CertificateList { req := client.NewRequest("GET", path) resp, err := client.RawRequest(req) @@ -225,7 +188,7 @@ func getParsedCrlAtPath(t *testing.T, client *api.Client, path string) *pkix.Cer } defer resp.Body.Close() - crlBytes, err := ioutil.ReadAll(resp.Body) + crlBytes, err := io.ReadAll(resp.Body) if err != nil { t.Fatalf("err: %s", err) } diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index b651da373..ba2ca7a65 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -117,7 +117,7 @@ func getKeyRefWithErr(data *framework.FieldData) (string, error) { keyRef := getKeyRef(data) if len(keyRef) == 0 { - return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_ref for existing type")} + return "", errutil.UserError{Err: "missing argument key_ref for existing type"} } return keyRef, nil