Skip to content

ACME Certificate Revocation #625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d53bcaf
Add base logic for ACME revoke-cert
hslatman Jul 2, 2021
84e7d46
Improve handling of ACME revocation
hslatman Jul 2, 2021
0e56932
Add support for revocation using JWK
hslatman Jul 2, 2021
16fe07d
Fix mockSignAuth
hslatman Jul 3, 2021
8f7e700
Merge branch 'master' into hs/acme-revocation
hslatman Jul 9, 2021
2b15230
Add Serial to Cert ID ACME table and lookup
hslatman Jul 9, 2021
97165f1
Fix test mocking for CreateCertificate
hslatman Jul 9, 2021
258efca
Improve revocation authorization
hslatman Jul 9, 2021
a4cfb66
Merge branch 'master' into hs/acme-revocation
hslatman Jul 17, 2021
3151255
Merge branch 'master' into hs/acme-revocation
hslatman Oct 30, 2021
c7a9c13
Add tests for extractOrLookupJWK middleware
hslatman Nov 12, 2021
023c64c
Merge branch 'master' into hs/acme-revocation
hslatman Nov 12, 2021
42f56d6
Set golangci-lint version to v1.41.0 instead of latest
hslatman Nov 12, 2021
29f9730
Satisfy golangci-lint
hslatman Nov 12, 2021
2d50c96
Merge branch 'master' into hs/acme-revocation
hslatman Nov 19, 2021
c9cd876
Merge branch 'master' into hs/acme-revocation
hslatman Nov 24, 2021
ed295ca
Fix linting issue
hslatman Nov 24, 2021
2d357da
Add tests for ACME revocation
hslatman Nov 26, 2021
4d01cf8
Increase test code coverage
hslatman Nov 28, 2021
a7fbbc4
Add tests for GetCertificateBySerial
hslatman Nov 28, 2021
bae1d25
Improve tests for JWK vs. KID revoke auth flow
hslatman Dec 2, 2021
06bb97c
Add logic for Account authorizations and improve tests
hslatman Dec 2, 2021
47a8a3c
Add test case for ACME Revoke to Authority
hslatman Dec 2, 2021
004fc05
Fix PR comments
hslatman Dec 3, 2021
0524122
Remove authorization flow for different Account private keys
hslatman Dec 8, 2021
3bc3957
Merge branch 'master' into hs/acme-revocation
hslatman Dec 9, 2021
00539d0
Add changelog entry for ACME revocation
hslatman Dec 9, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions acme/api/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,17 @@ func (h *Handler) Route(r api.Router) {
r.MethodFunc("GET", getPath(DirectoryLinkType, "{provisionerID}"), h.baseURLFromRequest(h.lookupProvisioner(h.GetDirectory)))
r.MethodFunc("HEAD", getPath(DirectoryLinkType, "{provisionerID}"), h.baseURLFromRequest(h.lookupProvisioner(h.GetDirectory)))

validatingMiddleware := func(next nextHTTP) nextHTTP {
return h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.addDirLink(h.verifyContentType(h.parseJWS(h.validateJWS(next)))))))
}
extractPayloadByJWK := func(next nextHTTP) nextHTTP {
return h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.addDirLink(h.verifyContentType(h.parseJWS(h.validateJWS(h.extractJWK(h.verifyAndExtractJWSPayload(next)))))))))
return validatingMiddleware(h.extractJWK(h.verifyAndExtractJWSPayload(next)))
}
extractPayloadByKid := func(next nextHTTP) nextHTTP {
return h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.addDirLink(h.verifyContentType(h.parseJWS(h.validateJWS(h.lookupJWK(h.verifyAndExtractJWSPayload(next)))))))))
return validatingMiddleware(h.lookupJWK(h.verifyAndExtractJWSPayload(next)))
}
extractPayloadByKidOrJWK := func(next nextHTTP) nextHTTP {
return validatingMiddleware(h.extractOrLookupJWK(h.verifyAndExtractJWSPayload(next)))
}

r.MethodFunc("POST", getPath(NewAccountLinkType, "{provisionerID}"), extractPayloadByJWK(h.NewAccount))
Expand All @@ -117,6 +123,7 @@ func (h *Handler) Route(r api.Router) {
r.MethodFunc("POST", getPath(AuthzLinkType, "{provisionerID}", "{authzID}"), extractPayloadByKid(h.isPostAsGet(h.GetAuthorization)))
r.MethodFunc("POST", getPath(ChallengeLinkType, "{provisionerID}", "{authzID}", "{chID}"), extractPayloadByKid(h.GetChallenge))
r.MethodFunc("POST", getPath(CertificateLinkType, "{provisionerID}", "{certID}"), extractPayloadByKid(h.isPostAsGet(h.GetCertificate)))
r.MethodFunc("POST", getPath(RevokeCertLinkType, "{provisionerID}"), extractPayloadByKidOrJWK(h.RevokeCert))
}

// GetNonce just sets the right header since a Nonce is added to each response
Expand Down
40 changes: 38 additions & 2 deletions acme/api/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,11 @@ func (h *Handler) extractJWK(next nextHTTP) nextHTTP {
// Store the JWK in the context.
ctx = context.WithValue(ctx, jwkContextKey, jwk)

// Get Account or continue to generate a new one.
// Get Account OR continue to generate a new one OR continue Revoke with certificate private key
acc, err := h.db.GetAccountByKeyID(ctx, jwk.KeyID)
switch {
case errors.Is(err, acme.ErrNotFound):
// For NewAccount requests ...
// For NewAccount and Revoke requests ...
break
case err != nil:
api.WriteError(w, err)
Expand Down Expand Up @@ -352,6 +352,42 @@ func (h *Handler) lookupJWK(next nextHTTP) nextHTTP {
}
}

// extractOrLookupJWK forwards handling to either extractJWK or
// lookupJWK based on the presence of a JWK or a KID, respectively.
func (h *Handler) extractOrLookupJWK(next nextHTTP) nextHTTP {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
jws, err := jwsFromContext(ctx)
if err != nil {
api.WriteError(w, err)
return
}

// at this point the JWS has already been verified (if correctly configured in middleware),
// and it can be used to check if a JWK exists. This flow is used when the ACME client
// signed the payload with a certificate private key.
if canExtractJWKFrom(jws) {
h.extractJWK(next)(w, r)
return
}

// default to looking up the JWK based on KeyID. This flow is used when the ACME client
// signed the payload with an account private key.
h.lookupJWK(next)(w, r)
}
}

// canExtractJWKFrom checks if the JWS has a JWK that can be extracted
func canExtractJWKFrom(jws *jose.JSONWebSignature) bool {
if jws == nil {
return false
}
if len(jws.Signatures) == 0 {
return false
}
return jws.Signatures[0].Protected.JSONWebKey != nil
}

// verifyAndExtractJWSPayload extracts the JWK from the JWS and saves it in the context.
// Make sure to parse and validate the JWS before running this middleware.
func (h *Handler) verifyAndExtractJWSPayload(next nextHTTP) nextHTTP {
Expand Down
184 changes: 184 additions & 0 deletions acme/api/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1472,3 +1472,187 @@ func TestHandler_validateJWS(t *testing.T) {
})
}
}

func Test_canExtractJWKFrom(t *testing.T) {
jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
assert.FatalError(t, err)
type args struct {
jws *jose.JSONWebSignature
}
tests := []struct {
name string
args args
want bool
}{
{
name: "no-jws",
args: args{
jws: nil,
},
want: false,
},
{
name: "no-signatures",
args: args{
jws: &jose.JSONWebSignature{
Signatures: []jose.Signature{},
},
},
want: false,
},
{
name: "no-jwk",
args: args{
jws: &jose.JSONWebSignature{
Signatures: []jose.Signature{
{
Protected: jose.Header{},
},
},
},
},
want: false,
},
{
name: "ok",
args: args{
jws: &jose.JSONWebSignature{
Signatures: []jose.Signature{
{
Protected: jose.Header{
JSONWebKey: jwk,
},
},
},
},
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := canExtractJWKFrom(tt.args.jws); got != tt.want {
t.Errorf("canExtractJWKFrom() = %v, want %v", got, tt.want)
}
})
}
}

func TestHandler_extractOrLookupJWK(t *testing.T) {
u := "https://ca.smallstep.com/acme/account"
type test struct {
db acme.DB
linker Linker
statusCode int
ctx context.Context
err *acme.Error
next func(w http.ResponseWriter, r *http.Request)
}
var tests = map[string]func(t *testing.T) test{
"ok/extract": func(t *testing.T) test {
jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
assert.FatalError(t, err)
kid, err := jwk.Thumbprint(crypto.SHA256)
assert.FatalError(t, err)
pub := jwk.Public()
pub.KeyID = base64.RawURLEncoding.EncodeToString(kid)
so := new(jose.SignerOptions)
so.WithHeader("jwk", pub) // JWK for certificate private key flow
signer, err := jose.NewSigner(jose.SigningKey{
Algorithm: jose.SignatureAlgorithm(jwk.Algorithm),
Key: jwk.Key,
}, so)
assert.FatalError(t, err)
signed, err := signer.Sign([]byte("foo"))
assert.FatalError(t, err)
raw, err := signed.CompactSerialize()
assert.FatalError(t, err)
parsedJWS, err := jose.ParseJWS(raw)
assert.FatalError(t, err)
return test{
linker: NewLinker("dns", "acme"),
db: &acme.MockDB{
MockGetAccountByKeyID: func(ctx context.Context, kid string) (*acme.Account, error) {
assert.Equals(t, kid, pub.KeyID)
return nil, acme.ErrNotFound
},
},
ctx: context.WithValue(context.Background(), jwsContextKey, parsedJWS),
statusCode: 200,
next: func(w http.ResponseWriter, r *http.Request) {
w.Write(testBody)
},
}
},
"ok/lookup": func(t *testing.T) test {
prov := newProv()
provName := url.PathEscape(prov.GetName())
baseURL := &url.URL{Scheme: "https", Host: "test.ca.smallstep.com"}
jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
assert.FatalError(t, err)
accID := "accID"
prefix := fmt.Sprintf("%s/acme/%s/account/", baseURL, provName)
so := new(jose.SignerOptions)
so.WithHeader("kid", fmt.Sprintf("%s%s", prefix, accID)) // KID for account private key flow
signer, err := jose.NewSigner(jose.SigningKey{
Algorithm: jose.SignatureAlgorithm(jwk.Algorithm),
Key: jwk.Key,
}, so)
assert.FatalError(t, err)
jws, err := signer.Sign([]byte("baz"))
assert.FatalError(t, err)
raw, err := jws.CompactSerialize()
assert.FatalError(t, err)
parsedJWS, err := jose.ParseJWS(raw)
assert.FatalError(t, err)
acc := &acme.Account{ID: "accID", Key: jwk, Status: "valid"}
ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
ctx = context.WithValue(ctx, baseURLContextKey, baseURL)
ctx = context.WithValue(ctx, jwsContextKey, parsedJWS)
return test{
linker: NewLinker("test.ca.smallstep.com", "acme"),
db: &acme.MockDB{
MockGetAccount: func(ctx context.Context, accID string) (*acme.Account, error) {
assert.Equals(t, accID, acc.ID)
return acc, nil
},
},
ctx: ctx,
statusCode: 200,
next: func(w http.ResponseWriter, r *http.Request) {
w.Write(testBody)
},
}
},
}
for name, prep := range tests {
tc := prep(t)
t.Run(name, func(t *testing.T) {
h := &Handler{db: tc.db, linker: tc.linker}
req := httptest.NewRequest("GET", u, nil)
req = req.WithContext(tc.ctx)
w := httptest.NewRecorder()
h.extractOrLookupJWK(tc.next)(w, req)
res := w.Result()

assert.Equals(t, res.StatusCode, tc.statusCode)

body, err := io.ReadAll(res.Body)
res.Body.Close()
assert.FatalError(t, err)

if res.StatusCode >= 400 && assert.NotNil(t, tc.err) {
var ae acme.Error
assert.FatalError(t, json.Unmarshal(bytes.TrimSpace(body), &ae))

assert.Equals(t, ae.Type, tc.err.Type)
assert.Equals(t, ae.Detail, tc.err.Detail)
assert.Equals(t, ae.Identifier, tc.err.Identifier)
assert.Equals(t, ae.Subproblems, tc.err.Subproblems)
assert.Equals(t, res.Header["Content-Type"], []string{"application/problem+json"})
} else {
assert.Equals(t, bytes.TrimSpace(body), testBody)
}
})
}
}
Loading