Skip to content

Commit e87ace7

Browse files
mcpherrinmj0ru
andauthored
Require HS256, HS384, or HS512 for EAB (#459)
During the go-jose/v4 upgrade, I accidentally required the same signature sets for EAB as for the account keys, which is incorrect. This allows the correct MAC-based algorithms. It drops the custom algorithm checks, which are now unreachable as go-jose will enforce the algorithms. This also adds a new EAB key to Pebble's test config which explicitly has base64url characters, from #428 Fixes #455 --------- Co-authored-by: Folke Gleumes <[email protected]>
1 parent 8250e65 commit e87ace7

File tree

3 files changed

+35
-10
lines changed

3 files changed

+35
-10
lines changed

.github/workflows/tests.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,33 @@ jobs:
5959
run: |
6060
git clone https://github.com/eggsampler/acme.git /tmp/eggsampler-acme
6161
cd /tmp/eggsampler-acme && make test
62+
lego-eab-linux:
63+
name: Test lego with EAB
64+
runs-on: ubuntu-latest
65+
steps:
66+
- uses: actions/checkout@v4
67+
- uses: actions/setup-go@v5
68+
with:
69+
go-version-file: go.mod
70+
- name: Install lego cli
71+
run: go install github.com/go-acme/lego/v4/cmd/lego@latest
72+
- name: go install commands
73+
run: go install -v ./cmd/...
74+
- name: launch pebble
75+
run: |
76+
GORACE="halt_on_error=1" PEBBLE_VA_ALWAYS_VALID=1 \
77+
pebble -config test/config/pebble-config-external-account-bindings.json &
78+
- run: |
79+
LEGO_CA_CERTIFICATES=./test/certs/pebble.minica.pem \
80+
lego --accept-tos \
81+
--server=https://localhost:14000/dir \
82+
--email="[email protected]" \
83+
--domains=example.letsencrypt.org \
84+
--eab \
85+
--kid kid-3 \
86+
--hmac=HjudV5qnbreN-n9WyFSH-t4HXuEx_XFen45zuxY-G1h6fr74V3cUM_dVlwQZBWmc \
87+
--http --http.port=:5002 \
88+
run
6289
go-linux:
6390
name: Run Go tests on Linux
6491
runs-on: ubuntu-latest

test/config/pebble-config-external-account-bindings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
"externalAccountBindingRequired": true,
1515
"externalAccountMACKeys": {
1616
"kid-1": "zWNDZM6eQGHWpSRTPal5eIUYFTu7EajVIoguysqZ9wG44nMEtx3MUAsUDkMTQ12W",
17-
"kid-2": "b10lLJs8l1GPIzsLP0s6pMt8O0XVGnfTaCeROxQM0BIt2XrJMDHJZBM5NuQmQJQH"
17+
"kid-2": "b10lLJs8l1GPIzsLP0s6pMt8O0XVGnfTaCeROxQM0BIt2XrJMDHJZBM5NuQmQJQH",
18+
"kid-3": "HjudV5qnbreN-n9WyFSH-t4HXuEx_XFen45zuxY-G1h6fr74V3cUM_dVlwQZBWmc"
1819
},
1920
"certificateValidityPeriod": 157766400
2021
}

wfe/wfe.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2786,14 +2786,18 @@ func (wfe *WebFrontEndImpl) verifyEAB(
27862786
fmt.Sprintf("failed to encode external account binding JSON structure: %s", err))
27872787
}
27882788

2789-
eab, err := jose.ParseSigned(string(eabBytes), goodJWSSignatureAlgorithms)
2789+
// The "alg" field MUST indicate a MAC-based algorithm
2790+
eabSignatureAlgorithms := []jose.SignatureAlgorithm{
2791+
jose.HS256, jose.HS384, jose.HS512,
2792+
}
2793+
2794+
eab, err := jose.ParseSigned(string(eabBytes), eabSignatureAlgorithms)
27902795
if err != nil {
27912796
return nil, acme.MalformedProblem(
27922797
fmt.Sprintf("failed to decode external account binding: %s", err))
27932798
}
27942799

27952800
// 2. Verify that the JWS protected field meets the following criteria
2796-
// - The "alg" field MUST indicate a MAC-based algorithm
27972801
// - The "kid" field MUST contain the key identifier provided by the CA
27982802
// - The "nonce" field MUST NOT be present
27992803
// - The "url" field MUST be set to the same value as the outer JWS
@@ -2839,13 +2843,6 @@ func (wfe *WebFrontEndImpl) verifyEABPayloadHeader(innerJWS *jose.JSONWebSignatu
28392843
}
28402844

28412845
header := innerJWS.Signatures[0].Protected
2842-
switch header.Algorithm {
2843-
case "HS256", "HS384", "HS512":
2844-
break
2845-
default:
2846-
return "", acme.BadPublicKeyProblem(
2847-
fmt.Sprintf("the 'alg' field is set to %q, which is not valid for external account binding, valid values are: HS256, HS384 or HS512", header.Algorithm))
2848-
}
28492846
if len(header.Nonce) > 0 {
28502847
return "", acme.MalformedProblem(
28512848
"the 'nonce' field must be absent in external account binding")

0 commit comments

Comments
 (0)