Skip to content
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

[RFC] Implement ACME RFC 8555, challenge retries #181

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,3 @@
coverage.txt
vendor
output
.idea
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ after_success:
not collect coverage reports"
notifications:
email: false
stage: Github Release
deploy:
provider: releases
skip_cleanup: true
Expand Down
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Copyright (c) 2020 Smallstep Labs, Inc.
Copyright (c) 2019 Smallstep Labs, Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ It's super easy to get started and to operate `step-ca` thanks to [streamlined i
### [Your own private ACME Server](https://smallstep.com/blog/private-acme-server/)
- Issue certificates using ACMEv2 ([RFC8555](https://tools.ietf.org/html/rfc8555)), **the protocol used by Let's Encrypt**
- Great for [using ACME in development & pre-production](https://smallstep.com/blog/private-acme-server/#local-development-pre-production)
- Supports the `http-01`, `tls-alpn-01`, and `dns-01` ACME challenge types
- Supports the `http-01` and `dns-01` ACME challenge types
- Works with any compliant ACME client including [certbot](https://smallstep.com/blog/private-acme-server/#certbot-uploads-acme-certbot-png-certbot-example), [acme.sh](https://smallstep.com/blog/private-acme-server/#acme-sh-uploads-acme-acme-sh-png-acme-sh-example), [Caddy](https://smallstep.com/blog/private-acme-server/#caddy-uploads-acme-caddy-png-caddy-example), and [traefik](https://smallstep.com/blog/private-acme-server/#traefik-uploads-acme-traefik-png-traefik-example)
- Get certificates programmatically (e.g., in [Go](https://smallstep.com/blog/private-acme-server/#golang-uploads-acme-golang-png-go-example), [Python](https://smallstep.com/blog/private-acme-server/#python-uploads-acme-python-png-python-example), [Node.js](https://smallstep.com/blog/private-acme-server/#node-js-uploads-acme-node-js-png-node-js-example))

Expand Down Expand Up @@ -342,8 +342,8 @@ Documentation can be found in a handful of different places:

1. The [docs](./docs/README.md) sub-repo has an index of documentation and tutorials.

2. On the command line with `step help ca xxx` where `xxx` is the subcommand
you are interested in. Ex: `step help ca provisioner list`.
2. On the command line with `step ca help xxx` where `xxx` is the subcommand
you are interested in. Ex: `step help ca provisioners list`.

3. On the web at https://smallstep.com/docs/certificates.

Expand Down
2 changes: 1 addition & 1 deletion acme/api/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (h *Handler) NewAccount(w http.ResponseWriter, r *http.Request) {
acc, err := accountFromContext(r)
if err != nil {
acmeErr, ok := err.(*acme.Error)
if !ok || acmeErr.Status != http.StatusBadRequest {
if !ok || acmeErr.Status != http.StatusNotFound {
// Something went wrong ...
api.WriteError(w, err)
return
Expand Down
10 changes: 5 additions & 5 deletions acme/api/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func TestHandlerGetOrdersByAccount(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -212,7 +212,7 @@ func TestHandlerGetOrdersByAccount(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down Expand Up @@ -378,7 +378,7 @@ func TestHandlerNewAccount(t *testing.T) {
ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: b})
return test{
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down Expand Up @@ -569,7 +569,7 @@ func TestHandlerGetUpdateAccount(t *testing.T) {
"fail/no-account": func(t *testing.T) test {
return test{
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -578,7 +578,7 @@ func TestHandlerGetUpdateAccount(t *testing.T) {
ctx = context.WithValue(ctx, accContextKey, nil)
return test{
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down
22 changes: 14 additions & 8 deletions acme/api/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ package api

import (
"fmt"
"net/http"

"github.com/go-chi/chi"
"github.com/pkg/errors"
"github.com/smallstep/certificates/acme"
"github.com/smallstep/certificates/api"
"github.com/smallstep/certificates/authority/provisioner"
"github.com/smallstep/cli/jose"
"net/http"
)

func link(url, typ string) string {
Expand Down Expand Up @@ -181,13 +180,20 @@ func (h *Handler) GetChallenge(w http.ResponseWriter, r *http.Request) {
ch, err = h.Auth.ValidateChallenge(prov, acc.GetID(), chID, acc.GetKey())
if err != nil {
api.WriteError(w, err)
return
} else if ch.Retry.Active {
retryAfter, err := h.Auth.BackoffChallenge(prov, acc.GetID(), chID, acc.GetKey())
if err != nil {
api.WriteError(w, err)
} else {
w.Header().Add("Retry-After", retryAfter.String())
api.WriteProcessing(w, ch)
}
} else {
getLink := h.Auth.GetLink
w.Header().Add("Link", link(getLink(acme.AuthzLink, acme.URLSafeProvisionerName(prov), true, ch.GetAuthzID()), "up"))
w.Header().Set("Location", getLink(acme.ChallengeLink, acme.URLSafeProvisionerName(prov), true, ch.GetID()))
api.JSON(w, ch)
}

getLink := h.Auth.GetLink
w.Header().Add("Link", link(getLink(acme.AuthzLink, acme.URLSafeProvisionerName(prov), true, ch.GetAuthzID()), "up"))
w.Header().Set("Location", getLink(acme.ChallengeLink, acme.URLSafeProvisionerName(prov), true, ch.GetID()))
api.JSON(w, ch)
}

// GetCertificate ACME api for retrieving a Certificate.
Expand Down
52 changes: 45 additions & 7 deletions acme/api/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ func TestHandlerGetAuthz(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -382,7 +382,7 @@ func TestHandlerGetAuthz(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down Expand Up @@ -504,7 +504,7 @@ func TestHandlerGetCertificate(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -513,7 +513,7 @@ func TestHandlerGetCertificate(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down Expand Up @@ -589,6 +589,7 @@ func ch() acme.Challenge {
URL: "https://ca.smallstep.com/acme/challenge/chID",
ID: "chID",
AuthzID: "authzID",
Retry: &acme.Retry{Called:0, Backoffs:1, Active:false},
}
}

Expand Down Expand Up @@ -623,7 +624,7 @@ func TestHandlerGetChallenge(t *testing.T) {
"fail/no-account": func(t *testing.T) test {
return test{
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -632,7 +633,7 @@ func TestHandlerGetChallenge(t *testing.T) {
ctx = context.WithValue(ctx, accContextKey, nil)
return test{
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down Expand Up @@ -733,6 +734,37 @@ func TestHandlerGetChallenge(t *testing.T) {
ch: ch,
}
},
"ok/retry-after": func(t *testing.T) test {
dcow marked this conversation as resolved.
Show resolved Hide resolved
key, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
assert.FatalError(t, err)
acc := &acme.Account{ID: "accID", Key: key}
ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
ctx = context.WithValue(ctx, accContextKey, acc)
// TODO: Add correct key such that challenge object is already "active"
chiCtxInactive := chi.NewRouteContext()
chiCtxInactive.URLParams.Add("chID", "chID")
//chiCtxInactive.URLParams.Add("Active", "true")
ctx = context.WithValue(ctx, chi.RouteCtxKey, chiCtxInactive)
ch := ch()
ch.Retry.Active = true
chJSON, err := json.Marshal(ch)
assert.FatalError(t, err)
ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: chJSON})
return test{
auth: &mockAcmeAuthority{
validateChallenge: func(p provisioner.Interface, accID, id string, jwk *jose.JSONWebKey) (*acme.Challenge, error) {
assert.Equals(t, p, prov)
assert.Equals(t, accID, acc.ID)
assert.Equals(t, id, ch.ID)
assert.Equals(t, jwk.KeyID, key.KeyID)
return &ch, nil
},
},
ctx: ctx,
statusCode: 200,
ch: ch,
}
},
}
for name, run := range tests {
tc := run(t)
Expand Down Expand Up @@ -760,13 +792,19 @@ func TestHandlerGetChallenge(t *testing.T) {
assert.Equals(t, ae.Identifier, prob.Identifier)
assert.Equals(t, ae.Subproblems, prob.Subproblems)
assert.Equals(t, res.Header["Content-Type"], []string{"application/problem+json"})
} else {
} else if res.StatusCode >= 200 {
expB, err := json.Marshal(tc.ch)
assert.FatalError(t, err)
assert.Equals(t, bytes.TrimSpace(body), expB)
assert.Equals(t, res.Header["Link"], []string{fmt.Sprintf("<https://ca.smallstep.com/acme/authz/%s>;rel=\"up\"", tc.ch.AuthzID)})
assert.Equals(t, res.Header["Location"], []string{url})
assert.Equals(t, res.Header["Content-Type"], []string{"application/json"})
} else if res.StatusCode >= 100 {
expB, err := json.Marshal(tc.ch)
wesgraham marked this conversation as resolved.
Show resolved Hide resolved
assert.FatalError(t, err)
assert.Equals(t, bytes.TrimSpace(body), expB)
assert.True(t, res.Header["Retry-After"] != nil)
assert.Equals(t, res.Header["Content-Type"], []string{"application/json"})
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion acme/api/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ func TestHandlerLookupJWK(t *testing.T) {
},
},
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down
12 changes: 6 additions & 6 deletions acme/api/order_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func TestHandlerGetOrder(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -215,7 +215,7 @@ func TestHandlerGetOrder(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down Expand Up @@ -343,7 +343,7 @@ func TestHandlerNewOrder(t *testing.T) {
"fail/no-account": func(t *testing.T) test {
return test{
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -352,7 +352,7 @@ func TestHandlerNewOrder(t *testing.T) {
ctx = context.WithValue(ctx, accContextKey, nil)
return test{
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down Expand Up @@ -597,7 +597,7 @@ func TestHandlerFinalizeOrder(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: context.WithValue(context.Background(), provisionerContextKey, prov),
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand All @@ -607,7 +607,7 @@ func TestHandlerFinalizeOrder(t *testing.T) {
return test{
auth: &mockAcmeAuthority{},
ctx: ctx,
statusCode: 400,
statusCode: 404,
problem: acme.AccountDoesNotExistErr(nil),
}
},
Expand Down
Loading