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

Handful of revocation pkg cleanups (#4801) 

When we originally added this package (4 years ago) x/crypto/ocsp didn't
have its own list of revocation reasons, so we added our own. Now it does
have its own list, so just use that list instead of duplicating code for
no real reason.

Also we build a list of the revocation reasons we support so that we can
tell users when they try to use an unsupported one. Instead of building
this string every time, just build it once it during package initialization.

Finally return the same error message in wfe that we use in wfe2 when a
user requests an unsupported reason.
This commit is contained in:
Roland Bracewell Shoemaker 2020-04-30 17:29:42 -07:00 committed by GitHub
parent 739686ba88
commit 97390560a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 46 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cmd/bad-key-revoker/main.go
View File

@ -21,9 +21,9 @@ import (
"github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/mail"
rapb "github.com/letsencrypt/boulder/ra/proto"
"github.com/letsencrypt/boulder/revocation"
"github.com/letsencrypt/boulder/sa"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/crypto/ocsp"
"google.golang.org/grpc"
)
@ -192,7 +192,7 @@ func (bkr *badKeyRevoker) sendMessage(addr string, serials []string) error {
return nil
}
var keyCompromiseCode = int64(revocation.KeyCompromise)
var keyCompromiseCode = int64(ocsp.KeyCompromise)
var revokerName = "bad-key-revoker"
// revokeCerts revokes all the certificates associated with a particular key hash and sends

3
ra/ra.go
View File

@ -39,6 +39,7 @@ import (
"github.com/letsencrypt/boulder/web"
"github.com/prometheus/client_golang/prometheus"
"github.com/weppos/publicsuffix-go/publicsuffix"
"golang.org/x/crypto/ocsp"
grpc "google.golang.org/grpc"
)
@ -1713,7 +1714,7 @@ func (ra *RegistrationAuthorityImpl) revokeCertificate(ctx context.Context, cert
if err != nil {
return err
}
if features.Enabled(features.BlockedKeyTable) && reason == revocation.KeyCompromise {
if features.Enabled(features.BlockedKeyTable) && reason == ocsp.KeyCompromise {
digest, err := core.KeyDigest(cert.PublicKey)
if err != nil {
return err

8
ra/ra_test.go
View File

@ -49,7 +49,6 @@ import (
pubpb "github.com/letsencrypt/boulder/publisher/proto"
rapb "github.com/letsencrypt/boulder/ra/proto"
"github.com/letsencrypt/boulder/ratelimit"
"github.com/letsencrypt/boulder/revocation"
"github.com/letsencrypt/boulder/sa"
sapb "github.com/letsencrypt/boulder/sa/proto"
"github.com/letsencrypt/boulder/test"
@ -57,6 +56,7 @@ import (
vaPB "github.com/letsencrypt/boulder/va/proto"
"github.com/prometheus/client_golang/prometheus"
"github.com/weppos/publicsuffix-go/publicsuffix"
"golang.org/x/crypto/ocsp"
"google.golang.org/grpc"
jose "gopkg.in/square/go-jose.v2"
)
@ -3863,11 +3863,11 @@ func TestRevocationAddBlockedKey(t *testing.T) {
test.AssertNotError(t, err, "x509.ParseCertificate failed")
ra.issuer = cert
err = ra.RevokeCertificateWithReg(context.Background(), *cert, revocation.Unspecified, 0)
err = ra.RevokeCertificateWithReg(context.Background(), *cert, ocsp.Unspecified, 0)
test.AssertNotError(t, err, "RevokeCertificateWithReg failed")
test.Assert(t, mockSA.added == nil, "blocked key was added when reason was not keyCompromise")
err = ra.RevokeCertificateWithReg(context.Background(), *cert, revocation.KeyCompromise, 0)
err = ra.RevokeCertificateWithReg(context.Background(), *cert, ocsp.KeyCompromise, 0)
test.AssertNotError(t, err, "RevokeCertificateWithReg failed")
test.Assert(t, mockSA.added != nil, "blocked key was not added when reason was keyCompromise")
test.Assert(t, bytes.Equal(digest[:], mockSA.added.KeyHash), "key hash mismatch")
@ -3875,7 +3875,7 @@ func TestRevocationAddBlockedKey(t *testing.T) {
test.Assert(t, mockSA.added.Comment == nil, "Comment is not nil")
mockSA.added = nil
err = ra.AdministrativelyRevokeCertificate(context.Background(), *cert, revocation.KeyCompromise, "root")
err = ra.AdministrativelyRevokeCertificate(context.Background(), *cert, ocsp.KeyCompromise, "root")
test.AssertNotError(t, err, "AdministrativelyRevokeCertificate failed")
test.Assert(t, mockSA.added != nil, "blocked key was not added when reason was keyCompromise")
test.Assert(t, bytes.Equal(digest[:], mockSA.added.KeyHash), "key hash mismatch")

61
revocation/reasons.go
View File

@ -4,58 +4,45 @@ import (
"fmt"
"sort"
"strings"
"golang.org/x/crypto/ocsp"
)
// Reason is used to specify a certificate revocation reason
type Reason int
const (
// Definitions for these codes can be found in Section 8.5.3.1 of ITU-T X.509
// http://www.itu.int/rec/T-REC-X.509-201210-I/en
Unspecified = 0
KeyCompromise = 1
CACompromise = 2
AffiliationChanged = 3
Superseded = 4
CessationOfOperation = 5
CertificateHold = 6
// 7 is unused
RemoveFromCRL = 8
PrivilegeWithdrawn = 9
AACompromise = 10
)
// RevocationReasons provides a map from reason code to string explaining the
// code
// ReasonToString provides a map from reason code to string
var ReasonToString = map[Reason]string{
Unspecified: "unspecified",
KeyCompromise: "keyCompromise",
CACompromise: "cACompromise",
AffiliationChanged: "affiliationChanged",
Superseded: "superseded",
CessationOfOperation: "cessationOfOperation",
CertificateHold: "certificateHold",
ocsp.Unspecified: "unspecified",
ocsp.KeyCompromise: "keyCompromise",
ocsp.CACompromise: "cACompromise",
ocsp.AffiliationChanged: "affiliationChanged",
ocsp.Superseded: "superseded",
ocsp.CessationOfOperation: "cessationOfOperation",
ocsp.CertificateHold: "certificateHold",
// 7 is unused
RemoveFromCRL: "removeFromCRL",
PrivilegeWithdrawn: "privilegeWithdrawn",
AACompromise: "aAcompromise",
ocsp.RemoveFromCRL: "removeFromCRL",
ocsp.PrivilegeWithdrawn: "privilegeWithdrawn",
ocsp.AACompromise: "aAcompromise",
}
// UserAllowedReasons contains the subset of Reasons which users are
// allowed to use
var UserAllowedReasons = map[Reason]struct{}{
Unspecified: {}, // unspecified
KeyCompromise: {}, // keyCompromise
AffiliationChanged: {}, // affiliationChanged
Superseded: {}, // superseded
CessationOfOperation: {}, // cessationOfOperation
ocsp.Unspecified: {}, // unspecified
ocsp.KeyCompromise: {}, // keyCompromise
ocsp.AffiliationChanged: {}, // affiliationChanged
ocsp.Superseded: {}, // superseded
ocsp.CessationOfOperation: {}, // cessationOfOperation
}
// UserAllowedReasonsMessage creates a string describing a list of user allowed
// UserAllowedReasonsMessage contains a string describing a list of user allowed
// revocation reasons. This is useful when a revocation is rejected because it
// is not a valid user supplied reason and the allowed values must be
// communicated.
func UserAllowedReasonsMessage() string {
// communicated. This variable is populated during package initialization.
var UserAllowedReasonsMessage = ""
func init() {
// Build a slice of ints from the allowed reason codes.
// We want a slice because iterating `UserAllowedReasons` will change order
// and make the message unpredictable and cumbersome for unit testing.
@ -71,5 +58,5 @@ func UserAllowedReasonsMessage() string {
reasonStrings = append(reasonStrings, fmt.Sprintf("%s (%d)",
ReasonToString[Reason(reason)], reason))
}
return strings.Join(reasonStrings, ", ")
UserAllowedReasonsMessage = strings.Join(reasonStrings, ", ")
}

4
test/load-generator/boulder-calls.go
View File

@ -23,8 +23,8 @@ import (
"github.com/letsencrypt/boulder/core"
"github.com/letsencrypt/boulder/identifier"
"github.com/letsencrypt/boulder/probs"
"github.com/letsencrypt/boulder/revocation"
"github.com/letsencrypt/boulder/test/load-generator/acme"
"golang.org/x/crypto/ocsp"
"gopkg.in/square/go-jose.v2"
)
@ -619,7 +619,7 @@ func revokeCertificate(s *State, ctx *context) error {
Reason int
}{
Certificate: base64.URLEncoding.EncodeToString(pemBlock.Bytes),
Reason: revocation.Unspecified,
Reason: ocsp.Unspecified,
}
revokeJSON, err := json.Marshal(revokeObj)

10
wfe/wfe.go
View File

@ -877,7 +877,15 @@ func (wfe *WebFrontEndImpl) RevokeCertificate(ctx context.Context, logEvent *web
reason := revocation.Reason(0)
if revokeRequest.Reason != nil {
if _, present := revocation.UserAllowedReasons[*revokeRequest.Reason]; !present {
wfe.sendError(response, logEvent, probs.Malformed("unsupported revocation reason code provided"), nil)
reasonStr, ok := revocation.ReasonToString[revocation.Reason(*revokeRequest.Reason)]
if !ok {
reasonStr = "unknown"
}
wfe.sendError(response, logEvent, probs.Malformed(
"unsupported revocation reason code provided: %s (%d). Supported reasons: %s",
reasonStr,
*revokeRequest.Reason,
revocation.UserAllowedReasonsMessage), nil)
return
}
reason = *revokeRequest.Reason

4
wfe/wfe_test.go
View File

@ -1615,7 +1615,7 @@ func TestRevokeCertificateReasons(t *testing.T) {
wfe.RevokeCertificate(ctx, newRequestEvent(), responseWriter,
makePostRequest(result.FullSerialize()))
test.AssertEquals(t, responseWriter.Code, 400)
test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{"type":"`+probs.V1ErrorNS+`malformed","detail":"unsupported revocation reason code provided","status":400}`)
test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{"type":"`+probs.V1ErrorNS+`malformed","detail":"unsupported revocation reason code provided: cACompromise (2). Supported reasons: unspecified (0), keyCompromise (1), affiliationChanged (3), superseded (4), cessationOfOperation (5)","status":400}`)
responseWriter = httptest.NewRecorder()
unsupported = revocation.Reason(100)
@ -1626,7 +1626,7 @@ func TestRevokeCertificateReasons(t *testing.T) {
wfe.RevokeCertificate(ctx, newRequestEvent(), responseWriter,
makePostRequest(result.FullSerialize()))
test.AssertEquals(t, responseWriter.Code, 400)
test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{"type":"`+probs.V1ErrorNS+`malformed","detail":"unsupported revocation reason code provided","status":400}`)
test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{"type":"`+probs.V1ErrorNS+`malformed","detail":"unsupported revocation reason code provided: unknown (100). Supported reasons: unspecified (0), keyCompromise (1), affiliationChanged (3), superseded (4), cessationOfOperation (5)","status":400}`)
}
// Valid revocation request for existing, non-revoked cert, signed with account

2
wfe2/wfe.go
View File

@ -827,7 +827,7 @@ func (wfe *WebFrontEndImpl) processRevocation(
"unsupported revocation reason code provided: %s (%d). Supported reasons: %s",
reasonStr,
*revokeRequest.Reason,
revocation.UserAllowedReasonsMessage())
revocation.UserAllowedReasonsMessage)
}
reason = *revokeRequest.Reason
}