Skip to content

Commit 8e20e63

Browse files
emlunemlun
committed
Don't enforce enforceCredentialPolicy when policy set to UV_OPTIONAL
1 parent 5c1566f commit 8e20e63

File tree

3 files changed

+144
-98
lines changed

3 files changed

+144
-98
lines changed

‎webauthn-server-core/src/main/java/com/yubico/webauthn/data/Extensions.java

+28-18
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,11 @@ public static class CredentialProtectionInput {
242242
private final CredentialProtectionPolicy credentialProtectionPolicy;
243243

244244
/**
245-
* If <code>true</code>, {@link RelyingParty#finishRegistration(FinishRegistrationOptions)}
246-
* will validate that the policy set in {@link #getCredentialProtectionPolicy()} was satisfied
247-
* and the browser is requested to fail the registration if the policy cannot be satisfied.
245+
* If this is <code>true</code> and {@link #getCredentialProtectionPolicy()
246+
* credentialProtectionPolicy} is not {@link CredentialProtectionPolicy#UV_OPTIONAL}, {@link
247+
* RelyingParty#finishRegistration(FinishRegistrationOptions)} will validate that the policy
248+
* set in {@link #getCredentialProtectionPolicy()} was satisfied and the browser is requested
249+
* to fail the registration if the policy cannot be satisfied.
248250
*
249251
* <p>{@link CredentialProtectionInput#prefer(CredentialProtectionPolicy)} sets this to <code>
250252
* false</code>. {@link CredentialProtectionInput#require(CredentialProtectionPolicy)} sets
@@ -300,10 +302,11 @@ public static CredentialProtectionInput prefer(
300302
* Create a Credential Protection (<code>credProtect</code>) extension input that requires the
301303
* given policy.
302304
*
303-
* <p>If the policy cannot be satisfied, the browser is requested to abort the registration
304-
* instead of proceeding. {@link RelyingParty#finishRegistration(FinishRegistrationOptions)}
305-
* will validate that the policy returned in the authenticator extension output equals this
306-
* input policy, and throw an exception otherwise. You can also use {@link
305+
* <p>If the policy is not {@link CredentialProtectionPolicy#UV_OPTIONAL} and cannot be
306+
* satisfied, the browser is requested to abort the registration instead of proceeding. {@link
307+
* RelyingParty#finishRegistration(FinishRegistrationOptions)} will validate that the policy
308+
* returned in the authenticator extension output equals this input policy, and throw an
309+
* exception otherwise. You can also use {@link
307310
* AuthenticatorRegistrationExtensionOutputs#getCredProtect()} to inspect the extension output
308311
* yourself.
309312
*
@@ -337,30 +340,34 @@ public static CredentialProtectionInput require(
337340
* RegistrationExtensionInputs.RegistrationExtensionInputsBuilder#credProtect(CredentialProtectionInput)
338341
* credProtect} extension is set in the request with {@link
339342
* CredentialProtectionInput#isEnforceCredentialProtectionPolicy()
340-
* enforceCredentialProtectionPolicy} set to <code>false</code>, this has no effect.
343+
* enforceCredentialProtectionPolicy} set to <code>false</code> or {@link
344+
* CredentialProtectionInput#getCredentialProtectionPolicy() credentialProtectionPolicy} set to
345+
* {@link CredentialProtectionPolicy#UV_OPTIONAL}, this has no effect.
341346
*
342347
* <p>If the {@link
343348
* RegistrationExtensionInputs.RegistrationExtensionInputsBuilder#credProtect(CredentialProtectionInput)
344349
* credProtect} extension is set in the request with {@link
345350
* CredentialProtectionInput#isEnforceCredentialProtectionPolicy()
346-
* enforceCredentialProtectionPolicy} set to <code>true</code>, then this throws an {@link
351+
* enforceCredentialProtectionPolicy} set to <code>true</code> and {@link
352+
* CredentialProtectionInput#getCredentialProtectionPolicy() credentialProtectionPolicy} is not
353+
* set to {@link CredentialProtectionPolicy#UV_OPTIONAL}, then this throws an {@link
347354
* IllegalArgumentException} if the <code>credProtect</code> authenticator extension output does
348355
* not equal the {@link CredentialProtectionInput#getCredentialProtectionPolicy()
349356
* credentialProtectionPolicy} set in the request.
350357
*
351358
* <p>This function is called automatically in {@link
352-
* RelyingParty#finishRegistration(FinishRegistrationOptions)} when the request has {@link
353-
* CredentialProtectionInput#isEnforceCredentialProtectionPolicy()
354-
* enforceCredentialProtectionPolicy} is set to <code>true</code>; you should not need to call
355-
* it yourself.
359+
* RelyingParty#finishRegistration(FinishRegistrationOptions)}; you should not need to call it
360+
* yourself.
356361
*
357362
* @param request the arguments to start the registration ceremony.
358363
* @param response the response from the registration ceremony.
359364
* @throws IllegalArgumentException if the {@link
360365
* RegistrationExtensionInputs.RegistrationExtensionInputsBuilder#credProtect(CredentialProtectionInput)
361366
* credProtect} extension is set in the request with {@link
362367
* CredentialProtectionInput#isEnforceCredentialProtectionPolicy()
363-
* enforceCredentialProtectionPolicy} set to <code>true</code> and the <code>credProtect
368+
* enforceCredentialProtectionPolicy} set to <code>true</code> and {@link
369+
* CredentialProtectionInput#getCredentialProtectionPolicy() credentialProtectionPolicy} not
370+
* set to {@link CredentialProtectionPolicy#UV_OPTIONAL}, and the <code>credProtect
364371
* </code> authenticator extension output does not equal the {@link
365372
* CredentialProtectionInput#getCredentialProtectionPolicy() credentialProtectionPolicy} set
366373
* in the request.
@@ -376,18 +383,21 @@ public static void validateExtensionOutput(
376383
.getExtensions()
377384
.getCredProtect()
378385
.ifPresent(
379-
credProtect -> {
380-
if (credProtect.isEnforceCredentialProtectionPolicy()) {
386+
credProtectInput -> {
387+
if (credProtectInput.isEnforceCredentialProtectionPolicy()
388+
&& credProtectInput.getCredentialProtectionPolicy()
389+
!= CredentialProtectionPolicy.UV_OPTIONAL) {
381390
Optional<CredentialProtectionPolicy> outputPolicy =
382391
response
383392
.getResponse()
384393
.getParsedAuthenticatorData()
385394
.getExtensions()
386395
.flatMap(CredentialProtection::parseAuthenticatorExtensionOutput);
387396
ExceptionUtil.assertTrue(
388-
outputPolicy.equals(Optional.of(credProtect.getCredentialProtectionPolicy())),
397+
outputPolicy.equals(
398+
Optional.of(credProtectInput.getCredentialProtectionPolicy())),
389399
"Unsatisfied credProtect policy: required %s, got: %s",
390-
credProtect.getCredentialProtectionPolicy(),
400+
credProtectInput.getCredentialProtectionPolicy(),
391401
outputPolicy);
392402
}
393403
});

‎webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala

+58-40
Original file line numberDiff line numberDiff line change
@@ -1134,8 +1134,12 @@ class RelyingPartyRegistrationSpec
11341134
}
11351135
}
11361136

1137-
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and no output policy is returned.") {
1138-
forAll { policy: CredentialProtectionPolicy =>
1137+
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and credProtectPolicy!=userVerificationOptional and no output policy is returned.") {
1138+
forAll(
1139+
arbitrary[CredentialProtectionPolicy].suchThat(
1140+
_ != CredentialProtectionPolicy.UV_OPTIONAL
1141+
)
1142+
) { policy: CredentialProtectionPolicy =>
11391143
val steps = finishRegistration(
11401144
testData = RegistrationTestData.Packed.BasicAttestation
11411145
.copy(
@@ -1153,47 +1157,63 @@ class RelyingPartyRegistrationSpec
11531157
}
11541158
}
11551159

1156-
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and the output policy does not match the input policy.") {
1157-
forAll(arbitrary[CredentialProtectionPolicy], Gen.oneOf(1, 2)) {
1158-
(policy: CredentialProtectionPolicy, diff) =>
1159-
val authenticatorExtensionOutputs = CBORObject.NewMap()
1160-
authenticatorExtensionOutputs.set(
1161-
"credProtect",
1162-
CBORObject.FromObject(
1163-
((ReexportHelpers.credProtectPolicyCborValue(
1164-
policy
1165-
) + diff - 1) % 3) + 1
1166-
),
1167-
)
1168-
val steps = finishRegistration(
1169-
testData = RegistrationTestData.Packed.BasicAttestation
1170-
.copy(
1171-
requestedExtensions = RegistrationExtensionInputs
1172-
.builder()
1173-
.credProtect(CredentialProtectionInput.require(policy))
1174-
.build()
1175-
)
1176-
.editAuthenticatorData(authData =>
1177-
new ByteArray(
1178-
authData.getBytes.updated(
1179-
32,
1180-
(authData.getBytes()(32) | 0x80).toByte,
1181-
) ++ authenticatorExtensionOutputs.EncodeToBytes()
1182-
)
1160+
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and credProtectPolicy!=userVerificationOptional and the output policy does not match the input policy.") {
1161+
forAll(
1162+
arbitrary[CredentialProtectionPolicy].suchThat(
1163+
_ != CredentialProtectionPolicy.UV_OPTIONAL
1164+
),
1165+
Gen.oneOf(1, 2),
1166+
) { (policy: CredentialProtectionPolicy, diff) =>
1167+
val authenticatorExtensionOutputs = CBORObject.NewMap()
1168+
authenticatorExtensionOutputs.set(
1169+
"credProtect",
1170+
CBORObject.FromObject(
1171+
((ReexportHelpers.credProtectPolicyCborValue(
1172+
policy
1173+
) + diff - 1) % 3) + 1
1174+
),
1175+
)
1176+
val steps = finishRegistration(
1177+
testData = RegistrationTestData.Packed.BasicAttestation
1178+
.copy(
1179+
requestedExtensions = RegistrationExtensionInputs
1180+
.builder()
1181+
.credProtect(CredentialProtectionInput.require(policy))
1182+
.build()
1183+
)
1184+
.editAuthenticatorData(authData =>
1185+
new ByteArray(
1186+
authData.getBytes.updated(
1187+
32,
1188+
(authData.getBytes()(32) | 0x80).toByte,
1189+
) ++ authenticatorExtensionOutputs.EncodeToBytes()
11831190
)
1184-
)
1185-
val stepAfter: Try[FinishRegistrationSteps#Step18] =
1186-
steps.begin.next.next.next.next.next.next.next.next.next.next.next.tryNext
1191+
)
1192+
)
1193+
val stepAfter: Try[FinishRegistrationSteps#Step18] =
1194+
steps.begin.next.next.next.next.next.next.next.next.next.next.next.tryNext
11871195

1188-
stepAfter shouldBe a[Failure[_]]
1189-
stepAfter.failed.get shouldBe an[IllegalArgumentException]
1196+
stepAfter shouldBe a[Failure[_]]
1197+
stepAfter.failed.get shouldBe an[IllegalArgumentException]
11901198
}
11911199
}
11921200

1193-
it("Succeeds regardless of output credProtect policy if credProtect is set with enforceCredentialProtectionPolicy=false.") {
1194-
forAll {
1201+
it("Succeeds regardless of output credProtect policy if credProtect is set with enforceCredentialProtectionPolicy=false or credProtectPolicy=userVerificationOptional.") {
1202+
val genCredPropsInput = for {
1203+
enforce <- arbitrary[Boolean]
1204+
policy <-
1205+
if (enforce) Gen.const(CredentialProtectionPolicy.UV_OPTIONAL)
1206+
else arbitrary[CredentialProtectionPolicy]
1207+
} yield {
1208+
if (enforce) CredentialProtectionInput.require(policy)
1209+
else CredentialProtectionInput.prefer(policy)
1210+
}
1211+
forAll(
1212+
genCredPropsInput,
1213+
arbitrary[Option[CredentialProtectionPolicy]],
1214+
) {
11951215
(
1196-
inputPolicy: CredentialProtectionPolicy,
1216+
credPropsInput: CredentialProtectionInput,
11971217
outputPolicy: Option[CredentialProtectionPolicy],
11981218
) =>
11991219
val authenticatorExtensionOutputs =
@@ -1212,9 +1232,7 @@ class RelyingPartyRegistrationSpec
12121232
.copy(
12131233
requestedExtensions = RegistrationExtensionInputs
12141234
.builder()
1215-
.credProtect(
1216-
CredentialProtectionInput.prefer(inputPolicy)
1217-
)
1235+
.credProtect(credPropsInput)
12181236
.build()
12191237
)
12201238
.editAuthenticatorData(authData =>

‎webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyV2RegistrationSpec.scala

+58-40
Original file line numberDiff line numberDiff line change
@@ -1126,8 +1126,12 @@ class RelyingPartyV2RegistrationSpec
11261126
}
11271127
}
11281128

1129-
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and no output policy is returned.") {
1130-
forAll { policy: CredentialProtectionPolicy =>
1129+
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and credProtectPolicy!=userVerificationOptional and no output policy is returned.") {
1130+
forAll(
1131+
arbitrary[CredentialProtectionPolicy].suchThat(
1132+
_ != CredentialProtectionPolicy.UV_OPTIONAL
1133+
)
1134+
) { policy: CredentialProtectionPolicy =>
11311135
val steps = finishRegistration(
11321136
testData = RegistrationTestData.Packed.BasicAttestation
11331137
.copy(
@@ -1145,47 +1149,63 @@ class RelyingPartyV2RegistrationSpec
11451149
}
11461150
}
11471151

1148-
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and the output policy does not match the input policy.") {
1149-
forAll(arbitrary[CredentialProtectionPolicy], Gen.oneOf(1, 2)) {
1150-
(policy: CredentialProtectionPolicy, diff) =>
1151-
val authenticatorExtensionOutputs = CBORObject.NewMap()
1152-
authenticatorExtensionOutputs.set(
1153-
"credProtect",
1154-
CBORObject.FromObject(
1155-
((ReexportHelpers.credProtectPolicyCborValue(
1156-
policy
1157-
) + diff - 1) % 3) + 1
1158-
),
1159-
)
1160-
val steps = finishRegistration(
1161-
testData = RegistrationTestData.Packed.BasicAttestation
1162-
.copy(
1163-
requestedExtensions = RegistrationExtensionInputs
1164-
.builder()
1165-
.credProtect(CredentialProtectionInput.require(policy))
1166-
.build()
1167-
)
1168-
.editAuthenticatorData(authData =>
1169-
new ByteArray(
1170-
authData.getBytes.updated(
1171-
32,
1172-
(authData.getBytes()(32) | 0x80).toByte,
1173-
) ++ authenticatorExtensionOutputs.EncodeToBytes()
1174-
)
1152+
it("Fails if credProtect is set with enforceCredentialProtectionPolicy=true and credProtectPolicy!=userVerificationOptional and the output policy does not match the input policy.") {
1153+
forAll(
1154+
arbitrary[CredentialProtectionPolicy].suchThat(
1155+
_ != CredentialProtectionPolicy.UV_OPTIONAL
1156+
),
1157+
Gen.oneOf(1, 2),
1158+
) { (policy: CredentialProtectionPolicy, diff) =>
1159+
val authenticatorExtensionOutputs = CBORObject.NewMap()
1160+
authenticatorExtensionOutputs.set(
1161+
"credProtect",
1162+
CBORObject.FromObject(
1163+
((ReexportHelpers.credProtectPolicyCborValue(
1164+
policy
1165+
) + diff - 1) % 3) + 1
1166+
),
1167+
)
1168+
val steps = finishRegistration(
1169+
testData = RegistrationTestData.Packed.BasicAttestation
1170+
.copy(
1171+
requestedExtensions = RegistrationExtensionInputs
1172+
.builder()
1173+
.credProtect(CredentialProtectionInput.require(policy))
1174+
.build()
1175+
)
1176+
.editAuthenticatorData(authData =>
1177+
new ByteArray(
1178+
authData.getBytes.updated(
1179+
32,
1180+
(authData.getBytes()(32) | 0x80).toByte,
1181+
) ++ authenticatorExtensionOutputs.EncodeToBytes()
11751182
)
1176-
)
1177-
val stepAfter: Try[FinishRegistrationSteps#Step18] =
1178-
steps.begin.next.next.next.next.next.next.next.next.next.next.next.tryNext
1183+
)
1184+
)
1185+
val stepAfter: Try[FinishRegistrationSteps#Step18] =
1186+
steps.begin.next.next.next.next.next.next.next.next.next.next.next.tryNext
11791187

1180-
stepAfter shouldBe a[Failure[_]]
1181-
stepAfter.failed.get shouldBe an[IllegalArgumentException]
1188+
stepAfter shouldBe a[Failure[_]]
1189+
stepAfter.failed.get shouldBe an[IllegalArgumentException]
11821190
}
11831191
}
11841192

1185-
it("Succeeds regardless of output credProtect policy if credProtect is set with enforceCredentialProtectionPolicy=false.") {
1186-
forAll {
1193+
it("Succeeds regardless of output credProtect policy if credProtect is set with enforceCredentialProtectionPolicy=false or credProtectPolicy=userVerificationOptional.") {
1194+
val genCredPropsInput = for {
1195+
enforce <- arbitrary[Boolean]
1196+
policy <-
1197+
if (enforce) Gen.const(CredentialProtectionPolicy.UV_OPTIONAL)
1198+
else arbitrary[CredentialProtectionPolicy]
1199+
} yield {
1200+
if (enforce) CredentialProtectionInput.require(policy)
1201+
else CredentialProtectionInput.prefer(policy)
1202+
}
1203+
forAll(
1204+
genCredPropsInput,
1205+
arbitrary[Option[CredentialProtectionPolicy]],
1206+
) {
11871207
(
1188-
inputPolicy: CredentialProtectionPolicy,
1208+
credPropsInput: CredentialProtectionInput,
11891209
outputPolicy: Option[CredentialProtectionPolicy],
11901210
) =>
11911211
val authenticatorExtensionOutputs =
@@ -1204,9 +1224,7 @@ class RelyingPartyV2RegistrationSpec
12041224
.copy(
12051225
requestedExtensions = RegistrationExtensionInputs
12061226
.builder()
1207-
.credProtect(
1208-
CredentialProtectionInput.prefer(inputPolicy)
1209-
)
1227+
.credProtect(credPropsInput)
12101228
.build()
12111229
)
12121230
.editAuthenticatorData(authData =>

0 commit comments

Comments
 (0)