From b85d6ec43411c68fa1269f8943e90f35c4e447c5 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Thu, 6 Oct 2022 12:01:12 -0400 Subject: [PATCH] Fix RevocationSigAlgo support in OCSP (#17436) * Allow OCSP to use issuer's RevocationSigAlgo When an issuer specifies a RevocationSigAlgo, we should largely follow this for both CRLs and OCSP. However, x/crypto/ocsp lacks support for PSS signatures, so we drop these down to PKCS#1v1.5 instead. Signed-off-by: Alexander Scheel * Add warning when issuer has PSS-based RevSigAlgo Signed-off-by: Alexander Scheel * Add note about OCSP and PSS support Signed-off-by: Alexander Scheel Signed-off-by: Alexander Scheel --- builtin/logical/pki/ocsp.go | 58 +++++++++++++------ builtin/logical/pki/path_fetch_issuers.go | 10 +++- website/content/api-docs/secret/pki.mdx | 1 + .../docs/secrets/pki/considerations.mdx | 7 +++ 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/builtin/logical/pki/ocsp.go b/builtin/logical/pki/ocsp.go index a3711c01d..da7e9e27d 100644 --- a/builtin/logical/pki/ocsp.go +++ b/builtin/logical/pki/ocsp.go @@ -122,7 +122,7 @@ func (b *backend) ocspHandler(ctx context.Context, request *logical.Request, dat return logAndReturnInternalError(b, err), nil } - caBundle, err := lookupOcspIssuer(sc, ocspReq, ocspStatus.issuerID) + caBundle, issuer, err := lookupOcspIssuer(sc, ocspReq, ocspStatus.issuerID) if err != nil { if errors.Is(err, ErrUnknownIssuer) { // Since we were not able to find a matching issuer for the incoming request @@ -140,7 +140,7 @@ func (b *backend) ocspHandler(ctx context.Context, request *logical.Request, dat return logAndReturnInternalError(b, err), nil } - byteResp, err := genResponse(cfg, caBundle, ocspStatus, ocspReq.HashAlgorithm) + byteResp, err := genResponse(cfg, caBundle, ocspStatus, ocspReq.HashAlgorithm, issuer.RevocationSigAlg) if err != nil { return logAndReturnInternalError(b, err), nil } @@ -188,7 +188,7 @@ func generateUnknownResponse(cfg *crlConfig, sc *storageContext, ocspReq *ocsp.R ocspStatus: ocsp.Unknown, } - byteResp, err := genResponse(cfg, caBundle, info, ocspReq.HashAlgorithm) + byteResp, err := genResponse(cfg, caBundle, info, ocspReq.HashAlgorithm, issuer.RevocationSigAlg) if err != nil { return logAndReturnInternalError(sc.Backend, err) } @@ -269,10 +269,10 @@ func getOcspStatus(sc *storageContext, request *logical.Request, ocspReq *ocsp.R return &info, nil } -func lookupOcspIssuer(sc *storageContext, req *ocsp.Request, optRevokedIssuer issuerID) (*certutil.ParsedCertBundle, error) { +func lookupOcspIssuer(sc *storageContext, req *ocsp.Request, optRevokedIssuer issuerID) (*certutil.ParsedCertBundle, *issuerEntry, error) { reqHash := req.HashAlgorithm if !reqHash.Available() { - return nil, x509.ErrUnsupportedAlgorithm + return nil, nil, x509.ErrUnsupportedAlgorithm } // This will prime up issuerIds, with either the optRevokedIssuer value if set @@ -280,7 +280,7 @@ func lookupOcspIssuer(sc *storageContext, req *ocsp.Request, optRevokedIssuer is // a list of all our issuers in this mount. issuerIds, err := lookupIssuerIds(sc, optRevokedIssuer) if err != nil { - return nil, err + return nil, nil, err } matchedButNoUsage := false @@ -294,7 +294,7 @@ func lookupOcspIssuer(sc *storageContext, req *ocsp.Request, optRevokedIssuer is // This skips either bad issuer ids, or root certs with no keys that we can't use. continue } - return nil, err + return nil, nil, err } // Make sure the client and Vault are talking about the same issuer, otherwise @@ -302,7 +302,7 @@ func lookupOcspIssuer(sc *storageContext, req *ocsp.Request, optRevokedIssuer is // we should not respond back in the affirmative about. matches, err := doesRequestMatchIssuer(parsedBundle, req) if err != nil { - return nil, err + return nil, nil, err } if matches { @@ -314,16 +314,16 @@ func lookupOcspIssuer(sc *storageContext, req *ocsp.Request, optRevokedIssuer is continue } - return parsedBundle, nil + return parsedBundle, issuer, nil } } if matchedButNoUsage { // We matched an issuer but it did not have an OCSP signing usage set so bail. - return nil, ErrMissingOcspUsage + return nil, nil, ErrMissingOcspUsage } - return nil, ErrUnknownIssuer + return nil, nil, ErrUnknownIssuer } func getOcspIssuerParsedBundle(sc *storageContext, issuerId issuerID) (*certutil.ParsedCertBundle, *issuerEntry, error) { @@ -383,20 +383,40 @@ func doesRequestMatchIssuer(parsedBundle *certutil.ParsedCertBundle, req *ocsp.R return bytes.Equal(req.IssuerKeyHash, issuerKeyHash) && bytes.Equal(req.IssuerNameHash, issuerNameHash), nil } -func genResponse(cfg *crlConfig, caBundle *certutil.ParsedCertBundle, info *ocspRespInfo, reqHash crypto.Hash) ([]byte, error) { +func genResponse(cfg *crlConfig, caBundle *certutil.ParsedCertBundle, info *ocspRespInfo, reqHash crypto.Hash, revSigAlg x509.SignatureAlgorithm) ([]byte, error) { curTime := time.Now() duration, err := time.ParseDuration(cfg.OcspExpiry) if err != nil { return nil, err } + + // x/ocsp lives outside of the standard library's crypto/x509 and includes + // ripped-off variants of many internal structures and functions. These + // lack support for PSS signatures altogether, so if we have revSigAlg + // that uses PSS, downgrade it to PKCS#1v1.5. This fixes the lack of + // support in x/ocsp, at the risk of OCSP requests failing due to lack + // of PKCS#1v1.5 (in say, PKCS#11 HSMs or GCP). + // + // Other restrictions, such as hash function selection, will still work + // however. + switch revSigAlg { + case x509.SHA256WithRSAPSS: + revSigAlg = x509.SHA256WithRSA + case x509.SHA384WithRSAPSS: + revSigAlg = x509.SHA384WithRSA + case x509.SHA512WithRSAPSS: + revSigAlg = x509.SHA512WithRSA + } + template := ocsp.Response{ - IssuerHash: reqHash, - Status: info.ocspStatus, - SerialNumber: info.serialNumber, - ThisUpdate: curTime, - NextUpdate: curTime.Add(duration), - Certificate: caBundle.Certificate, - ExtraExtensions: []pkix.Extension{}, + IssuerHash: reqHash, + Status: info.ocspStatus, + SerialNumber: info.serialNumber, + ThisUpdate: curTime, + NextUpdate: curTime.Add(duration), + Certificate: caBundle.Certificate, + ExtraExtensions: []pkix.Extension{}, + SignatureAlgorithm: revSigAlg, } if info.ocspStatus == ocsp.Revoked { diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index 2f4402a50..b052689e0 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -239,9 +239,15 @@ func respondReadIssuer(issuer *issuerEntry) (*logical.Response, error) { data["ocsp_servers"] = issuer.AIAURIs.OCSPServers } - return &logical.Response{ + response := &logical.Response{ Data: data, - }, nil + } + + if issuer.RevocationSigAlg == x509.SHA256WithRSAPSS || issuer.RevocationSigAlg == x509.SHA384WithRSAPSS || issuer.RevocationSigAlg == x509.SHA512WithRSAPSS { + response.AddWarning("Issuer uses a PSS Revocation Signature Algorithm. This algorithm will be downgraded to PKCS#1v1.5 signature scheme on OCSP responses, due to limitations in the OCSP library.") + } + + return response, nil } func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index 63bbad807..b2c64ec53 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -1200,6 +1200,7 @@ At this time there are certain limitations of the OCSP implementation at this pa 1. None of the extensions defined in the RFC are supported for requests or responses 1. Ed25519 backed CA's are not supported for OCSP requests 1. Note that this api will not work with the Vault client as both request and responses are DER encoded + 1. Note that KMS based issuers which require PSS support are not supported either (such as PKCS#11 HSMs or GCP in certain scenarios). These are unauthenticated endpoints. diff --git a/website/content/docs/secrets/pki/considerations.mdx b/website/content/docs/secrets/pki/considerations.mdx index d4ebe3ed4..ae259bb6d 100644 --- a/website/content/docs/secrets/pki/considerations.mdx +++ b/website/content/docs/secrets/pki/considerations.mdx @@ -597,6 +597,13 @@ attempting to generate a CSR will fail signature verification. In this case the CSR will need to be generated outside of Vault and the signed version can be imported into the mount. +Go additionally lacks support for creating OCSP responses with the PSS +signature algorithm. Vault will automatically downgrade issuers with +PSS-based revocation signature algorithms to PKCS#1v1.5, but note that +certain KMS devices (like HSMs and GCP) may not support this with the +same key. As a result, the OCSP responder may fail to sign responses, +returning an internal error. + ## Issuer Subjects and CRLs As noted on several [GitHub issues](https://github.com/hashicorp/vault/issues/10176),