Re: RFR: 8347946: Add API note that caller should validate/trust signers to the getCertificates and getCodeSigners methods of JarEntry and JarURLConnection
On Thu, 13 Feb 2025 16:27:03 GMT, Sean Mullan wrote:
> This change adds an API note to these methods recommending that the caller
> should perform further validation steps on the code signers that signed the
> JAR file, such as validating the code signer's certificate chain, and
> determining if the signer should be trusted. There was already a similar
> warning in the `JarFile` and `JarInputStream` class descriptions, but this
> adds a similar and more direct warning at the methods that return the code
> signer's certificates.
>
> 2 other smaller changes:
> - In `JarEntry.getCertificates`, added a recommendation to use the
> `getCodeSigners` method instead
> - Added details of the order of the returned certificates to
> `JarURLConnection.getCertificates` (copied from `JarEntry.getCertificates`)
src/java.base/share/classes/java/util/jar/JarEntry.java line 100:
> 98: * reached. Otherwise, this method will return {@code null}.
> 99: *
> 100: * It is recommended to use the {@link getCodeSigners} method
> instead,
Isn't this missing a `#` to be a valid link?
Suggestion:
* It is recommended to use the {@link #getCodeSigners} method instead,
(Please don't use the "Commit suggestion" button to not include me as author)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/23616#discussion_r1961440218
Re: RFR: 8347946: Add API note that caller should validate/trust signers to the getCertificates and getCodeSigners methods of JarEntry and JarURLConnection
On Wed, 19 Feb 2025 10:43:06 GMT, Marcono1234 wrote:
>> This change adds an API note to these methods recommending that the caller
>> should perform further validation steps on the code signers that signed the
>> JAR file, such as validating the code signer's certificate chain, and
>> determining if the signer should be trusted. There was already a similar
>> warning in the `JarFile` and `JarInputStream` class descriptions, but this
>> adds a similar and more direct warning at the methods that return the code
>> signer's certificates.
>>
>> 2 other smaller changes:
>> - In `JarEntry.getCertificates`, added a recommendation to use the
>> `getCodeSigners` method instead
>> - Added details of the order of the returned certificates to
>> `JarURLConnection.getCertificates` (copied from `JarEntry.getCertificates`)
>
> src/java.base/share/classes/java/util/jar/JarEntry.java line 100:
>
>> 98: * reached. Otherwise, this method will return {@code null}.
>> 99: *
>> 100: * It is recommended to use the {@link getCodeSigners} method
>> instead,
>
> Isn't this missing a `#` to be a valid link?
> Suggestion:
>
> * It is recommended to use the {@link #getCodeSigners} method instead,
>
>
> (Please don't use the "Commit suggestion" button to not include me as author)
I had a similar reaction when I first saw it. So I checked the javadoc
specification the other day and it states
https://docs.oracle.com/en/java/javase/23/docs/specs/javadoc/doc-comment-spec.html#references:
The most general form of a reference is:
module/package.class#member
This form is used by the see, link and linkplain tags.
Leading components can be omitted when they can be inferred from the
surrounding context. Trailing components can be omitted when they are not
required.
... When the reference is to a member of the same class as that containing the
documentation comment, all parts of the reference up to and including the # may
be omitted, although the '#' may be retained for clarity.
So, syntactically, it's fine in the current form.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/23616#discussion_r1961450370
Integrated: 8347946: Add API note that caller should validate/trust signers to the getCertificates and getCodeSigners methods of JarEntry and JarURLConnection
On Thu, 13 Feb 2025 16:27:03 GMT, Sean Mullan wrote: > This change adds an API note to these methods recommending that the caller > should perform further validation steps on the code signers that signed the > JAR file, such as validating the code signer's certificate chain, and > determining if the signer should be trusted. There was already a similar > warning in the `JarFile` and `JarInputStream` class descriptions, but this > adds a similar and more direct warning at the methods that return the code > signer's certificates. > > 2 other smaller changes: > - In `JarEntry.getCertificates`, added a recommendation to use the > `getCodeSigners` method instead > - Added details of the order of the returned certificates to > `JarURLConnection.getCertificates` (copied from `JarEntry.getCertificates`) This pull request has now been integrated. Changeset: 577ff98a Author:Sean Mullan URL: https://git.openjdk.org/jdk/commit/577ff98a6733a99ea49510f15d631beff39c34a5 Stats: 38 lines in 3 files changed: 32 ins; 0 del; 6 mod 8347946: Add API note that caller should validate/trust signers to the getCertificates and getCodeSigners methods of JarEntry and JarURLConnection Reviewed-by: lancea, jpai - PR: https://git.openjdk.org/jdk/pull/23616
RFR: 8325766: Review seclibs tests for cert expiry
This PR updates the CertificateBuilder with a new method that creates a new instance with common fields (subject name, public key, serial number, validity, and key uses) filled-in. One test, IPIdentities.java, is updated to show how the method can be used to create various certificates. I attached screenshots that compare the old hard-coded certificates (left) with the new generated certificates.    - Commit messages: - 8325766: Review seclibs tests for cert expiry Changes: https://git.openjdk.org/jdk/pull/23700/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23700&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8325766 Stats: 779 lines in 3 files changed: 154 ins; 599 del; 26 mod Patch: https://git.openjdk.org/jdk/pull/23700.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/23700/head:pull/23700 PR: https://git.openjdk.org/jdk/pull/23700
Withdrawn: 8227493: Return a more useful error message from lookupAllHostAddr if getaddrinfo results in EAI_SYSTEM error
On Mon, 2 Dec 2024 13:53:00 GMT, Jaikiran Pai wrote: > Can I please get a review of this minor enhancement to the error text that is > reported if the `getaddrinfo()` native call returns the `EAI_SYSTEM` error? > This addresses https://bugs.openjdk.org/browse/JDK-8227493. > > The `java.net.InetAddress` class, in its implementation for resolving > addresses for a host name, calls the `getaddrinfo()` native call on *nix > platforms. If `getaddrinfo()` returns an error then we use the > `gai_strerror()` native call to convert the error number into an error string > that is then propagated to the application. Among other errors, the > `getaddrinfo()` is specified to return the error code `EAI_SYSTEM` which as > per its documentation represents > >> EAI_SYSTEMsystem error returned in errno > > So calling `gai_strerror()` merely returns a generic "System error" text. The > real underlying error is present in the `errno` and that has more useful > information. > > The commit in this PR checks the error for `EAI_SYSTEM` and if it matches > then it additionally gets the error text corresponding to `errno`. So the > error text that gets propagated will now be "System error: Illegal byte > sequence", assuming `EILSEQ` was the underlying `errno` for the `getaddrinfo` > call (my use of `EILSEQ` in this example is arbitrary and it's merely to show > what the error text will look like after this change). > > Given the nature of this change no new test has been introduced. Existing > tests in tier1, tier2 and tier3 continue to pass with this change. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/22484
Re: RFR: 8349705: java.net.URI.scanIPv4Address throws unnecessary URISyntaxException
On Mon, 10 Feb 2025 10:53:12 GMT, Daniel Fuchs wrote:
>> java.net.URI.scanIPv4Address is a private method, it is only called by
>> java.net.URI.takeIPv4Address and java.net.URI.parseIPv4Address, the
>> URISyntaxException("Malformed IPv4 address") is not necessary, returning -1
>> should be good. In one of our systems, we noticed the exception consumes
>> ~0.3% CPU.
>>
>> Additional test:
>> - [x] make test TEST=jdk/java/net
>> - [x] all tier2 tests(except DirectIOTest.java which is a [known
>> issue](https://bugs.openjdk.org/browse/JDK-8333783))
>
> What tests did you run against those changes? You will need to run at least
> tier2 tests.
@dfuch
Hi Daniel, thank you for the review of the changes, do you have other concerns
on the PR?
-
PR Comment: https://git.openjdk.org/jdk/pull/23538#issuecomment-2670172037
