Re: RFR: 8280494: (D)TLS signature schemes [v13]
On Mon, 7 Feb 2022 22:56:45 GMT, Xue-Lei Andrew Fan wrote:
>> Sorry, you will have to bear with me as I am still not sure how it works - I
>> want to know who wins, the API or the properties, if both are set and I
>> can't find where it answers that above. Maybe I need to read the code. Are
>> you maybe saying that this method returns the value of the system properties
>> if they are set?
>
>> "If set, these properties will override the signature schemes returned by
>> this method."
>
> If I understand your ideas correctly, the behavior should be "the returned
> value of this method will override these properties", except the case if the
> returned value is null.
>
> But let me trying if I could understand the spec by myself.
>
> For the getSignatureSchemes() method's spec:
>
> * If the returned array is {@code null}, then the underlying
> * provider-specific default signature schemes will be used over the
> * SSL/TLS/DTLS connections.
>
> With the spec above, the API getSignatureSchemes() returns null, and the
> provider-specific default signature schemes will be used for the connection.
> As the properties could be used to customize the default signature schemes,
> the properties wins in this situation for your question. However, I would
> like to express that the default signature schemes win (See bellow about why
> I don't want to use the properties).
>
>
> * If the returned array is empty (zero-length), then the signature scheme
> * negotiation mechanism is turned off for SSL/TLS/DTLS protocols, and
> * the connections may not be able to be established if the negotiation
> * mechanism is required by a certain SSL/TLS/DTLS protocol.
>
> With the spec above, the API getSignatureSchemes() returns empty array, and
> no signature schemes will be used. The API wins, the default signature
> schemes are not used.
>
>
> *
> * If the returned array is not {@code null} or empty (zero-length),
> * then the signature schemes in the returned array will be used over
> * the SSL/TLS/DTLS connections.
>
> With the spec above, the API getSignatureSchemes() returns non-null and
> non-empty, and the returned array will be used. The API wins, the default
> signature schemes are not used.
>
>
> * @implNote
> * Note that the underlying provider may define the default signature
> * schemes for each SSL/TLS/DTLS connection. Applications may also use
> * the {@systemProperty jdk.tls.client.SignatureSchemes} and/or
> * {@systemProperty jdk.tls.server.SignatureSchemes} system properties to
> * customize the provider-specific default signature schemes.
>
> With the spec above, I could understand what are the default signature
> schemes, and how they could be customized with properties.
>
> So back to my question above, why I don't want to use the properties, and use
> the default signature schemes instead? We need to take care of three things:
> the properties, the API set values and the provider default values. If I use
> the properties values, I would have to describe the provider default values
> as well. As the properties values are used to set the provider default
> values, it should be sufficient to use the provider default values for the
> description in this context.
>
> What if both the properties and the APIs are set? The properties are used to
> set the provider default values, and the properties will change the provider
> default values. So we only need to consider the provider default values and
> the API, then I think we can refer to the 3 spec sections before the
> @implNote tag above. Simply, if both set, the API wins.
>
> Similar to the set method.
>
> Please let me know if you are on the same page as well. I am open to make
> more changes if the spec is still not clear.
> Are you maybe saying that this method returns the value of the system
> properties if they are set?
The return value of this method depends on the following specs:
* If the {@link #setSignatureSchemes} method has not been called, this
* method should return the default signature schemes for connection
* populated objects, or {@code null} for pre-populated objects.
```
If the setSignatureSchemes() method has been called, the get method will return
the set values. I may miss this spec. But I think it is a common sense that
the getter method returns the values set with setter method. Let me know if
you would like to have an explicit clarification in the spec.
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v2]
On Mon, 7 Feb 2022 20:16:43 GMT, Lance Andersen wrote: >> If you are pretty sure the only other case are as above, I wonder if a >> simpler fix would be to change `verifiableEntry()` to check for these null >> cases and throw a `ZipException` which will get directly propagated by >> `getInputStream()`? > > I can certainly throw a ZipException from `verifiableEntry`. I am a bit > reluctant to not catch any other `Exception` and then throw a `ZipException` > from `getInputStream()` as it is certainly possible of encountering some > other issue due to some stray value in the CEN. > > So I will update `verifiableEntry` to validate `ZipEntry` and` > ZipEntry::getName()` potential issues Per an offline-discussion with Sean, I narrowed the Exception checking to JarFile::verifiableEntry. - PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v2]
> Hi all, > > Please review the attached patch to address > > - That JarFile::getInputStream did not check for a null ZipEntry passed as a > parameter > - Have Zip/JarFile::getInputStream throw a ZipException in the event that an > unexpected exception occurs > > Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar > test runs > > Best > Lance Lance Andersen has updated the pull request incrementally with one additional commit since the last revision: Reduce Exception checking to JarFile::verifiableEntry - Changes: - all: https://git.openjdk.java.net/jdk/pull/7348/files - new: https://git.openjdk.java.net/jdk/pull/7348/files/b50ebf05..6c75384a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7348&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7348&range=00-01 Stats: 162 lines in 3 files changed: 90 ins; 21 del; 51 mod Patch: https://git.openjdk.java.net/jdk/pull/7348.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7348/head:pull/7348 PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280494: (D)TLS signature schemes [v13]
On Mon, 7 Feb 2022 22:18:03 GMT, Sean Mullan wrote:
>> I think lines 714-816/723-725 describe the behavior already.
>>
>> I was hesitate to use "override", as the System Property values and the
>> default signature schemes are not actually overrode. The default signature
>> schemes are still there, and they are not just used for this specific
>> connection, when the connection use the non-default values.
>>
>> It might be something like, "If the returned array of this method is not
>> {@code null} or empty, the default signature schemes are not used, and
>> signature schemes in the returned array of this method will be used
>> instead". But I think it is a duplicate of lines 714-816/723-725 .
>
> Sorry, you will have to bear with me as I am still not sure how it works - I
> want to know who wins, the API or the properties, if both are set and I can't
> find where it answers that above. Maybe I need to read the code. Are you
> maybe saying that this method returns the value of the system properties if
> they are set?
> "If set, these properties will override the signature schemes returned by
> this method."
If I understand your ideas correctly, the behavior should be "the returned
value of this method will override these properties", except the case if the
returned value is null.
But let me trying if I could understand the spec by myself.
For the getSignatureSchemes() method's spec:
* If the returned array is {@code null}, then the underlying
* provider-specific default signature schemes will be used over the
* SSL/TLS/DTLS connections.
With the spec above, the API getSignatureSchemes() returns null, and the
provider-specific default signature schemes will be used for the connection.
As the properties could be used to customize the default signature schemes, the
properties wins in this situation for your question. However, I would like to
express that the default signature schemes win (See bellow about why I don't
want to use the properties).
* If the returned array is empty (zero-length), then the signature scheme
* negotiation mechanism is turned off for SSL/TLS/DTLS protocols, and
* the connections may not be able to be established if the negotiation
* mechanism is required by a certain SSL/TLS/DTLS protocol.
With the spec above, the API getSignatureSchemes() returns empty array, and no
signature schemes will be used. The API wins, the default signature schemes
are not used.
*
* If the returned array is not {@code null} or empty (zero-length),
* then the signature schemes in the returned array will be used over
* the SSL/TLS/DTLS connections.
With the spec above, the API getSignatureSchemes() returns non-null and
non-empty, and the returned array will be used. The API wins, the default
signature schemes are not used.
* @implNote
* Note that the underlying provider may define the default signature
* schemes for each SSL/TLS/DTLS connection. Applications may also use
* the {@systemProperty jdk.tls.client.SignatureSchemes} and/or
* {@systemProperty jdk.tls.server.SignatureSchemes} system properties to
* customize the provider-specific default signature schemes.
With the spec above, I could understand what are the default signature schemes,
and how they could be customized with properties.
So back to my question above, why I don't want to use the properties, and use
the default signature schemes instead? We need to take care of three things:
the properties, the API set values and the provider default values. If I use
the properties values, I would have to describe the provider default values as
well. As the properties values are used to set the provider default values, it
should be sufficient to use the provider default values for the description in
this context.
What if both the properties and the APIs are set? The properties are used to
set the provider default values, and the properties will change the provider
default values. So we only need to consider the provider default values and
the API, then I think we can refer to the 3 spec sections before the @implNote
tag above. Simply, if both set, the API wins.
Similar to the set method.
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v13]
On Mon, 7 Feb 2022 22:00:21 GMT, Xue-Lei Andrew Fan wrote:
>> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 744:
>>
>>> 742: * the {@systemProperty jdk.tls.client.SignatureSchemes} and/or
>>> 743: * {@systemProperty jdk.tls.server.SignatureSchemes} system
>>> properties to
>>> 744: * customize the provider-specific default signature schemes.
>>
>> This still doesn't say if the properties override the API. I would suggest
>> adding a sentence: "If set, these properties will override the signature
>> schemes returned by this method."
>>
>> Similar comment in `setSignatureSchemes`.
>
> I think lines 714-816/723-725 describe the behavior already.
>
> I was hesitate to use "override", as the System Property values and the
> default signature schemes are not actually overrode. The default signature
> schemes are still there, and they are not just used for this specific
> connection, when the connection use the non-default values.
>
> It might be something like, "If the returned array of this method is not
> {@code null} or empty, the default signature schemes are not used, and
> signature schemes in the returned array of this method will be used instead".
> But I think it is a duplicate of lines 714-816/723-725 .
Sorry, you will have to bear with me as I am still not sure how it works - I
want to know who wins, the API or the properties, if both are set and I can't
find where it answers that above. Maybe I need to read the code. Are you maybe
saying that this method returns the value of the system properties if they are
set?
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v13]
On Mon, 7 Feb 2022 19:59:32 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> correct null tags
>
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 47:
>
>> 45: *
>> 46: * SSLParameters can be created via the constructors in this class.
>> 47: * Objects can also be obtained using the {@code getSSLParameters()}
>
> Since you introduce the terms "pre-populated" and "connection populated" in
> the new methods, I think it would be useful to describe them up front in the
> summary, ex:
>
> `{@code SSLParameter} objects can be created via the constructors in this
> class, and can be described as pre-populated objects. {@code SSLParameter}
> objects can also be obtained using the ... , and can be
> described as connection populated objects."
Nice! It is something I want to add for quite a while, but never get a chance.
The suggested description is really simple and great!
Updated!
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 747:
>
>> 745: *
>> 746: * @return an array of signature scheme {@code Strings} or {@code
>> null} if
>> 747: * none have been set. For non-null returns, this method
>> willu
>
> Typo, s/willu/will/
Thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v14]
> This update is to support signature schemes customization for individual > (D)TLS connection. Please review the CSR as well: > CSR: https://bugs.openjdk.java.net/browse/JDK-8280495 > RFE: https://bugs.openjdk.java.net/browse/JDK-8280494 > Release-note: https://bugs.openjdk.java.net/browse/JDK-8281290 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: Update class summary - Changes: - all: https://git.openjdk.java.net/jdk/pull/7252/files - new: https://git.openjdk.java.net/jdk/pull/7252/files/be947693..8302c592 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=12-13 Stats: 6 lines in 1 file changed: 1 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7252.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7252/head:pull/7252 PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v13]
On Mon, 7 Feb 2022 19:51:28 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> correct null tags
>
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 744:
>
>> 742: * the {@systemProperty jdk.tls.client.SignatureSchemes} and/or
>> 743: * {@systemProperty jdk.tls.server.SignatureSchemes} system
>> properties to
>> 744: * customize the provider-specific default signature schemes.
>
> This still doesn't say if the properties override the API. I would suggest
> adding a sentence: "If set, these properties will override the signature
> schemes returned by this method."
>
> Similar comment in `setSignatureSchemes`.
I think lines 714-816/723-725 describe the behavior already.
I was hesitate to use "override", as the System Property values and the default
signature schemes are not actually overrode. The default signature schemes are
still there, and they are not just used for this specific connection, when the
connection use the non-default values.
It might be something like, "If the returned array of this method is not {@code
null} or empty, the default signature schemes are not used, and signature
schemes in the returned array of this method will be used instead". But I
think it is a duplicate of lines 714-816/723-725 .
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Mon, 7 Feb 2022 18:44:10 GMT, Sean Mullan wrote: >> Looking at this a bit more, it looks like `JariFile::initializeVerifier` is >> the only place currently in `JarFile` that could throw a `JarException` and >> that method could be called from `JarFile::getInputStream` >> >> As `verifiableEntry` is only called from `JarFile::getInputStream `and this >> change validates the `ZipEntry` for being null, which is should, the only >> other possible error would be in the unlikely event. `ZipEntry` was >> subclassed passed into `JarFile::getInputStream` resulting in the call to >> `ZipEntry::getName` returning null(in which case `ZipFile::getEntry` will >> return a `NullPointerException`) or the name being invalid resulting once >> again in a possible null value for the returned `ZipEntry` from >> `JarFile::getJarEntry` (which calls `ZipFile::getEntry`) >> >> >> I am still leaning towards a `ZipException`, not a `JarException`, thrown >> from `verifiableEntry`. >> >> That being said, I will go with whatever the consensus is. > > If you are pretty sure the only other case are as above, I wonder if a > simpler fix would be to change `verifiableEntry()` to check for these null > cases and throw a `ZipException` which will get directly propagated by > `getInputStream()`? I can certainly throw a ZipException from `verifiableEntry`. I am a bit reluctant to not catch any other `Exception` and then throw a `ZipException` from `getInputStream()` as it is certainly possible of encountering some other issue due to some stray value in the CEN. So I will update `verifiableEntry` to validate `ZipEntry` and` ZipEntry::getName()` potential issues - PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280494: (D)TLS signature schemes [v13]
On Fri, 4 Feb 2022 20:58:46 GMT, Xue-Lei Andrew Fan wrote:
>> This update is to support signature schemes customization for individual
>> (D)TLS connection. Please review the CSR as well:
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8280495
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8280494
>> Release-note: https://bugs.openjdk.java.net/browse/JDK-8281290
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> correct null tags
Changes requested by mullan (Reviewer).
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 47:
> 45: *
> 46: * SSLParameters can be created via the constructors in this class.
> 47: * Objects can also be obtained using the {@code getSSLParameters()}
Since you introduce the terms "pre-populated" and "connection populated" in the
new methods, I think it would be useful to describe them up front in the
summary, ex:
`{@code SSLParameter} objects can be created via the constructors in this
class, and can be described as pre-populated objects. {@code SSLParameter}
objects can also be obtained using the ... , and can be
described as connection populated objects."
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 744:
> 742: * the {@systemProperty jdk.tls.client.SignatureSchemes} and/or
> 743: * {@systemProperty jdk.tls.server.SignatureSchemes} system
> properties to
> 744: * customize the provider-specific default signature schemes.
This still doesn't say if the properties override the API. I would suggest
adding a sentence: "If set, these properties will override the signature
schemes returned by this method."
Similar comment in `setSignatureSchemes`.
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 747:
> 745: *
> 746: * @return an array of signature scheme {@code Strings} or {@code
> null} if
> 747: * none have been set. For non-null returns, this method
> willu
Typo, s/willu/will/
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Mon, 7 Feb 2022 16:52:07 GMT, Lance Andersen wrote: >> `JarException` is a subclass of `ZipException` though, so I think this would >> be ok to throw and still be compliant with the specification. > > Looking at this a bit more, it looks like `JariFile::initializeVerifier` is > the only place currently in `JarFile` that could throw a `JarException` and > that method could be called from `JarFile::getInputStream` > > As `verifiableEntry` is only called from `JarFile::getInputStream `and this > change validates the `ZipEntry` for being null, which is should, the only > other possible error would be in the unlikely event. `ZipEntry` was > subclassed passed into `JarFile::getInputStream` resulting in the call to > `ZipEntry::getName` returning null(in which case `ZipFile::getEntry` will > return a `NullPointerException`) or the name being invalid resulting once > again in a possible null value for the returned `ZipEntry` from > `JarFile::getJarEntry` (which calls `ZipFile::getEntry`) > > > I am still leaning towards a `ZipException`, not a `JarException`, thrown > from `verifiableEntry`. > > That being said, I will go with whatever the consensus is. If you are pretty sure the only other case are as above, I wonder if a simpler fix would be to change `verifiableEntry()` to check for these null cases and throw a `ZipException` which will get directly propagated by `getInputStream()`? - PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Mon, 7 Feb 2022 15:16:43 GMT, Sean Mullan wrote: >> JarFile::getInputStream. mentions ZipException but not JarException which is >> why I chose this. If we change this to JarException, I would need to update >> the javadoc and create a CSR. >> >> Please let me know your preference > > `JarException` is a subclass of `ZipException` though, so I think this would > be ok to throw and still be compliant with the specification. Looking at this a bit more, it looks like `JariFile::initializeVerifier` is the only place currently in `JarFile` that could throw a `JarException` and that method could be called from `JarFile::getInputStream` As `verifiableEntry` is only called from `JarFile::getInputStream `and this change validates the `ZipEntry` for being null, which is should, the only other possible error would be in the unlikely event. `ZipEntry` was subclassed passed into `JarFile::getInputStream` resulting in the call to `ZipEntry::getName` returning null(in which case `ZipFile::getEntry` will return a `NullPointerException`) or the name being invalid resulting once again in a possible null value for the returned `ZipEntry` from `JarFile::getJarEntry` (which calls `ZipFile::getEntry`) I am still leaning towards a `ZipException`, not a `JarException`, thrown from `verifiableEntry`. That being said, I will go with whatever the consensus is. - PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Fri, 4 Feb 2022 15:19:11 GMT, Lance Andersen wrote:
>> src/java.base/share/classes/java/util/jar/JarFile.java line 866:
>>
>>> 864: } catch (Exception e2) {
>>> 865: // Any other Exception should be a ZipException
>>> 866: throw (ZipException) new ZipException("Zip file format
>>> error").initCause(e2);
>>
>> If there is ZIP format error then I would expect ZipException or the more
>> general IOException is already thrown. So I think this is catching other
>> cases, maybe broken manifests or signed JAR files, in which case a
>> JarException may be better.
>
> JarFile::getInputStream. mentions ZipException but not JarException which is
> why I chose this. If we change this to JarException, I would need to update
> the javadoc and create a CSR.
>
> Please let me know your preference
`JarException` is a subclass of `ZipException` though, so I think this would be
ok to throw and still be compliant with the specification.
-
PR: https://git.openjdk.java.net/jdk/pull/7348
Integrated: 8281175: Add a -providerPath option to jarsigner
On Thu, 3 Feb 2022 17:12:05 GMT, Weijun Wang wrote: > Add the `-providerPath` option to jarsigner to be consistent with keytool. This pull request has now been integrated. Changeset: 2ed1f4cf Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/2ed1f4cf32b1cef4ccb129d622f9368c3469d1d4 Stats: 40 lines in 4 files changed: 22 ins; 7 del; 11 mod 8281175: Add a -providerPath option to jarsigner Reviewed-by: xuelei, hchao - PR: https://git.openjdk.java.net/jdk/pull/7338
Integrated: 8280890: Cannot use '-Djava.system.class.loader' with class loader in signed JAR
On Tue, 1 Feb 2022 21:54:29 GMT, Sean Mullan wrote: > This fixes a bootstrapping issue if a custom system class loader is set with > the `-Djava.system.class.loader` option and the custom class loader is inside > a signed JAR. In order to load the custom class loader, the runtime must > verify the signed JAR first, and the algorithm constraint code tries to load > a `Locale` provider using a `ServiceLoader` before the class loader is set, > and this causes a `ServiceConfigurationError`. > > The fix removes a dependency from the security algorithm "denyAfter" > constraint parsing code on the `Calendar` API which uses a `ServiceLoader` > for gathering default locale information. Instead the `ZonedDateTime` API is > now used, which simplifies the code and removes some unnecessary code from > `keytool` as well. This pull request has now been integrated. Changeset: a0f6f240 Author:Sean Mullan URL: https://git.openjdk.java.net/jdk/commit/a0f6f2409ea61ff9ed9dc2e2b46e309c751d456d Stats: 186 lines in 4 files changed: 142 ins; 28 del; 16 mod 8280890: Cannot use '-Djava.system.class.loader' with class loader in signed JAR Reviewed-by: weijun, hchao - PR: https://git.openjdk.java.net/jdk/pull/7316
