Add validation of `handle` and `token` to Wire verification

acme/challenge.go

@@ -389,7 +389,7 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO
 			"error retrieving claims from ID token"))
 	}
 
-	challengeValues, err := wire.ParseID([]byte(ch.Value))
+	wireID, err := wire.ParseID([]byte(ch.Value))
 	if err != nil {
 		return WrapErrorISE(err, "error unmarshalling challenge data")
 	}
@@ -400,11 +400,11 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO
 	}
 	if expectedKeyAuth != oidcPayload.KeyAuth {
 		return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType,
-			"keyAuthorization does not match; expected %s, but got %s", expectedKeyAuth, oidcPayload.KeyAuth))
+			"keyAuthorization does not match; expected %q, but got %q", expectedKeyAuth, oidcPayload.KeyAuth))
 	}
 
-	if challengeValues.Name != claims.Name || challengeValues.Handle != claims.Handle {
-		return storeError(ctx, db, ch, false, NewError(ErrorRejectedIdentifierType, "OIDC claims don't match"))
+	if wireID.Name != claims.Name || wireID.Handle != claims.Handle {
+		return storeError(ctx, db, ch, false, NewError(ErrorRejectedIdentifierType, "claims in OIDC ID token don't match"))
 	}
 
 	// Update and store the challenge.
@@ -578,43 +578,40 @@ func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopT
 	if accessToken.Cnf == nil {
 		return nil, nil, errors.New("'cnf' is nil")
 	}
-
 	if accessToken.Cnf.Kid != v.kid {
 		return nil, nil, fmt.Errorf("expected kid %q; got %q", v.kid, accessToken.Cnf.Kid)
 	}
-
 	if accessToken.ClientID != v.wireID.ClientID {
 		return nil, nil, fmt.Errorf("invalid Wire client ID %q", accessToken.ClientID)
 	}
 
-	parsedDpopToken, err := jose.ParseSigned(accessToken.Proof)
+	dpopJWT, err := jose.ParseSigned(accessToken.Proof)
 	if err != nil {
 		return nil, nil, fmt.Errorf("invalid Wire DPoP token: %w", err)
 	}
 	var dpopToken wireDpopToken
-	if err := parsedDpopToken.UnsafeClaimsWithoutVerification(&dpopToken); err != nil {
+	if err := dpopJWT.UnsafeClaimsWithoutVerification(&dpopToken); err != nil {
 		return nil, nil, fmt.Errorf("failed parsing Wire DPoP token: %w", err)
 	}
 
-	// TODO: validate DPoP too? Which key(s)?
+	challenge, ok := dpopToken["chal"].(string)
+	if !ok {
+		return nil, nil, fmt.Errorf("invalid challenge in Wire DPoP token")
+	}
+	if challenge != v.challenge.Token {
+		return nil, nil, fmt.Errorf("invalid Wire DPoP challenge %q", challenge)
+	}
 
 	handle, ok := dpopToken["handle"].(string)
 	if !ok {
 		return nil, nil, fmt.Errorf("invalid handle in Wire DPoP token")
 	}
-
-	_ = handle
+	if handle != v.wireID.Handle {
+		return nil, nil, fmt.Errorf("invalid Wire client handle %q", handle)
+	}
 
 	// TODO(hs): what to do with max expiry?
 	// maxExpiry:= strconv.FormatInt(time.Now().Add(time.Hour*24*365).Unix(), 10),
-
-	// TODO: compare handle?
-	// TODO: compare challenge token / value?
-	// TODO: max expiry?
-	// 	"--handle",
-	// 	challengeValues.Handle,
-	// 	"--challenge",
-	// 	ch.Token,
 	// 	"--max-expiry",
 	// 	expiry,
 

acme/challenge_test.go

@@ -4313,9 +4313,13 @@ MCowBQYDK2VwAyEA5c+4NKZSNQcR1T8qN6SjwgdPZQ0Ge12Ylx/YeGAJ35k=
 	kid := "QAv6C9q47Cyfd1u9z6uX3V_o-t11S8p81wLH-oTRlh0"
 	wireID := wire.ID{
 		ClientID: "wireapp://Jrj3af6YQ7efXqIPa3KTfw!45e8ec286dfdb69@wire.com",
+		Handle:   "wireapp://%40beltram_wire@wire.com",
 	}
 	token := `eyJhbGciOiJFZERTQSIsInR5cCI6ImF0K2p3dCIsImp3ayI6eyJrdHkiOiJPS1AiLCJjcnYiOiJFZDI1NTE5IiwieCI6IjVjLTROS1pTTlFjUjFUOHFONlNqd2dkUFpRMEdlMTJZbHhfWWVHQUozNWsifX0.eyJpYXQiOjE3MDQ4NDA4OTYsImV4cCI6MTcwNDg0NDg1NiwibmJmIjoxNzA0ODQwODk2LCJpc3MiOiJodHRwczovL3dpcmUuZXhhbXBsZS5jb20vY2xpZW50cy8zMTQ4NDU5OTAxMDAxMzA2NjUvYWNjZXNzLXRva2VuIiwic3ViIjoid2lyZWFwcDovL0pyajNhZjZZUTdlZlhxSVBhM0tUZnchNDVlOGVjMjg2ZGZkYjY5QHdpcmUuY29tIiwiYXVkIjoiaHR0cHM6Ly93aXJlLmV4YW1wbGUuY29tL2NsaWVudHMvMzE0ODQ1OTkwMTAwMTMwNjY1L2FjY2Vzcy10b2tlbiIsImp0aSI6IjI2MjUxNzgzLWUxNDItNDNhNC04ZWE5LWU0MTk3MTJmYjE0MSIsIm5vbmNlIjoiY1hKVU9EUnROemxzWmtocGQwOHhSVEpTVHpSVFowWnBiRGhLWkZKUFdYayIsImNoYWwiOiJiRzlZYVRObk4yUTJiVU5IVEhWb016aHRaREo1WTNSQ05VaFZaR3hXUjBvIiwiY25mIjp7ImtpZCI6IlFBdjZDOXE0N0N5ZmQxdTl6NnVYM1Zfby10MTFTOHA4MXdMSC1vVFJsaDAifSwicHJvb2YiOiJleUpoYkdjaU9pSkZaRVJUUVNJc0luUjVjQ0k2SW1Sd2IzQXJhbmQwSWl3aWFuZHJJanA3SW10MGVTSTZJazlMVUNJc0ltTnlkaUk2SWtWa01qVTFNVGtpTENKNElqb2laMk5VUWpCQmRHUjNkR0pqYmxoUFNtaHlTM2RKWmpob1F6ZFNVR2xtZEV0bFEzSjFUMmRpUVRCRVl5SjlmUS5leUpwWVhRaU9qRTNNRFE0TkRBNE9UWXNJbVY0Y0NJNk1UY3dORGt6TURnNU5pd2libUptSWpveE56QTBPRFF3T0RrMkxDSnpkV0lpT2lKM2FYSmxZWEJ3T2k4dlNuSnFNMkZtTmxsUk4yVm1XSEZKVUdFelMxUm1keUUwTldVNFpXTXlPRFprWm1SaU5qbEFkMmx5WlM1amIyMGlMQ0pxZEdraU9pSTBPVEl4WW1FMk5DMWhOVE0yTFRRd05qSXRZamhoTkMwNVpHVXlaR1l3WlRBMlpEWWlMQ0p1YjI1alpTSTZJbU5ZU2xWUFJGSjBUbnBzYzFwcmFIQmtNRGg0VWxSS1UxUjZVbFJhTUZwd1lrUm9TMXBHU2xCWFdHc2lMQ0pvZEcwaU9pSlFUMU5VSWl3aWFIUjFJam9pYUhSMGNITTZMeTkzYVhKbExtVjRZVzF3YkdVdVkyOXRMMk5zYVdWdWRITXZNekUwT0RRMU9Ua3dNVEF3TVRNd05qWTFMMkZqWTJWemN5MTBiMnRsYmlJc0ltTm9ZV3dpT2lKaVJ6bFpZVlJPYms0eVVUSmlWVTVJVkVoV2IwMTZhSFJhUkVvMVdUTlNRMDVWYUZaYVIzaFhVakJ2SWl3aWFHRnVaR3hsSWpvaWQybHlaV0Z3Y0Rvdkx5VTBNR0psYkhSeVlXMWZkMmx5WlVCM2FYSmxMbU52YlNJc0luUmxZVzBpT2lKM2FYSmxJbjAuUGVMaXEtZWlVWXhDREszT3dHMGtsN25lR0RQYUhtYW5KY1BlOEJOZ0pJemRHUm1nVEE1UVZQNTJsdzcwendJcy0yZ0JZTWxyOVVPb1VXX1l1bnN4RHciLCJjbGllbnRfaWQiOiJ3aXJlYXBwOi8vSnJqM2FmNllRN2VmWHFJUGEzS1RmdyE0NWU4ZWMyODZkZmRiNjlAd2lyZS5jb20iLCJhcGlfdmVyc2lvbiI6NSwic2NvcGUiOiJ3aXJlX2NsaWVudF9pZCJ9.VwMJGkXRaP0lC9UDe5iGU8fxOSeBKCXfHXhqcbu_n5JiP5b7WTJAymiCFmVyAaKWZIK6S9qxncqSj5AUPAfQAg`
-	ch := &Challenge{} // TODO(hs): fill after adding validation
+	ch := &Challenge{
+		Type:  WIREDPOP01,
+		Token: "bG9YaTNnN2Q2bUNHTHVoMzhtZDJ5Y3RCNUhVZGxWR0o",
+	}
 
 	issuedAtUnix, err := strconv.ParseInt("1704840896", 10, 64)
 	require.NoError(t, err)