Skip to content

Commit

Permalink
Require HS256, HS384, or HS512 for EAB
Browse files Browse the repository at this point in the history
During the 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.
  • Loading branch information
mcpherrinm committed May 1, 2024
1 parent f3d2567 commit ac5d6f1
Showing 1 changed file with 4 additions and 9 deletions.
13 changes: 4 additions & 9 deletions wfe/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -2786,14 +2786,16 @@ func (wfe *WebFrontEndImpl) verifyEAB(
fmt.Sprintf("failed to encode external account binding JSON structure: %s", err))
}

eab, err := jose.ParseSigned(string(eabBytes), goodJWSSignatureAlgorithms)
// The "alg" field MUST indicate a MAC-based algorithm
eab, err := jose.ParseSigned(string(eabBytes), []jose.SignatureAlgorithm{
jose.HS256, jose.HS384, jose.HS512,
})
if err != nil {
return nil, acme.MalformedProblem(
fmt.Sprintf("failed to decode external account binding: %s", err))
}

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

header := innerJWS.Signatures[0].Protected
switch header.Algorithm {
case "HS256", "HS384", "HS512":
break
default:
return "", acme.BadPublicKeyProblem(
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))
}
if len(header.Nonce) > 0 {
return "", acme.MalformedProblem(
"the 'nonce' field must be absent in external account binding")
Expand Down

0 comments on commit ac5d6f1

Please sign in to comment.