[crypto] acme/autocert: support External Account Binding (EAB) tokens
Ben Burkert (Gerrit)
Ben Burkert has uploaded this change for review.
acme/autocert: support External Account Binding (EAB) tokens
Support External Account Binding (EAB) tokens to the Manager as defined
in RFC 8555, Section 7.3.4. If the ExternalAccountBinding field is set
on Manager, pass it into the acme Account during registration.
Fixes golang/go#48809
Change-Id: I64c38b05ab577acbde9f526638cc8104d15ff055
---
M acme/autocert/autocert_test.go
M acme/autocert/internal/acmetest/ca.go
M acme/autocert/autocert.go
3 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go
index c7fbc54..dc565f3 100644
--- a/acme/autocert/autocert.go
+++ b/acme/autocert/autocert.go
@@ -168,6 +168,11 @@
// in the template's ExtraExtensions field as is.
ExtraExtensions []pkix.Extension
+ // ExternalAccountBinding optionally represents an arbitrary binding to an
+ // account of the CA to which the ACME server is tied.
+ // See https://tools.ietf.org/html/rfc8555#section-7.3.4 for more details.
+ ExternalAccountBinding *acme.ExternalAccountBinding
+
clientMu sync.Mutex
client *acme.Client // initialized by acmeClient method
@@ -1068,7 +1073,7 @@
if m.Email != "" {
contact = []string{"mailto:" + m.Email}
}
- a := &acme.Account{Contact: contact}
+ a := &acme.Account{Contact: contact, ExternalAccountBinding: m.ExternalAccountBinding}
_, err := client.Register(ctx, a, m.Prompt)
if err == nil || isAccountAlreadyExist(err) {
m.client = client
diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go
index 59f39c1..895c7f2 100644
--- a/acme/autocert/autocert_test.go
+++ b/acme/autocert/autocert_test.go
@@ -1181,13 +1181,17 @@
const domain = "example.org"
// ACME CA server
- ca := acmetest.NewCAServer([]string{"tls-alpn-01"}, []string{domain})
+ ca := acmetest.NewCAServer([]string{"tls-alpn-01"}, []string{domain}, true)
defer ca.Close()
// User dummy server.
m := &Manager{
Prompt: AcceptTOS,
Client: &acme.Client{DirectoryURL: ca.URL},
+ ExternalAccountBinding: &acme.ExternalAccountBinding{
+ KID: "test-key",
+ Key: make([]byte, 32),
+ },
}
us := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte("OK"))
diff --git a/acme/autocert/internal/acmetest/ca.go b/acme/autocert/internal/acmetest/ca.go
index faffd20..b8dfc8e 100644
--- a/acme/autocert/internal/acmetest/ca.go
+++ b/acme/autocert/internal/acmetest/ca.go
@@ -46,6 +46,7 @@
server *httptest.Server
challengeTypes []string // supported challenge types
domainsWhitelist []string // only these domains are valid for issuing, unless empty
+ eabRequired bool // EAB JWS required for account registration
mu sync.Mutex
certCount int // number of issued certs
@@ -63,7 +64,7 @@
// sent to a client in a response for a domain authorization.
// If domainsWhitelist is non-empty, the certs will be issued only for the specified
// list of domains. Otherwise, any domain name is allowed.
-func NewCAServer(challengeTypes []string, domainsWhitelist []string) *CAServer {
+func NewCAServer(challengeTypes []string, domainsWhitelist []string, eabRequired bool) *CAServer {
var whitelist []string
for _, name := range domainsWhitelist {
whitelist = append(whitelist, name)
@@ -74,6 +75,7 @@
domainsWhitelist: whitelist,
domainAddr: make(map[string]string),
authorizations: make(map[string]*authorization),
+ eabRequired: eabRequired,
}
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -150,6 +152,12 @@
NewReg string `json:"newAccount"`
NewOrder string `json:"newOrder"`
NewAuthz string `json:"newAuthz"`
+
+ Meta discoveryMeta `json:"meta"`
+}
+
+type discoveryMeta struct {
+ ExternalAccountRequired bool `json:"externalAccountRequired"`
}
type challenge struct {
@@ -190,6 +198,9 @@
NewReg: ca.serverURL("/new-reg"),
NewOrder: ca.serverURL("/new-order"),
NewAuthz: ca.serverURL("/new-authz"),
+ Meta: discoveryMeta{
+ ExternalAccountRequired: ca.eabRequired,
+ },
}
if err := json.NewEncoder(w).Encode(resp); err != nil {
panic(fmt.Sprintf("discovery response: %v", err))
@@ -202,6 +213,22 @@
// Client key registration request.
case r.URL.Path == "/new-reg":
+ var req struct {
+ Contact []string
+ TermsOfServiceAgreed bool
+ ExternalAccountBinding json.RawMessage
+ }
+
+ if err := decodePayload(&req, r.Body); err != nil {
+ ca.httpErrorf(w, http.StatusBadRequest, err.Error())
+ return
+ }
+
+ if ca.eabRequired && len(req.ExternalAccountBinding) == 0 {
+ ca.httpErrorf(w, http.StatusBadRequest, "registration failed: no jws for eab")
+ return
+ }
+
// TODO: Check the user account key against a ca.accountKeys?
w.Header().Set("Location", ca.serverURL("/accounts/1"))
w.WriteHeader(http.StatusCreated)
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
1 comment:
Patchset:
Ping, Filippo/Roland.
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Attention is currently required from: Ben Burkert.
2 comments:
File acme/autocert/internal/acmetest/ca.go:
Patch Set #1, Line 215:
case r.URL.Path == "/new-reg":I forget from limited context here: is this the pre-RFC or RFC way?
I was trying to delete all the old unused pre-RFC code (in https://go-review.googlesource.com/c/crypto/+/342449) but didn't quite finish yet, as I got stuck by the tests all assuming they could work in pre-RFC mode only (our tests were the only user of the pre-RFC code)
I don't want to add more dependence on the old way, if this isn't the new RFC way.
Patch Set #1, Line 228:
ca.httpErrorf(w, http.StatusBadRequest, "registration failed: no jws for eab")no JWS for EAB
(capitalize acronyms for humans)
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Ben Burkert.
Ben Burkert uploaded patch set #2 to this change.
acme/autocert: support External Account Binding (EAB) tokens
Support External Account Binding (EAB) tokens to the Manager as defined
in RFC 8555, Section 7.3.4. If the ExternalAccountBinding field is set
on Manager, pass it into the acme Account during registration.
Fixes golang/go#48809
Change-Id: I64c38b05ab577acbde9f526638cc8104d15ff055
---
M acme/autocert/autocert_test.go
M acme/autocert/internal/acmetest/ca.go
M acme/autocert/autocert.go
3 files changed, 54 insertions(+), 3 deletions(-)
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
2 comments:
File acme/autocert/internal/acmetest/ca.go:
Patch Set #1, Line 215:
case r.URL.Path == "/new-reg":
I forget from limited context here: is this the pre-RFC or RFC way? […]
It's the old path style, but it's an RFC compliant server. I have a branch that swaps "/new-reg" for "/new-account", and also picks up where CL342449 left off. Would you like that submitted as a new CL?
Patch Set #1, Line 228:
ca.httpErrorf(w, http.StatusBadRequest, "registration failed: no jws for eab")
no JWS for EAB […]
Done
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
1 comment:
File acme/autocert/internal/acmetest/ca.go:
Patch Set #1, Line 215:
case r.URL.Path == "/new-reg":
It's the old path style, but it's an RFC compliant server. […]
here's the commit: https://github.com/benburkert/crypto/commit/74cd8c674e4246fba978d298cd65e8851fe6c20b
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Patchset:
Ping
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Patchset:
Ping
I've submitted a different CL for the pre-rfc cleanup stuff, so I think this is good to go.
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
Ben Burkert uploaded patch set #3 to this change.
acme/autocert: support External Account Binding (EAB) tokens
Support External Account Binding (EAB) tokens to the Manager as defined
in RFC 8555, Section 7.3.4. If the ExternalAccountBinding field is set
on Manager, pass it into the acme Account during registration.
Fixes golang/go#48809
Change-Id: I64c38b05ab577acbde9f526638cc8104d15ff055
---
M acme/autocert/autocert.go
M acme/autocert/autocert_test.go
M acme/autocert/internal/acmetest/ca.go
3 files changed, 68 insertions(+), 1 deletion(-)
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
Ben Burkert uploaded patch set #4 to this change.
acme/autocert: support External Account Binding (EAB) tokens
Support External Account Binding (EAB) tokens to the Manager as defined
in RFC 8555, Section 7.3.4. If the ExternalAccountBinding field is set
on Manager, pass it into the acme Account during registration.
Fixes golang/go#48809
Change-Id: I64c38b05ab577acbde9f526638cc8104d15ff055
---
M acme/autocert/autocert.go
M acme/autocert/autocert_test.go
M acme/autocert/internal/acmetest/ca.go
3 files changed, 68 insertions(+), 1 deletion(-)
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
1 comment:
Patchset:
Ping bradfitz
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Attention is currently required from: Ben Burkert.
Patch set 5:Run-TryBot +1Code-Review +2Trust +1
3 comments:
Patchset:
LGTM at least. Roland, Filippo?
File acme/autocert/internal/acmetest/ca.go:
Patch Set #5, Line 156:
// ExternalAccountRequired makes an EAB JWS required for account registration.I didn't realize we used this chained return-the-receiver style anywhere but I guess this package does.
Maybe we can make that a bit more explicit in the docs?
// ExternalAccountRequired marks ca as requiring an EAB JWS for account registration.
//
// It modifies and returns ca.Patch Set #5, Line 238:
Meta discoveryMeta `json:"meta"`should either this and/or ExternalAccountRequired be omitempty? nothing will choke on the extra fields if they don't understand meta, will they? that's part of the base spec?
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
3 comments:
Patchset:
Thanks Brad.
File acme/autocert/internal/acmetest/ca.go:
Patch Set #5, Line 156:
// ExternalAccountRequired makes an EAB JWS required for account registration.
I didn't realize we used this chained return-the-receiver style anywhere but I guess this package do […]
I followed the signature and comments for the ChallengeTypes method. Happy to make these more explicit as part of CL 384698, does that sound ok?
Patch Set #5, Line 238:
Meta discoveryMeta `json:"meta"`
should either this and/or ExternalAccountRequired be omitempty? nothing will choke on the extra fiel […]
LE's directory endpoint always includes a `meta` section, but it omits the `externalAccountRequired` field. And acme.Client handles it fine, which seems to be the only consumer of this internal ACME implementation.
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ben Burkert.
Patch set 5:Auto-Submit +1Code-Review +2
3 comments:
File acme/autocert/autocert.go:
Patch Set #5, Line 175:
https://tools.ietf.org/html/rfc8555#section-7.3.4RFC 8555, Section 7.3.4
(which gets autolinked by some godoc implementations)
File acme/autocert/internal/acmetest/ca.go:
Patch Set #5, Line 156:
// ExternalAccountRequired makes an EAB JWS required for account registration.
I followed the signature and comments for the ChallengeTypes method. […]
Ah that was me in CL 381715, I'm used to it in crypto packages and found it convenient setting up the test server here.
Patch Set #5, Line 238:
Meta discoveryMeta `json:"meta"`
LE's directory endpoint always includes a `meta` section, but it omits the `externalAccountRequired` […]
Still make `meta` `omitempty`, so we have tests for the case where it's missing, and would catch a nil dereference, for example.
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ben Burkert.
Ben Burkert uploaded patch set #6 to this change.
acme/autocert: support External Account Binding (EAB) tokens
Support External Account Binding (EAB) tokens to the Manager as defined
in RFC 8555, Section 7.3.4. If the ExternalAccountBinding field is set
on Manager, pass it into the acme Account during registration.
Fixes golang/go#48809
Change-Id: I64c38b05ab577acbde9f526638cc8104d15ff055
---
M acme/autocert/autocert.go
M acme/autocert/autocert_test.go
M acme/autocert/internal/acmetest/ca.go
3 files changed, 68 insertions(+), 1 deletion(-)
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Ben Burkert (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
3 comments:
Patchset:
Thanks Filippo.
File acme/autocert/autocert.go:
RFC 8555, Section 7.3.4 […]
Done
File acme/autocert/internal/acmetest/ca.go:
Patch Set #5, Line 238:
Meta discoveryMeta `json:"meta"`Still make `meta` `omitempty`, so we have tests for the case where it's missing, and would catch a n […]
Done
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
Attention is currently required from: Ben Burkert, Brad Fitzpatrick.
Patch set 6:Run-TryBot +1Auto-Submit +1Code-Review +2
Gopher Robot (Gerrit)
Gopher Robot submitted this change.
acme/autocert: support External Account Binding (EAB) tokens
Support External Account Binding (EAB) tokens to the Manager as defined
in RFC 8555, Section 7.3.4. If the ExternalAccountBinding field is set
on Manager, pass it into the acme Account during registration.
Fixes golang/go#48809
Change-Id: I64c38b05ab577acbde9f526638cc8104d15ff055
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/354189
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
Trust: Brad Fitzpatrick <brad...@golang.org>
Reviewed-by: Filippo Valsorda <fil...@golang.org>
Run-TryBot: Filippo Valsorda <fil...@golang.org>
Auto-Submit: Filippo Valsorda <fil...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M acme/autocert/autocert.go
M acme/autocert/autocert_test.go
M acme/autocert/internal/acmetest/ca.go
3 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go
index 1858184..0061c28 100644
--- a/acme/autocert/autocert.go
+++ b/acme/autocert/autocert.go
@@ -170,6 +170,11 @@
// in the template's ExtraExtensions field as is.
ExtraExtensions []pkix.Extension
+ // ExternalAccountBinding optionally represents an arbitrary binding to an
+ // account of the CA to which the ACME server is tied.
+ // See RFC 8555, Section 7.3.4 for more details.
+ ExternalAccountBinding *acme.ExternalAccountBinding
+
clientMu sync.Mutex
client *acme.Client // initialized by acmeClient method
@@ -996,7 +1001,7 @@
if m.Email != "" {
contact = []string{"mailto:" + m.Email}
}
- a := &acme.Account{Contact: contact}
+ a := &acme.Account{Contact: contact, ExternalAccountBinding: m.ExternalAccountBinding}
_, err := client.Register(ctx, a, m.Prompt)
if err == nil || isAccountAlreadyExist(err) {
m.client = client
diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go
index 4ae408f..ab7504a 100644
--- a/acme/autocert/autocert_test.go
+++ b/acme/autocert/autocert_test.go
@@ -394,6 +394,19 @@
}
},
},
+ {
+ name: "provideExternalAuth",
+ hello: clientHelloInfo("example.org", algECDSA),
+ domain: "example.org",
+ prepare: func(t *testing.T, man *Manager, s *acmetest.CAServer) {
+ s.ExternalAccountRequired()
+
+ man.ExternalAccountBinding = &acme.ExternalAccountBinding{
+ KID: "test-key",
+ Key: make([]byte, 32),
+ }
+ },
+ },
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
diff --git a/acme/autocert/internal/acmetest/ca.go b/acme/autocert/internal/acmetest/ca.go
index 8c4c642..fa33987 100644
--- a/acme/autocert/internal/acmetest/ca.go
+++ b/acme/autocert/internal/acmetest/ca.go
@@ -49,6 +49,7 @@
challengeTypes []string
url string
roots *x509.CertPool
+ eabRequired bool
mu sync.Mutex
certCount int // number of issued certs
@@ -152,6 +153,15 @@
return ca.roots
}
+// ExternalAccountRequired makes an EAB JWS required for account registration.
+func (ca *CAServer) ExternalAccountRequired() *CAServer {
+ if ca.url != "" {
+ panic("ExternalAccountRequired must be called before Start")
+ }
+ ca.eabRequired = true
+ return ca
+}
+
// Start starts serving requests. The server address becomes available in the
// URL field.
func (ca *CAServer) Start() *CAServer {
@@ -224,6 +234,12 @@
NewAccount string `json:"newAccount"`
NewOrder string `json:"newOrder"`
NewAuthz string `json:"newAuthz"`
+
+ Meta discoveryMeta `json:"meta,omitempty"`
+}
+
+type discoveryMeta struct {
+ ExternalAccountRequired bool `json:"externalAccountRequired,omitempty"`
}
type challenge struct {
@@ -264,6 +280,9 @@
NewNonce: ca.serverURL("/new-nonce"),
NewAccount: ca.serverURL("/new-account"),
NewOrder: ca.serverURL("/new-order"),
+ Meta: discoveryMeta{
+ ExternalAccountRequired: ca.eabRequired,
+ },
}
if err := json.NewEncoder(w).Encode(resp); err != nil {
panic(fmt.Sprintf("discovery response: %v", err))
@@ -283,6 +302,21 @@
return
}
ca.acctRegistered = true
+
+ var req struct {
+ ExternalAccountBinding json.RawMessage
+ }
+
+ if err := decodePayload(&req, r.Body); err != nil {
+ ca.httpErrorf(w, http.StatusBadRequest, err.Error())
+ return
+ }
+
+ if ca.eabRequired && len(req.ExternalAccountBinding) == 0 {
+ ca.httpErrorf(w, http.StatusBadRequest, "registration failed: no JWS for EAB")
+ return
+ }
+
// TODO: Check the user account key against a ca.accountKeys?
w.Header().Set("Location", ca.serverURL("/accounts/1"))
w.WriteHeader(http.StatusCreated)
To view, visit change 354189. To unsubscribe, or for help writing mail filters, visit settings.