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.

Sign up for GitHub

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

8346094: Harden X509CertImpl.getExtensionValue for NPE cases #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+220 −58
Open

8346094: Harden X509CertImpl.getExtensionValue for NPE cases #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 16 additions & 56 deletions src/java.base/share/classes/sun/security/x509/X509CertImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1996, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1996, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -120,7 +120,7 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
*/
public X509CertImpl(X509CertInfo info, AlgorithmId algId, byte[] signature,
byte[] signedCert) {
this.info = info;
this.info = Objects.requireNonNull(info);
this.algId = algId;
this.signature = signature;
this.signedCert = Objects.requireNonNull(signedCert);
Expand Down Expand Up @@ -553,7 +553,7 @@ public X509CertInfo getInfo() {
* before this function may be called.
*/
public String toString() {
if (info == null || algId == null || signature == null)
if (algId == null || signature == null)
return "";

HexDumpEncoder encoder = new HexDumpEncoder();
Expand All @@ -570,8 +570,6 @@ public String toString() {
* @return the publickey.
*/
public PublicKey getPublicKey() {
if (info == null)
return null;
return info.getKey().getKey();
}

Expand All @@ -581,8 +579,6 @@ public PublicKey getPublicKey() {
* @return the version number, i.e. 1, 2 or 3.
*/
public int getVersion() {
if (info == null)
return -1;
try {
int vers = info.getVersion().getVersion();
return vers + 1;
Expand All @@ -609,8 +605,6 @@ public BigInteger getSerialNumber() {
* @return the serial number.
*/
public SerialNumber getSerialNumberObject() {
if (info == null)
return null;
return info.getSerialNumber().getSerial();
}

Expand All @@ -622,8 +616,6 @@ public SerialNumber getSerialNumberObject() {
*/
@SuppressWarnings("deprecation")
public Principal getSubjectDN() {
if (info == null)
return null;
return info.getSubject();
}

Expand All @@ -633,9 +625,6 @@ public Principal getSubjectDN() {
* also aware of X509CertImpl mutability.
*/
public X500Principal getSubjectX500Principal() {
if (info == null) {
return null;
}
try {
return info.getSubject().asX500Principal();
} catch (Exception e) {
Expand All @@ -650,8 +639,6 @@ public X500Principal getSubjectX500Principal() {
*/
@SuppressWarnings("deprecation")
public Principal getIssuerDN() {
if (info == null)
return null;
return info.getIssuer();
}

Expand All @@ -661,9 +648,6 @@ public Principal getIssuerDN() {
* also aware of X509CertImpl mutability.
*/
public X500Principal getIssuerX500Principal() {
if (info == null) {
return null;
}
try {
return info.getIssuer().asX500Principal();
} catch (Exception e) {
Expand All @@ -677,8 +661,6 @@ public X500Principal getIssuerX500Principal() {
* @return the start date of the validity period.
*/
public Date getNotBefore() {
if (info == null)
return null;
return info.getValidity().getNotBefore();
}

Expand All @@ -688,8 +670,6 @@ public Date getNotBefore() {
* @return the end date of the validity period.
*/
public Date getNotAfter() {
if (info == null)
return null;
return info.getValidity().getNotAfter();
}

Expand All @@ -702,10 +682,7 @@ public Date getNotAfter() {
* @exception CertificateEncodingException if an encoding error occurs.
*/
public byte[] getTBSCertificate() throws CertificateEncodingException {
if (info != null) {
return info.getEncodedInfo();
} else
throw new CertificateEncodingException("Uninitialized certificate");
return info.getEncodedInfo();
}

/**
Expand Down Expand Up @@ -766,8 +743,6 @@ public byte[] getSigAlgParams() {
* @return the Issuer Unique Identity.
*/
public boolean[] getIssuerUniqueID() {
if (info == null)
return null;
UniqueIdentity id = info.getIssuerUniqueId();
if (id == null)
return null;
Expand All @@ -781,8 +756,6 @@ public boolean[] getIssuerUniqueID() {
* @return the Subject Unique Identity.
*/
public boolean[] getSubjectUniqueID() {
if (info == null)
return null;
UniqueIdentity id = info.getSubjectUniqueId();
if (id == null)
return null;
Expand Down Expand Up @@ -935,8 +908,6 @@ public CRLDistributionPointsExtension getCRLDistributionPointsExtension() {
* not supported, otherwise return false.
*/
public boolean hasUnsupportedCriticalExtension() {
if (info == null)
return false;
CertificateExtensions exts = info.getExtensions();
if (exts == null)
return false;
Expand All @@ -952,9 +923,6 @@ public boolean hasUnsupportedCriticalExtension() {
* certificate that are marked critical.
*/
public Set<String> getCriticalExtensionOIDs() {
if (info == null) {
return null;
}
try {
CertificateExtensions exts = info.getExtensions();
if (exts == null) {
Expand All @@ -981,9 +949,6 @@ public Set<String> getCriticalExtensionOIDs() {
* certificate that are NOT marked critical.
*/
public Set<String> getNonCriticalExtensionOIDs() {
if (info == null) {
return null;
}
try {
CertificateExtensions exts = info.getExtensions();
if (exts == null) {
Expand All @@ -1010,9 +975,6 @@ public Set<String> getNonCriticalExtensionOIDs() {
* extension
*/
public Extension getExtension(ObjectIdentifier oid) {
if (info == null) {
return null;
}
CertificateExtensions extensions = info.getExtensions();
if (extensions != null) {
Extension ex = extensions.getExtension(oid.toString());
Expand All @@ -1031,9 +993,6 @@ public Extension getExtension(ObjectIdentifier oid) {
}

public Extension getUnparseableExtension(ObjectIdentifier oid) {
if (info == null) {
return null;
}
CertificateExtensions extensions = info.getExtensions();
if (extensions == null) {
return null;
Expand All @@ -1045,7 +1004,8 @@ public Extension getUnparseableExtension(ObjectIdentifier oid) {
/**
* Gets the DER encoded extension identified by the given
* oid String.
*
* @return the DER-encoded extension value, or {@code null} if
* the extensions are not present or the value is not found
* @param oid the Object Identifier value for the extension.
*/
public byte[] getExtensionValue(String oid) {
Expand All @@ -1054,13 +1014,11 @@ public byte[] getExtensionValue(String oid) {
String extAlias = OIDMap.getName(findOID);
Extension certExt = null;
CertificateExtensions exts = info.getExtensions();

if (exts == null) {
return null;
}
if (extAlias == null) { // may be unknown
// get the extensions, search through' for this oid
if (exts == null) {
return null;
}

for (Extension ex : exts.getAllExtensions()) {
ObjectIdentifier inCertOID = ex.getExtensionId();
if (inCertOID.equals(findOID)) {
Expand All @@ -1069,12 +1027,10 @@ public byte[] getExtensionValue(String oid) {
}
}
} else { // there's subclass that can handle this extension
certExt = getInfo().getExtensions().getExtension(extAlias);
certExt = exts.getExtension(extAlias);
}
if (certExt == null) {
if (exts != null) {
certExt = exts.getUnparseableExtensions().get(oid);
}
certExt = exts.getUnparseableExtensions().get(oid);
if (certExt == null) {
return null;
}
Expand All @@ -1098,8 +1054,12 @@ public byte[] getExtensionValue(String oid) {
*/
public boolean[] getKeyUsage() {
try {
CertificateExtensions extensions = info.getExtensions();
if (extensions == null) {
return null;
}
KeyUsageExtension certExt = (KeyUsageExtension)
getInfo().getExtensions().getExtension(KeyUsageExtension.NAME);
extensions.getExtension(KeyUsageExtension.NAME);
if (certExt == null)
return null;

Expand Down
5 changes: 3 additions & 2 deletions test/jdk/sun/security/util/HostnameChecker/TestHostnameChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -36,6 +36,7 @@
import jdk.test.lib.security.CertUtils;
import sun.security.util.*;
import sun.security.x509.X509CertImpl;
import sun.security.x509.X509CertInfo;

/**
* Certificate 1:
Expand Down Expand Up @@ -225,7 +226,7 @@ public static void main(String[] args) throws Exception {
}

private static X509Certificate mock(String domain) {
return new X509CertImpl(null, null, null, new byte[0]) {
return new X509CertImpl(new X509CertInfo(), null, null, new byte[0]) {
@Override
public Collection<List<?>> getSubjectAlternativeNames() {
return List.of(List.of(2, domain));
Expand Down
Loading