RFR: 8346094: Harden X509CertImpl.getExtensionValue for NPE cases
Weijun Wang
weijun at openjdk.org
Thu Feb 6 14:39:12 UTC 2025
On Mon, 27 Jan 2025 12:39:45 GMT, Konanki Sreenath <duke at openjdk.org> wrote:
> Earlier code will trigger NPE if the certificate does not contain the extensions or if the requested extensions does not exist. The better approach for hardening **getExtensionValue** here is to to check for NULL explicitly before calling **getExtensionValue()** and avoding try-catch block which ensures the readability and maintainability.
>
> After scanning in multiple places where invokng getExtensions on the X509CertInfo reference, the check for NULL is added in the **getKeyUsage()** as well while calling before **getExtensionValue()**
>
> The associated tests are written and added in test class **CertificateExtensions**. Which will ensure to validate the
> **getExtensionValue()** and **getKeyUsage()** methods in **X509CertImpl** class.
I'm wondering how necessary this fix is. These are internal classes, only called inside JDK, where some pre-conditions are always met. Unless someone explicitly calls `x509Certimpl.getInfo().setExtensions(null)` (as done in the test), it seems like both the `info` and `extensions` fields should never be null.
If you’re concerned about misuse of these methods leading to potential future issues, consider adding comments to clarify their expected usage. You could also include `assert` statements or `Objects.requireNonNull` calls to enforce these preconditions.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23315#issuecomment-2640005099
More information about the security-dev
mailing list