diff --git a/acme/api/revoke.go b/acme/api/revoke.go index e92058e8..7ae93152 100644 --- a/acme/api/revoke.go +++ b/acme/api/revoke.go @@ -91,9 +91,9 @@ func (h *Handler) RevokeCert(w http.ResponseWriter, r *http.Request) { } else { // if account doesn't need to be checked, the JWS should be verified to be signed by the // private key that belongs to the public key in the certificate to be revoked. - // TODO: implement test case for this _, err := jws.Verify(certToBeRevoked.PublicKey) if err != nil { + // TODO(hs): possible to determine an error vs. unauthorized and thus provide an ISE vs. Unauthorized? api.WriteError(w, wrapUnauthorizedError(certToBeRevoked, "verification of jws using certificate public key failed", err)) return } @@ -141,7 +141,7 @@ func (h *Handler) RevokeCert(w http.ResponseWriter, r *http.Request) { // revocation into an ACME error, so that clients can understand the error. func wrapRevokeErr(err error) *acme.Error { t := err.Error() - if strings.Contains(t, "has already been revoked") { + if strings.Contains(t, "is already revoked") { return acme.NewError(acme.ErrorAlreadyRevokedType, t) } return acme.WrapErrorISE(err, "error when revoking certificate") @@ -157,7 +157,7 @@ func wrapUnauthorizedError(cert *x509.Certificate, msg string, err error) *acme. acmeErr = acme.WrapError(acme.ErrorUnauthorizedType, err, msg) } acmeErr.Status = http.StatusForbidden - acmeErr.Detail = fmt.Sprintf("No authorization provided for name %s", cert.Subject.String()) // TODO: what about other SANs? + acmeErr.Detail = fmt.Sprintf("No authorization provided for name %s", cert.Subject.String()) // TODO: what about other SANs? When no Subject is in the certificate? return acmeErr } diff --git a/acme/api/revoke_test.go b/acme/api/revoke_test.go index 033934c6..2feae989 100644 --- a/acme/api/revoke_test.go +++ b/acme/api/revoke_test.go @@ -3,66 +3,267 @@ package api import ( "bytes" "context" + "crypto" + "crypto/ecdsa" + "crypto/rand" + "crypto/rsa" "crypto/x509" + "crypto/x509/pkix" "encoding/base64" "encoding/json" - "encoding/pem" "fmt" "io" + "math/big" "net/http/httptest" "net/url" - "reflect" "testing" + "time" "github.com/go-chi/chi" + "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "github.com/smallstep/assert" "github.com/smallstep/certificates/acme" "github.com/smallstep/certificates/authority" "github.com/smallstep/certificates/authority/provisioner" "go.step.sm/crypto/jose" + "go.step.sm/crypto/keyutil" + "go.step.sm/crypto/x509util" "golang.org/x/crypto/ocsp" ) -const ( - certPEM = `-----BEGIN CERTIFICATE----- -MIIDujCCAqKgAwIBAgIIE31FZVaPXTUwDQYJKoZIhvcNAQEFBQAwSTELMAkGA1UE -BhMCVVMxEzARBgNVBAoTCkdvb2dsZSBJbmMxJTAjBgNVBAMTHEdvb2dsZSBJbnRl -cm5ldCBBdXRob3JpdHkgRzIwHhcNMTQwMTI5MTMyNzQzWhcNMTQwNTI5MDAwMDAw -WjBpMQswCQYDVQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwN -TW91bnRhaW4gVmlldzETMBEGA1UECgwKR29vZ2xlIEluYzEYMBYGA1UEAwwPbWFp -bC5nb29nbGUuY29tMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEfRrObuSW5T7q -5CnSEqefEmtH4CCv6+5EckuriNr1CjfVvqzwfAhopXkLrq45EQm8vkmf7W96XJhC -7ZM0dYi1/qOCAU8wggFLMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAa -BgNVHREEEzARgg9tYWlsLmdvb2dsZS5jb20wCwYDVR0PBAQDAgeAMGgGCCsGAQUF -BwEBBFwwWjArBggrBgEFBQcwAoYfaHR0cDovL3BraS5nb29nbGUuY29tL0dJQUcy -LmNydDArBggrBgEFBQcwAYYfaHR0cDovL2NsaWVudHMxLmdvb2dsZS5jb20vb2Nz -cDAdBgNVHQ4EFgQUiJxtimAuTfwb+aUtBn5UYKreKvMwDAYDVR0TAQH/BAIwADAf -BgNVHSMEGDAWgBRK3QYWG7z2aLV29YG2u2IaulqBLzAXBgNVHSAEEDAOMAwGCisG -AQQB1nkCBQEwMAYDVR0fBCkwJzAloCOgIYYfaHR0cDovL3BraS5nb29nbGUuY29t -L0dJQUcyLmNybDANBgkqhkiG9w0BAQUFAAOCAQEAH6RYHxHdcGpMpFE3oxDoFnP+ -gtuBCHan2yE2GRbJ2Cw8Lw0MmuKqHlf9RSeYfd3BXeKkj1qO6TVKwCh+0HdZk283 -TZZyzmEOyclm3UGFYe82P/iDFt+CeQ3NpmBg+GoaVCuWAARJN/KfglbLyyYygcQq -0SgeDh8dRKUiaW3HQSoYvTvdTuqzwK4CXsr3b5/dAOY8uMuG/IAR3FgwTbZ1dtoW -RvOTa8hYiU6A475WuZKyEHcwnGYe57u2I2KbMgcKjPniocj4QzgYsVAVKW3IwaOh -yE+vPxsiUkvQHdO2fojCkY8jg70jxM+gu59tPDNbw3Uh/2Ij310FgTHsnGQMyA== ------END CERTIFICATE-----` -) - +// v is a utility function to return the pointer to an integer func v(v int) *int { return &v } -func parseCertificate(data string) *x509.Certificate { - block, _ := pem.Decode([]byte(data)) - if block == nil { - panic("failed to parse certificate PEM") - } - cert, err := x509.ParseCertificate(block.Bytes) +// generateCertKeyPair generates fresh x509 certificate/key pairs for testing +func generateCertKeyPair() (*x509.Certificate, crypto.Signer, error) { + + pub, priv, err := keyutil.GenerateKeyPair("EC", "P-256", 0) if err != nil { - panic("failed to parse certificate: " + err.Error()) + return nil, nil, err } - return cert + + serial, err := rand.Int(rand.Reader, big.NewInt(1000000000000000000)) + if err != nil { + return nil, nil, err + } + + now := time.Now() + template := &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test ACME Revoke Certificate"}, + Issuer: pkix.Name{CommonName: "Test ACME Revoke Certificate"}, + IsCA: false, + MaxPathLen: 0, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + NotBefore: now, + NotAfter: now.Add(time.Hour), + SerialNumber: serial, + } + + signer, ok := priv.(crypto.Signer) + if !ok { + return nil, nil, errors.Errorf("result is not a crypto.Signer: type %T", priv) + } + + cert, err := x509util.CreateCertificate(template, template, pub, signer) + + return cert, signer, err +} + +var errUnsupportedKey = fmt.Errorf("unknown key type; only RSA and ECDSA are supported") + +// keyID is the account identity provided by a CA during registration. +type keyID string + +// noKeyID indicates that jwsEncodeJSON should compute and use JWK instead of a KID. +// See jwsEncodeJSON for details. +const noKeyID = keyID("") + +// jwsEncodeJSON signs claimset using provided key and a nonce. +// The result is serialized in JSON format containing either kid or jwk +// fields based on the provided keyID value. +// +// If kid is non-empty, its quoted value is inserted in the protected head +// as "kid" field value. Otherwise, JWK is computed using jwkEncode and inserted +// as "jwk" field value. The "jwk" and "kid" fields are mutually exclusive. +// +// See https://tools.ietf.org/html/rfc7515#section-7. +// +// If nonce is empty, it will not be encoded into the header. +// Implementation taken from github.com/mholt/acmez, which seems to be based on +// https://github.com/golang/crypto/blob/master/acme/jws.go. +func jwsEncodeJSON(claimset interface{}, key crypto.Signer, kid keyID, nonce, u string) ([]byte, error) { + alg, sha := jwsHasher(key.Public()) + if alg == "" || !sha.Available() { + return nil, errUnsupportedKey + } + + phead, err := jwsHead(alg, nonce, u, kid, key) + if err != nil { + return nil, err + } + + var payload string + if claimset != nil { + cs, err := json.Marshal(claimset) + if err != nil { + return nil, err + } + payload = base64.RawURLEncoding.EncodeToString(cs) + } + + payloadToSign := []byte(phead + "." + payload) + hash := sha.New() + _, _ = hash.Write(payloadToSign) + digest := hash.Sum(nil) + + sig, err := jwsSign(key, sha, digest) + if err != nil { + return nil, err + } + + return jwsFinal(sha, sig, phead, payload) +} + +// jwsHasher indicates suitable JWS algorithm name and a hash function +// to use for signing a digest with the provided key. +// It returns ("", 0) if the key is not supported. +// Implementation taken from github.com/mholt/acmez, which seems to be based on +// https://github.com/golang/crypto/blob/master/acme/jws.go. +func jwsHasher(pub crypto.PublicKey) (string, crypto.Hash) { + switch pub := pub.(type) { + case *rsa.PublicKey: + return "RS256", crypto.SHA256 + case *ecdsa.PublicKey: + switch pub.Params().Name { + case "P-256": + return "ES256", crypto.SHA256 + case "P-384": + return "ES384", crypto.SHA384 + case "P-521": + return "ES512", crypto.SHA512 + } + } + return "", 0 +} + +// jwsSign signs the digest using the given key. +// The hash is unused for ECDSA keys. +// +// Note: non-stdlib crypto.Signer implementations are expected to return +// the signature in the format as specified in RFC7518. +// See https://tools.ietf.org/html/rfc7518 for more details. +// Implementation taken from github.com/mholt/acmez, which seems to be based on +// https://github.com/golang/crypto/blob/master/acme/jws.go. +func jwsSign(key crypto.Signer, hash crypto.Hash, digest []byte) ([]byte, error) { + if key, ok := key.(*ecdsa.PrivateKey); ok { + // The key.Sign method of ecdsa returns ASN1-encoded signature. + // So, we use the package Sign function instead + // to get R and S values directly and format the result accordingly. + r, s, err := ecdsa.Sign(rand.Reader, key, digest) + if err != nil { + return nil, err + } + rb, sb := r.Bytes(), s.Bytes() + size := key.Params().BitSize / 8 + if size%8 > 0 { + size++ + } + sig := make([]byte, size*2) + copy(sig[size-len(rb):], rb) + copy(sig[size*2-len(sb):], sb) + return sig, nil + } + return key.Sign(rand.Reader, digest, hash) +} + +// jwsHead constructs the protected JWS header for the given fields. +// Since jwk and kid are mutually-exclusive, the jwk will be encoded +// only if kid is empty. If nonce is empty, it will not be encoded. +// Implementation taken from github.com/mholt/acmez, which seems to be based on +// https://github.com/golang/crypto/blob/master/acme/jws.go. +func jwsHead(alg, nonce, u string, kid keyID, key crypto.Signer) (string, error) { + phead := fmt.Sprintf(`{"alg":%q`, alg) + if kid == noKeyID { + jwk, err := jwkEncode(key.Public()) + if err != nil { + return "", err + } + phead += fmt.Sprintf(`,"jwk":%s`, jwk) + } else { + phead += fmt.Sprintf(`,"kid":%q`, kid) + } + if nonce != "" { + phead += fmt.Sprintf(`,"nonce":%q`, nonce) + } + phead += fmt.Sprintf(`,"url":%q}`, u) + phead = base64.RawURLEncoding.EncodeToString([]byte(phead)) + return phead, nil +} + +// jwkEncode encodes public part of an RSA or ECDSA key into a JWK. +// The result is also suitable for creating a JWK thumbprint. +// https://tools.ietf.org/html/rfc7517 +// Implementation taken from github.com/mholt/acmez, which seems to be based on +// https://github.com/golang/crypto/blob/master/acme/jws.go. +func jwkEncode(pub crypto.PublicKey) (string, error) { + switch pub := pub.(type) { + case *rsa.PublicKey: + // https://tools.ietf.org/html/rfc7518#section-6.3.1 + n := pub.N + e := big.NewInt(int64(pub.E)) + // Field order is important. + // See https://tools.ietf.org/html/rfc7638#section-3.3 for details. + return fmt.Sprintf(`{"e":%q,"kty":"RSA","n":%q}`, + base64.RawURLEncoding.EncodeToString(e.Bytes()), + base64.RawURLEncoding.EncodeToString(n.Bytes()), + ), nil + case *ecdsa.PublicKey: + // https://tools.ietf.org/html/rfc7518#section-6.2.1 + p := pub.Curve.Params() + n := p.BitSize / 8 + if p.BitSize%8 != 0 { + n++ + } + x := pub.X.Bytes() + if n > len(x) { + x = append(make([]byte, n-len(x)), x...) + } + y := pub.Y.Bytes() + if n > len(y) { + y = append(make([]byte, n-len(y)), y...) + } + // Field order is important. + // See https://tools.ietf.org/html/rfc7638#section-3.3 for details. + return fmt.Sprintf(`{"crv":%q,"kty":"EC","x":%q,"y":%q}`, + p.Name, + base64.RawURLEncoding.EncodeToString(x), + base64.RawURLEncoding.EncodeToString(y), + ), nil + } + return "", errUnsupportedKey +} + +// jwsFinal constructs the final JWS object. +// Implementation taken from github.com/mholt/acmez, which seems to be based on +// https://github.com/golang/crypto/blob/master/acme/jws.go. +func jwsFinal(sha crypto.Hash, sig []byte, phead, payload string) ([]byte, error) { + enc := struct { + Protected string `json:"protected"` + Payload string `json:"payload"` + Sig string `json:"signature"` + }{ + Protected: phead, + Payload: payload, + Sig: base64.RawURLEncoding.EncodeToString(sig), + } + result, err := json.Marshal(&enc) + if err != nil { + return nil, err + } + return result, nil } type mockCA struct { @@ -138,17 +339,6 @@ func Test_validateReasonCode(t *testing.T) { } func Test_reason(t *testing.T) { - - // case ocsp.RemoveFromCRL: - // return "remove from crl" - // case ocsp.PrivilegeWithdrawn: - // return "privilege withdrawn" - // case ocsp.AACompromise: - // return "aa compromised" - // default: - // return "unspecified reason" - // } - // } tests := []struct { name string reasonCode int @@ -220,7 +410,8 @@ func Test_reason(t *testing.T) { } func Test_revokeOptions(t *testing.T) { - var cert *x509.Certificate + cert, _, err := generateCertKeyPair() + assert.FatalError(t, err) type args struct { serial string certToBeRevoked *x509.Certificate @@ -239,7 +430,7 @@ func Test_revokeOptions(t *testing.T) { }, want: &authority.RevokeOptions{ Serial: "1234", - Crt: nil, + Crt: cert, ACME: true, }, }, @@ -252,7 +443,7 @@ func Test_revokeOptions(t *testing.T) { }, want: &authority.RevokeOptions{ Serial: "1234", - Crt: nil, + Crt: cert, ACME: true, ReasonCode: 1, Reason: "key compromised", @@ -261,8 +452,8 @@ func Test_revokeOptions(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := revokeOptions(tt.args.serial, tt.args.certToBeRevoked, tt.args.reasonCode); !reflect.DeepEqual(got, tt.want) { - t.Errorf("revokeOptions() = %v, want %v", got, tt.want) + if got := revokeOptions(tt.args.serial, tt.args.certToBeRevoked, tt.args.reasonCode); !cmp.Equal(got, tt.want) { + t.Errorf("revokeOptions() diff = %s", cmp.Diff(got, tt.want)) } }) } @@ -279,7 +470,8 @@ func TestHandler_RevokeCert(t *testing.T) { chiCtx := chi.NewRouteContext() revokeURL := fmt.Sprintf("%s/acme/%s/revoke-cert", baseURL.String(), escProvName) - cert := parseCertificate(certPEM) + cert, key, err := generateCertKeyPair() + assert.FatalError(t, err) rp := &revokePayload{ Certificate: base64.RawURLEncoding.EncodeToString(cert.Raw), } @@ -295,6 +487,134 @@ func TestHandler_RevokeCert(t *testing.T) { } var tests = map[string]func(t *testing.T) test{ + "fail/no-jws": func(t *testing.T) test { + ctx := context.Background() + return test{ + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("jws expected in request context"), + } + }, + "fail/nil-jws": func(t *testing.T) test { + ctx := context.WithValue(context.Background(), jwsContextKey, nil) + return test{ + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("jws expected in request context"), + } + }, + "fail/no-provisioner": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + ctx := context.WithValue(context.Background(), jwsContextKey, jws) + return test{ + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("provisioner does not exist"), + } + }, + "fail/nil-provisioner": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + ctx := context.WithValue(context.Background(), jwsContextKey, jws) + ctx = context.WithValue(ctx, provisionerContextKey, nil) + return test{ + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("provisioner does not exist"), + } + }, + "fail/no-payload": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + ctx := context.WithValue(context.Background(), jwsContextKey, jws) + ctx = context.WithValue(ctx, provisionerContextKey, prov) + return test{ + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("payload does not exist"), + } + }, + "fail/nil-payload": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + ctx := context.WithValue(context.Background(), jwsContextKey, jws) + ctx = context.WithValue(ctx, provisionerContextKey, prov) + ctx = context.WithValue(ctx, payloadContextKey, nil) + return test{ + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("payload does not exist"), + } + }, + "fail/unmarshal-payload": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + malformedPayload := []byte(`{"payload":malformed?}`) + ctx := context.WithValue(context.Background(), jwsContextKey, jws) + ctx = context.WithValue(ctx, provisionerContextKey, prov) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: malformedPayload}) + return test{ + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("error unmarshaling payload"), + } + }, "fail/wrong-certificate-encoding": func(t *testing.T) test { rp := &revokePayload{ Certificate: base64.StdEncoding.EncodeToString(cert.Raw), @@ -314,18 +634,10 @@ func TestHandler_RevokeCert(t *testing.T) { }, }, } - acc := &acme.Account{ID: "accountID", Status: acme.StatusInvalid} ctx := context.WithValue(context.Background(), provisionerContextKey, prov) - ctx = context.WithValue(ctx, accContextKey, acc) ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: wronglyEncodedPayloadBytes}) ctx = context.WithValue(ctx, jwsContextKey, jws) - ctx = context.WithValue(ctx, baseURLContextKey, baseURL) - ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx) - db := &acme.MockDB{} - ca := &mockCA{} return test{ - db: db, - ca: ca, ctx: ctx, statusCode: 400, err: &acme.Error{ @@ -354,18 +666,10 @@ func TestHandler_RevokeCert(t *testing.T) { }, }, } - acc := &acme.Account{ID: "accountID", Status: acme.StatusInvalid} ctx := context.WithValue(context.Background(), provisionerContextKey, prov) - ctx = context.WithValue(ctx, accContextKey, acc) ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: wrongPayloadBytes}) ctx = context.WithValue(ctx, jwsContextKey, jws) - ctx = context.WithValue(ctx, baseURLContextKey, baseURL) - ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx) - db := &acme.MockDB{} - ca := &mockCA{} return test{ - db: db, - ca: ca, ctx: ctx, statusCode: 400, err: &acme.Error{ @@ -375,6 +679,96 @@ func TestHandler_RevokeCert(t *testing.T) { }, } }, + "fail/db.GetCertificateBySerial": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + ctx := context.WithValue(context.Background(), provisionerContextKey, prov) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + return nil, errors.New("force") + }, + } + return test{ + db: db, + ctx: ctx, + statusCode: 500, + err: acme.NewErrorISE("error retrieving certificate by serial"), + } + }, + "fail/no-account": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + ctx := context.WithValue(context.Background(), provisionerContextKey, prov) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + assert.Equals(t, cert.SerialNumber.String(), serial) + return &acme.Certificate{}, nil + }, + } + return test{ + db: db, + ctx: ctx, + statusCode: 400, + err: acme.NewError(acme.ErrorAccountDoesNotExistType, "account not in context"), + } + }, + "fail/nil-account": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + ctx := context.WithValue(context.Background(), provisionerContextKey, prov) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + ctx = context.WithValue(ctx, accContextKey, nil) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + assert.Equals(t, cert.SerialNumber.String(), serial) + return &acme.Certificate{}, nil + }, + } + return test{ + db: db, + ctx: ctx, + statusCode: 400, + err: acme.NewError(acme.ErrorAccountDoesNotExistType, "account not in context"), + } + }, "fail/account-not-valid": func(t *testing.T) test { jws := &jose.JSONWebSignature{ Signatures: []jose.Signature{ @@ -459,6 +853,43 @@ func TestHandler_RevokeCert(t *testing.T) { }, } }, + "fail/unauthorized-certificate-key": func(t *testing.T) test { + _, unauthorizedKey, err := generateCertKeyPair() + assert.FatalError(t, err) + rp := &revokePayload{ + Certificate: base64.RawURLEncoding.EncodeToString(cert.Raw), + ReasonCode: v(1), + } + jwsBytes, err := jwsEncodeJSON(rp, unauthorizedKey, "", "nonce", revokeURL) + assert.FatalError(t, err) + jws, err := jose.ParseJWS(string(jwsBytes)) + assert.FatalError(t, err) + unauthorizedPayloadBytes, err := json.Marshal(rp) + assert.FatalError(t, err) + ctx := context.WithValue(context.Background(), provisionerContextKey, prov) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: unauthorizedPayloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + ctx = context.WithValue(ctx, baseURLContextKey, baseURL) + ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + assert.Equals(t, cert.SerialNumber.String(), serial) + return &acme.Certificate{ + AccountID: "accountID", + }, nil + }, + } + ca := &mockCA{} + acmeErr := acme.NewError(acme.ErrorUnauthorizedType, "verification of jws using certificate public key failed") + acmeErr.Detail = "No authorization provided for name CN=Test ACME Revoke Certificate" + return test{ + db: db, + ca: ca, + ctx: ctx, + statusCode: 403, + err: acmeErr, + } + }, "fail/certificate-revoked-check-fails": func(t *testing.T) test { jws := &jose.JSONWebSignature{ Signatures: []jose.Signature{ @@ -524,8 +955,6 @@ func TestHandler_RevokeCert(t *testing.T) { ctx = context.WithValue(ctx, accContextKey, acc) ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) ctx = context.WithValue(ctx, jwsContextKey, jws) - ctx = context.WithValue(ctx, baseURLContextKey, baseURL) - ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtx) db := &acme.MockDB{ MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { assert.Equals(t, cert.SerialNumber.String(), serial) @@ -551,7 +980,194 @@ func TestHandler_RevokeCert(t *testing.T) { }, } }, - "fail/certificate-revoke-fails": func(t *testing.T) test { + "fail/invalid-reasoncode": func(t *testing.T) test { + rp := &revokePayload{ + Certificate: base64.RawURLEncoding.EncodeToString(cert.Raw), + ReasonCode: v(7), + } + wrongReasonCodePayloadBytes, err := json.Marshal(rp) + assert.FatalError(t, err) + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + acc := &acme.Account{ID: "accountID", Status: acme.StatusValid} + ctx := context.WithValue(context.Background(), provisionerContextKey, prov) + ctx = context.WithValue(ctx, accContextKey, acc) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: wrongReasonCodePayloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + assert.Equals(t, cert.SerialNumber.String(), serial) + return &acme.Certificate{ + AccountID: "accountID", + }, nil + }, + } + ca := &mockCA{ + MockIsRevoked: func(sn string) (bool, error) { + return false, nil + }, + } + return test{ + db: db, + ca: ca, + ctx: ctx, + statusCode: 400, + err: &acme.Error{ + Type: "urn:ietf:params:acme:error:badRevocationReason", + Detail: "The revocation reason provided is not allowed by the server", + Status: 400, + }, + } + }, + "fail/prov.AuthorizeRevoke": func(t *testing.T) test { + assert.FatalError(t, err) + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + mockACMEProv := &acme.MockProvisioner{ + MauthorizeRevoke: func(ctx context.Context, token string) error { + return errors.New("force") + }, + } + acc := &acme.Account{ID: "accountID", Status: acme.StatusValid} + ctx := context.WithValue(context.Background(), provisionerContextKey, mockACMEProv) + ctx = context.WithValue(ctx, accContextKey, acc) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + assert.Equals(t, cert.SerialNumber.String(), serial) + return &acme.Certificate{ + AccountID: "accountID", + }, nil + }, + } + ca := &mockCA{ + MockIsRevoked: func(sn string) (bool, error) { + return false, nil + }, + } + return test{ + db: db, + ca: ca, + ctx: ctx, + statusCode: 500, + err: &acme.Error{ + Type: "urn:ietf:params:acme:error:serverInternal", + Detail: "The server experienced an internal error", + Status: 500, + }, + } + }, + "fail/ca.Revoke": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + acc := &acme.Account{ID: "accountID", Status: acme.StatusValid} + ctx := context.WithValue(context.Background(), provisionerContextKey, prov) + ctx = context.WithValue(ctx, accContextKey, acc) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + assert.Equals(t, cert.SerialNumber.String(), serial) + return &acme.Certificate{ + AccountID: "accountID", + }, nil + }, + } + ca := &mockCA{ + MockRevoke: func(ctx context.Context, opts *authority.RevokeOptions) error { + return errors.New("force") + }, + } + return test{ + db: db, + ca: ca, + ctx: ctx, + statusCode: 500, + err: &acme.Error{ + Type: "urn:ietf:params:acme:error:serverInternal", + Detail: "The server experienced an internal error", + Status: 500, + }, + } + }, + "fail/ca.Revoke-already-revoked": func(t *testing.T) test { + jws := &jose.JSONWebSignature{ + Signatures: []jose.Signature{ + { + Protected: jose.Header{ + Algorithm: jose.ES256, + KeyID: "bar", + ExtraHeaders: map[jose.HeaderKey]interface{}{ + "url": revokeURL, + }, + }, + }, + }, + } + acc := &acme.Account{ID: "accountID", Status: acme.StatusValid} + ctx := context.WithValue(context.Background(), provisionerContextKey, prov) + ctx = context.WithValue(ctx, accContextKey, acc) + ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) + ctx = context.WithValue(ctx, jwsContextKey, jws) + db := &acme.MockDB{ + MockGetCertificateBySerial: func(ctx context.Context, serial string) (*acme.Certificate, error) { + assert.Equals(t, cert.SerialNumber.String(), serial) + return &acme.Certificate{ + AccountID: "accountID", + }, nil + }, + } + ca := &mockCA{ + MockIsRevoked: func(sn string) (bool, error) { + return false, nil + }, + MockRevoke: func(ctx context.Context, opts *authority.RevokeOptions) error { + return fmt.Errorf("certificate with serial number '%s' is already revoked", cert.SerialNumber.String()) + }, + } + return test{ + db: db, + ca: ca, + ctx: ctx, + statusCode: 400, + err: acme.NewError(acme.ErrorAlreadyRevokedType, "certificate with serial number '%s' is already revoked", cert.SerialNumber.String()), + } + }, + "ok/using-account-key": func(t *testing.T) test { jws := &jose.JSONWebSignature{ Signatures: []jose.Signature{ { @@ -580,40 +1196,26 @@ func TestHandler_RevokeCert(t *testing.T) { }, nil }, } - ca := &mockCA{ - MockRevoke: func(ctx context.Context, opts *authority.RevokeOptions) error { - return errors.New("force") - }, - } + ca := &mockCA{} return test{ db: db, ca: ca, ctx: ctx, - statusCode: 500, - err: &acme.Error{ - Type: "urn:ietf:params:acme:error:serverInternal", - Detail: "The server experienced an internal error", - Status: 500, - }, + statusCode: 200, } }, - "ok/using-account-key": func(t *testing.T) test { - jws := &jose.JSONWebSignature{ - Signatures: []jose.Signature{ - { - Protected: jose.Header{ - Algorithm: jose.ES256, - KeyID: "bar", - ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": revokeURL, - }, - }, - }, - }, + "ok/using-certificate-key": func(t *testing.T) test { + rp := &revokePayload{ + Certificate: base64.RawURLEncoding.EncodeToString(cert.Raw), + ReasonCode: v(1), } - acc := &acme.Account{ID: "accountID", Status: acme.StatusValid} + jwsBytes, err := jwsEncodeJSON(rp, key, "", "nonce", revokeURL) + assert.FatalError(t, err) + jws, err := jose.ParseJWS(string(jwsBytes)) + assert.FatalError(t, err) + payloadBytes, err := json.Marshal(rp) + assert.FatalError(t, err) ctx := context.WithValue(context.Background(), provisionerContextKey, prov) - ctx = context.WithValue(ctx, accContextKey, acc) ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: payloadBytes}) ctx = context.WithValue(ctx, jwsContextKey, jws) ctx = context.WithValue(ctx, baseURLContextKey, baseURL)