Skip to content

Commit

Permalink
8296742: Illegal X509 Extension should not be created
Browse files Browse the repository at this point in the history
Reviewed-by: mullan
  • Loading branch information
wangweij committed Nov 22, 2022
1 parent a6c418e commit e174558
Show file tree
Hide file tree
Showing 28 changed files with 303 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1582,7 +1582,11 @@ private void doGenCRL(PrintStream out)
int d = id.indexOf(':');
if (d >= 0) {
CRLExtensions ext = new CRLExtensions();
ext.setExtension("Reason", new CRLReasonCodeExtension(Integer.parseInt(id.substring(d+1))));
int code = Integer.parseInt(id.substring(d+1));
if (code <= 0) {
throw new Exception("Reason code must be positive");
}
ext.setExtension("Reason", new CRLReasonCodeExtension(code));
badCerts[i] = new X509CRLEntryImpl(new BigInteger(id.substring(0, d)),
firstDate, ext);
} else {
Expand Down Expand Up @@ -4632,6 +4636,9 @@ private CertificateExtensions createV3Extensions(
continue;
}
int exttype = oneOf(name, extSupported);
if (exttype != -1 && value != null && value.isEmpty()) {
throw new Exception(rb.getString("Illegal.value.") + extstr);
}
switch (exttype) {
case 0: // BC
int pathLen = -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,15 @@ public class AuthorityInfoAccessExtension extends Extension {
* Create an AuthorityInfoAccessExtension from a List of
* AccessDescription; the criticality is set to false.
*
* @param accessDescriptions the List of AccessDescription
* @param accessDescriptions the List of AccessDescription,
* cannot be null or empty.
* @throws IOException on error
*/
public AuthorityInfoAccessExtension(
List<AccessDescription> accessDescriptions) throws IOException {
if (accessDescriptions == null || accessDescriptions.isEmpty()) {
throw new IllegalArgumentException("accessDescriptions is null or empty");
}
this.extensionId = PKIXExtensions.AuthInfoAccess_Id;
this.critical = false;
this.accessDescriptions = accessDescriptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ private void encodeThis() throws IOException {
}

/**
* The default constructor for this extension. Null parameters make
* the element optional (not present).
* The default constructor for this extension. At least one parameter
* must be non null. Null parameters make the element optional (not present).
*
* @param kid the KeyIdentifier associated with this extension.
* @param names the GeneralNames associated with this extension
Expand All @@ -110,7 +110,11 @@ private void encodeThis() throws IOException {
*/
public AuthorityKeyIdentifierExtension(KeyIdentifier kid, GeneralNames names,
SerialNumber sn)
throws IOException {
throws IOException {
if (kid == null && names == null && sn == null) {
throw new IllegalArgumentException(
"AuthorityKeyIdentifierExtension cannot be empty");
}
this.id = kid;
this.names = names;
this.serialNum = sn;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ public CRLDistributionPointsExtension(
* DistributionPoint.
*
* @param isCritical the criticality setting.
* @param distributionPoints the list of distribution points
* @param distributionPoints the list of distribution points,
* cannot be null or empty.
* @throws IOException on error
*/
public CRLDistributionPointsExtension(boolean isCritical,
Expand All @@ -120,9 +121,14 @@ public CRLDistributionPointsExtension(boolean isCritical,
* Creates the extension (also called by the subclass).
*/
protected CRLDistributionPointsExtension(ObjectIdentifier extensionId,
boolean isCritical, List<DistributionPoint> distributionPoints,
boolean isCritical, List<DistributionPoint> distributionPoints,
String extensionName) throws IOException {

if (distributionPoints == null || distributionPoints.isEmpty()) {
throw new IllegalArgumentException(
"distribution points cannot be null or empty");
}

this.extensionId = extensionId;
this.critical = isCritical;
this.distributionPoints = distributionPoints;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public CRLNumberExtension(int crlNum) throws IOException {
* Create a CRLNumberExtension with the BigInteger value .
* The criticality is set to false.
*
* @param crlNum the value to be set for the extension.
* @param crlNum the value to be set for the extension, cannot be null
*/
public CRLNumberExtension(BigInteger crlNum) throws IOException {
this(PKIXExtensions.CRLNumber_Id, false, crlNum, NAME, LABEL);
Expand All @@ -91,6 +91,9 @@ protected CRLNumberExtension(ObjectIdentifier extensionId,
boolean isCritical, BigInteger crlNum, String extensionName,
String extensionLabel) throws IOException {

if (crlNum == null) {
throw new IllegalArgumentException("CRL number cannot be null");
}
this.extensionId = extensionId;
this.critical = isCritical;
this.crlNumber = crlNum;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,13 @@ public CRLReasonCodeExtension(int reason) throws IOException {
* Create a CRLReasonCodeExtension with the passed in reason.
*
* @param critical true if the extension is to be treated as critical.
* @param reason the enumerated value for the reason code.
* @param reason the enumerated value for the reason code, must be positive.
*/
public CRLReasonCodeExtension(boolean critical, int reason)
throws IOException {
throws IOException {
if (reason <= 0) {
throw new IllegalArgumentException("reason code must be positive");
}
this.extensionId = PKIXExtensions.ReasonCode_Id;
this.critical = critical;
this.reasonCode = reason;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,13 @@ private void encodeThis() throws IOException {
* Create a CertificateIssuerExtension containing the specified issuer name.
* Criticality is automatically set to true.
*
* @param issuer the certificate issuer
* @param issuer the certificate issuer, cannot be null or empty.
* @throws IOException on error
*/
public CertificateIssuerExtension(GeneralNames issuer) throws IOException {
if (issuer == null || issuer.isEmpty()) {
throw new IllegalArgumentException("issuer cannot be null or empty");
}
this.extensionId = PKIXExtensions.CertificateIssuer_Id;
this.critical = true;
this.names = issuer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,14 @@ public CertificatePoliciesExtension(List<PolicyInformation> certPolicies)
* a List of PolicyInformation with specified criticality.
*
* @param critical true if the extension is to be treated as critical.
* @param certPolicies the List of PolicyInformation.
* @param certPolicies the List of PolicyInformation, cannot be null or empty.
*/
public CertificatePoliciesExtension(Boolean critical,
List<PolicyInformation> certPolicies) throws IOException {
if (certPolicies == null || certPolicies.isEmpty()) {
throw new IllegalArgumentException(
"certificate policies cannot be null or empty");
}
this.certPolicies = certPolicies;
this.extensionId = PKIXExtensions.CertificatePolicies_Id;
this.critical = critical.booleanValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
package sun.security.x509;

import java.io.IOException;
import java.util.Objects;

import sun.security.util.*;


Expand All @@ -44,7 +46,7 @@ public class CertificatePolicyId {
* @param id the ObjectIdentifier for the policy id.
*/
public CertificatePolicyId(ObjectIdentifier id) {
this.id = id;
this.id = Objects.requireNonNull(id);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package sun.security.x509;

import java.io.IOException;
import java.util.Objects;

import sun.security.util.*;

Expand All @@ -47,8 +48,8 @@ public class CertificatePolicyMap {
*/
public CertificatePolicyMap(CertificatePolicyId issuer,
CertificatePolicyId subject) {
this.issuerDomain = issuer;
this.subjectDomain = subject;
this.issuerDomain = Objects.requireNonNull(issuer);
this.subjectDomain = Objects.requireNonNull(subject);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,15 @@ public ExtendedKeyUsageExtension(Vector<ObjectIdentifier> keyUsages)
* a Vector of KeyUsages with specified criticality.
*
* @param critical true if the extension is to be treated as critical.
* @param keyUsages the Vector of KeyUsages (ObjectIdentifiers)
* @param keyUsages the Vector of KeyUsages (ObjectIdentifiers),
* cannot be null or empty.
*/
public ExtendedKeyUsageExtension(Boolean critical, Vector<ObjectIdentifier> keyUsages)
throws IOException {
throws IOException {
if (keyUsages == null || keyUsages.isEmpty()) {
throw new IllegalArgumentException(
"key usages cannot be null or empty");
}
this.keyUsages = keyUsages;
this.extensionId = PKIXExtensions.ExtendedKeyUsage_Id;
this.critical = critical.booleanValue();
Expand Down
10 changes: 6 additions & 4 deletions src/java.base/share/classes/sun/security/x509/Extension.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import java.io.IOException;
import java.io.OutputStream;
import java.util.Arrays;
import java.util.Objects;

import sun.security.util.*;

/**
Expand Down Expand Up @@ -172,10 +174,10 @@ public final void encode(OutputStream out) throws IOException {
@Override
public void encode(DerOutputStream out) throws IOException {

if (extensionId == null)
throw new IOException("Null OID to encode for the extension!");
if (extensionValue == null)
throw new IOException("No value to encode for the extension!");
Objects.requireNonNull(extensionId,
"No OID to encode for the extension");
Objects.requireNonNull(extensionValue,
"No value to encode for the extension");

DerOutputStream dos = new DerOutputStream();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package sun.security.x509;

import java.io.*;
import java.util.Objects;

import sun.security.util.*;

Expand Down Expand Up @@ -61,7 +62,7 @@ public class GeneralSubtree {
* @param max the maximum BaseDistance
*/
public GeneralSubtree(GeneralName name, int min, int max) {
this.name = name;
this.name = Objects.requireNonNull(name);
this.minimum = min;
this.maximum = max;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private void encodeThis() throws IOException {
*/
public InhibitAnyPolicyExtension(int skipCerts) throws IOException {
if (skipCerts < -1)
throw new IOException("Invalid value for skipCerts");
throw new IllegalArgumentException("Invalid value for skipCerts");
if (skipCerts == -1)
this.skipCerts = Integer.MAX_VALUE;
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,13 @@ public InvalidityDateExtension(Date date) throws IOException {
* Create a InvalidityDateExtension with the passed in date.
*
* @param critical true if the extension is to be treated as critical.
* @param date the invalidity date
* @param date the invalidity date, cannot be null.
*/
public InvalidityDateExtension(boolean critical, Date date)
throws IOException {
throws IOException {
if (date == null) {
throw new IllegalArgumentException("date cannot be null");
}
this.extensionId = PKIXExtensions.InvalidityDate_Id;
this.critical = critical;
this.date = date;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,38 +69,29 @@ private void encodeThis() throws IOException {
* @exception IOException on error.
*/
public IssuerAlternativeNameExtension(GeneralNames names)
throws IOException {
this.names = names;
this.extensionId = PKIXExtensions.IssuerAlternativeName_Id;
this.critical = false;
encodeThis();
throws IOException {
this(false, names);
}

/**
* Create a IssuerAlternativeNameExtension with the passed criticality
* and GeneralNames.
*
* @param critical true if the extension is to be treated as critical.
* @param names the GeneralNames for the issuer.
* @param names the GeneralNames for the issuer, cannot be null or empty.
* @exception IOException on error.
*/
public IssuerAlternativeNameExtension(Boolean critical, GeneralNames names)
throws IOException {
throws IOException {
if (names == null || names.isEmpty()) {
throw new IllegalArgumentException("names cannot be null or empty");
}
this.names = names;
this.extensionId = PKIXExtensions.IssuerAlternativeName_Id;
this.critical = critical.booleanValue();
encodeThis();
}

/**
* Create a default IssuerAlternativeNameExtension.
*/
public IssuerAlternativeNameExtension() {
extensionId = PKIXExtensions.IssuerAlternativeName_Id;
critical = false;
names = new GeneralNames();
}

/**
* Create the extension from the passed DER encoded value.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ public class IssuingDistributionPointExtension extends Extension {
* issuer CRL entry extension.
* @throws IllegalArgumentException if more than one of
* <code>hasOnlyUserCerts</code>, <code>hasOnlyCACerts</code>,
* <code>hasOnlyAttributeCerts</code> is set to <code>true</code>.
* <code>hasOnlyAttributeCerts</code> is set to <code>true</code>,
* or all arguments are either <code>null</code> or <code>false</code>.
* @throws IOException on encoding error.
*/
public IssuingDistributionPointExtension(
Expand All @@ -119,6 +120,14 @@ public IssuingDistributionPointExtension(
boolean hasOnlyAttributeCerts, boolean isIndirectCRL)
throws IOException {

if (distributionPoint == null &&
revocationReasons == null &&
!hasOnlyUserCerts &&
!hasOnlyCACerts &&
!hasOnlyAttributeCerts &&
!isIndirectCRL) {
throw new IllegalArgumentException("elements cannot be empty");
}
if ((hasOnlyUserCerts && (hasOnlyCACerts || hasOnlyAttributeCerts)) ||
(hasOnlyCACerts && (hasOnlyUserCerts || hasOnlyAttributeCerts)) ||
(hasOnlyAttributeCerts && (hasOnlyUserCerts || hasOnlyCACerts))) {
Expand Down

1 comment on commit e174558

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.