luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

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 <alex.scheel@hashicorp.com>

* Add warning when issuer has PSS-based RevSigAlgo

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add note about OCSP and PSS support

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2022-10-06 12:01:12 -04:00 committed by GitHub
parent c32fb1bd61
commit b85d6ec434
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 55 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
builtin/logical/pki/ocsp.go
View File

@ -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 {

10
builtin/logical/pki/path_fetch_issuers.go
View File

@ -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) {

1
website/content/api-docs/secret/pki.mdx
View File

@ -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.

7
website/content/docs/secrets/pki/considerations.mdx
View File

@ -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),