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
This commit is contained in:
Steven Clark
2022-08-31 16:25:14 -04:00
committed by GitHub
parent 4099ca7704
commit 9cbd80b51e
17 changed files with 81 additions and 132 deletions

View File

@@ -128,7 +128,7 @@ func TestPKI_RequireCN(t *testing.T) {
// Issue a cert with require_cn set to true and with common name supplied. // Issue a cert with require_cn set to true and with common name supplied.
// It should succeed. // 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", "common_name": "foobar.com",
}) })
if err != nil { 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 // Issue a cert with require_cn set to true and with out supplying the
// common name. It should error out. // 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 { if err == nil {
t.Fatalf("expected an error due to missing common_name") 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 cert := parsedCertBundle.Certificate
actualDiff := time.Now().Sub(cert.NotBefore) actualDiff := time.Since(cert.NotBefore)
certRoleDiff := (role.NotBeforeDuration - actualDiff).Truncate(time.Second) certRoleDiff := (role.NotBeforeDuration - actualDiff).Truncate(time.Second)
// These times get truncated, so give a 1 second buffer on each side // These times get truncated, so give a 1 second buffer on each side
if certRoleDiff >= -1*time.Second && certRoleDiff <= 1*time.Second { 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) return fmt.Errorf("error parsing cert bundle: %s", err)
} }
cert := parsedCertBundle.Certificate cert := parsedCertBundle.Certificate
var emptyIPs []net.IP var expected []net.IP
var expected []net.IP = append(emptyIPs, expectedIp...) expected = append(expected, expectedIp...)
if diff := deep.Equal(cert.IPAddresses, expected); len(diff) > 0 { if diff := deep.Equal(cert.IPAddresses, expected); len(diff) > 0 {
return fmt.Errorf("wrong SAN IPs, diff: %v", diff) 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 { if err != nil {
return err return err
} }
var emptyOthers []otherNameUtf8 var expected []otherNameUtf8
var expected []otherNameUtf8 = append(emptyOthers, expectedOthers...) expected = append(expected, expectedOthers...)
if diff := deep.Equal(foundOthers, expected); len(diff) > 0 { if diff := deep.Equal(foundOthers, expected); len(diff) > 0 {
return fmt.Errorf("wrong SAN IPs, diff: %v", diff) 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) t.Fatalf("failed read ca/pem, %#v", resp)
} }
// check the raw cert matches the response body // 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") 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, Operation: logical.UpdateOperation,
Path: "roles/example", Path: "roles/example",
Storage: storage, Storage: storage,
@@ -1927,7 +1927,7 @@ func TestBackend_PathFetchValidRaw(t *testing.T) {
// check the raw cert matches the response body // check the raw cert matches the response body
rawBody := resp.Data[logical.HTTPRawBody].([]byte) rawBody := resp.Data[logical.HTTPRawBody].([]byte)
bodyAsPem := []byte(strings.TrimSpace(string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: rawBody})))) 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) t.Fatalf("failed to get raw cert for serial number: %s", expectedSerial)
} }
if resp.Data[logical.HTTPContentType] != "application/pkix-cert" { 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 // 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") t.Fatalf("failed to get pem cert")
} }
if resp.Data[logical.HTTPContentType] != "application/pem-certificate-chain" { 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) 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{ resp, err = b.HandleRequest(context.Background(), &logical.Request{
Operation: logical.UpdateOperation, Operation: logical.UpdateOperation,
Path: "root/sign-self-issued", Path: "root/sign-self-issued",
@@ -2765,7 +2765,7 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) {
// Test with flag present and true // Test with flag present and true
ss, _ = getSelfSigned(t, template, template, key) ss, _ = getSelfSigned(t, template, template, key)
resp, err = b.HandleRequest(context.Background(), &logical.Request{ _, err = b.HandleRequest(context.Background(), &logical.Request{
Operation: logical.UpdateOperation, Operation: logical.UpdateOperation,
Path: "root/sign-self-issued", Path: "root/sign-self-issued",
Storage: storage, Storage: storage,
@@ -2866,7 +2866,7 @@ func TestBackend_OID_SANs(t *testing.T) {
} }
// First test some bad stuff that shouldn't work // 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", "common_name": "foobar.com",
"ip_sans": "1.2.3.4", "ip_sans": "1.2.3.4",
"alt_names": "foo.foobar.com,bar.foobar.com", "alt_names": "foo.foobar.com,bar.foobar.com",
@@ -2878,7 +2878,7 @@ func TestBackend_OID_SANs(t *testing.T) {
t.Fatal("expected error") 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", "common_name": "foobar.com",
"ip_sans": "1.2.3.4", "ip_sans": "1.2.3.4",
"alt_names": "foo.foobar.com,bar.foobar.com", "alt_names": "foo.foobar.com,bar.foobar.com",
@@ -2890,7 +2890,7 @@ func TestBackend_OID_SANs(t *testing.T) {
t.Fatal("expected error") 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", "common_name": "foobar.com",
"ip_sans": "1.2.3.4", "ip_sans": "1.2.3.4",
"alt_names": "foo.foobar.com,bar.foobar.com", "alt_names": "foo.foobar.com,bar.foobar.com",
@@ -2902,7 +2902,7 @@ func TestBackend_OID_SANs(t *testing.T) {
t.Fatal("expected error") 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", "common_name": "foobar.com",
"ip_sans": "1.2.3.4", "ip_sans": "1.2.3.4",
"alt_names": "foo.foobar.com,bar.foobar.com", "alt_names": "foo.foobar.com,bar.foobar.com",
@@ -2914,7 +2914,7 @@ func TestBackend_OID_SANs(t *testing.T) {
t.Fatal("expected error") 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", "common_name": "foobar.com",
"ip_sans": "1.2.3.4", "ip_sans": "1.2.3.4",
"alt_names": "foo.foobar.com,bar.foobar.com", "alt_names": "foo.foobar.com,bar.foobar.com",
@@ -3058,7 +3058,7 @@ func TestBackend_AllowedSerialNumbers(t *testing.T) {
t.Fatal(err) 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", "common_name": "foobar",
"ttl": "1h", "ttl": "1h",
}) })
@@ -3066,7 +3066,7 @@ func TestBackend_AllowedSerialNumbers(t *testing.T) {
t.Fatal(err) 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", "common_name": "foobar",
"ttl": "1h", "ttl": "1h",
"serial_number": "foobar", "serial_number": "foobar",
@@ -3085,7 +3085,7 @@ func TestBackend_AllowedSerialNumbers(t *testing.T) {
t.Fatal(err) 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", "common_name": "foobar",
"ttl": "1h", "ttl": "1h",
// Not a valid serial number // Not a valid serial number
@@ -3669,7 +3669,7 @@ func setCerts() {
if err != nil { if err != nil {
panic(err) panic(err)
} }
subjKeyID, err = certutil.GetSubjKeyID(rak) _, err = certutil.GetSubjKeyID(rak)
if err != nil { if err != nil {
panic(err) panic(err)
} }
@@ -3699,7 +3699,7 @@ func setCerts() {
if err != nil { if err != nil {
panic(err) panic(err)
} }
subjKeyID, err = certutil.GetSubjKeyID(edk) _, err = certutil.GetSubjKeyID(edk)
if err != nil { if err != nil {
panic(err) 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") 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. // 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", "allowed_domains": "example.com",
"allow_subdomains": "true", "allow_subdomains": "true",
"max_ttl": "1h", "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[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") 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", "certificate": intermediateCert + "\n" + rootCert + "\n",
}) })
if err != nil { 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") 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. // 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", "allowed_domains": "example.com",
"allow_subdomains": "true", "allow_subdomains": "true",
"max_ttl": "1h", "max_ttl": "1h",
@@ -4112,7 +4112,7 @@ func runFullCAChainTest(t *testing.T, keyType string) {
// "external" CAs behave as expected. // "external" CAs behave as expected.
b_ext, s_ext := createBackendWithStorage(t) 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", "pem_bundle": intermediateKey + "\n" + intermediateCert + "\n" + rootCert + "\n",
}) })
if err != nil { if err != nil {
@@ -4137,7 +4137,7 @@ func runFullCAChainTest(t *testing.T, keyType string) {
} }
// Now issue a short-lived certificate from our pki-external. // 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", "allowed_domains": "example.com",
"allow_subdomains": "true", "allow_subdomains": "true",
"max_ttl": "1h", "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 _, AllowLocalhost := range test.AllowLocalhost.ToValues() {
for _, AllowWildcardCertificates := range test.AllowWildcardCertificates.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) 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, "allowed_domains": test.AllowedDomains,
"allow_bare_domains": AllowBareDomains, "allow_bare_domains": AllowBareDomains,
"allow_glob_domains": AllowGlobDomains, "allow_glob_domains": AllowGlobDomains,
@@ -4254,7 +4254,7 @@ func RoleIssuanceRegressionHelper(t *testing.T, b *backend, s logical.Storage, i
t.Fatal(err) 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, "common_name": test.CommonName,
"exclude_cn_from_sans": true, "exclude_cn_from_sans": true,
}) })
@@ -4470,7 +4470,7 @@ func TestBackend_Roles_IssuanceRegression(t *testing.T) {
tested += RoleIssuanceRegressionHelper(t, b, s, index, test) 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 { type KeySizeRegression struct {
@@ -4520,7 +4520,7 @@ func RoleKeySizeRegressionHelper(t *testing.T, b *backend, s logical.Storage, in
for _, roleKeyBits := range test.RoleKeyBits { for _, roleKeyBits := range test.RoleKeyBits {
for _, roleSignatureBits := range test.RoleSignatureBits { for _, roleSignatureBits := range test.RoleSignatureBits {
role := fmt.Sprintf("key-size-regression-%d-keytype-%v-keybits-%d-signature-bits-%d", index, test.RoleKeyType, roleKeyBits, 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_type": test.RoleKeyType,
"key_bits": roleKeyBits, "key_bits": roleKeyBits,
"signature_bits": roleSignatureBits, "signature_bits": roleSignatureBits,
@@ -4625,7 +4625,7 @@ func TestBackend_Roles_KeySizeRegression(t *testing.T) {
tested += RoleKeySizeRegressionHelper(t, b, s, index, test) 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) { 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 // Sleep until the parent cert expires and the clock rolls over
// to the next second. // 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{}{ resp, err = CBWrite(b, s, "issuer/root", map[string]interface{}{
"issuer_name": "root", "issuer_name": "root",

View File

@@ -745,11 +745,10 @@ func signCert(b *backend,
csrString := data.apiData.Get("csr").(string) csrString := data.apiData.Get("csr").(string)
if csrString == "" { if csrString == "" {
return nil, errutil.UserError{Err: fmt.Sprintf("\"csr\" is empty")} return nil, errutil.UserError{Err: "\"csr\" is empty"}
} }
pemBytes := []byte(csrString) pemBlock, _ := pem.Decode([]byte(csrString))
pemBlock, pemBytes := pem.Decode(pemBytes)
if pemBlock == nil { if pemBlock == nil {
return nil, errutil.UserError{Err: "csr contains no data"} 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 csr != nil && data.role.UseCSRSANs {
if len(csr.IPAddresses) > 0 { if len(csr.IPAddresses) > 0 {
if !data.role.AllowIPSANs { if !data.role.AllowIPSANs {
return nil, errutil.UserError{Err: fmt.Sprintf( return nil, errutil.UserError{Err: "IP Subject Alternative Names are not allowed in this role, but was provided some via CSR"}
"IP Subject Alternative Names are not allowed in this role, but was provided some via CSR")}
} }
ipAddresses = csr.IPAddresses ipAddresses = csr.IPAddresses
} }
@@ -1225,8 +1223,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn
if len(csr.URIs) > 0 { if len(csr.URIs) > 0 {
if len(data.role.AllowedURISANs) == 0 { if len(data.role.AllowedURISANs) == 0 {
return nil, errutil.UserError{ return nil, errutil.UserError{
Err: fmt.Sprintf( Err: "URI Subject Alternative Names are not allowed in this role, but were provided via CSR",
"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()) valid := validateURISAN(b, data, uri.String())
if !valid { if !valid {
return nil, errutil.UserError{ return nil, errutil.UserError{
Err: fmt.Sprintf( Err: "URI Subject Alternative Names were provided via CSR which are not valid for this role",
"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(uriAlt) > 0 {
if len(data.role.AllowedURISANs) == 0 { if len(data.role.AllowedURISANs) == 0 {
return nil, errutil.UserError{ return nil, errutil.UserError{
Err: fmt.Sprintf( Err: "URI Subject Alternative Names are not allowed in this role, but were provided via the API",
"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) valid := validateURISAN(b, data, uri)
if !valid { if !valid {
return nil, errutil.UserError{ return nil, errutil.UserError{
Err: fmt.Sprintf( Err: "URI Subject Alternative Names were provided via the API which are not valid for this role",
"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 != "" { if ttl > 0 && notAfterAlt != "" {
return nil, errutil.UserError{ return nil, errutil.UserError{
Err: fmt.Sprintf( Err: "Either ttl or not_after should be provided. Both should not be provided in the same request.",
"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 return err
} }
if len(certTemplate.ExtraExtensions) > 0 { if len(certTemplate.ExtraExtensions) > 0 {
for _, v := range certTemplate.ExtraExtensions { in.ExtraExtensions = append(in.ExtraExtensions, certTemplate.ExtraExtensions...)
in.ExtraExtensions = append(in.ExtraExtensions, v)
}
} }
return nil return nil
} }

View File

@@ -267,9 +267,7 @@ func (sc *storageContext) rebuildIssuersChains(referenceCert *issuerEntry /* opt
continue continue
} }
for _, child := range children { toVisit = append(toVisit, children...)
toVisit = append(toVisit, child)
}
} }
// Setup the toVisit queue. // Setup the toVisit queue.
@@ -582,9 +580,7 @@ func processAnyCliqueOrCycle(
continue continue
} }
for _, child := range children { cliquesToProcess = append(cliquesToProcess, children...)
cliquesToProcess = append(cliquesToProcess, child)
}
// While we're here, add this cycle node to the closure. // While we're here, add this cycle node to the closure.
closure[cycleNode] = true closure[cycleNode] = true

View File

@@ -235,7 +235,7 @@ func crlEnableDisableIntermediateTestForBackend(t *testing.T, withRoot bool) {
t.Fatal("expected signed intermediate info") t.Fatal("expected signed intermediate info")
} }
intermediateSignedData := resp.Data intermediateSignedData := resp.Data
var certs string = intermediateSignedData["certificate"].(string) certs := intermediateSignedData["certificate"].(string)
caSerial := intermediateSignedData["serial_number"].(string) caSerial := intermediateSignedData["serial_number"].(string)
caSerials := []string{caSerial} caSerials := []string{caSerial}
if withRoot { if withRoot {
@@ -244,10 +244,12 @@ func crlEnableDisableIntermediateTestForBackend(t *testing.T, withRoot bool) {
caSerials = append(caSerials, rootSerial) 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, "certificate": certs,
}) })
if err != nil {
t.Fatal(err)
}
crlEnableDisableTestForBackend(t, b_int, s_int, caSerials) 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 // Make sure we have ticked over to the next second
for { for {
diff := time.Now().Sub(crl1.ThisUpdate) diff := time.Since(crl1.ThisUpdate)
if diff.Seconds() >= 1 { if diff.Seconds() >= 1 {
break break
} }
@@ -1097,8 +1099,6 @@ func TestAutoRebuild(t *testing.T) {
mainCRL := getParsedCrlAtPath(t, client, "/v1/pki/crl").TBSCertList mainCRL := getParsedCrlAtPath(t, client, "/v1/pki/crl").TBSCertList
requireSerialNumberInCRL(t, mainCRL, newLeafSerial) requireSerialNumberInCRL(t, mainCRL, newLeafSerial)
} }
break
} }
} }

View File

@@ -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 // TODO: In this case, remove it and continue? How likely is this to
// happen? Alternately, could skip it entirely, or could implement a // happen? Alternately, could skip it entirely, or could implement a
// delete function so that there is a way to remove these // 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) err = revokedEntry.DecodeJSON(&revInfo)

View File

@@ -31,11 +31,10 @@ const (
) )
type ocspRespInfo struct { type ocspRespInfo struct {
formattedSerialNumber string serialNumber *big.Int
serialNumber *big.Int ocspStatus int
ocspStatus int revocationTimeUTC *time.Time
revocationTimeUTC *time.Time issuerID issuerID
issuerID issuerID
} }
// These response variables should not be mutated, instead treat them as constants // These response variables should not be mutated, instead treat them as constants

View File

@@ -608,12 +608,12 @@ func requireOcspResponseSignedBy(t *testing.T, ocspResp *ocsp.Response, key cryp
hasher.Write(ocspResp.TBSResponseData) hasher.Write(ocspResp.TBSResponseData)
hashData := hasher.Sum(nil) hashData := hasher.Sum(nil)
switch key.(type) { switch typedKey := key.(type) {
case *rsa.PublicKey: 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.") require.NoError(t, err, "the ocsp response was not signed by the expected public rsa key.")
case *ecdsa.PublicKey: 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.") require.True(t, verify, "the certificate was not signed by the expected public ecdsa key.")
} }
} }

View File

@@ -198,7 +198,7 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
} }
if config.EnableDelta && !config.AutoRebuild { 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) entry, err := logical.StorageEntryJSON("config/crl", config)

View File

@@ -361,7 +361,7 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
if newUsage != issuer.Usage { if newUsage != issuer.Usage {
if issuer.Revoked && newUsage.HasUsage(IssuanceUsage) { if issuer.Revoked && newUsage.HasUsage(IssuanceUsage) {
// Forbid allowing cert signing on its usage. // 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 // 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) return nil, fmt.Errorf("unable to parse issuer's certificate: %v", err)
} }
if (cert.KeyUsage&x509.KeyUsageCRLSign) == 0 && newUsage.HasUsage(CRLSigningUsage) { 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 issuer.Usage = newUsage
@@ -576,7 +576,7 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat
if newUsage != issuer.Usage { if newUsage != issuer.Usage {
if issuer.Revoked && newUsage.HasUsage(IssuanceUsage) { if issuer.Revoked && newUsage.HasUsage(IssuanceUsage) {
// Forbid allowing cert signing on its usage. // 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() 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) return nil, fmt.Errorf("unable to parse issuer's certificate: %v", err)
} }
if (cert.KeyUsage&x509.KeyUsageCRLSign) == 0 && newUsage.HasUsage(CRLSigningUsage) { 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 issuer.Usage = newUsage

View File

@@ -385,7 +385,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
switch { switch {
case role.GenerateLease == nil: case role.GenerateLease == nil:
return nil, fmt.Errorf("generate lease in role is 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 // If lease generation is disabled do not populate `Secret` field in
// the response // the response
resp = &logical.Response{ resp = &logical.Response{

View File

@@ -156,7 +156,7 @@ func (b *backend) pathRevokeWriteHandleCertificate(ctx context.Context, req *log
// Ensure we have a well-formed serial number before continuing. // Ensure we have a well-formed serial number before continuing.
serial := serialFromCert(certReference) serial := serialFromCert(certReference)
if len(serial) == 0 { 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 // 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, 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 { func (b *backend) pathRevokeWriteHandleKey(ctx context.Context, req *logical.Request, cert []byte, keyPem string) error {

View File

@@ -933,7 +933,7 @@ func (b *backend) pathRolePatch(ctx context.Context, req *logical.Request, data
// no_store implies generate_lease := false // no_store implies generate_lease := false
if entry.NoStore { if entry.NoStore {
*entry.GenerateLease = false *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" warning = "mutually exclusive values no_store=true and generate_lease=true were both specified; no_store=true takes priority"
} }
} else { } else {

View File

@@ -603,7 +603,7 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ
} }
if config.Enabled && !(config.CertStore || config.RevokedCerts || config.IssuerAssocs) { 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) return nil, sc.writeAutoTidyConfig(config)

View File

@@ -112,7 +112,7 @@ func TestAutoTidy(t *testing.T) {
require.NotEmpty(t, resp.Data["certificate"]) require.NotEmpty(t, resp.Data["certificate"])
// Wait for cert to expire and the safety buffer to elapse. // 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. // Wait for auto-tidy to run afterwards.
var foundTidyRunning string var foundTidyRunning string

View File

@@ -210,7 +210,7 @@ func (sc *storageContext) listKeys() ([]keyID, error) {
func (sc *storageContext) fetchKeyById(keyId keyID) (*keyEntry, error) { func (sc *storageContext) fetchKeyById(keyId keyID) (*keyEntry, error) {
if len(keyId) == 0 { 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()) 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. // fetchIssuerById returns an issuerEntry based on issuerId, if none found an error is returned.
func (sc *storageContext) fetchIssuerById(issuerId issuerID) (*issuerEntry, error) { func (sc *storageContext) fetchIssuerById(issuerId issuerID) (*issuerEntry, error) {
if len(issuerId) == 0 { 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()) 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 { 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 { func (sc *storageContext) setLocalCRLConfig(mapping *localCRLConfigEntry) error {

View File

@@ -14,7 +14,7 @@ import (
"encoding/pem" "encoding/pem"
"fmt" "fmt"
"hash" "hash"
"io/ioutil" "io"
"strings" "strings"
"testing" "testing"
@@ -41,8 +41,7 @@ func createBackendWithStorage(t testing.TB) (*backend, logical.Storage) {
} }
func mountPKIEndpoint(t testing.TB, client *api.Client, path string) { 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", Type: "pki",
Config: api.MountConfigInput{ Config: api.MountConfigInput{
DefaultLeaseTTL: "16h", DefaultLeaseTTL: "16h",
@@ -54,13 +53,13 @@ func mountPKIEndpoint(t testing.TB, client *api.Client, path string) {
// Signing helpers // Signing helpers
func requireSignedBy(t *testing.T, cert *x509.Certificate, key crypto.PublicKey) { func requireSignedBy(t *testing.T, cert *x509.Certificate, key crypto.PublicKey) {
switch key.(type) { switch typedKey := key.(type) {
case *rsa.PublicKey: case *rsa.PublicKey:
requireRSASignedBy(t, cert, key.(*rsa.PublicKey)) requireRSASignedBy(t, cert, typedKey)
case *ecdsa.PublicKey: case *ecdsa.PublicKey:
requireECDSASignedBy(t, cert, key.(*ecdsa.PublicKey)) requireECDSASignedBy(t, cert, typedKey)
case ed25519.PublicKey: case ed25519.PublicKey:
requireED25519SignedBy(t, cert, key.(ed25519.PublicKey)) requireED25519SignedBy(t, cert, typedKey)
default: default:
require.Fail(t, "unknown public key type %#v", key) 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) 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 { func getParsedCrlAtPath(t *testing.T, client *api.Client, path string) *pkix.CertificateList {
req := client.NewRequest("GET", path) req := client.NewRequest("GET", path)
resp, err := client.RawRequest(req) 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() defer resp.Body.Close()
crlBytes, err := ioutil.ReadAll(resp.Body) crlBytes, err := io.ReadAll(resp.Body)
if err != nil { if err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }

View File

@@ -117,7 +117,7 @@ func getKeyRefWithErr(data *framework.FieldData) (string, error) {
keyRef := getKeyRef(data) keyRef := getKeyRef(data)
if len(keyRef) == 0 { 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 return keyRef, nil