Skip to content

Commit 18eb6d9

Browse files
coffeyscoffeys
committed
8255348: NPE in PKIXCertPathValidator event logging code
Reviewed-by: mullan
1 parent a97f3c1 commit 18eb6d9

File tree

6 files changed

+75
-24
lines changed

6 files changed

+75
-24
lines changed

‎src/java.base/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,13 +241,13 @@ private static PKIXCertPathValidatorResult validate(TrustAnchor anchor,
241241
X509ValidationEvent xve = new X509ValidationEvent();
242242
if (xve.shouldCommit() || EventHelper.isLoggingSecurity()) {
243243
int[] certIds = params.certificates().stream()
244-
.mapToInt(x -> x.hashCode())
244+
.mapToInt(Certificate::hashCode)
245245
.toArray();
246-
int anchorCertId =
247-
anchor.getTrustedCert().hashCode();
246+
int anchorCertId = (anchorCert != null) ?
247+
anchorCert.hashCode() : anchor.getCAPublicKey().hashCode();
248248
if (xve.shouldCommit()) {
249249
xve.certificateId = anchorCertId;
250-
int certificatePos = 1; //anchor cert
250+
int certificatePos = 1; // most trusted CA
251251
xve.certificatePosition = certificatePos;
252252
xve.validationCounter = validationCounter.incrementAndGet();
253253
xve.commit();

‎test/jdk/jdk/jfr/event/security/TestX509CertificateEvent.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,11 @@ public static void main(String[] args) throws Exception {
4848
recording.enable(EventNames.X509Certificate);
4949
recording.start();
5050

51-
CertificateFactory cf = CertificateFactory.getInstance("X.509");
52-
TestCertificate.ONE.generate(cf);
53-
TestCertificate.TWO.generate(cf);
51+
TestCertificate.ONE.certificate();
52+
TestCertificate.TWO.certificate();
5453
// Generate twice to make sure only one event per certificate is generated
55-
TestCertificate.ONE.generate(cf);
56-
TestCertificate.TWO.generate(cf);
54+
TestCertificate.ONE.certificate();
55+
TestCertificate.TWO.certificate();
5756

5857
recording.stop();
5958

‎test/jdk/jdk/jfr/event/security/TestX509ValidationEvent.java

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ public static void main(String[] args) throws Exception {
4747
try (Recording recording = new Recording()) {
4848
recording.enable(EventNames.X509Validation);
4949
recording.start();
50-
// intermeditate certificate test
51-
TestCertificate.generateChain(false);
50+
// intermediate certificate test
51+
TestCertificate.generateChain(false, true);
5252
recording.stop();
5353
List<RecordedEvent> events = Events.fromRecording(recording);
5454
Asserts.assertEquals(events.size(), 3, "Incorrect number of events");
@@ -59,12 +59,23 @@ public static void main(String[] args) throws Exception {
5959
recording.enable(EventNames.X509Validation);
6060
recording.start();
6161
// self signed certificate test
62-
TestCertificate.generateChain(true);
62+
TestCertificate.generateChain(true, true);
6363
recording.stop();
6464
List<RecordedEvent> events = Events.fromRecording(recording);
6565
Asserts.assertEquals(events.size(), 2, "Incorrect number of events");
6666
assertEvent2(events);
6767
}
68+
69+
try (Recording recording = new Recording()) {
70+
recording.enable(EventNames.X509Validation);
71+
recording.start();
72+
// intermediate certificate test, with no Cert for trust anchor
73+
TestCertificate.generateChain(true, false);
74+
recording.stop();
75+
List<RecordedEvent> events = Events.fromRecording(recording);
76+
Asserts.assertEquals(events.size(), 2, "Incorrect number of events");
77+
assertEvent3(events);
78+
}
6879
}
6980

7081
private static void assertEvent1(List<RecordedEvent> events) throws Exception {
@@ -108,4 +119,26 @@ private static void assertEvent2(List<RecordedEvent> events) throws Exception {
108119
}
109120
}
110121
}
122+
/*
123+
* Self signed certificate test
124+
*/
125+
private static void assertEvent3(List<RecordedEvent> events) throws Exception {
126+
for (RecordedEvent e : events) {
127+
int pos = e.getInt("certificatePosition");
128+
switch (pos) {
129+
// use public key of cert provided in TrustAnchor
130+
case 1:
131+
Asserts.assertEquals(e.getLong("certificateId"),
132+
Long.valueOf(TestCertificate.ROOT_CA.certificate().getPublicKey().hashCode()));
133+
break;
134+
case 2:
135+
Events.assertField(e, "certificateId")
136+
.equal(TestCertificate.ROOT_CA.certId);
137+
break;
138+
default:
139+
System.out.println(events);
140+
throw new Exception("Unexpected position:" + pos);
141+
}
142+
}
143+
}
111144
}

‎test/jdk/jdk/security/logging/TestX509CertificateLog.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,8 @@ public static void main(String[] args) throws Exception {
5858

5959
public static class GenerateX509Certicate {
6060
public static void main(String[] args) throws Exception {
61-
CertificateFactory cf = CertificateFactory.getInstance("X.509");
62-
TestCertificate.ONE.generate(cf);
63-
TestCertificate.TWO.generate(cf);
61+
TestCertificate.ONE.certificate();
62+
TestCertificate.TWO.certificate();
6463
}
6564
}
6665
}

‎test/jdk/jdk/security/logging/TestX509ValidationLog.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,19 @@ public static void main(String[] args) throws Exception {
4343
l.addExpected("FINE: ValidationChain: " +
4444
TestCertificate.ROOT_CA.certId + ", " +
4545
TestCertificate.ROOT_CA.certId);
46+
l.addExpected("FINE: ValidationChain: " +
47+
TestCertificate.ROOT_CA.certificate().getPublicKey().hashCode() +
48+
", " + TestCertificate.ROOT_CA.certId);
4649
l.testExpected();
4750
}
4851

4952
public static class GenerateCertificateChain {
5053
public static void main(String[] args) throws Exception {
51-
TestCertificate.generateChain(false);
54+
TestCertificate.generateChain(false, true);
5255
// self signed test
53-
TestCertificate.generateChain(true);
56+
TestCertificate.generateChain(true, true);
57+
// no cert for trust anchor
58+
TestCertificate.generateChain(true, false);
5459
}
5560
}
5661
}

‎test/lib/jdk/test/lib/security/TestCertificate.java

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ public enum TestCertificate {
122122
"J2GyCaJINsyaI/I2\n" +
123123
"-----END CERTIFICATE-----");
124124

125+
private static final CertificateFactory CERTIFICATE_FACTORY = getCertificateFactory();
126+
125127
public String serialNumber;
126128
public String algorithm;
127129
public String subject;
@@ -143,22 +145,35 @@ public enum TestCertificate {
143145
this.keyLength = 2048;
144146
}
145147

146-
public X509Certificate generate(CertificateFactory cf) throws CertificateException {
148+
private static CertificateFactory getCertificateFactory() {
149+
try {
150+
return CertificateFactory.getInstance("X.509");
151+
} catch (CertificateException e) {
152+
throw new RuntimeException(e);
153+
}
154+
}
155+
156+
public X509Certificate certificate() throws CertificateException {
147157
ByteArrayInputStream is = new ByteArrayInputStream(encoded.getBytes());
148-
return (X509Certificate) cf.generateCertificate(is);
158+
return (X509Certificate) CERTIFICATE_FACTORY.generateCertificate(is);
149159
}
150160

151-
public static void generateChain(boolean selfSignedTest) throws Exception {
161+
public static void generateChain(boolean selfSignedTest, boolean trustAnchorCert) throws Exception {
152162
// Do path validation as if it is always Tue, 06 Sep 2016 22:12:21 GMT
153163
// This value is within the lifetimes of all certificates.
154164
Date testDate = new Date(1473199941000L);
155165

156166
CertificateFactory cf = CertificateFactory.getInstance("X.509");
157-
X509Certificate c1 = TestCertificate.ONE.generate(cf);
158-
X509Certificate c2 = TestCertificate.TWO.generate(cf);
159-
X509Certificate ca = TestCertificate.ROOT_CA.generate(cf);
167+
X509Certificate c1 = TestCertificate.ONE.certificate();
168+
X509Certificate c2 = TestCertificate.TWO.certificate();
169+
X509Certificate ca = TestCertificate.ROOT_CA.certificate();
160170

161-
TrustAnchor ta = new TrustAnchor(ca, null);
171+
TrustAnchor ta;
172+
if (trustAnchorCert) {
173+
ta = new TrustAnchor(ca, null);
174+
} else {
175+
ta = new TrustAnchor(ca.getIssuerX500Principal(), ca.getPublicKey(), null);
176+
}
162177
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
163178

164179
PKIXParameters params = new PKIXParameters(Collections.singleton(ta));

0 commit comments

Comments
 (0)