fix(verifier): accept padded base64 vp_token and stop panicking on malformed VPs

openapi/api_api.go

@@ -53,6 +53,7 @@ var ErrorMessageNoToken = ErrorMessage{"no_token_provided", "Authentication requ
 var ErrorMessageNoPresentationSubmission = ErrorMessage{"no_presentation_submission_provided", "Authentication requires a presentation submission provided as a form parameter."}
 var ErrorMessageNoCallback = ErrorMessage{"NoCallbackProvided", "A callback address has to be provided as query-parameter."}
 var ErrorMessageUnableToDecodeToken = ErrorMessage{"invalid_token", "Token could not be decoded."}
+var ErrorMessageInvalidTokenEncoding = ErrorMessage{"invalid_token", "The vp_token has to be base64url encoded without padding."}
 var ErrorMessageUnableToDecodeCredential = ErrorMessage{"invalid_token", "Could not read the credential(s) inside the token."}
 var ErrorMessageUnableToDecodeHolder = ErrorMessage{"invalid_token", "Could not read the holder inside the token."}
 var ErrorMessageNoSuchSession = ErrorMessage{"no_session", "Session with the requested id is not available."}
@@ -68,6 +69,8 @@ var ErrorMessageInvalidRequestedTokenType = ErrorMessage{"invalid_requested_toke
 var ErrorMessageNoRefreshToken = ErrorMessage{"no_refresh_token_provided", "Refresh token requests require a refresh_token."}
 var ErrorMessageInvalidRefreshToken = ErrorMessage{"invalid_refresh_token", "The provided refresh token is invalid or expired."}
 
+var ErrorInvalidTokenEncoding = errors.New("invalid_token_encoding")
+
 func getApiVerifier() verifier.Verifier {
 	if apiVerifier == nil {
 		apiVerifier = verifier.GetVerifier()
@@ -633,7 +636,12 @@ func extractVpFromToken(c *gin.Context, vpToken string) (parsedPresentation *com
 }
 
 func tokenToPresentation(c *gin.Context, vpToken string) (parsedPresentation *common.Presentation, err error) {
-	tokenBytes := decodeVpString(vpToken)
+	tokenBytes, err := decodeVpString(vpToken)
+	if err != nil {
+		logging.Log().Infof("Was not able to decode the token %s. Err: %v", vpToken, err)
+		c.AbortWithStatusJSON(http.StatusBadRequest, ErrorMessageInvalidTokenEncoding)
+		return nil, err
+	}
 
 	isSdJWT, parsedPresentation, err := isSdJWT(c, vpToken)
 	if isSdJWT && err != nil {
@@ -670,7 +678,12 @@ func tokenToPresentation(c *gin.Context, vpToken string) (parsedPresentation *co
 }
 
 func getPresentationFromQuery(c *gin.Context, vpToken string) (parsedPresentation *common.Presentation, err error) {
-	tokenBytes := decodeVpString(vpToken)
+	tokenBytes, err := decodeVpString(vpToken)
+	if err != nil {
+		logging.Log().Infof("Was not able to decode the token %s. Err: %v", vpToken, err)
+		c.AbortWithStatusJSON(http.StatusBadRequest, ErrorMessageInvalidTokenEncoding)
+		return nil, err
+	}
 
 	// First, try the OID4VP drafts 22-24 shape:
 	//   {"<query_id>": "<single-presentation>"}
@@ -750,13 +763,23 @@ func isSdJWT(c *gin.Context, vpToken string) (isSdJwt bool, presentation *common
 	return true, presentation, nil
 }
 
-// decodeVpString - In newer versions of OID4VP the token is not encoded as a whole but only its segments separately. This function covers the older and newer versions
-func decodeVpString(vpToken string) (tokenBytes []byte) {
-	tokenBytes, err := base64.RawURLEncoding.DecodeString(vpToken)
-	if err != nil {
-		return []byte(vpToken)
+// decodeVpString - In newer versions of OID4VP the token is not encoded as a whole but only its segments separately. This function covers the older and newer versions.
+// OID4VP requires the vp_token to be base64url encoded without padding, thus only base64.RawURLEncoding is accepted. A token that is not base64 at all(a compact JWT,
+// an SD-JWT or a plain json dcql response) is returned unchanged, since '.', '~', '{', '"' and ':' are part of no base64 alphabet and thus can never be decoded by
+// accident. A token that decodes only with padding or only with the standard alphabet is none of those either, so instead of handing the still-encoded string on as
+// an opaque failure further down the parsers, it is reported as the malformed request that it is.
+func decodeVpString(vpToken string) (tokenBytes []byte, err error) {
+	if decodedBytes, decodeErr := base64.RawURLEncoding.DecodeString(vpToken); decodeErr == nil {
+		return decodedBytes, nil
+	}
+	for _, encoding := range []*base64.Encoding{base64.URLEncoding, base64.RawStdEncoding, base64.StdEncoding} {
+		if _, decodeErr := encoding.DecodeString(vpToken); decodeErr == nil {
+			logging.Log().Warnf("The vp_token is base64 encoded, but not with the raw(unpadded) url-safe alphabet required by OID4VP.")
+			return nil, ErrorInvalidTokenEncoding
+		}
 	}
-	return tokenBytes
+	logging.Log().Debugf("The token is not base64 encoded, using it as is.")
+	return []byte(vpToken), nil
 }
 
 func handleAuthenticationResponse(c *gin.Context, state string, presentation *common.Presentation) {

openapi/api_api_test.go

@@ -675,7 +675,9 @@ func buildSignedVPToken(t *testing.T) string {
 //   - OID4VP draft 25+:    {"<query_id>": ["<presentation>", ...]}
 // Both must be accepted (backward-compatible). Inputs that are neither a query
 // map nor parseable as a single token must return (nil, nil) so the caller
-// falls through to the flat-string presentation parsing path.
+// falls through to the flat-string presentation parsing path. A token that is
+// base64, but not in the raw url-safe encoding OID4VP requires, must be rejected
+// with a 400 instead of falling through as an undecoded string.
 func TestGetPresentationFromQuery(t *testing.T) {
 
 	logging.Configure(LOGGING_CONFIG)
@@ -690,18 +692,28 @@ func TestGetPresentationFromQuery(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to marshal draft 25+ query map: %v", err)
 	}
+	// a spec-conform client base64url encodes the query map without padding
+	encodedStringShapeMap := base64.RawURLEncoding.EncodeToString(stringShapeMap)
+	// clients that encode it with a padding-emitting encoder(java, python) send a trailing '=' instead, which
+	// used to make the raw string fall through to the sd-jwt parser. It has to be reported, not compensated.
+	paddedStringShapeMap := base64.URLEncoding.EncodeToString(withBase64Padding(stringShapeMap))
+	standardAlphabetArrayShapeMap := base64.StdEncoding.EncodeToString(withBase64Padding(arrayShapeMap))
 
 	type test struct {
 		testName       string
 		vpToken        string
 		expectedNonNil bool
+		expectedErr    error
 	}
 
 	tests := []test{
-		{"OID4VP drafts 22-24 single-string-shape query map should be parsed.", string(stringShapeMap), true},
-		{"OID4VP draft 25+ array-shape query map should be parsed.", string(arrayShapeMap), true},
-		{"A flat sd-jwt (no query map) should return nil so the caller falls through.", sdJwt, false},
-		{"A non-JSON token should return nil so the caller falls through.", "this-is-not-json", false},
+		{"OID4VP drafts 22-24 single-string-shape query map should be parsed.", string(stringShapeMap), true, nil},
+		{"OID4VP draft 25+ array-shape query map should be parsed.", string(arrayShapeMap), true, nil},
+		{"A raw base64url encoded query map should be parsed.", encodedStringShapeMap, true, nil},
+		{"A padded base64url encoded query map should be rejected.", paddedStringShapeMap, false, ErrorInvalidTokenEncoding},
+		{"A standard-alphabet base64 encoded query map should be rejected.", standardAlphabetArrayShapeMap, false, ErrorInvalidTokenEncoding},
+		{"A flat sd-jwt (no query map) should return nil so the caller falls through.", sdJwt, false, nil},
+		{"A non-JSON token should return nil so the caller falls through.", "this-is-not-json", false, nil},
 	}
 
 	for _, tc := range tests {
@@ -718,6 +730,16 @@ func TestGetPresentationFromQuery(t *testing.T) {
 
 			parsed, parseErr := getPresentationFromQuery(testContext, tc.vpToken)
 
+			if tc.expectedErr != nil {
+				if parseErr != tc.expectedErr {
+					t.Errorf("%s - Expected error %v but was %v", tc.testName, tc.expectedErr, parseErr)
+					return
+				}
+				if recorder.Code != http.StatusBadRequest {
+					t.Errorf("%s - Expected the request to be answered with a 400 but was %d.", tc.testName, recorder.Code)
+				}
+				return
+			}
 			if parseErr != nil {
 				t.Errorf("%s - Unexpected error: %v", tc.testName, parseErr)
 				return
@@ -753,6 +775,55 @@ func TestBuildFrontendV2Address(t *testing.T) {
 	}
 }
 
+// withBase64Padding appends json-insignificant whitespace until the length is not a multiple of 3, so
+// that encoding the result is guaranteed to emit '=' padding regardless of the fixture that is used.
+func withBase64Padding(raw []byte) []byte {
+	for len(raw)%3 == 0 {
+		raw = append(raw, ' ')
+	}
+	return raw
+}
+
+func TestDecodeVpString(t *testing.T) {
+
+	logging.Configure(LOGGING_CONFIG)
+
+	// 4 bytes, so the encodings emit padding, and chosen so that the standard alphabet uses '+' and '/'
+	// where the url-safe one uses '-' and '_' - the four encodings therefore produce four distinct strings
+	decoded := []byte{0xfb, 0xff, 0xbe, 0x2a}
+
+	type test struct {
+		testName    string
+		vpToken     string
+		expected    []byte
+		expectedErr error
+	}
+
+	tests := []test{
+		{"Raw base64url, the only encoding OID4VP allows, should be decoded.", base64.RawURLEncoding.EncodeToString(decoded), decoded, nil},
+		{"Padded base64url should be rejected.", base64.URLEncoding.EncodeToString(decoded), nil, ErrorInvalidTokenEncoding},
+		{"Raw standard base64 should be rejected.", base64.RawStdEncoding.EncodeToString(decoded), nil, ErrorInvalidTokenEncoding},
+		{"Padded standard base64 should be rejected.", base64.StdEncoding.EncodeToString(decoded), nil, ErrorInvalidTokenEncoding},
+		{"A compact jwt should be returned unchanged.", "eyJhbGciOiJFUzI1NiJ9.eyJpc3MiOiJtZSJ9.sig", []byte("eyJhbGciOiJFUzI1NiJ9.eyJpc3MiOiJtZSJ9.sig"), nil},
+		{"An sd-jwt should be returned unchanged.", "eyJhbGciOiJFUzI1NiJ9.eyJpc3MiOiJtZSJ9.sig~disclosure~", []byte("eyJhbGciOiJFUzI1NiJ9.eyJpc3MiOiJtZSJ9.sig~disclosure~"), nil},
+		{"A plain json query map should be returned unchanged.", `{"lpc-query":"a.b.c"}`, []byte(`{"lpc-query":"a.b.c"}`), nil},
+		{"A non-base64 string should be returned unchanged.", "this-is-not-base64!", []byte("this-is-not-base64!"), nil},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.testName, func(t *testing.T) {
+			actual, err := decodeVpString(tc.vpToken)
+			if err != tc.expectedErr {
+				t.Errorf("%s - Expected error %v but was %v", tc.testName, tc.expectedErr, err)
+				return
+			}
+			if !bytes.Equal(actual, tc.expected) {
+				t.Errorf("%s - Expected %s but was %s", tc.testName, tc.expected, actual)
+			}
+		})
+	}
+}
+
 func getNoHolderVPToken() string {
 	return "ewogICJAY29udGV4dCI6IFsKICAgICJodHRwczovL3d3dy53My5vcmcvMjAxOC9jcmVkZW50aWFscy92MSIKICBdLAogICJ0eXBlIjogWwogICAgIlZlcmlmaWFibGVQcmVzZW50YXRpb24iCiAgXSwKICAidmVyaWZpYWJsZUNyZWRlbnRpYWwiOiBbCiAgICB7CiAgICAgICJ0eXBlcyI6IFsKICAgICAgICAiUGFja2V0RGVsaXZlcnlTZXJ2aWNlIiwKICAgICAgICAiVmVyaWZpYWJsZUNyZWRlbnRpYWwiCiAgICAgIF0sCiAgICAgICJAY29udGV4dCI6IFsKICAgICAgICAiaHR0cHM6Ly93d3cudzMub3JnLzIwMTgvY3JlZGVudGlhbHMvdjEiLAogICAgICAgICJodHRwczovL3czaWQub3JnL3NlY3VyaXR5L3N1aXRlcy9qd3MtMjAyMC92MSIKICAgICAgXSwKICAgICAgImNyZWRlbnRpYWxzU3ViamVjdCI6IHt9LAogICAgICAiYWRkaXRpb25hbFByb3AxIjoge30KICAgIH0KICBdLAogICJpZCI6ICJlYmM2ZjFjMiIsCiAgImhvbGRlciI6IHsKICAgICJub3RhIjogImhvbGRlciIKICB9LAogICJwcm9vZiI6IHsKICAgICJ0eXBlIjogIkpzb25XZWJTaWduYXR1cmUyMDIwIiwKICAgICJjcmVhdG9yIjogImRpZDprZXk6ejZNa3M5bTlpZkx3eTNKV3FINGM1N0ViQlFWUzJTcFJDamZhNzl3SGI1dldNNnZoIiwKICAgICJjcmVhdGVkIjogIjIwMjMtMDEtMDZUMDc6NTE6MzZaIiwKICAgICJ2ZXJpZmljYXRpb25NZXRob2QiOiAiZGlkOmtleTp6Nk1rczltOWlmTHd5M0pXcUg0YzU3RWJCUVZTMlNwUkNqZmE3OXdIYjV2V002dmgjejZNa3M5bTlpZkx3eTNKV3FINGM1N0ViQlFWUzJTcFJDamZhNzl3SGI1dldNNnZoIiwKICAgICJqd3MiOiAiZXlKaU5qUWlPbVpoYkhObExDSmpjbWwwSWpwYkltSTJOQ0pkTENKaGJHY2lPaUpGWkVSVFFTSjkuLjZ4U3FvWmphME53akYwYWY5WmtucXgzQ2JoOUdFTnVuQmY5Qzh1TDJ1bEdmd3VzM1VGTV9abmhQald0SFBsLTcyRTlwM0JUNWYycHRab1lrdE1LcERBIgogIH0KfQ"
 }

verifier/presentation_parser.go

@@ -519,9 +519,11 @@ func (sjp *ConfigurableSdJwtParser) ClaimsToCredential(claims map[string]interfa
 func (sjp *ConfigurableSdJwtParser) ParseWithSdJwt(tokenBytes []byte) (presentation *common.Presentation, err error) {
 	logging.Log().Debug("Parse with SD-Jwt")
 
-	tokenString := string(tokenBytes)
-	payloadString := strings.Split(tokenString, ".")[1]
-	payloadBytes, _ := base64.RawURLEncoding.DecodeString(payloadString)
+	payloadBytes, err := extractJWTPayload(tokenBytes)
+	if err != nil {
+		logging.Log().Warnf("Failed to extract the VP payload: %v", err)
+		return nil, err
+	}
 
 	var vpMap map[string]interface{}
 	if err := json.Unmarshal(payloadBytes, &vpMap); err != nil {
@@ -546,12 +548,27 @@ func (sjp *ConfigurableSdJwtParser) ParseWithSdJwt(tokenBytes []byte) (presentat
 		return nil, err
 	}
 
-	presentation.Holder = vp[common.VPKeyHolder].(string)
+	// the holder is optional here, mirroring parseJSONLDPresentation and parseJWTPresentation. Holder binding
+	// for sd-jwts is done via the kb-jwt, not via this claim, so a missing one must not reject the presentation.
+	if holder, ok := vp[common.VPKeyHolder].(string); ok {
+		presentation.Holder = holder
+	}
+
+	vcArray, ok := vcs.([]interface{})
+	if !ok {
+		logging.Log().Warn("The verifiableCredential entry is not an array")
+		return nil, ErrorVCNotArray
+	}
 
 	// due to dcql, we only need to take care of presentations containing credentials of the same type.
-	for _, vc := range vcs.([]interface{}) {
-		logging.Log().Debugf("The vc %s", vc.(string))
-		parsed, err := sjp.Parse(vc.(string))
+	for _, vc := range vcArray {
+		vcString, ok := vc.(string)
+		if !ok {
+			logging.Log().Warn("The presentation contains a credential that is not an sd-jwt string")
+			return nil, ErrorInvalidSdJwt
+		}
+		logging.Log().Debugf("The vc %s", vcString)
+		parsed, err := sjp.Parse(vcString)
 		if err != nil {
 			logging.Log().Warnf("Failed to parse SD-JWT VC: %v", err)
 			return nil, err

verifier/presentation_parser_test.go

@@ -276,6 +276,91 @@ func TestParseWithSdJwt_MalformedPayload(t *testing.T) {
 	}
 }
 
+// TestParseWithSdJwt_RejectsTokensWithoutPayloadSegment covers tokens that do not have a payload
+// segment at all. Such a token reaches the parser whenever decodeVpString cannot decode the vp_token
+// and hands the raw string over, e.g. for a base64 encoded dcql response - and must not panic.
+func TestParseWithSdJwt_RejectsTokensWithoutPayloadSegment(t *testing.T) {
+	tests := []struct {
+		name  string
+		token string
+	}{
+		// base64 of {"mc-query":"a.b.c"}, the shape a padded dcql response degrades into
+		{"padded_base64_dcql_response", "eyJtYy1xdWVyeSI6ImEuYi5jIn0="},
+		{"empty_token", ""},
+		{"no_separator", "notajwt"},
+		{"header_only", "eyJhbGciOiJFUzI1NiJ9"},
+	}
+
+	parser := &ConfigurableSdJwtParser{}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := parser.ParseWithSdJwt([]byte(tc.token))
+			if err != ErrorInvalidJWTFormat {
+				t.Errorf("Expected ErrorInvalidJWTFormat, got %v", err)
+			}
+		})
+	}
+}
+
+// TestParseWithSdJwt_MissingHolderIsAccepted documents that the holder is optional, just like it is in
+// parseJSONLDPresentation and parseJWTPresentation. Wallets do not have to send it for sd-jwt presentations,
+// since holder binding is done via the kb-jwt.
+func TestParseWithSdJwt_MissingHolderIsAccepted(t *testing.T) {
+	tests := []struct {
+		name string
+		vp   map[string]interface{}
+	}{
+		{"absent_holder", map[string]interface{}{"verifiableCredential": []interface{}{}}},
+		{"non_string_holder", map[string]interface{}{"holder": map[string]interface{}{"not": "a string"}, "verifiableCredential": []interface{}{}}},
+	}
+
+	parser := &ConfigurableSdJwtParser{}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			token := buildFakeJWT(map[string]interface{}{"vp": tc.vp})
+
+			presentation, err := parser.ParseWithSdJwt([]byte(token))
+			if err != nil {
+				t.Errorf("Expected no error, got %v", err)
+				return
+			}
+			if presentation.Holder != "" {
+				t.Errorf("Expected an empty holder, got %s", presentation.Holder)
+			}
+		})
+	}
+}
+
+func TestParseWithSdJwt_VerifiableCredentialNotAnArray(t *testing.T) {
+	parser := &ConfigurableSdJwtParser{}
+	token := buildFakeJWT(map[string]interface{}{
+		"vp": map[string]interface{}{
+			"holder":               "did:web:holder",
+			"verifiableCredential": map[string]interface{}{"id": "urn:vc:1"},
+		},
+	})
+
+	_, err := parser.ParseWithSdJwt([]byte(token))
+	if err != ErrorVCNotArray {
+		t.Errorf("Expected ErrorVCNotArray, got %v", err)
+	}
+}
+
+func TestParseWithSdJwt_CredentialNotAString(t *testing.T) {
+	parser := &ConfigurableSdJwtParser{}
+	token := buildFakeJWT(map[string]interface{}{
+		"vp": map[string]interface{}{
+			"holder":               "did:web:holder",
+			"verifiableCredential": []interface{}{map[string]interface{}{"id": "urn:vc:1"}},
+		},
+	})
+
+	_, err := parser.ParseWithSdJwt([]byte(token))
+	if err != ErrorInvalidSdJwt {
+		t.Errorf("Expected ErrorInvalidSdJwt, got %v", err)
+	}
+}
+
 // --- Tests for VP signature verification ---
 
 // buildSignedJWT creates a properly signed JWT with the given payload using a random EC key.