Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebAuthn: Add user id to PublicKeyCredentialsCreateOptions, Authenticator and WebAuthnCredentials (#580, #581) #582

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions vertx-auth-webauthn/src/main/asciidoc/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -143,27 +143,54 @@ established a secure session.
{@link examples.WebAuthNExamples#example1}
----

After this, store the generated challenge somewhere where it can be retrieved from in
the next step. Some options for this are user session, JWT token handed to the client, or
database. Implementers should pay special care to scope the stored challenge to WebAuthn
registration to ensure that a solution to the registration challenge can't be used for
logging in. To prevent replay attacks, the challenge should have a reasonably short
time-to-live (for example, no longer than the `timeout` in the generated
`PublicKeyCredentialCreationOptions`) and be invalidated after a valid solution has been
provided.

=== Verify the registration request

Retrieve the challenge generated in the previous step, ensure the challenge is meant only
for authenticator registration and call `authenticate` with the `AuthenticatorAttestationResponse`
from `navigator.credentials.create()`.

[source,$lang]
----
{@link examples.WebAuthNExamples#example2}
----

To prevent replay attacks, it would be a good idea to invalidate the challenge after use, so the same
challenge can't be used to register another authenticator.

=== Create a Login request

[source,$lang]
----
{@link examples.WebAuthNExamples#example3}
----

Like in registration, store the generated challenge so it can be retrieved in the next step. Make sure
to scope the challenge to WebAuthn login to ensure that a solution to a registration challenge can't be
used for logging in. Also, same considerations as in registration should be given to the TTL of the
challenge.

=== Verify the Login request

Again, retrieve the challenge generated in the previous step and ensure it's only valid in login
context, and then call `authenticate` with the `AuthenticatorAssertionResponse` from
`navigator.credentials.get()`:

[source,$lang]
----
{@link examples.WebAuthNExamples#example4}
----

Here too, to prevent replay attacks, it would be a good idea to invalidate the challenge after use.

== Metadata Service

The current module passes all FIDO2 compliance tests **including** the yet to be final FIDO2 Metadata Service API.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ public static void fromJson(Iterable<java.util.Map.Entry<String, Object>> json,
obj.setUserName((String)member.getValue());
}
break;
case "userId":
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realized I modified these by hand... how do I run the code generator?

if (member.getValue() instanceof String) {
obj.setUserId((String)member.getValue());
}
}
}
}
Expand Down Expand Up @@ -91,5 +95,8 @@ public static void toJson(Authenticator obj, java.util.Map<String, Object> json)
if (obj.getUserName() != null) {
json.put("userName", obj.getUserName());
}
if (obj.getUserId() != null) {
json.put("userId", obj.getUserId());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ public static void fromJson(Iterable<java.util.Map.Entry<String, Object>> json,
obj.setWebauthn(((JsonObject)member.getValue()).copy());
}
break;
case "userId":
if (member.getValue() instanceof String) {
obj.setUserId((String)member.getValue());
}
break;
}
}
}
Expand All @@ -69,5 +74,8 @@ public static void toJson(WebAuthnCredentials obj, java.util.Map<String, Object>
if (obj.getWebauthn() != null) {
json.put("webauthn", obj.getWebauthn());
}
if (obj.getUserId() != null) {
json.put("userId", obj.getUserId());
}
}
}
15 changes: 9 additions & 6 deletions vertx-auth-webauthn/src/main/java/examples/WebAuthNExamples.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.vertx.core.Future;
import io.vertx.core.Vertx;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.auth.impl.Codec;
import io.vertx.ext.auth.webauthn.Authenticator;
import io.vertx.ext.auth.webauthn.RelyingParty;
import io.vertx.ext.auth.webauthn.WebAuthn;
Expand Down Expand Up @@ -49,8 +50,7 @@ public void example1(Vertx vertx, List<Authenticator> authenticators) {
// some user
JsonObject user = new JsonObject()
// id is expected to be a base64url string
.put("id", "000000000000000000000000")
.put("rawId", "000000000000000000000000")
.put("id", Codec.base64UrlEncode("8d7c481b-b60f-4833-9f07-1a299a38ad85".getBytes()))
.put("name", "[email protected]")
// optionally
.put("displayName", "John Doe")
Expand All @@ -59,8 +59,10 @@ public void example1(Vertx vertx, List<Authenticator> authenticators) {
webAuthN
.createCredentialsOptions(user)
.onSuccess(challengeResponse -> {
// return the challenge to the browser
// for further processing
// Extract the challenge from the challengeResponse
// and store it somewhere (e.g. user session) for verifying the
// registration in the next step, and then return the
// challengeResponse to the browser.
});
}

Expand All @@ -70,8 +72,8 @@ public void example2(Vertx vertx, List<Authenticator> authenticators) {
new WebAuthnOptions()
.setRelyingParty(new RelyingParty().setName("ACME Corporation")))
.authenticatorFetcher(query -> {
// function that fetches some authenticators from a
// persistence storage
// function that fetches authenticators from a
// persistence storage with either user id or name
return Future.succeededFuture(authenticators);
})
.authenticatorUpdater(authenticator -> {
Expand All @@ -92,6 +94,7 @@ public void example2(Vertx vertx, List<Authenticator> authenticators) {
webAuthN
.authenticate(
new JsonObject()
.put("userId", Codec.base64UrlEncode("8d7c481b-b60f-4833-9f07-1a299a38ad85".getBytes()))
// the username you want to link to
.put("username", "paulo")
// the server origin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ public class Authenticator {
private AttestationCertificates attestationCertificates;
private String fmt;

/**
* The base64 url encoded user handle associated with this authenticator.
*/
private String userId;

public Authenticator() {}
public Authenticator(JsonObject json) {
AuthenticatorConverter.fromJson(json, this);
Expand Down Expand Up @@ -168,4 +173,13 @@ public Authenticator setAaguid(String aaguid) {
public String getAaguid() {
return aaguid;
}

public String getUserId() {
return userId;
}

public Authenticator setUserId(String userId) {
this.userId = userId;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.vertx.codegen.annotations.VertxGen;
import io.vertx.core.*;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.auth.User;
import io.vertx.ext.auth.authentication.AuthenticationProvider;
import io.vertx.ext.auth.webauthn.impl.WebAuthnImpl;

Expand Down Expand Up @@ -59,7 +60,24 @@ static WebAuthn create(Vertx vertx, WebAuthnOptions options) {
* Gets a challenge and any other parameters for the {@code navigator.credentials.create()} call.
*
* The object being returned is described here <a href="https://w3c.github.io/webauthn/#dictdef-publickeycredentialcreationoptions">https://w3c.github.io/webauthn/#dictdef-publickeycredentialcreationoptions</a>
* @param user - the user object with name and optionally displayName and icon
*
* The caller should extract the generated challenge and store it, so it can be fetched later for the
* {@link #authenticate(JsonObject)} call. The challenge could for example be stored in a session and later
* pulled from there.
*
* The user object should contain base64 url encoded id (the user handle), name and, optionally, displayName and icon.
* See the above link for more documentation on the content of the different fields. The user handle should be base64
* url encoded. You can use <code>java.util.Base64.getUrlEncoder().withoutPadding().encodeToString(byte[])</code>
* to encode any user id bytes to base64 url format.
*
* For backwards compatibility, if user id is not defined, a random UUID will be generated instead. This has some
* drawbacks, as it might cause user to register the same authenticator multiple times.
*
* Will use the configured {@link #authenticatorFetcher(Function)} to fetch any existing authenticators
* by the user id or name. Any authenticators found will be added as excludedCredentials, so the application
* knows not to register those again.
*
* @param user - the user object with id, name and optionally displayName and icon
* @param handler server encoded make credentials request
* @return fluent self
*/
Expand Down Expand Up @@ -106,6 +124,7 @@ default WebAuthn getCredentialsOptions(@Nullable String name, Handler<AsyncResul
*
* The implementation must consider the following fields <strong>exclusively</strong>, while performing the lookup:
* <ul>
* <li>{@link Authenticator#getUserId()}</li>
* <li>{@link Authenticator#getUserName()}</li>
* <li>{@link Authenticator#getCredID()} ()}</li>
* </ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class WebAuthnCredentials implements Credentials {
private String challenge;
private JsonObject webauthn;
private String username;
private String userId;
private String origin;
private String domain;

Expand Down Expand Up @@ -80,6 +81,15 @@ public WebAuthnCredentials setDomain(String domain) {
return this;
}

public String getUserId() {
return userId;
}

public WebAuthnCredentials setUserId(String userId) {
this.userId = userId;
return this;
}

@Override
public <V> void checkValid(V arg) throws CredentialValidationException {
if (challenge == null || challenge.length() == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,20 @@ public WebAuthn authenticatorUpdater(Function<Authenticator, Future<Void>> updat

@Override
public Future<JsonObject> createCredentialsOptions(JsonObject user) {
String userId;

if (user.getString("id") != null) {
userId = user.getString("id");
} else if (user.getString("rawId") != null) {
// For backwards compatibility, allow using rawId in place of id. Should be removed in future.
userId = user.getString("rawId");
} else {
// For backwards compatibility, if both id and rawId is missing, use a random base64url encoded UUID
userId = uUIDtoBase64Url(UUID.randomUUID());
}

return fetcher
.apply(new Authenticator().setUserName(user.getString("name")))
.apply(new Authenticator().setUserId(userId).setUserName(user.getString("name")))
.map(authenticators -> {
// empty structure with all required fields
JsonObject json = new JsonObject()
Expand All @@ -177,10 +188,11 @@ public Future<JsonObject> createCredentialsOptions(JsonObject user) {
putOpt(json.getJsonObject("rp"), "icon", options.getRelyingParty().getIcon());

// put non null values for User
putOpt(json.getJsonObject("user"), "id", uUIDtoBase64Url(UUID.randomUUID()));
putOpt(json.getJsonObject("user"), "id", userId);
putOpt(json.getJsonObject("user"), "name", user.getString("name"));
putOpt(json.getJsonObject("user"), "displayName", user.getString("displayName"));
putOpt(json.getJsonObject("user"), "icon", user.getString("icon"));

// put the public key credentials parameters
for (PublicKeyCredential pubKeyCredParam : options.getPubKeyCredParams()) {
addOpt(
Expand Down Expand Up @@ -294,6 +306,7 @@ public void authenticate(Credentials credentials, Handler<AsyncResult<User>> han
WebAuthnCredentials authInfo = (WebAuthnCredentials) credentials;
// check
authInfo.checkValid(null);

// The basic data supplied with any kind of validation is:
// {
// "rawId": "base64url",
Expand Down Expand Up @@ -339,6 +352,7 @@ public void authenticate(Credentials credentials, Handler<AsyncResult<User>> han
}

// optional data

if (clientData.containsKey("tokenBinding")) {
JsonObject tokenBinding = clientData.getJsonObject("tokenBinding");
if (tokenBinding == null) {
Expand All @@ -358,6 +372,7 @@ public void authenticate(Credentials credentials, Handler<AsyncResult<User>> han
}
}

final String userId = authInfo.getUserId();
final String username = authInfo.getUsername();

// Step #4
Expand All @@ -379,6 +394,7 @@ public void authenticate(Credentials credentials, Handler<AsyncResult<User>> han
final Authenticator authrInfo = verifyWebAuthNCreate(authInfo, clientDataJSON);
// by default the store can upsert if a credential is missing, the user has been verified so it is valid
// the store however might disallow this operation
authrInfo.setUserId(userId);
authrInfo.setUserName(username);

// the create challenge is complete we can finally safe this
Expand All @@ -393,6 +409,7 @@ public void authenticate(Credentials credentials, Handler<AsyncResult<User>> han
return;
case "webauthn.get":
Authenticator query = new Authenticator();

if (options.getRequireResidentKey()) {
// username are not provided (RK) we now need to lookup by id
query.setCredID(webauthn.getString("id"));
Expand All @@ -402,9 +419,14 @@ public void authenticate(Credentials credentials, Handler<AsyncResult<User>> han
handler.handle(Future.failedFuture("username can't be null!"));
return;
}

query.setUserName(username);
}

if (userId != null) {
query.setUserId(userId);
}

fetcher.apply(query)
.onFailure(err -> handler.handle(Future.failedFuture(err)))
.onSuccess(authenticators -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

public class DummyStore {
Expand All @@ -20,17 +21,25 @@ public void clear() {
}

public Future<List<Authenticator>> fetch(Authenticator query) {
if (query.getUserName() == null && query.getCredID() == null && query.getUserId() == null) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was better to explicitly fail the query in the tests if all conditions were null (which should not happen with proper usage / implementation)

return Future.failedFuture(new IllegalArgumentException("Bad authenticator query! All conditions were null"));
}

return Future.succeededFuture(
database.stream()
.filter(entry -> {
boolean matches = true;
if (query.getUserName() != null) {
return query.getUserName().equals(entry.getUserName());
matches = query.getUserName().equals(entry.getUserName());
}
if (query.getCredID() != null) {
return query.getCredID().equals(entry.getCredID());
matches = matches || query.getCredID().equals(entry.getCredID());
}
// This is a bad query! both username and credID are null
return false;
if (query.getUserId() != null) {
matches = matches || query.getUserId().equals(entry.getUserId());
}

return matches;
})
.collect(Collectors.toList())
);
Expand Down
Loading