From 1ab5b68916633dc95da2eda176d98a109ca8fe73 Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Wed, 23 Mar 2022 15:19:56 -0400 Subject: [PATCH] PKI: Add missing default cases within switch statements (#14661) * Misc PKI code fixes. - Harden the code base a bit adding default's to switch statements to various error handlers and processing statements. - Fixup some error messages to include proper values we support. * Additional default case missing within PKI * Fix typo in PKI error message --- builtin/logical/pki/ca_util.go | 4 +-- builtin/logical/pki/cert_util.go | 8 +++-- builtin/logical/pki/crl_util.go | 42 ++++++++++++++---------- builtin/logical/pki/path_intermediate.go | 2 ++ builtin/logical/pki/path_issue_sign.go | 2 ++ builtin/logical/pki/path_root.go | 4 +++ 6 files changed, 39 insertions(+), 23 deletions(-) diff --git a/builtin/logical/pki/ca_util.go b/builtin/logical/pki/ca_util.go index 93aa48f1e4..e7a9e87004 100644 --- a/builtin/logical/pki/ca_util.go +++ b/builtin/logical/pki/ca_util.go @@ -25,14 +25,14 @@ func (b *backend) getGenerationParams(ctx context.Context, case "kms": default: errorResp = logical.ErrorResponse( - `the "exported" path parameter must be "internal" or "exported"`) + `the "exported" path parameter must be "internal", "exported" or "kms"`) return } format = getFormat(data) if format == "" { errorResp = logical.ErrorResponse( - `the "format" path parameter must be "pem", "der", "der_pkcs", or "pem_bundle"`) + `the "format" path parameter must be "pem", "der", or "pem_bundle"`) return } diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 1fb3cb1b91..301f4534d6 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -114,7 +114,7 @@ func fetchCAInfo(ctx context.Context, b *backend, req *logical.Request) (*certut return nil, errutil.InternalError{Err: "stored CA information not able to be parsed"} } - caInfo := &certutil.CAInfoBundle{*parsedBundle, nil} + caInfo := &certutil.CAInfoBundle{ParsedCertBundle: *parsedBundle, URLs: nil} entries, err := getURLs(ctx, req) if err != nil { @@ -721,7 +721,7 @@ func signCert(b *backend, case "ed25519": // Verify that the key matches the role type - if csr.PublicKeyAlgorithm != x509.PublicKeyAlgorithm(x509.Ed25519) { + if csr.PublicKeyAlgorithm != x509.Ed25519 { return nil, errutil.UserError{Err: fmt.Sprintf( "role requires keys of type %s", data.role.KeyType)} @@ -747,6 +747,8 @@ func signCert(b *backend, return nil, errutil.UserError{Err: "RSA keys < 2048 bits are unsafe and not supported"} } + default: + return nil, errutil.InternalError{Err: fmt.Sprintf("unsupported key type value: %s", data.role.KeyType)} } creation, err := generateCreationBundle(b, data, caSign, csr) @@ -1441,5 +1443,5 @@ func stringToOid(in string) (asn1.ObjectIdentifier, error) { } ret = append(ret, i) } - return asn1.ObjectIdentifier(ret), nil + return ret, nil } diff --git a/builtin/logical/pki/crl_util.go b/builtin/logical/pki/crl_util.go index 94b286c0f7..2fd0e33fd2 100644 --- a/builtin/logical/pki/crl_util.go +++ b/builtin/logical/pki/crl_util.go @@ -33,12 +33,15 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st } signingBundle, caErr := fetchCAInfo(ctx, b, req) - switch caErr.(type) { - case errutil.UserError: - return logical.ErrorResponse(fmt.Sprintf("could not fetch the CA certificate: %s", caErr)), nil - case errutil.InternalError: - return nil, fmt.Errorf("error fetching CA certificate: %s", caErr) + if caErr != nil { + switch caErr.(type) { + case errutil.UserError: + return logical.ErrorResponse(fmt.Sprintf("could not fetch the CA certificate: %s", caErr)), nil + default: + return nil, fmt.Errorf("error fetching CA certificate: %s", caErr) + } } + if signingBundle == nil { return nil, errors.New("CA info not found") } @@ -55,7 +58,7 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st switch err.(type) { case errutil.UserError: return logical.ErrorResponse(err.Error()), nil - case errutil.InternalError: + default: return nil, err } } @@ -74,7 +77,7 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st switch err.(type) { case errutil.UserError: return logical.ErrorResponse(err.Error()), nil - case errutil.InternalError: + default: return nil, err } } @@ -123,15 +126,16 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st if err != nil { return nil, fmt.Errorf("error saving revoked certificate to new location") } - } crlErr := buildCRL(ctx, b, req, false) - switch crlErr.(type) { - case errutil.UserError: - return logical.ErrorResponse(fmt.Sprintf("Error during CRL building: %s", crlErr)), nil - case errutil.InternalError: - return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr) + if crlErr != nil { + switch crlErr.(type) { + case errutil.UserError: + return logical.ErrorResponse(fmt.Sprintf("Error during CRL building: %s", crlErr)), nil + default: + return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr) + } } resp := &logical.Response{ @@ -220,11 +224,13 @@ func buildCRL(ctx context.Context, b *backend, req *logical.Request, forceNew bo WRITE: signingBundle, caErr := fetchCAInfo(ctx, b, req) - switch caErr.(type) { - case errutil.UserError: - return errutil.UserError{Err: fmt.Sprintf("could not fetch the CA certificate: %s", caErr)} - case errutil.InternalError: - return errutil.InternalError{Err: fmt.Sprintf("error fetching CA certificate: %s", caErr)} + if caErr != nil { + switch caErr.(type) { + case errutil.UserError: + return errutil.UserError{Err: fmt.Sprintf("could not fetch the CA certificate: %s", caErr)} + default: + return errutil.InternalError{Err: fmt.Sprintf("error fetching CA certificate: %s", caErr)} + } } crlBytes, err := signingBundle.Certificate.CreateCRL(rand.Reader, signingBundle.PrivateKey, revokedCerts, time.Now(), time.Now().Add(crlLifetime)) diff --git a/builtin/logical/pki/path_intermediate.go b/builtin/logical/pki/path_intermediate.go index 77e4d4a545..4e1e766eae 100644 --- a/builtin/logical/pki/path_intermediate.go +++ b/builtin/logical/pki/path_intermediate.go @@ -124,6 +124,8 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req resp.Data["private_key"] = base64.StdEncoding.EncodeToString(parsedBundle.PrivateKeyBytes) resp.Data["private_key_type"] = csrb.PrivateKeyType } + default: + return nil, fmt.Errorf("unsupported format argument: %s", format) } if data.Get("private_key_format").(string) == "pkcs8" { diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 5123a3ad3a..218964f3ac 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -263,6 +263,8 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d respData["private_key"] = base64.StdEncoding.EncodeToString(parsedBundle.PrivateKeyBytes) respData["private_key_type"] = cb.PrivateKeyType } + default: + return nil, fmt.Errorf("unsupported format: %s", format) } var resp *logical.Response diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 4cc607e125..2edd4b181f 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -220,6 +220,8 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, resp.Data["private_key"] = base64.StdEncoding.EncodeToString(parsedBundle.PrivateKeyBytes) resp.Data["private_key_type"] = cb.PrivateKeyType } + default: + return nil, fmt.Errorf("unsupported format argument: %s", format) } if data.Get("private_key_format").(string) == "pkcs8" { @@ -396,6 +398,8 @@ func (b *backend) pathCASignIntermediate(ctx context.Context, req *logical.Reque if caChain != nil && len(caChain) > 0 { resp.Data["ca_chain"] = cb.CAChain } + default: + return nil, fmt.Errorf("unsupported format argument: %s", format) } err = req.Storage.Put(ctx, &logical.StorageEntry{