letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Match revocation reason and request signing method (#5713) 

Match revocation reason and request signing method

Add more detailed logging about request signing methods
This commit is contained in:
Andrew Gabbitas 2021-10-14 15:39:22 -06:00 committed by GitHub
parent 18e5f405ed
commit ba673673a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 113 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
test/integration/revocation_test.go
View File

@ -1,3 +1,4 @@
//go:build integration
// +build integration
package integration
@ -15,6 +16,7 @@ import (
"testing"
"time"
"github.com/eggsampler/acme/v3"
"github.com/letsencrypt/boulder/test"
ocsp_helper "github.com/letsencrypt/boulder/test/ocsp/helper"
"golang.org/x/crypto/ocsp"
@ -153,9 +155,9 @@ func TestRevokeWithKeyCompromise(t *testing.T) {
cert := res.certs[0]
err = c.RevokeCertificate(
c.Account,
acme.Account{},
cert,
c.Account.PrivateKey,
certKey,
ocsp.KeyCompromise,
)
test.AssertNotError(t, err, "failed to revoke certificate")
@ -202,9 +204,9 @@ func TestBadKeyRevoker(t *testing.T) {
}
err = cA.RevokeCertificate(
cA.Account,
acme.Account{},
badCert.certs[0],
cA.Account.PrivateKey,
certKey,
ocsp.KeyCompromise,
)
test.AssertNotError(t, err, "failed to revoke certificate")
@ -243,5 +245,5 @@ func TestBadKeyRevoker(t *testing.T) {
defer func() { _ = zeroCountResp.Body.Close() }()
body, err = ioutil.ReadAll(zeroCountResp.Body)
test.AssertNotError(t, err, "failed to read body")
test.AssertEquals(t, string(body), "0\n")
test.AssertEquals(t, string(body), "1\n")
}

6
test/v2_integration.py
View File

@ -1565,8 +1565,8 @@ def ocsp_resigning_setup():
cert = OpenSSL.crypto.load_certificate(
OpenSSL.crypto.FILETYPE_PEM, order.fullchain_pem)
# Revoke for reason 1: keyCompromise
client.revoke(josepy.ComparableX509(cert), 1)
# Revoke for reason 3: affiliationChanged
client.revoke(josepy.ComparableX509(cert), 3)
ocsp_response, reason = get_ocsp_response_and_reason(
cert_file.name, "/tmp/intermediate-cert-rsa-a.pem", "http://localhost:4002")
@ -1596,5 +1596,5 @@ def test_ocsp_resigning():
if reason != ocsp_resigning_setup_data['reason']:
raise(Exception("re-signed ocsp response has different reason %s expected %s" % (
reason, ocsp_resigning_setup_data['reason'])))
if reason != "keyCompromise":
if reason != "affiliationChanged":
raise(Exception("re-signed ocsp response has wrong reason %s" % reason))

49
wfe2/wfe.go
View File

@ -36,6 +36,7 @@ import (
sapb "github.com/letsencrypt/boulder/sa/proto"
"github.com/letsencrypt/boulder/web"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/crypto/ocsp"
"google.golang.org/protobuf/types/known/emptypb"
jose "gopkg.in/square/go-jose.v2"
)
@ -771,7 +772,7 @@ func (wfe *WebFrontEndImpl) acctHoldsAuthorizations(ctx context.Context, acctID
// revoke the certificate a problem is returned. It is expected to be a closure
// containing additional state (an account ID or key) that will be used to make
// the decision.
type authorizedToRevokeCert func(*x509.Certificate) *probs.ProblemDetails
type authorizedToRevokeCert func(*x509.Certificate, revocation.Reason) *probs.ProblemDetails
// processRevocation accepts the payload for a revocation request along with
// an account ID and a callback used to decide if the requester is authorized to
@ -846,12 +847,6 @@ func (wfe *WebFrontEndImpl) processRevocation(
return probs.AlreadyRevoked("Certificate already revoked")
}
// Validate that the requester is authenticated to revoke the given certificate
prob := authorizedToRevoke(parsedCertificate)
if prob != nil {
return prob
}
// Verify the revocation reason supplied is allowed
reason := revocation.Reason(0)
if revokeRequest.Reason != nil {
@ -869,6 +864,12 @@ func (wfe *WebFrontEndImpl) processRevocation(
reason = *revokeRequest.Reason
}
// Validate that the requester is authenticated to revoke the given certificate
prob := authorizedToRevoke(parsedCertificate, reason)
if prob != nil {
return prob
}
// Revoke the certificate. AcctID may be 0 if there is no associated account
// (e.g. it was a self-authenticated JWS using the certificate public key)
_, err = wfe.RA.RevokeCertificateWithReg(ctx, &rapb.RevokeCertificateWithRegRequest{
@ -897,6 +898,13 @@ func (wfe *WebFrontEndImpl) processRevocation(
return nil
}
type revocationEvidence struct {
Serial string
Reason revocation.Reason
RegID int64
Method string
}
// revokeCertByKeyID processes an outer JWS as a revocation request that is
// authenticated by a KeyID and the associated account.
func (wfe *WebFrontEndImpl) revokeCertByKeyID(
@ -913,7 +921,12 @@ func (wfe *WebFrontEndImpl) revokeCertByKeyID(
// For Key ID revocations we decide if an account is able to revoke a specific
// certificate by checking that the account has valid authorizations for all
// of the names in the certificate or was the issuing account
authorizedToRevoke := func(parsedCertificate *x509.Certificate) *probs.ProblemDetails {
authorizedToRevoke := func(parsedCertificate *x509.Certificate, reason revocation.Reason) *probs.ProblemDetails {
// If revocation reason is keyCompromise, reject the request.
if reason == revocation.Reason(ocsp.KeyCompromise) {
return probs.Unauthorized("Revocation with reason keyCompromise is only supported by signing with the certificate private key")
}
// Try to find a stored final certificate for the serial number
serial := core.SerialToString(parsedCertificate.SerialNumber)
cert, err := wfe.SA.GetCertificate(ctx, &sapb.Serial{Serial: serial})
@ -938,6 +951,12 @@ func (wfe *WebFrontEndImpl) revokeCertByKeyID(
// If the cert/precert is owned by the requester then return nil, it is an
// authorized revocation.
if cert.RegistrationID == acct.ID {
wfe.log.AuditObject("Authorizing revocation", revocationEvidence{
Serial: core.SerialToString(parsedCertificate.SerialNumber),
Reason: reason,
RegID: acct.ID,
Method: "owner",
})
return nil
}
// Otherwise check if the account, while not the owner, has equivalent authorizations
@ -950,6 +969,12 @@ func (wfe *WebFrontEndImpl) revokeCertByKeyID(
return probs.Unauthorized(
"The key ID specified in the revocation request does not hold valid authorizations for all names in the certificate to be revoked")
}
wfe.log.AuditObject("Authorizing revocation", revocationEvidence{
Serial: core.SerialToString(parsedCertificate.SerialNumber),
Reason: reason,
RegID: acct.ID,
Method: "authorizations",
})
// If it does, return nil. It is an an authorized revocation.
return nil
}
@ -980,11 +1005,17 @@ func (wfe *WebFrontEndImpl) revokeCertByJWK(
// For embedded JWK revocations we decide if a requester is able to revoke a specific
// certificate by checking that to-be-revoked certificate has the same public
// key as the JWK that was used to authenticate the request
authorizedToRevoke := func(parsedCertificate *x509.Certificate) *probs.ProblemDetails {
authorizedToRevoke := func(parsedCertificate *x509.Certificate, reason revocation.Reason) *probs.ProblemDetails {
if !core.KeyDigestEquals(requestKey, parsedCertificate.PublicKey) {
return probs.Unauthorized(
"JWK embedded in revocation request must be the same public key as the cert to be revoked")
}
wfe.log.AuditObject("Authorizing revocation", revocationEvidence{
Serial: core.SerialToString(parsedCertificate.SerialNumber),
Reason: reason,
RegID: 0,
Method: "privkey",
})
return nil
}
// We use `0` as the account ID provided to `processRevocation` because this

63
wfe2/wfe_test.go
View File

@ -26,6 +26,7 @@ import (
"github.com/jmhodges/clock"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/crypto/ocsp"
"google.golang.org/grpc"
"google.golang.org/protobuf/types/known/emptypb"
jose "gopkg.in/square/go-jose.v2"
@ -2934,6 +2935,56 @@ func TestRevokeCertificateValid(t *testing.T) {
test.AssertEquals(t, responseWriter.Body.String(), "")
}
// A revocation request with reason == keyCompromise should only succeed
// if it was signed by the private key.
func TestRevokeCertificateKeyCompromiseValid(t *testing.T) {
wfe, _ := setupWFE(t)
wfe.SA = newMockSAWithCert(t, wfe.SA, core.OCSPStatusGood)
mockLog := wfe.log.(*blog.Mock)
mockLog.Clear()
keyPemBytes, err := ioutil.ReadFile("../test/hierarchy/ee-r3.key.pem")
test.AssertNotError(t, err, "Failed to load key")
key := loadKey(t, keyPemBytes)
revocationReason := revocation.Reason(ocsp.KeyCompromise)
revokeRequestJSON, err := makeRevokeRequestJSON(&revocationReason)
test.AssertNotError(t, err, "Failed to make revokeRequestJSON")
_, _, jwsBody := signRequestEmbed(t,
key, "http://localhost/revoke-cert", string(revokeRequestJSON), wfe.nonceService)
responseWriter := httptest.NewRecorder()
wfe.RevokeCertificate(ctx, newRequestEvent(), responseWriter,
makePostRequestWithPath("revoke-cert", jwsBody))
test.AssertEquals(t, responseWriter.Code, 200)
test.AssertEquals(t, responseWriter.Body.String(), "")
test.AssertDeepEquals(t, mockLog.GetAllMatching("Authorizing revocation"), []string{
`INFO: [AUDIT] Authorizing revocation JSON={"Serial":"000000000000000000001d72443db5189821","Reason":1,"RegID":0,"Method":"privkey"}`,
})
}
func TestRevokeCertificateKeyCompromiseInvalid(t *testing.T) {
wfe, _ := setupWFE(t)
wfe.SA = newMockSAWithCert(t, wfe.SA, core.OCSPStatusGood)
revocationReason := revocation.Reason(ocsp.KeyCompromise)
revokeRequestJSON, err := makeRevokeRequestJSON(&revocationReason)
test.AssertNotError(t, err, "Failed to make revokeRequestJSON")
// NOTE: this account doesn't have any authorizations for the
// names in the cert, but it is the account that issued it
// originally
_, _, jwsBody := signRequestKeyID(
t, 1, nil, "http://localhost/revoke-cert", string(revokeRequestJSON), wfe.nonceService)
responseWriter := httptest.NewRecorder()
wfe.RevokeCertificate(ctx, newRequestEvent(), responseWriter,
makePostRequestWithPath("revoke-cert", jwsBody))
test.AssertEquals(t, responseWriter.Code, 403)
test.AssertEquals(t, responseWriter.Body.String(), "{\n \"type\": \"urn:ietf:params:acme:error:unauthorized\",\n \"detail\": \"Revocation with reason keyCompromise is only supported by signing with the certificate private key\",\n \"status\": 403\n}")
}
// Invalid revocation request: although signed with the cert key, the cert
// wasn't issued by any issuer the Boulder is aware of.
func TestRevokeCertificateNotIssued(t *testing.T) {
@ -3048,6 +3099,9 @@ func TestRevokeCertificateIssuingAccount(t *testing.T) {
wfe, _ := setupWFE(t)
wfe.SA = newMockSAWithCert(t, wfe.SA, core.OCSPStatusGood)
mockLog := wfe.log.(*blog.Mock)
mockLog.Clear()
revokeRequestJSON, err := makeRevokeRequestJSON(nil)
test.AssertNotError(t, err, "Failed to make revokeRequestJSON")
// NOTE: this account doesn't have any authorizations for the
@ -3062,6 +3116,9 @@ func TestRevokeCertificateIssuingAccount(t *testing.T) {
test.AssertEquals(t, responseWriter.Code, 200)
test.AssertEquals(t, responseWriter.Body.String(), "")
test.AssertDeepEquals(t, mockLog.GetAllMatching("Authorizing revocation"), []string{
`INFO: [AUDIT] Authorizing revocation JSON={"Serial":"000000000000000000001d72443db5189821","Reason":0,"RegID":1,"Method":"owner"}`,
})
}
type mockSAWithValidAuthz struct {
@ -3085,6 +3142,9 @@ func TestRevokeCertificateWithAuthorizations(t *testing.T) {
wfe, _ := setupWFE(t)
wfe.SA = mockSAWithValidAuthz{newMockSAWithCert(t, wfe.SA, core.OCSPStatusGood)}
mockLog := wfe.log.(*blog.Mock)
mockLog.Clear()
revokeRequestJSON, err := makeRevokeRequestJSON(nil)
test.AssertNotError(t, err, "Failed to make revokeRequestJSON")
@ -3099,6 +3159,9 @@ func TestRevokeCertificateWithAuthorizations(t *testing.T) {
makePostRequestWithPath("revoke-cert", jwsBody))
test.AssertEquals(t, responseWriter.Code, 200)
test.AssertEquals(t, responseWriter.Body.String(), "")
test.AssertDeepEquals(t, mockLog.GetAllMatching("Authorizing revocation"), []string{
`INFO: [AUDIT] Authorizing revocation JSON={"Serial":"000000000000000000001d72443db5189821","Reason":0,"RegID":5,"Method":"authorizations"}`,
})
}
// A revocation request signed by an unauthorized key.