Skip to content

Commit d981db8

Browse files
committed
8255348: NPE in PKIXCertPathValidator event logging code
Backport-of: 18eb6d9
1 parent d5d3981 commit d981db8

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,13 @@ private static PKIXCertPathValidatorResult validate(TrustAnchor anchor,
228228
X509ValidationEvent xve = new X509ValidationEvent();
229229
if (xve.shouldCommit() || EventHelper.isLoggingSecurity()) {
230230
int[] certIds = params.certificates().stream()
231-
.mapToInt(x -> x.hashCode())
231+
.mapToInt(Certificate::hashCode)
232232
.toArray();
233-
int anchorCertId =
234-
anchor.getTrustedCert().hashCode();
233+
int anchorCertId = (anchorCert != null) ?
234+
anchorCert.hashCode() : anchor.getCAPublicKey().hashCode();
235235
if (xve.shouldCommit()) {
236236
xve.certificateId = anchorCertId;
237-
int certificatePos = 1; //anchor cert
237+
int certificatePos = 1; // most trusted CA
238238
xve.certificatePosition = certificatePos;
239239
xve.validationCounter = validationCounter.incrementAndGet();
240240
xve.commit();

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

+4-5
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

+36-3
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ public static void main(String[] args) throws Exception {
5050
try (Recording recording = new Recording()) {
5151
recording.enable(EventNames.X509Validation);
5252
recording.start();
53-
// intermeditate certificate test
54-
TestCertificate.generateChain(false);
53+
// intermediate certificate test
54+
TestCertificate.generateChain(false, true);
5555
recording.stop();
5656
List<RecordedEvent> events = Events.fromRecording(recording);
5757
Asserts.assertEquals(events.size(), 3, "Incorrect number of events");
@@ -62,12 +62,23 @@ public static void main(String[] args) throws Exception {
6262
recording.enable(EventNames.X509Validation);
6363
recording.start();
6464
// self signed certificate test
65-
TestCertificate.generateChain(true);
65+
TestCertificate.generateChain(true, true);
6666
recording.stop();
6767
List<RecordedEvent> events = Events.fromRecording(recording);
6868
Asserts.assertEquals(events.size(), 2, "Incorrect number of events");
6969
assertEvent2(events);
7070
}
71+
72+
try (Recording recording = new Recording()) {
73+
recording.enable(EventNames.X509Validation);
74+
recording.start();
75+
// intermediate certificate test, with no Cert for trust anchor
76+
TestCertificate.generateChain(true, false);
77+
recording.stop();
78+
List<RecordedEvent> events = Events.fromRecording(recording);
79+
Asserts.assertEquals(events.size(), 2, "Incorrect number of events");
80+
assertEvent3(events);
81+
}
7182
}
7283

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

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

+2-3
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

+7-2
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

+22-7
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)