From 9bb1b24bf1a23407c5626c29f7ed1fb1f021c536 Mon Sep 17 00:00:00 2001
From: Herman Slatman <hermanslatman@hotmail.com>
Date: Fri, 12 Jan 2024 10:44:49 +0100
Subject: [PATCH] Change `kid` and `dpop` validation

---
 acme/challenge.go      | 55 ++++++++++++++++++------------------------
 acme/challenge_test.go | 11 ++++++---
 2 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/acme/challenge.go b/acme/challenge.go
index 5d190519..f3c23d34 100644
--- a/acme/challenge.go
+++ b/acme/challenge.go
@@ -352,9 +352,9 @@ func dns01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebK
 
 type wireOidcPayload struct {
 	// IDToken contains the OIDC identity token
-	IDToken string `json:"id_token,omitempty"`
+	IDToken string `json:"id_token"`
 	// KeyAuth ({challenge-token}.{jwk-thumbprint})
-	KeyAuth string `json:"keyauth,omitempty"`
+	KeyAuth string `json:"keyauth"`
 }
 
 func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebKey, payload []byte) error {
@@ -448,7 +448,7 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO
 
 type wireDpopPayload struct {
 	// AccessToken is the token generated by wire-server
-	AccessToken string `json:"access_token,omitempty"`
+	AccessToken string `json:"access_token"`
 }
 
 func wireDPOP01Validate(ctx context.Context, ch *Challenge, db DB, accountJWK *jose.JSONWebKey, payload []byte) error {
@@ -484,13 +484,14 @@ func wireDPOP01Validate(ctx context.Context, ch *Challenge, db DB, accountJWK *j
 		return WrapErrorISE(err, "invalid Go template registered for 'target'")
 	}
 
-	params := verifyParams{
+	params := wireVerifyParams{
 		token:     dpopPayload.AccessToken,
 		tokenKey:  dpopOptions.GetSigningKey(),
-		dpopKey:   accountJWK,
+		dpopKey:   accountJWK.Public(),
+		dpopKeyID: accountJWK.KeyID,
 		issuer:    issuer,
 		wireID:    wireID,
-		challenge: ch,
+		chToken:   ch.Token,
 		t:         clock.Now().UTC(),
 	}
 	_, dpop, err := parseAndVerifyWireAccessToken(params)
@@ -523,33 +524,34 @@ func wireDPOP01Validate(ctx context.Context, ch *Challenge, db DB, accountJWK *j
 	return nil
 }
 
-type cnf struct {
-	Kid string `json:"kid,omitempty"`
+type wireCnf struct {
+	Kid string `json:"kid"`
 }
 
 type wireAccessToken struct {
 	jose.Claims
-	Challenge  string `json:"chal,omitempty"`
-	Cnf        *cnf   `json:"cnf,omitempty"`
-	Proof      string `json:"proof,omitempty"`
-	ClientID   string `json:"client_id,omitempty"`
-	APIVersion int    `json:"api_version,omitempty"`
-	Scope      string `json:"scope,omitempty"`
+	Challenge  string  `json:"chal"`
+	Cnf        wireCnf `json:"cnf"`
+	Proof      string  `json:"proof"`
+	ClientID   string  `json:"client_id"`
+	APIVersion int     `json:"api_version"`
+	Scope      string  `json:"scope"`
 }
 
 type wireDpopToken map[string]any
 
-type verifyParams struct {
+type wireVerifyParams struct {
 	token     string
 	tokenKey  crypto.PublicKey
-	dpopKey   *jose.JSONWebKey
+	dpopKey   crypto.PublicKey
+	dpopKeyID string
 	issuer    string
 	wireID    wire.ID
-	challenge *Challenge
+	chToken   string
 	t         time.Time
 }
 
-func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopToken, error) {
+func parseAndVerifyWireAccessToken(v wireVerifyParams) (*wireAccessToken, *wireDpopToken, error) {
 	jwt, err := jose.ParseSigned(v.token)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed parsing token: %w", err)
@@ -567,17 +569,8 @@ func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopT
 		return nil, nil, fmt.Errorf("failed validation: %w", err)
 	}
 
-	rawKid, err := v.dpopKey.Thumbprint(crypto.SHA256)
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to compute JWK thumbprint")
-	}
-	kid := base64.RawURLEncoding.EncodeToString(rawKid)
-
-	if accessToken.Cnf == nil {
-		return nil, nil, errors.New("'cnf' is nil")
-	}
-	if accessToken.Cnf.Kid != kid {
-		return nil, nil, fmt.Errorf("expected kid %q; got %q", kid, accessToken.Cnf.Kid)
+	if accessToken.Cnf.Kid != v.dpopKeyID {
+		return nil, nil, fmt.Errorf("expected kid %q; got %q", v.dpopKeyID, accessToken.Cnf.Kid)
 	}
 	if accessToken.ClientID != v.wireID.ClientID {
 		return nil, nil, fmt.Errorf("invalid Wire client ID %q", accessToken.ClientID)
@@ -591,7 +584,7 @@ func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopT
 		return nil, nil, fmt.Errorf("invalid Wire DPoP token: %w", err)
 	}
 	var dpopToken wireDpopToken
-	if err := dpopJWT.Claims(v.dpopKey.Key, &dpopToken); err != nil {
+	if err := dpopJWT.Claims(v.dpopKey, &dpopToken); err != nil {
 		return nil, nil, fmt.Errorf("failed validating Wire DPoP token claims: %w", err)
 	}
 
@@ -599,7 +592,7 @@ func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopT
 	if !ok {
 		return nil, nil, fmt.Errorf("invalid challenge in Wire DPoP token")
 	}
-	if challenge != v.challenge.Token {
+	if challenge != v.chToken {
 		return nil, nil, fmt.Errorf("invalid Wire DPoP challenge %q", challenge)
 	}
 
diff --git a/acme/challenge_test.go b/acme/challenge_test.go
index d917b1e2..f3f65a21 100644
--- a/acme/challenge_test.go
+++ b/acme/challenge_test.go
@@ -4332,13 +4332,18 @@ MCowBQYDK2VwAyEAB2IYqBWXAouDt3WcCZgCM3t9gumMEKMlgMsGenSu+fA=
 	var accountJWK jose.JSONWebKey
 	json.Unmarshal(jwkBytes, &accountJWK)
 
-	at, dpop, err := parseAndVerifyWireAccessToken(verifyParams{
+	rawKid, err := accountJWK.Thumbprint(crypto.SHA256)
+	require.NoError(t, err)
+	accountJWK.KeyID = base64.RawURLEncoding.EncodeToString(rawKid)
+
+	at, dpop, err := parseAndVerifyWireAccessToken(wireVerifyParams{
 		token:     token,
 		tokenKey:  publicKey,
-		dpopKey:   &accountJWK,
+		dpopKey:   accountJWK.Public(),
+		dpopKeyID: accountJWK.KeyID,
 		issuer:    issuer,
 		wireID:    wireID,
-		challenge: ch,
+		chToken:   ch.Token,
 		t:         issuedAt.Add(1 * time.Minute), // set validation time to be one minute after issuance
 	})
 	if assert.NoError(t, err) {