Skip to content

Commit 083a518

Browse files
emlunemlun
committed
Add policy tree validator setting
1 parent 8474644 commit 083a518

File tree

4 files changed

+141
-9
lines changed

4 files changed

+141
-9
lines changed

‎NEWS

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ New features:
99

1010
* Added support for the `"tpm"` attestation statement format.
1111
* Added support for ES384 and ES512 signature algorithms.
12+
* Added property `policyTreeValidator` to `TrustRootsResult`. If set, the given
13+
predicate function will be used to validate the certificate policy tree after
14+
successful attestation certificate path validation. This may be required for
15+
some JCA providers to accept attestation certificates with critical
16+
certificate policy extensions.
1217

1318
Fixes:
1419

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import java.security.cert.CertPathValidatorException;
5151
import java.security.cert.CertificateException;
5252
import java.security.cert.CertificateFactory;
53+
import java.security.cert.PKIXCertPathValidatorResult;
5354
import java.security.cert.PKIXParameters;
5455
import java.security.cert.TrustAnchor;
5556
import java.security.cert.X509Certificate;
@@ -536,10 +537,25 @@ public boolean attestationTrusted() {
536537
pathParams.setDate(Date.from(clock.instant()));
537538
pathParams.setRevocationEnabled(trustRoots.get().isEnableRevocationChecking());
538539
pathParams.setPolicyQualifiersRejected(
539-
false); // TODO: Add parameter to configure policy qualifier processor
540+
!trustRoots.get().getPolicyTreeValidator().isPresent());
540541
trustRoots.get().getCertStore().ifPresent(pathParams::addCertStore);
541-
cpv.validate(certPath, pathParams);
542-
return true;
542+
final PKIXCertPathValidatorResult result =
543+
(PKIXCertPathValidatorResult) cpv.validate(certPath, pathParams);
544+
return trustRoots
545+
.get()
546+
.getPolicyTreeValidator()
547+
.map(
548+
policyNodePredicate -> {
549+
if (policyNodePredicate.test(result.getPolicyTree())) {
550+
return true;
551+
} else {
552+
log.info(
553+
"Failed to derive trust in attestation statement: Certificate path policy tree does not satisfy policy tree validator. Attestation object: {}",
554+
response.getResponse().getAttestationObject());
555+
return false;
556+
}
557+
})
558+
.orElse(true);
543559
}
544560

545561
} catch (CertPathValidatorException e) {

‎webauthn-server-core/src/main/java/com/yubico/webauthn/attestation/AttestationTrustSource.java

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
import com.yubico.internal.util.CollectionUtil;
2828
import com.yubico.webauthn.data.ByteArray;
2929
import java.security.cert.CertStore;
30+
import java.security.cert.PolicyNode;
3031
import java.security.cert.X509Certificate;
3132
import java.util.List;
3233
import java.util.Optional;
3334
import java.util.Set;
35+
import java.util.function.Predicate;
3436
import lombok.Builder;
3537
import lombok.NonNull;
3638
import lombok.Value;
@@ -60,11 +62,21 @@ TrustRootsResult findTrustRoots(
6062
List<X509Certificate> attestationCertificateChain, Optional<ByteArray> aaguid);
6163

6264
/**
63-
* A result of looking up attestation trust roots for a particular attestation statement. This
64-
* primarily consists of a set of trust root certificates, but may also include a {@link
65-
* CertStore} of additional CRLs and/or intermediate certificate to use during certificate path
66-
* validation, and may also disable certificate revocation checking for the relevant attestation
67-
* statement.
65+
* A result of looking up attestation trust roots for a particular attestation statement.
66+
*
67+
* <p>This primarily consists of a set of trust root certificates - see {@link
68+
* TrustRootsResultBuilder#trustRoots(Set) trustRoots(Set)} - but may also:
69+
*
70+
* <ul>
71+
* <li>include a {@link CertStore} of additional CRLs and/or intermediate certificates to use
72+
* during certificate path validation - see {@link
73+
* TrustRootsResultBuilder#certStore(CertStore) certStore(CertStore)};
74+
* <li>disable certificate revocation checking for the relevant attestation statement - see
75+
* {@link TrustRootsResultBuilder#enableRevocationChecking(boolean)
76+
* enableRevocationChecking(boolean)}; and/or
77+
* <li>define a policy tree validator for the PKIX policy tree result - see {@link
78+
* TrustRootsResultBuilder#policyTreeValidator(Predicate) policyTreeValidator(Predicate)}.
79+
* </ul>
6880
*/
6981
@Value
7082
@Builder(toBuilder = true)
@@ -97,19 +109,47 @@ class TrustRootsResult {
97109
*/
98110
@Builder.Default private final boolean enableRevocationChecking = true;
99111

112+
/**
113+
* If non-null, the PolicyQualifiersRejected flag will be set to false during certificate path
114+
* validation. See {@link
115+
* java.security.cert.PKIXParameters#setPolicyQualifiersRejected(boolean)}.
116+
*
117+
* <p>The given {@link Predicate} will be used to validate the policy tree. The {@link
118+
* Predicate} should return <code>true</code> if the policy tree is acceptable, and <code>false
119+
* </code> otherwise.
120+
*
121+
* <p>This may be required if any certificate in the certificate path contains a certificate
122+
* policies extension marked critical. If this is not set, then such a certificate will be
123+
* rejected by the certificate path validator.
124+
*
125+
* <p>Consult the <a
126+
* href="https://docs.oracle.com/en/java/javase/17/security/java-pki-programmers-guide.html#GUID-3AD41382-E729-469B-83EE-CB2FE66D71D8">Java
127+
* PKI Programmer's Guide</a> for how to use the {@link PolicyNode} argument of the {@link
128+
* Predicate}.
129+
*
130+
* <p>The default is <code>null</code>.
131+
*/
132+
@Builder.Default private final Predicate<PolicyNode> policyTreeValidator = null;
133+
100134
private TrustRootsResult(
101135
@NonNull Set<X509Certificate> trustRoots,
102136
CertStore certStore,
103-
boolean enableRevocationChecking) {
137+
boolean enableRevocationChecking,
138+
Predicate<PolicyNode> policyTreeValidator) {
104139
this.trustRoots = CollectionUtil.immutableSet(trustRoots);
105140
this.certStore = certStore;
106141
this.enableRevocationChecking = enableRevocationChecking;
142+
this.policyTreeValidator = policyTreeValidator;
107143
}
108144

109145
public Optional<CertStore> getCertStore() {
110146
return Optional.ofNullable(certStore);
111147
}
112148

149+
public Optional<Predicate<PolicyNode>> getPolicyTreeValidator() {
150+
return Optional.ofNullable(policyTreeValidator);
151+
}
152+
113153
public static TrustRootsResultBuilder.Step1 builder() {
114154
return new TrustRootsResultBuilder.Step1();
115155
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ import org.bouncycastle.asn1.x509.Extension
8181
import org.bouncycastle.asn1.x509.GeneralName
8282
import org.bouncycastle.asn1.x509.GeneralNamesBuilder
8383
import org.bouncycastle.cert.jcajce.JcaX500NameUtil
84+
import org.bouncycastle.jce.provider.BouncyCastleProvider
8485
import org.junit.runner.RunWith
8586
import org.mockito.Mockito
8687
import org.scalacheck.Arbitrary.arbitrary
@@ -98,10 +99,12 @@ import java.security.KeyFactory
9899
import java.security.KeyPair
99100
import java.security.MessageDigest
100101
import java.security.PrivateKey
102+
import java.security.Security
101103
import java.security.SignatureException
102104
import java.security.cert.CRL
103105
import java.security.cert.CertStore
104106
import java.security.cert.CollectionCertStoreParameters
107+
import java.security.cert.PolicyNode
105108
import java.security.cert.X509Certificate
106109
import java.security.interfaces.ECPublicKey
107110
import java.security.interfaces.RSAPublicKey
@@ -111,6 +114,7 @@ import java.time.ZoneOffset
111114
import java.util
112115
import java.util.Collections
113116
import java.util.Optional
117+
import java.util.function.Predicate
114118
import javax.security.auth.x500.X500Principal
115119
import scala.jdk.CollectionConverters._
116120
import scala.jdk.OptionConverters.RichOption
@@ -193,6 +197,7 @@ class RelyingPartyRegistrationSpec
193197
trustedCert: X509Certificate,
194198
crls: Option[Set[CRL]] = None,
195199
enableRevocationChecking: Boolean = true,
200+
policyTreeValidator: Option[Predicate[PolicyNode]] = None,
196201
): AttestationTrustSource =
197202
(_: util.List[X509Certificate], _: Optional[ByteArray]) => {
198203
TrustRootsResult
@@ -209,6 +214,7 @@ class RelyingPartyRegistrationSpec
209214
.orNull
210215
)
211216
.enableRevocationChecking(enableRevocationChecking)
217+
.policyTreeValidator(policyTreeValidator.orNull)
212218
.build()
213219
}
214220

@@ -2088,6 +2094,7 @@ class RelyingPartyRegistrationSpec
20882094
trustSourceWith(
20892095
testData.attestationRootCertificate.get,
20902096
enableRevocationChecking = false,
2097+
policyTreeValidator = Some(_ => true),
20912098
)
20922099
),
20932100
)
@@ -3364,6 +3371,7 @@ class RelyingPartyRegistrationSpec
33643371
trustedRootCert: Option[X509Certificate] = None,
33653372
enableRevocationChecking: Boolean = true,
33663373
origins: Option[Set[String]] = None,
3374+
policyTreeValidator: Option[Predicate[PolicyNode]] = None,
33673375
): Unit = {
33683376
it("is rejected if untrusted attestation is not allowed and the trust source does not trust it.") {
33693377
val steps = finishRegistration(
@@ -3418,6 +3426,7 @@ class RelyingPartyRegistrationSpec
34183426
)
34193427
}),
34203428
enableRevocationChecking = enableRevocationChecking,
3429+
policyTreeValidator = policyTreeValidator,
34213430
)
34223431
)
34233432
val steps = finishRegistration(
@@ -3469,6 +3478,7 @@ class RelyingPartyRegistrationSpec
34693478
.trustRoots(Collections.emptySet())
34703479
.certStore(certStore)
34713480
.enableRevocationChecking(enableRevocationChecking)
3481+
.policyTreeValidator(policyTreeValidator.orNull)
34723482
.build()
34733483
}
34743484
val steps = finishRegistration(
@@ -3497,6 +3507,7 @@ class RelyingPartyRegistrationSpec
34973507
.trustRoots(Collections.singleton(rootCert))
34983508
.certStore(certStore)
34993509
.enableRevocationChecking(enableRevocationChecking)
3510+
.policyTreeValidator(policyTreeValidator.orNull)
35003511
.build()
35013512
}
35023513
val steps = finishRegistration(
@@ -3573,8 +3584,67 @@ class RelyingPartyRegistrationSpec
35733584
origins = Some(Set(testData.clientData.getOrigin)),
35743585
trustedRootCert = Some(testData.attestationRootCertificate.get),
35753586
enableRevocationChecking = false,
3587+
policyTreeValidator = Some(_ => true),
35763588
)
35773589
}
3590+
3591+
describe("Critical certificate policy extensions") {
3592+
def init(
3593+
policyTreeValidator: Option[Predicate[PolicyNode]]
3594+
): FinishRegistrationSteps#Step21 = {
3595+
val testData = RegistrationTestData.Tpm.RealExample
3596+
val clock = Clock.fixed(
3597+
Instant.parse("2022-08-25T16:00:00Z"),
3598+
ZoneOffset.UTC,
3599+
)
3600+
val steps = finishRegistration(
3601+
allowUntrustedAttestation = false,
3602+
origins = Some(Set(testData.clientData.getOrigin)),
3603+
testData = testData,
3604+
attestationTrustSource = Some(
3605+
trustSourceWith(
3606+
testData.attestationRootCertificate.get,
3607+
enableRevocationChecking = false,
3608+
policyTreeValidator = policyTreeValidator,
3609+
)
3610+
),
3611+
clock = clock,
3612+
)
3613+
3614+
steps.begin.next.next.next.next.next.next.next.next.next.next.next.next.next.next
3615+
}
3616+
3617+
it("are rejected if no policy tree validator is set.") {
3618+
// BouncyCastle provider does not reject critical policy extensions
3619+
// TODO Mark test as ignored instead of just skipping (assume() and cancel() currently break pitest)
3620+
if (
3621+
!Security.getProviders
3622+
.exists(p => p.isInstanceOf[BouncyCastleProvider])
3623+
) {
3624+
val step = init(policyTreeValidator = None)
3625+
3626+
step.validations shouldBe a[Failure[_]]
3627+
step.attestationTrusted should be(false)
3628+
step.tryNext shouldBe a[Failure[_]]
3629+
}
3630+
}
3631+
3632+
it("are accepted if a policy tree validator is set and accepts the policy tree.") {
3633+
val step = init(policyTreeValidator = Some(_ => true))
3634+
3635+
step.validations shouldBe a[Success[_]]
3636+
step.attestationTrusted should be(true)
3637+
step.tryNext shouldBe a[Success[_]]
3638+
}
3639+
3640+
it("are rejected if a policy tree validator is set and does not accept the policy tree.") {
3641+
val step = init(policyTreeValidator = Some(_ => false))
3642+
3643+
step.validations shouldBe a[Failure[_]]
3644+
step.attestationTrusted should be(false)
3645+
step.tryNext shouldBe a[Failure[_]]
3646+
}
3647+
}
35783648
}
35793649
}
35803650

@@ -4413,6 +4483,7 @@ class RelyingPartyRegistrationSpec
44134483
trustSourceWith(
44144484
testData.attestationRootCertificate.get,
44154485
enableRevocationChecking = false,
4486+
policyTreeValidator = Some(_ => true),
44164487
)
44174488
),
44184489
credentialRepository = Helpers.CredentialRepository.empty,

0 commit comments

Comments
 (0)