On Mon, 14 Nov 2022 16:47:22 GMT, Weijun Wang <[email protected]> wrote:
> Inside JDK we support a lot of X.509 certificate extensions. Almost every
> extension has a rule about what is legal or not. For example, the names in
> `SubjectAlternativeNameExtension` cannot be missing or empty. Usually, a rule
> is enforced in the `encode()` method, where the extension value is assigned
> null for illegal extension and the method throws an `IOException`. However,
> before the `encode()` method is called, the illegal extension can always be
> created successfully, whether from a constructor using extension components
> (For example, `new SubjectAlternativeNameExtension(names)`) or using the
> encoded value (for example, `new
> SubjectAlternativeNameExtension(derEncoding)`).
>
> This code change tries to prevent illegal extensions from being created right
> from the beginning but the solution is not complete. Precisely, for
> constructors using extension components, new checks are added to ensure the
> correct components are provided and the extension can be encoded correctly.
> Fortunately, most of these conditions are already met inside JDK calls to
> them. The only exception is inside the `keytool -gencrl` command where the
> reason code of a revoked certificate could be zero. This has been fixed in
> this code change. There are some constructors having no arguments at all.
> These are useless and also removed.
>
> On the other hand, constructors using the encoded value are complicated. Some
> of them check for legal values, some do not. However, since the encoding is
> read from the argument and already stored inside the object, there is no need
> to calculate the encoding in the `encode()` method and this method always
> succeed.
>
> In short, while we cannot ensure the extensions created are perfectly legal,
> we ensure their `encode()` methods are always able to find a non-null
> extension value to write out.
>
> More fine comments in the code change.
src/java.base/share/classes/sun/security/x509/AuthorityKeyIdentifierExtension.java
line 114:
> 112: SerialNumber sn)
> 113: throws IOException {
> 114: if (kid == null && names == null && serialNum == null) {
Might want to mention in javadoc comment that at least one element must be
non-null.
src/java.base/share/classes/sun/security/x509/NameConstraintsExtension.java
line 142:
> 140: if (permitted == null && excluded == null) {
> 141: throw new IllegalArgumentException(
> 142: "permitted and exclude cannot be all null");
s/be all/both be/
src/java.base/share/classes/sun/security/x509/PolicyConstraintsExtension.java
line 115:
> 113: if (require == -1 && inhibit == -1) {
> 114: throw new IllegalArgumentException(
> 115: "require and inhibit cannot be all -1");
s/be all/both be/
src/java.base/share/classes/sun/security/x509/PrivateKeyUsageExtension.java
line 107:
> 105: if (notBefore == null && notAfter == null) {
> 106: throw new IllegalArgumentException(
> 107: "notBefore and notAfter cannot be all null");
s/be all/both be/
-------------
PR: https://git.openjdk.org/jdk/pull/11137
