Subtle Change in Authenticator Assertion Verification Logic

[philsmart]

I realise you should key assertion results on the returned credential (and userHandle associated with it). However, I’ve hit a confusing scenario, and I’m trying to understand whether the change in FinishAssertionSteps from 2.5.1 to 2.8.1 is intentional.

In 2.5.1, I don’t think the AssertionResult could contain a different username than the one in the AssertionRequest. In Step 6, the library checked:

• if the request contained a username
• and the response contained a userHandle
• then the userHandle derived from the request’s username must match the response’s userHandle

java-webauthn-server/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java

Lines 187 to 196 in a3698be

	final Optional<String> usernameFromRequest = request.getUsername();
	final Optional<ByteArray> userHandleFromResponse = response.getResponse().getUserHandle();
	if (usernameFromRequest.isPresent() && userHandleFromResponse.isPresent()) {
	assertTrue(
	userHandleFromResponse.equals(
	credentialRepository.getUserHandleForUsername(usernameFromRequest.get())),
	"User handle %s in response does not match username %s in request",
	userHandleFromResponse,
	usernameFromRequest);
	}

If the username in the request either never had credentials or no longer had any credentials, getUserHandleForUsername() returned an empty Optional, which did not equal the response userHandle — causing the assertion to fail. This seemed sensible, if an unlikely scenario.

In 2.8.1, the logic has changed slightly. If the request contains a username but no userHandle, the library now has the logic:

java-webauthn-server/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java

Lines 199 to 204 in c9383d5

	effectiveRequestUserHandle =
	OptionalUtil.orElseOptional(
	requestedUserHandle,
	() ->
	usernameRepository.flatMap(
	unr -> requestedUsername.flatMap(unr::getUserHandleForUsername)));

java-webauthn-server/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java

Lines 248 to 255 in c9383d5

	if (userHandleDerivedFromUsername && responseUserHandle.isPresent()) {
	assertTrue(
	effectiveRequestUserHandle.get().equals(responseUserHandle.get()),
	"User handle in request (%s) (derived from username: %s) does not match user handle in response (%s).",
	effectiveRequestUserHandle.get(),
	requestedUsername.get(),
	responseUserHandle.get());
	}

If the username does not correspond to a stored credential (e.g., no entry in the repository), then:

• effectiveRequestUserHandle is empty
• userHandleDerivedFromUsername is false
• the mismatch check is not executed and the final username becomes the username from the request, even though the authenticated credential belongs to a different user

I assume this is a pretty unlikely scenario, and it might be intentional, but the result from the older version seems less confusing (arguably safer). Although possible that the original check was overly strict compared to the spec.

The validation logic seems to imply it detects this kind of mismatch (as the old one did), and I’d still assume the result to contain a username that matches the userhandle in the verified response.

Phil

[philsmart]

I've just noticed that AssertionResultV2 does not even expose username. Maybe that avoids any confusion from:

java-webauthn-server/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java

Line 74 in c9383d5

* The username of the authenticated user.

[philsmart]

Actually, the README does suggest you use this username:

java-webauthn-server/README

Line 367 in 2584188

return result.getUsername();

So IMHO it might be best just to cover off the 'unresolvable/empty' credentials case in Step 6.