Re: RFR: 8284893: Fix typos in java.base [v4]

[Sean Mullan]

On Tue, 19 Apr 2022 16:50:12 GMT, Magnus Ihse Bursie  wrote:

>> I ran `codespell` on the `src/java.base` directory, and accepted those 
>> changes where it indeed discovered real typos.
>> 
>> (Due to false positives this can unfortunately not be run automatically) 
>> 
>> The majority of fixes are in comments. A handful is in strings, one in a 
>> local variable name, and a couple in parameter declarations.
>> 
>> Annoyingly, there are several instances of "childs" (instead of "children") 
>> in the source code, but they were not local and I dared not change them. 
>> Someone braver than me might take a stab at it, perhaps..
>
> Magnus Ihse Bursie has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Update Oracle copyrights
>  - Also revert changes in ASM (3rd party code)

Marked as reviewed by mullan (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8250

Re: RFR: 8186958: Need method to create pre-sized HashMap [v21]

[Sean Mullan]

On Thu, 14 Apr 2022 20:16:38 GMT, Sean Mullan  wrote:

>>> Are the changes necessary for this part?
>> 
>> @seanjmullan no, they are just performance refinement.
>> 
>> If you really that wanna 100% sync ,
>> 
>> I can use the old 1.8 api to migrate that part, and make a mirror pr to that 
>> part of https://github.com/apache/santuario-xml-security-java
>> 
>> Is this solution acceptable then?
>
>> > Are the changes necessary for this part?
>> 
>> @seanjmullan no, they are just performance refinement.
>> 
>> If you really that wanna 100% sync ,
>> 
>> I can use the old 1.8 api to migrate that part, and make a mirror pr to that 
>> part of https://github.com/apache/santuario-xml-security-java
>> 
>> Is this solution acceptable then?
> 
> Yes, that would be preferred. Thanks!

> I'd like to see a confirmation from @seanjmullan to make sure the issues with 
> Santuario are resolved satisfactorily. Other than that I think it's pretty 
> well covered. I've already run these changes through our internal test system 
> and they look fine. I'll do a final recheck and let you know.

I am fine with this being integrated. @XenoAmess already [submitted a PR to the 
Santuario 
Project](https://github.com/apache/santuario-xml-security-java/pull/64) using 
the existing `HashMap` constructor which I have approved. So the code will be 
in sync the next time we upgrade the JDK to a newer version of Santuario.

-

PR: https://git.openjdk.java.net/jdk/pull/7928

Re: RFR: 8284893: Fix typos in java.base

[Sean Mullan]

On Thu, 14 Apr 2022 20:16:21 GMT, Bradford Wetmore  wrote:

>> I ran `codespell` on the `src/java.base` directory, and accepted those 
>> changes where it indeed discovered real typos.
>> 
>> (Due to false positives this can unfortunately not be run automatically) 
>> 
>> The majority of fixes are in comments. A handful is in strings, one in a 
>> local variable name, and a couple in parameter declarations.
>> 
>> Annoyingly, there are several instances of "childs" (instead of "children") 
>> in the source code, but they were not local and I dared not change them. 
>> Someone braver than me might take a stab at it, perhaps..
>
> src/java.base/share/classes/sun/security/provider/certpath/AdjacencyList.java 
> line 128:
> 
>> 126: // Each time this method is called, we're examining a new list
>> 127: // from the global list. So, we have to start by getting the 
>> list
>> 128: // that contains the set of Vertices we're considering.
> 
> The class being affected is a Vertex, so either change to vertices, or leave 
> as is...

Agree, suggest changing it to "vertices".

-

PR: https://git.openjdk.java.net/jdk/pull/8250

Re: RFR: 8186958: Need method to create pre-sized HashMap [v21]

[Sean Mullan]

On Thu, 14 Apr 2022 20:11:37 GMT, XenoAmess  wrote:

> > Are the changes necessary for this part?
> 
> @seanjmullan no, they are just performance refinement.
> 
> If you really that wanna 100% sync ,
> 
> I can use the old 1.8 api to migrate that part, and make a mirror pr to that 
> part of https://github.com/apache/santuario-xml-security-java
> 
> Is this solution acceptable then?

Yes, that would be preferred. Thanks!

-

PR: https://git.openjdk.java.net/jdk/pull/7928

Re: RFR: 8186958: Need method to create pre-sized HashMap [v21]

[Sean Mullan]

On Thu, 14 Apr 2022 18:10:28 GMT, XenoAmess  wrote:

>> 8186958: Need method to create pre-sized HashMap
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add `@LastModified: Apr 2022` to DocumentCache

Right, we generally try to avoid making too many changes to the implementation 
code in the java.xml.crypto module in order to stay consistent with Apache 
Santuario. They also would not accept this change because it is a new API and 
they need to run on older releases. I haven't had time yet to understand this 
Enhancement, but are the changes necessary for this part?

-

PR: https://git.openjdk.java.net/jdk/pull/7928

Re: RFR: 8281561: Disable http DIGEST mechanism with MD5 by default [v6]

[Sean Mullan]

On Tue, 15 Mar 2022 16:00:41 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> Could I get the following change reviewed please, which is to disable the 
>> MD5 message digest algorithm by default in the HTTP Digest authentication 
>> mechanism? The algorithm can be opted into by setting a new system property 
>> "http.auth.digest.reEnabledAlgs" to include the value MD5. The change also 
>> updates the Digest authentication implementation to use some of the more 
>> secure features defined in RFC7616, such as username hashing and additional 
>> digest algorithms like SHA256 and SHA512-256.
>> 
>> - Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   made disabledAlgorithms immutable

Suggest changing the bug title to reflect that SHA-1 is now also disabled by 
default, i.e. "Disable http DIGEST mechanisms with MD5 or SHA-1 by default".

-

PR: https://git.openjdk.java.net/jdk/pull/7688

Re: RFR: 8281561: Disable http DIGEST mechanism with MD5 by default [v2]

[Sean Mullan]

On Thu, 10 Mar 2022 16:43:23 GMT, Michael McMahon  wrote:

>> src/java.base/share/conf/security/java.security line 711:
>> 
>>> 709: # separated list of algorithms to be allowed.
>>> 710: #
>>> 711: jdk.httpdigest.defaultDisabledAlgorithms = MD5, MD-5, SHA1, SHA-1
>> 
>> I haven't seen people using "MD-5". It's also not an alias of "MD5" in our 
>> own security providers. On the other hand, we support both "SHA1" and "SHA" 
>> (and its OID) as aliases of "SHA-1". So, either we list all these aliases 
>> here, or we only put the standard names here and "canonicalize" the name 
>> when we see one. `sun.security.util.KnownOIDs.findMatch("SHA-1").stdName()` 
>> can be used.
>
> The aliases are a PITA. It's a shame there isn't support in the public API to 
> canonicalize the names. But, what I will do is canonicalize the incoming 
> strings from the HTTP server and then we only have to list canonical names in 
> the properties.

I think it would be more consistent if the namespace of the two properties were 
the same. For example, consider renaming the security property to 
"http.auth.digest.disabledAlgorithms". I don't think the "default" word adds 
anything meaningful.

To avoid any confusion, I think you may also want to add a sentence describing 
that these algorithms are only disabled when used over HTTP, and not HTTPS.  
Because I think technically are they still called HTTP digest authentication 
schemes even if used over HTTPS.

-

PR: https://git.openjdk.java.net/jdk/pull/7688

Re: RFR: 8280494: (D)TLS signature schemes [v20]

[Sean Mullan]

On Wed, 9 Mar 2022 08:25:48 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 test case issues and more

Marked as reviewed by mullan (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v19]

[Sean Mullan]

On Sun, 6 Mar 2022 05:40:59 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:
> 
>   add test for DTLS

Changes requested by mullan (Reviewer).

src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 501:

> 499: 
> 500: // Note that if the System Property value is not defined (JDK
> 501: // default value) or empty, the provider-specific default is 
> used.

I think you can remove this comment as it is repeated on lines 507-508 (and 
makes more sense there).

src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 564:

> 562: String[] signatureSchemes) {
> 563: 
> 564: if (signatureSchemes == null ||  signatureSchemes.length == 0) {

Nit: remove extra space after `||`.

src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 576:

> 574: SSLLogger.finest(
> 575: "Ignore the signature algorithm (" + ss
> 576:   + "), unsupported or unavailable");

Should this message be more consistent with the message in 
`getCustomizedSignatureScheme`?: "The current installed providers do not 
support signature scheme: " + schemeName

test/jdk/javax/net/ssl/DTLS/DTLSSignatureSchemes.java line 125:

> 123: testCase.runTest(testCase);
> 124: if (exceptionExpected) {
> 125: throw new RuntimeException("Unexpected success!");

The catch block on line 127 will end up catching this exception and swallowing 
it, and the test will incorrectly pass.

test/jdk/javax/net/ssl/SSLParameters/SignatureSchemes.java line 81:

> 79: super.runClientApplication(sslSocket);
> 80: if (exceptionExpected) {
> 81: throw new RuntimeException("Unexpected success!");

The catch block on line 83 will end up catching this exception and swallowing 
it, and the test will incorrectly pass.

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8281561: Disable http DIGEST mechanism with MD5 by default

[Sean Mullan]

On Fri, 4 Mar 2022 09:37:21 GMT, Michael McMahon  wrote:

> Hi,
> 
> Could I get the following change reviewed please, which is to disable the MD5 
> message digest algorithm by default in the HTTP Digest authentication 
> mechanism? The algorithm can be opted into by setting a new system property 
> "http.auth.digest.reEnabledAlgs" to include the value MD5. The change also 
> updates the Digest authentication implementation to use some of the more 
> secure features defined in RFC7616, such as username hashing and additional 
> digest algorithms like SHA256 and SHA512-256.
> 
> - Michael

src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java 
line 514:

> 512: if (getAuthType() == AuthCacheValue.Type.Server &&
> 513: getProtocolScheme().equals("https")) {
> 514: // HTTPS server authentication can use any algorithm

A more security conscious user may want to disable MD5 digest authentication, 
even when used over HTTPS, even though the risks are far less than when used 
over HTTP. Is there a way to do that?

-

PR: https://git.openjdk.java.net/jdk/pull/7688

Re: RFR: 8280494: (D)TLS signature schemes [v18]

[Sean Mullan]

On Thu, 17 Feb 2022 18:57:02 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:
> 
>   more spec update per CSR feedback

Since this new API is also intended to be supported for DTLS, have you added 
implementation support for that, and if so have you added a test for it?

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v15]

[Sean Mullan]

On Wed, 9 Feb 2022 18:24:56 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:
> 
>   Spec update

If you agree with my last couple of comments, I think the API looks good now 
that you can finalize the CSR. I have not really reviewed the impl code and 
tests previously and may not have time to do a thorough review although I think 
Jamil may have done that - if he is ok with the rest of the code, then I think 
you can integrate once the CSR is approved. Thanks for the patience.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 749:

> 747:  * @implNote
> 748:  * Note that the underlying provider may define the default signature
> 749:  * schemes for each SSL/TLS/DTLS connection.  Applications may also 
> use

I think you can remove the first sentence that starts with "Note ..." as you 
already talk about provider-specific defaults before this. Also @implNotes are 
usually about the JDK implementation and not any provider. In the second 
sentence I would be more specific that this applies to the SunJSSE provider 
(see changes in italics), ex: "Applications ... system properties _with the 
SunJSSE provider_ to ..."

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v13]

[Sean Mullan]

On Tue, 8 Feb 2022 23:36:05 GMT, Xue-Lei Andrew Fan  wrote:

>> Ok, I get it now, the API wins if both are set. But I could not discern that 
>> from the current text. I think it is ok to be more clear about this. I 
>> suggest adding something like the following:
>> 
>> "The set of signature schemes that will be used is determined in this order 
>> of preference: 1. explicitly set by application; 2. specified by system 
>> property; 3. specified by provider defaults. For example, setting the 
>> signature schemes in your application overrides settings specified in 
>> jdk.tls.client.SignatureSchemes or jdk.tls.server.SignatureSchemes, as well 
>> as JDK provider defaults."
>> 
>> Does that accurately capture the implementation behavior?
>
> Basically, the suggestion captures the implementation behaviors correctly.  
> To make it more accuracy, if we want to use it, we may need to consider more 
> cases:
> 1. _explicitly set by application_, with null, empty or 1+ schemes.
> 2. _specified by system property_, with unset, empty or 1+ schemes.
> 
> I think the description could be in the following logic:
> A. the system properties are used to customize the provider default schemes.
> B. If the API set it to null, use the the provider default schemes (including 
> system properties customized default schemes).
> C. If the API set it to empty, don't use any schemes.
> D. if the API set it to 1+ scheme, use the API set schemes.
> 
> With logic A, we don't have to describe both _system properties_ and the 
> _provider default schemes_ each time when we talk about the interaction 
> behaviors of them. And then we can simplify the 3 interactive factors into 2 
> factors.
> 
> Here is the general 3 interactive factors:
> 1. the API.
> 2. the System properties.
> 3. the provider default.
> 
> Here is set 1 of the 2 interactive factors:
> 1. The System properties;
> 2. the provider default.
> 
> Here is set 2 of the 2 interactive factors:
> 1. the API
> 2. the provider default.
> 
> Then, we could describe set 1 and set 2 individually.  I think it is easier 
> to understand.
> 
> Then, we could have the sections accordingly.
> For A, we have (for set 1 of the 2 interactive factors):
> 
>  * @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.
> ``` 
> 
> For B, we have (for set 2 of the 2 interactive factors):
> 
>  * If the returned array is {@code null}, then the underlying
>  * provider-specific default signature schemes will be used over the
>  * SSL/TLS/DTLS connections.
> 
> 
> For C, we have (for set 2 of the 2 interactive factors):
> 
>  * 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.
> 
> 
> For D, we have (for set 2 of the 2 interactive factors):
> 
>  * 
>  * 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.
> 
> 
> That's the current description we have.  That's, in order to explain behavior 
> of the null, empty and 1+ schemes setting, I cut the sentence into 4 sections 
> above.
> 
> I understand it could be confusing when multiple facts are involved.  It may 
> be clear if re-org the description. What do you think of the following 
> descriptions?
> 
> In the get method:
> 
>  * 
>  * 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.
>  * 
>  * The set of signature schemes that will be used over the SSL/TLS/DTLS
>  * connections is determined by the returned array of this method and the 
>  * underlying provider-specific default signature schemes.
>  * 
>  * If the returned array is {@code null}, then the underlying
>  * provider-specific default signature schemes will be used over the
>  * SSL/TLS/DTLS connections.
>  * 
>  * 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.
>  

Re: RFR: 8280494: (D)TLS signature schemes [v13]

[Sean Mullan]

On Mon, 7 Feb 2022 23:13:13 GMT, Xue-Lei Andrew Fan  wrote:

>>> "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.

Ok, I get it now, the API wins if both are set. But I could not discern that 
from the current text. I think it is ok to be more clear about this. I suggest 
adding something like the following:

"The set of signature schemes that will be used is determined in this order of 
preference: 1. explicitly set by application; 2. specified by system property; 
3. specified by provider defaults. For example, setting the signature schemes 
in your application overrides settings specified in 
jdk.tls.client.SignatureSchemes or jdk.tls.client.SignatureSchemes, as well as 
JDK provider defaults."

Does that accurately capture the implementation behavior?

Re: RFR: 8280494: (D)TLS signature schemes [v13]

[Sean Mullan]

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]

[Sean Mullan]

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: 8280494: (D)TLS signature schemes [v9]

[Sean Mullan]

On Wed, 2 Feb 2022 19:12:56 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
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   More update for the sec and impl

Changes requested by mullan (Reviewer).

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 713:

> 711:  * in this list or may not use the recommended name for a certain
> 712:  * signature scheme.
> 713:  * 

Regarding my previous comment about providers not supporting these new methods, 
I think something like the following might suffice:


@apiNote Note that a provider may not have been updated to support this method 
and in that case may return null instead of the default signature schemes for 
pre-populated objects.
@implNote The SunJSSE provider supports this method.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 735:

> 733:  * {@link #setSignatureSchemes} method has not been called, this 
> method
> 734:  * should return the default signature schemes for connection 
> populated
> 735:  * objects, or {@code null} for pre-populated objects.

I think this sentence should be part of the specification and not the 
implementation note since ideally you would want all provider implementations 
to behave this way (assuming they supported this method): 

"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."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 752:

> 750: 
> 751: /**
> 752:  * Sets the prioritized array of signature scheme names that

If the system properties are set, does it override this API? I think it is 
important to mention that here in an @implNote. I know you discuss them in 
`getSignatureSchemes` but I think a developer is more likely to want to know 
the behavior of the properties for this method especially if they will override 
this setting.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 761:

> 759:  * Names Specification.  Providers may support signature schemes not 
> defined
> 760:  * in this list or may not use the recommended name for a certain
> 761:  * signature scheme.

Did you want to say something about what the behavior should be if a provider 
does not support one of these schemes? Is it ignored, or is an exception thrown?

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 762:

> 760:  * in this list or may not use the recommended name for a certain
> 761:  * signature scheme.
> 762:  *

Regarding my previous comment about providers not supporting these new methods, 
I think something like the following might suffice:


@apiNote Note that a provider may not have been updated to support this method 
and in that case may ignore the schemes that are set.
@implNote The SunJSSE provider supports this method.

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v9]

[Sean Mullan]

On Wed, 2 Feb 2022 22:41:56 GMT, Xue-Lei Andrew Fan  wrote:

> > On a related issue, have you given any thought as to what the behavior 
> > should be if a 3rd-party JSSE provider is not updated to support these new 
> > methods? I don't know of a good way to address that since the API is not 
> > part of the provider implementation. We need a way to query what parameters 
> > a given provider supports, I think.
> 
> It's a good question. A 3rd party provider need to support to the new APIs. 
> Otherwise, the spec does not really work except to use the default values. It 
> might be overweighted if the Java SE API has to detect the implementation 
> details.

Fair point. However, I think then we have to make some changes to the 
specification wording that state that these new methods depend on support from 
the underlying provider. In other words something like 
`SSLParameters.setSignatureSchemes(new String[] {"ecdsa_secp521r1_sha512"})` 
could be silently ignored if the provider doesn't support the new method. It's 
unfortunate because a developer may miss this point or it may go undetected, 
whereas throwing an `UnsupportedOperationException` would probably be detected 
(but which won't work for this API). I do note that previous methods we have 
added to SSLParameters have probably had this same issue. But I think we should 
try to address it better with these methods. I'll suggest some wording changes.

To help reduce this potential issue, we can make sure we mention this issue in 
the release notes, and encourage 3rd party providers to add support for these 
methods when they add support for JDK 18.

Open to other ideas though.

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v9]

[Sean Mullan]

On Wed, 2 Feb 2022 19:12:56 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
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   More update for the sec and impl

On a related issue, have you given any thought as to what the behavior should 
be if a 3rd-party JSSE provider is not updated to support these new methods? I 
don't know of a good way to address that since the API is not part of the 
provider implementation. We need a way to query what parameters a given 
provider supports, I think.

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v9]

[Sean Mullan]

On Wed, 2 Feb 2022 19:12:56 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
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   More update for the sec and impl

Ok, great. I'll need another day or so to review the implementation changes.

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v2]

[Sean Mullan]

On Tue, 1 Feb 2022 06:42:30 GMT, Xue-Lei Andrew Fan  wrote:

>> Ok. You should specify what the default value of the signature schemes 
>> parameter is for this constructor as it does for the other parameters.
>
> Good catch.  Updated.

Looks good.

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v8]

[Sean Mullan]

On Tue, 1 Feb 2022 06:47:00 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
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Allow null input and return values

A few more comments on the API.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 706:

> 704:  * over the SSL/TLS/DTLS protocols.
> 705:  * 
> 706:  * Note that the standard list of signature scheme names may be 
> found in

s/may be found/are defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 710:

> 708:  * 
> "{@docRoot}/../specs/security/standard-names.html#signature-schemes">
> 709:  * Signature Schemes section of the Java Security Standard 
> Algorithm
> 710:  * Names Specification. Providers may support signature schemes not 
> found

s/found/defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 711:

> 709:  * Signature Schemes section of the Java Security Standard 
> Algorithm
> 710:  * Names Specification. Providers may support signature schemes not 
> found
> 711:  * in this list or might not use the recommended name for a certain

s/might/may/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 714:

> 712:  * signature scheme.
> 713:  * 
> 714:  * The returned array could be {@code null}, in which case the 
> underlying

s/could be/may be/

Suggest rewording as "If the returned array is {@code null}, then the 
underlying ..."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 718:

> 716:  * SSL/TLS/DTLS connections.
> 717:  * 
> 718:  * The returned array could be empty (zero-length), in which case the

Suggest rewording as "If the returned array is empty (zero-length), then the 
..."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 720:

> 718:  * The returned array could be empty (zero-length), in which case the
> 719:  * signature scheme negotiation mechanism is turned off for 
> SSL/TLS/DTLS
> 720:  * protocols, and the connections may not be able estabilished if the

s/estabilished/to be established/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 723:

> 721:  * negotiation mechanism is required by a certain SSL/TLS/DTLS 
> protocol.
> 722:  * 
> 723:  * The returned array could be neither {@code null} nor empty 
> (zero-length),

Suggest rewording as "If the returned array is not {@code null} or empty 
(zero-length), then the signature schemes .."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 730:

> 728:  * For non-null returns, this method will return a new array each 
> time it
> 729:  * is invoked. Providers should ignore unknown signature scheme names
> 730:  * while establishing the SSL/TLS/DTLS connections.

I think this condition should be part of the API specification, and not the 
implementation specification. This needs to be a required part of the API, 
otherwise applications cannot safely rely on other implementations of this 
class to return copies. I suggest moving it to the `@returns` clause.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 736:

> 734:  * schemes for each SSL/TLS/DTLS connection.  Applications may also 
> use
> 735:  * System Property, {@systemProperty jdk.tls.client.SignatureSchemes}
> 736:  * and/or {@systemProperty jdk.tls.server.SignatureSchemes}, to 
> customize

Suggest rewording as "the {@systemProperty jdk.tls.client.SignatureSchemes} 
and/or {@systemProperty jdk.tls.server.SignatureSchemes} system properties to 
customize ..."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 742:

> 740:  * objects, or {@code null} for pre-populated objects.
> 741:  *
> 742:  * @return {@code null} or an array of signature scheme {@code 
> String}s.

Suggest rephrasing as "an array of signature scheme {@code Strings} or {@null} 
if none have been set."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 757:

> 755:  * can be used over the SSL/TLS/DTLS protocols.
> 756:  * 
> 757:  * Note that the standard list of signature scheme names may be 
> found in

s/may be found/are defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 761:

> 759:  * 
> "{@docRoot}/../specs/security/standard-names.html#signature-schemes">
> 760:  * Signature Schemes section of the Java Security Standard 
> Algorithm
> 761:  * Names Specification. Providers may support signature schemes not 
> found

s/found/defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 762:

> 760:  * Signature Schemes section of the Java Security Standard 

Re: RFR: 8280494: (D)TLS signature schemes [v2]

[Sean Mullan]

On Sat, 29 Jan 2022 05:26:33 GMT, Xue-Lei Andrew Fan  wrote:

>> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 94:
>> 
>>> 92: 
>>> 93: /**
>>> 94:  * Constructs SSLParameters.
>> 
>> Would it be useful to add another ctor that takes a signature schemes array 
>> parameter?
>
> As there is increasing number of SSL parameters, applications may just use 
> the constructor with no argument, and use the set methods individually.

Ok. You should specify what the default value of the signature schemes 
parameter is for this constructor as it does for the other parameters.

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v7]

[Sean Mullan]

On Mon, 31 Jan 2022 20:24:47 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
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Rollback to use captialized S

A few more comments on the API.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 727:

> 725:  * Note that the underlying provider may define the default signature
> 726:  * schemes for each SSL/TLS/DTLS connection.  Applications may also 
> use
> 727:  * System Property, "jdk.tls.client.SignatureSchemes" and/or

Use the javadoc annotation @systemProperty when referring to the system 
properties. Look at other code for examples.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 734:

> 732:  * pre-populated objects.
> 733:  *
> 734:  * @return a non-null, possibly zero-length array of signature scheme

The other methods that return arrays like `getCipherSuites` and `getProtocols` 
return `null` if none have been set. While one could argue whether returning 
`null` or an empty array is better, there is already a precedent in this API of 
returning `null`, and with this API programmers will have to check for two 
different ways of checking for parameters that are not set. I'm not really sure 
if the inconsistency is worth it.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 765:

> 763:  * System Property, "jdk.tls.client.SignatureSchemes" and/or
> 764:  * "jdk.tls.server.SignatureSchemes", to customize the 
> provider-specific
> 765:  * default signature schemes. If the {@code signatureSchemes} array 
> is

In this sentence does `signatureSchemes` mean the "*.SignatureSchemes" property 
or the `signatureSchemes` parameter? I don't think I understand this sentence. 
I think you should explain when the system property overrides the parameter in 
this API, and/or vice-versa.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 774:

> 772:  *is empty (zero-length), the provider-specific default 
> signature
> 773:  *schemes will be used for the SSL/TLS/DTLS connection.
> 774:  * @throws IllegalArgumentException if signatureSchemes is null, or 
> if

The other methods that accept arrays like `setCipherSuites` and `setProtocols` 
accept `null` as a parameter. Like my previous comment, it seems more important 
to keep this behavior consistent in the API and allow `null` as an acceptable 
value, which is also useful for clearing the current value of the parameter.

-

Changes requested by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280949: Correct the references for the Java Security Standard Algorithm Names specification [v2]

[Sean Mullan]

On Mon, 31 Jan 2022 19:19:28 GMT, Xue-Lei Andrew Fan  wrote:

> > Please capitalize "specification" (i.e. "Specification") to be consistent 
> > with other references in the javadoc. Looks good otherwise.
> 
> OK. Just curious, why we want to use capitalized "specification"? This word 
> "specification" is not a part of the document title.

True (although in hindsight we could have added it to the title). However, I 
think capitalizing when referencing it signifies that it is an important and 
normative document, similar to when you refer to standard documents even if it 
is not in title (ex: "The AES Standard"). If we referred to it again in 
subsequent text, then it is not as necessary, i.e. you could say "This 
specification includes list of names that are defined as standard ...

-

PR: https://git.openjdk.java.net/jdk/pull/7290

Re: RFR: 8280494: (D)TLS signature schemes [v2]

[Sean Mullan]

On Fri, 28 Jan 2022 15:17:28 GMT, Sean Mullan  wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Copyright correction
>
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 710:
> 
>> 708:  * Signature Schemes section of the Java Cryptography
>> 709:  * Architecture Standard Algorithm Name Documentation, and may also
>> 710:  * include other signature schemes that the provider supports.
> 
> There doesn't seem to be anything preventing a user from setting a bogus 
> signature scheme (ex: named "foo") - which is neither a standard name or a 
> provider specific name.
> I think the method may be too tightly specified, and you should make this 
> more general and not put any constraints on the names of the signature 
> schemes. (Although we should still link to the specification for a list of 
> standard names). 
> 
> It would be useful to explain when this method returns pre-populated scheme 
> names as supported by the underlying provider and when it may return an empty 
> list.

You should also define the interaction with the system properties (probably as 
an @implNote). Does `getSignatureSchemes` ever return the value of the system 
properties? Does the `setSignatureSchemes` API always override the properties 
even if it is called by the provider?

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280494: (D)TLS signature schemes [v2]

[Sean Mullan]

On Fri, 28 Jan 2022 07:21:56 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
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Copyright correction

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 94:

> 92: 
> 93: /**
> 94:  * Constructs SSLParameters.

Would it be useful to add another ctor that takes a signature schemes array 
parameter?

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 709:

> 707:  * 
> "{@docRoot}/../specs/security/standard-names.html#signature-schemes">
> 708:  * Signature Schemes section of the Java Cryptography
> 709:  * Architecture Standard Algorithm Name Documentation, and may also

The correct name is "Java Security Standard Algorithm Names Specification". 
Same comment below for `setSignatureSchemes`.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 710:

> 708:  * Signature Schemes section of the Java Cryptography
> 709:  * Architecture Standard Algorithm Name Documentation, and may also
> 710:  * include other signature schemes that the provider supports.

There doesn't seem to be anything preventing a user from setting a bogus 
signature scheme (ex: named "foo") - which is neither a standard name or a 
provider specific name.
I think the method may be too tightly specified, and you should make this more 
general and not put any constraints on the names of the signature schemes. 
(Although we should still link to the specification for a list of standard 
names). 

It would be useful to explain when this method returns pre-populated scheme 
names as supported by the underlying provider and when it may return an empty 
list.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 746:

> 744:  * @param signatureSchemes an ordered array of signature scheme 
> names,
> 745:  *with the first entry being the most preferred. If the array
> 746:  *is empty (zero-length), the prodiver-specific default 
> signature

typo: prodiver -> provider

-

PR: https://git.openjdk.java.net/jdk/pull/7252

Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters [v2]

[Sean Mullan]

On Thu, 20 Jan 2022 14:46:28 GMT, Xue-Lei Andrew Fan  wrote:

>> In the getApplicationProtocols() method in javax.net.ssl.SSLParameters, the 
>> return statement says that "The array is ordered based on protocol 
>> preference, with protocols[0] being the most preferred.". However, there is 
>> no "protocols" variable in this method.
>> 
>> The update is a minor correction so that the specification is not rely on 
>> the "protocols" variable.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update per feedback

Marked as reviewed by mullan (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7152

Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters

[Sean Mullan]

On Thu, 20 Jan 2022 07:12:42 GMT, Xue-Lei Andrew Fan  wrote:

> In the getApplicationProtocols() method in javax.net.ssl.SSLParameters, the 
> return statement says that "The array is ordered based on protocol 
> preference, with protocols[0] being the most preferred.". However, there is 
> no "protocols" variable in this method.
> 
> The update is a minor correction so that the specification is not rely on the 
> "protocols" variable.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 619:

> 617:  *
> 618:  * @return a non-null, possibly zero-length array of application 
> protocol
> 619:  * {@code String}s.  The array is placed in descending order 
> of

The phrase "descending order" seems more appropriate for numerical values. I 
think the previous wording was more clear, with a small change: "The array is 
ordered based on protocol preference, with the first entry being the most 
preferred."

-

PR: https://git.openjdk.java.net/jdk/pull/7152

Re: RFR: 8274809: Update java.base classes to use try-with-resources

[Sean Mullan]

On Tue, 5 Oct 2021 09:36:23 GMT, Andrey Turbanov 
 wrote:

> 8274809: Update java.base classes to use try-with-resources

The security related files look fine.

-

Marked as reviewed by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5818

Re: RFR: 8274835: Remove unnecessary castings in java.base

[Sean Mullan]

On Thu, 9 Sep 2021 20:12:47 GMT, Andrey Turbanov 
 wrote:

> Redundant castings make code harder to read.
> Found them by IntelliJ IDEA.
> I tried to select only casts which are definitely safe to remove. Also didn't 
> touch primitive types casts.

The security related files look fine.

-

Marked as reviewed by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5454

Re: RFR: 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base

[Sean Mullan]

On Wed, 1 Sep 2021 07:37:53 GMT, Andrey Turbanov 
 wrote:

> There are few places in code where manual while loop is used with Iterator to 
> iterate over Collection.
> Instead of manual while cycles it's preferred to use enhanced-for cycle 
> instead: it's less verbose, makes code easier to read and it's less 
> error-prone.
> It doesn't have any performance impact: java compiler generates similar code 
> when compiling enhanced-for cycle.
> 
> Similar cleanups:
> * https://bugs.openjdk.java.net/browse/JDK-8258006
> * https://bugs.openjdk.java.net/browse/JDK-8257912

The security related changes look fine to me.

-

Marked as reviewed by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5328

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Sean Mullan]

On Fri, 21 May 2021 15:27:39 GMT, Daniel Fuchs  wrote:

>> Weijun Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
>
> src/java.base/share/classes/java/lang/SecurityManager.java line 104:
> 
>> 102:  * method will throw an {@code UnsupportedOperationException}). If the
>> 103:  * {@systemProperty java.security.manager} system property is set to the
>> 104:  * special token "{@code allow}", then a security manager will not be 
>> set at
> 
> Can/should the `{@systemProperty ...}` tag be used more than once for a given 
> system property? I thought it should be used only once, at the place where 
> the system property is defined. Maybe @jonathan-gibbons can offer some more 
> guidance on this.

Good point. I would remove the extra @systemProperty tags on lines 103, 106, 
and 113. Also, in `System.setSecurityManager` there are 3 @systemProperty 
java.security.manager tags, I would just keep the first one. (I think it's ok 
to have more than one, if they are defined in different APIs).

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v2]

[Sean Mullan]

On Tue, 18 May 2021 21:44:43 GMT, Weijun Wang  wrote:

>> Please review the test changes for [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> With JEP 411 and the default value of `-Djava.security.manager` becoming 
>> `disallow`, tests calling `System.setSecurityManager()`  need 
>> `-Djava.security.manager=allow` when launched. This PR covers such changes 
>> for tier1 to tier3 (except for the JCK tests).
>> 
>> To make it easier to focus your review on the tests in your area, this PR is 
>> divided into multiple commits for different areas (~~serviceability~~, 
>> ~~hotspot-compiler~~, ~~i18n~~, ~~rmi~~, ~~javadoc~~, swing, 2d, 
>> ~~security~~, ~~hotspot-runtime~~, ~~nio~~, ~~xml~~, ~~beans~~, 
>> ~~core-libs~~, ~~net~~, ~~compiler~~, ~~hotspot-gc~~). Mostly the rule is 
>> the same as how Skara adds labels, but there are several small tweaks:
>> 
>> 1. When a file is covered by multiple labels, only one is chosen. I make my 
>> best to avoid putting too many tests into `core-libs`. If a file is covered 
>> by `core-libs` and another label, I categorized it into the other label.
>> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the 
>> `xml` commit.
>> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the 
>> `rmi` commit.
>> 4. One file not covered by any label -- 
>> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in 
>> the `swing` commit.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> all files to minimize unnecessary merge conflict.
>> 
>> Please note that this PR can be integrated before the source changes for JEP 
>> 411, as the possible values of this system property was already defined long 
>> time ago in JDK 9.
>> 
>> Most of the change in this PR is a simple adding of 
>> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes 
>> it was not `othervm` and we add one. Sometimes there's no `@run` at all and 
>> we add the line.
>> 
>> There are several tests that launch another Java process that needs to call 
>> the `System.setSecurityManager()` method, and the system property is added 
>> to `ProcessBuilder`, `ProcessTools`, or the java command line (if the test 
>> is a shell test).
>> 
>> 3 langtools tests are added into problem list due to 
>> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
>> 
>> 2 SQL tests are moved because they need different options on the `@run` line 
>> but they are inside a directory that has a `TEST.properties`:
>> 
>> rename test/jdk/java/sql/{testng/test/sql/othervm => 
>> permissionTests}/DriverManagerPermissionsTests.java (93%)
>> rename test/jdk/javax/sql/{testng/test/rowset/spi => 
>> permissionTests}/SyncFactoryPermissionsTests.java (95%)
>>  ```
>> 
>> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   adjust order of VM options

The changes to the security tests look good.

-

Marked as reviewed by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4071

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

[Sean Mullan]

On Tue, 18 May 2021 15:19:21 GMT, Alan Bateman  wrote:

>> It includes both:
>> ![Screen Shot 2021-05-18 at 8 41 11 
>> AM](https://user-images.githubusercontent.com/35072269/118652730-dfb35400-b7b4-11eb-83ee-92be9136fea2.jpg)
>
> Thanks for checking, I assumed that was the case so wondering if it should be 
> dropped from the deprecation text to avoid the repeated sentence.

My feeling is that it is better to be more specific than the generic removal 
text as this is the most significant class that is being deprecated for 
removal. Also, this says "the Security Manager" (note the space between) which 
really encompasses more than the `java.lang.SecurityManager` API and which is 
explained more completely in the JEP.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

[Sean Mullan]

On Tue, 18 May 2021 06:31:06 GMT, Alan Bateman  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>
> src/java.base/share/classes/java/lang/SecurityManager.java line 315:
> 
>> 313:  *
>> 314:  * @since   1.0
>> 315:  * @deprecated The Security Manager is deprecated and subject to 
>> removal in a
> 
> Javadoc will prefix, in bold, "Deprecated, for removal: This API element is 
> subject to removal in a future version". I'm just wondering if the sentence 
> will be repeated here, can you check?

It includes both:
![Screen Shot 2021-05-18 at 8 41 11 
AM](https://user-images.githubusercontent.com/35072269/118652730-dfb35400-b7b4-11eb-83ee-92be9136fea2.jpg)

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v11]

[Sean Mullan]

On Fri, 19 Feb 2021 08:05:06 GMT, Andrey Turbanov 
 wrote:

>> src/java.base/share/classes/sun/security/provider/certpath/X509CertPath.java 
>> line 228:
>> 
>>> 226: try {
>>> 227: if (is.markSupported() == false) {
>>> 228: // Copy the entire input stream into an InputStream 
>>> that does
>> 
>> I don't think you should remove lines 228-232. These methods are called by 
>> methods of CertificateFactory that take InputStream (which may contain a 
>> stream of security data) and they are designed such that they try to read 
>> one Certificate, CRL, or CertPath from the InputStream and leave the 
>> InputStream ready to parse the next structure instead of consuming all of 
>> the bytes. Thus they check if the InputStream supports mark in order to try 
>> to preserve that behavior. If mark is not supported, then it's ok to use 
>> InputStream.readAllBytes, otherwise, leave the stream as-is.
>
> As I can see only ByteArrayInputStream is actually passed in `InputStream` in 
> current JDK code:
> 
> PKCS7 pkcs7 = new PKCS7(is.readAllBytes());
> private static List parsePKCS7(InputStream is)
> certs = parsePKCS7(is);
> public X509CertPath(InputStream is, String encoding)
> return new X509CertPath(new ByteArrayInputStream(data), 
> encoding);
> 
> PKCS7 pkcs7 = new PKCS7(is.readAllBytes());
> private static List parsePKCS7(InputStream is)
> certs = parsePKCS7(is);
> public X509CertPath(InputStream is, String encoding)
> this(is, PKIPATH_ENCODING);
> public X509CertPath(InputStream is) throws 
> CertificateException {
> return new X509CertPath(new 
> ByteArrayInputStream(encoding));
> 
> ![изображение](https://user-images.githubusercontent.com/741251/108475587-f4f61080-72a1-11eb-91e0-ac2b98c7c490.png)
> 
> Perhaps original marking approach was lost during refactoring?

You are right, the code was refactored (way back in 2010) [1] to read one block 
at a time, so this check on mark can be removed. So, in this case, I think it 
is probably safe to just pass the InputStream as-is to PKCS7(InputStream), but 
maybe you can add a comment that says this should always be a 
ByteArrayInputStream. We can look at refactoring this code and clean it up a 
bit more later.

[1] https://hg.openjdk.java.net/jdk/jdk/rev/337ae296b6d6

-

PR: https://git.openjdk.java.net/jdk/pull/1853

Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v11]

[Sean Mullan]

On Mon, 15 Feb 2021 19:47:00 GMT, Andrey Turbanov 
 wrote:

>> 8080272  Refactor I/O stream copying to use 
>> InputStream.transferTo/readAllBytes and Files.copy
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo
>   remove unnecessary file.exists() check

src/java.base/share/classes/sun/security/provider/certpath/X509CertPath.java 
line 228:

> 226: try {
> 227: if (is.markSupported() == false) {
> 228: // Copy the entire input stream into an InputStream that 
> does

I don't think you should remove lines 228-232. These methods are called by 
methods of CertificateFactory that take InputStream (which may contain a stream 
of security data) and they are designed such that they try to read one 
Certificate, CRL, or CertPath from the InputStream and leave the InputStream 
ready to parse the next structure instead of consuming all of the bytes. Thus 
they check if the InputStream supports mark in order to try to preserve that 
behavior. If mark is not supported, then it's ok to use 
InputStream.readAllBytes, otherwise, leave the stream as-is.

-

PR: https://git.openjdk.java.net/jdk/pull/1853

Re: RFR: 8260520: Avoid getting permissions in JarFileFactory when no SecurityManager installed [v2]

[Sean Mullan]

On Wed, 27 Jan 2021 18:59:00 GMT, Claes Redestad  wrote:

>> 8260520: Avoid getting permissions in JarFileFactory when no SecurityManager 
>> installed
>
> Claes Redestad has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Copyrights
>  - Same for Windows

Marked as reviewed by mullan (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2263

Re: RFR: 8260520: Avoid getting permissions in JarFileFactory when no SecurityManager installed

[Sean Mullan]

On Wed, 27 Jan 2021 15:10:16 GMT, Michael McMahon  wrote:

>> 8260520: Avoid getting permissions in JarFileFactory when no SecurityManager 
>> installed
>
> Marked as reviewed by michaelm (Reviewer).

Will you make the same change to 
`src/java.base/windows/classes/sun/net/www/protocol/jar/JarFileFactory.java`?

-

PR: https://git.openjdk.java.net/jdk/pull/2263

Re: RFR: 8250564: Remove terminally deprecated constructor in GSSUtil

[Sean Mullan]

On Tue, 5 Jan 2021 21:02:21 GMT, Joe Darcy  wrote:

> Back in JDK 16, two unintended default constructors were identified and 
> deprecated for removal. The time has come to remove them.
> 
> Please also review the corresponding CSRs:
> 
> JDK-8258521 Remove terminally deprecated constructor in GSSUtil 
> https://bugs.openjdk.java.net/browse/JDK-8258521
> 
>  JDK-8258522 Remove terminally deprecated constructor in java.net.URLDecoder 
> https://bugs.openjdk.java.net/browse/JDK-8258522
> 
> Calling a static method using an instance of a class, as done in the test 
> B6463990.java, is a coding anti-pattern that generates a lint warning; that 
> warning in enabled in the JDK build.
> 
> Thanks,

The GSSUtil bug needs an appropriate noreg keyword, but otherwise looks fine.

-

Marked as reviewed by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1948

Integrated: 8202343: Disable TLS 1.0 and 1.1

[Sean Mullan]

On Mon, 16 Nov 2020 20:18:16 GMT, Sean Mullan  wrote:

> This change disables the TLSv1 and TLSv1.1 protocols by adding them to the 
> jdk.tls.disabledAlgorithms security property in the java.security file. These 
> protocols use weak algorithms and are being deprecated by the IETF. They 
> should be disabled by default to improve the default security configuration 
> of the JDK. See the CSR for more rationale: 
> https://bugs.openjdk.java.net/browse/JDK-8254713
> 
> The fix mostly involves changes to existing tests that for one reason or 
> another depend on the TLSv1 and TLSv1.1 protocols being enabled. There is a 
> new test specifically for this issue: 
> test/jdk/sun/security/ssl/SSLContextImpl/SSLContextDefault.java

This pull request has now been integrated.

Changeset: 3a4b90f0
Author:Sean Mullan 
URL:   https://git.openjdk.java.net/jdk/commit/3a4b90f0
Stats: 396 lines in 21 files changed: 273 ins; 97 del; 26 mod

8202343: Disable TLS 1.0 and 1.1

Reviewed-by: xuelei, dfuchs, coffeys

-

PR: https://git.openjdk.java.net/jdk/pull/1235

Re: RFR: 8202343: Disable TLS 1.0 and 1.1 [v2]

[Sean Mullan]

On Wed, 18 Nov 2020 15:45:02 GMT, Sean Coffey  wrote:

>> Sean Mullan has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   More test changes.
>
> test/lib/jdk/test/lib/security/SecurityUtils.java line 64:
> 
>> 62: }
>> 63: 
>> 64: private static void removeFromDisabledAlgs(String prop, List 
>> algs) {
> 
> Useful utility method. Maybe it can be made public/ opened up and renamed to 
> something like "removeFromSecurityProperty" perhaps  ? could be done at a 
> later time perhaps.

Yes, that's the main reason why I put that code into a separate method, so that 
we could consider adding more methods that call it or using it more generally 
in subsequent fixes where we need to re-enable disabled algorithms.

-

PR: https://git.openjdk.java.net/jdk/pull/1235

Re: RFR: 8202343: Disable TLS 1.0 and 1.1 [v2]

[Sean Mullan]

> This change disables the TLSv1 and TLSv1.1 protocols by adding them to the 
> jdk.tls.disabledAlgorithms security property in the java.security file. These 
> protocols use weak algorithms and are being deprecated by the IETF. They 
> should be disabled by default to improve the default security configuration 
> of the JDK. See the CSR for more rationale: 
> https://bugs.openjdk.java.net/browse/JDK-8254713
> 
> The fix mostly involves changes to existing tests that for one reason or 
> another depend on the TLSv1 and TLSv1.1 protocols being enabled. There is a 
> new test specifically for this issue: 
> test/jdk/sun/security/ssl/SSLContextImpl/SSLContextDefault.java

Sean Mullan has updated the pull request incrementally with one additional 
commit since the last revision:

  More test changes.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1235/files
  - new: https://git.openjdk.java.net/jdk/pull/1235/files/71d49643..4daf0154

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1235=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1235=00-01

  Stats: 10 lines in 2 files changed: 7 ins; 3 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1235.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1235/head:pull/1235

PR: https://git.openjdk.java.net/jdk/pull/1235

Re: Fix for Javadoc errors in java.base

[Sean Mullan]

On 8/13/20 1:21 PM, Jonathan Gibbons wrote:
--- 
old/src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java 


2020-07-25 23:46:21.233726447 +0530
+++ 
new/src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java 


2020-07-25 23:46:20.721720857 +0530
@@ -96,8 +96,6 @@
   * @param p the prime modulus
   * @param g the base generator
   * @param l the private-value length
- *
- * @exception InvalidKeyException if the key cannot be encoded


This should actually remain, but it should be ProviderException which 
is a RuntimeException. See the other DHPrivateKey ctor as that 
specifies it correctly.


--Sean



I note the use of `@exception`, as compared to `@throws`, which is more 
common.


Stats:
`@exception` 7322 occurrences
`@throws` 21173 occurrences

That's probably too many `@exception` to clean up. :-(


Right, that's probably a separate cleanup activity. However, if you want 
to change the 3 instances of @exception to @throws in DHPrivateKey, I'm 
fine with that.


--Sean

Re: Fix for Javadoc errors in java.base

[Sean Mullan]

On 8/13/20 11:05 AM, Julia Boes wrote:

Hi Vipin,

Thanks for providing this fix, I'm happy to sponsor your change. To 
complete the review, we still need someone to green light the remaining 
changes below. I'm looping in net-dev and security-dev to have a look.


Bug: https://bugs.openjdk.java.net/browse/JDK-8251542

Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8251542/webrev/

Once the review is completed, please provide me with a patch that 
includes a changeset comment as described here: 
https://openjdk.java.net/guide/producingChangeset.html


If you have any questions, let me know.

Cheers,

Julia

--- 
old/src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java

2020-07-25 23:46:21.233726447 +0530
+++ 
new/src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java

2020-07-25 23:46:20.721720857 +0530
@@ -96,8 +96,6 @@
   * @param p the prime modulus
   * @param g the base generator
   * @param l the private-value length
- *
- * @exception InvalidKeyException if the key cannot be encoded


This should actually remain, but it should be ProviderException which is 
a RuntimeException. See the other DHPrivateKey ctor as that specifies it 
correctly.


--Sean

Re: Browser's accepting certificates that Java does not

[Sean Mullan]

Also, in case you did not know, the JDK "PKIX" CertPathBuilder 
implementation (which is also the default used by the JSSE TrustManager) 
supports retrieving certificates via the AIA extension, but it is 
disabled by default. To enable it, set the 
"com.sun.security.enableAIAcaIssuers" system property to the value "true".


--Sean

On 7/8/20 5:27 AM, Bernd Eckenfels wrote:
Note that many browsers also download certs from the AIA and even "well 
known" mechanisms. It won't help to access more truststores, that would 
be a function you need to prove directly. Also the dynamic installation 
from Windows Updates or offline from crypt32.dll is not triggered when 
only browsing the existing stores.


If you need that kind of integration it's probably better do not use java :)

Chrome does install those dynamically loaded intermiates to Windows 
truststores, I think - it would not hurt to get access to it.



--
http://bernd.eckenfels.net

*Von:* security-dev  im Auftrag von 
Daniel Fuchs 

*Gesendet:* Wednesday, July 8, 2020 11:09:26 AM
*An:* Mark A. Claassen ; OpenJDK 


*Cc:* net-dev@openjdk.java.net 
*Betreff:* Re: Browser's accepting certificates that Java does not
Hi Mark,

This is probably a question for the security-dev mailing list, which
I have put in the to: of my reply.

best regards,

-- daniel

On 07/07/2020 20:24, Mark A. Claassen wrote:
I was curious if there has been any thought to allowing accessing to other certificate stores in Windows besides the "Trusted Root Certification Authorities" and the "Personal" ones.  It seems like web servers omitting intermediate certificates in the certificate  chain is pretty common.  Browsers seems to fill in the gaps, but Java 

does not.


We very recently encountered this again when a customer started proxying their SSL requests, creating a new certificate on the fly, resigning ours with their corporate CA.  (The browser handled this fine, but our Java app detected a chain length of 2, instead  of 4 like in the browser.)  Having them put their intermediate 
certificates in the "Trusted Root Certification Authorities" solved the 
issue, but they are unwilling to do this on a corporate-wide basis.


If Java was able to access more keystores through the MSCAPI interface, is seems like it would fill in the gaps as well and remove a pain point we are experiencing where Java does not accept a certificate even though all their browsers will.  I think all  intermediate certificates are supposed to be in the chain sent from 
the server (https://tools.ietf.org/html/rfc5246) in the TLS negotiation, 
but since browser's don't enforce care, people are left thinking 
everything is great (until Java tries to connect).


Thanks,

Mark Claassen
Senior Software Engineer

Donnell Systems, Inc.
130 South Main Street
Leighton Plaza Suite 375
South Bend, IN  46601
E-mail: mailto:mclaas...@ocie.net
Voice: (574)232-3784
Fax: (574)232-4014

Disclaimer:
The opinions provided herein do not necessarily state or reflect
those of Donnell Systems, Inc.(DSI). DSI makes no warranty for and
assumes no legal liability or responsibility for the posting.

Re: 8248865: Document JNDI/LDAP timeout properties

[Sean Mullan]

On 7/7/20 9:02 AM, Daniel Fuchs wrote:

Hi Sean,

Good point.

It appears it throws a java.lang.NumberFormatException which is not
documented as being thrown by any InitialContext constructors or its
subclasses, nor by `addToEnvironment`.


Ugh, ok.


I believe this is a more general problem - and I would beg to tackle
that in a followup issue if you don't mind.


Ok.

--Sean



best regards,

-- daniel

On 07/07/2020 13:12, Sean Mullan wrote:
You should document what the behavior is if an invalid string value is 
set (ex: not an integer).


--Sean

Re: 8248865: Document JNDI/LDAP timeout properties

[Sean Mullan]

You should document what the behavior is if an invalid string value is 
set (ex: not an integer).


--Sean

On 7/7/20 7:11 AM, Aleks Efimov wrote:

Hi Daniel,

Thanks for documenting the system properties. It looks good to me.

NIT: You might want to update the copyright's last modification year to 
2020


Best Regards,
Aleksei

On 06/07/2020 16:39, Daniel Fuchs wrote:

Hi,

Please find below a doc only change to document two
JDK specific JNDI/LDAP timeout properties:

8248865: Document JNDI/LDAP timeout properties
https://bugs.openjdk.java.net/browse/JDK-8248865

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8248865/webrev.00/

CSR:
https://bugs.openjdk.java.net/browse/JDK-8248866

best regards,

-- daniel

Re: Need sponsor to fix Javadoc warnings

[Sean Mullan]

The security changes look fine to me.

--Sean

On 4/8/20 7:50 AM, Pavel Rappo wrote:

Vipin, here you go:

 https://bugs.openjdk.java.net/browse/JDK-8242366
 http://cr.openjdk.java.net/~prappo/8242366/webrev.00/

I took the liberty of additionally fixing a couple of parameters' names,
a typo, and `@exception` tags for checked exceptions that were neither thrown
nor imported.

The bulk of the change is in Security. Some changes are in Networking. The
appropriate mailing lists are in CC for this email. We should wait for their
feedback.

Changes in core area look good to me and I'd be surprised if there are any
problems with the remaining portion of the changeset.

-Pavel


On 7 Apr 2020, at 19:50, Vipin Sharma  wrote:

Hi Pavel,


On Apr 7, 2020, at 11:11 PM, Pavel Rappo  wrote:

I assume you have signed the OCA [1]. If not and you want to continue, please 
do it. If you've already done so, which is probably the case [2], please attach 
your patch as text to this thread with the next email. Do not use zip or the 
like. I will take it from there and sponsor that for you.

Yes I have signed OCA.


-Pavel

[1] https://www.oracle.com/technetwork/community/oca-486395.html
[2] changeset:   58344:65f30e209890
user:clanger
date:Wed Mar 11 13:50:13 2020 +0100
files:   test/jdk/java/lang/Boolean/GetBoolean.java 
test/jdk/java/lang/Boolean/MakeBooleanComparable.java 
test/jdk/java/lang/Boolean/ParseBoolean.java
description:
8240524: Remove explicit type argument in test 
jdk/java/lang/Boolean/MakeBooleanComparable.java
Reviewed-by: clanger, vtewari
Contributed-by: vipinsharma85 at gmail.com


Yes this is my first contribution.

Patch text:

--- old/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java  
2020-04-06 00:19:10.546117441 +0530
+++ new/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java  
2020-04-06 00:19:10.130115855 +0530
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -202,7 +202,7 @@
 /**
  * Sets the padding mechanism of this cipher.
  *
- * @param padding the padding mechanism
+ * @param paddingScheme the padding mechanism
  *
  * @exception NoSuchPaddingException if the requested padding mechanism
  * does not exist
--- old/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java  
2020-04-06 00:19:11.526121179 +0530
+++ new/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java  
2020-04-06 00:19:11.118119622 +0530
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2004, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -313,10 +313,10 @@
  * current Cipher.engineInit(...) implementation,
  * IllegalStateException will always be thrown upon invocation.
  *
- * @param in the input buffer
- * @param inOffset the offset in in where the input
+ * @param input the input buffer
+ * @param inputOffset the offset in in where the input
  * starts
- * @param inLen the input length.
+ * @param inputLen the input length.
  *
  * @return n/a.
  *
--- old/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java  
2020-04-06 00:19:12.462124749 +0530
+++ new/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java  
2020-04-06 00:19:12.054123193 +0530
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2007, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -130,7 +130,6 @@
  *
  * @param plain the buffer with the input data to be encrypted
  * @param plainOffset the offset in plain
- * @param plainLen the length of the input data
  * @param cipher the buffer for the result
  * @param cipherOffset the offset in cipher
  */
@@ -154,7 +153,6 @@
  *
  * @param cipher the buffer with the input data to be decrypted
  * @param cipherOffset the offset in cipherOffset
- * @param cipherLen the length of the input data
  * @param plain the buffer for the result
  * @param plainOffset the offset in plain
  */
--- old/src/java.base/share/classes/com/sun/crypto/provider/DESCrypt.java   
2020-04-06 00:19:13.414128382 +0530
+++ new/src/java.base/share/classes/com/sun/crypto/provider/DESCrypt.java   
2020-04-06 00:19:12.998126795 +0530
@@ -1,5 +1,5 @@
/*
- * 

Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

[Sean Mullan]

I think you should mark one of the two bugs a duplicate. Typically I 
mark the more recent one as a duplicate, unless there is a good reason 
to do otherwise.


--Sean

On 3/26/20 12:28 PM, Sean Mullan wrote:

Cross-posting to security-dev as this involves TLS/SSL configuration.

--Sean

On 3/26/20 10:02 AM, rahul.r.ya...@oracle.com wrote:

Hello,

Request to have my fix reviewed for issues:

 JDK-8239595 : ssl context version is not respected
 JDK-8239594 : jdk.tls.client.protocols is not respected

The fix updates 
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of 
ctx.getSupportedSSLParameters(),

as the latter does not respect the context parameters set by the user.

Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594

Webrev: 
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/


-- Rahul

Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

[Sean Mullan]

Cross-posting to security-dev as this involves TLS/SSL configuration.

--Sean

On 3/26/20 10:02 AM, rahul.r.ya...@oracle.com wrote:

Hello,

Request to have my fix reviewed for issues:

     JDK-8239595 : ssl context version is not respected
     JDK-8239594 : jdk.tls.client.protocols is not respected

The fix updates 
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of 
ctx.getSupportedSSLParameters(),

as the latter does not respect the context parameters set by the user.

Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594

Webrev: 
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/


-- Rahul

Re: Reading from a closed socket: different behavior between Linux and other operating systems

[Sean Mullan]

Cross-posting to security-dev as SSL is involved.

--Sean

On 12/29/19 4:01 PM, Dawid Weiss wrote:

Hello,

I am a committer to the Apache Lucene project. We have been looking
into a problem in which SSL connections were handled differently in
tests on different operating systems and narrowed it down to
essentially the following scenario (full repro code at [1]):

Server side:

   try (ServerSocketChannel serverChannel = ServerSocketChannel.open()) {
 SocketChannel clientChannel = serverChannel.accept();
 clientChannel.close();
   }

Client side:

   Socket socket = new Socket();
   socket.connect(target);
   // ... server closes the socket here.
   // Queue some data for writing to the closed socket. This succeeds.
   socket.getOutputStream().write("will succeed?!".getBytes("UTF-8"));
   // Try to read something from the closed socket.
   socket.getInputStream().read(new byte[100]);

The last line of the client results in different behavior between
operating systems.

1) Linux, JDK 11, 13, 14: succeeds with -1 (EOF).
2) Windows, JDK 11: SocketException ("recv failed") is thrown
3) Windows, JDK 13, 14: SocketException (localized message) is thrown
4) FreeBSD: SocketException (connection reset) is thrown
5) Mac OS X: SocketException (connection reset) is thrown

I admit my original thinking on the Lucene issue (see full  discussion
at [2]) was that it was Windows that was off here (due to
WSAECONNRESET not being handled at all in SocketInputStream.c [3].
Since then (JDK11) the underlying socket implementation has changed
due to JEP 353 [4] (which Alan Bateman kindly pointed out to me).

But the difference in runtime behavior between Linux and other
operating systems still exists on both the old and the new
implementation. I don't know whether it's something that should be
qualified as platform-specific but it causes additional problems when
it triggers somewhere deep inside the SSL handling layer -- then the
application-level code receives a different exception depending on
where it's run (an SSLException with a suppressed SocketException or a
SocketException directly).

I don't have any ideas about what a "good" fix for this is but I'm
curious what others think.

Dawid

[1] https://issues.apache.org/jira/secure/attachment/12989538/RecvRepro.java
[2] https://issues.apache.org/jira/browse/SOLR-13778
[3] 
https://github.com/openjdk/jdk14/blob/f58a8cbed2ba984ceeb9a1ea59f917e3f9530f1e/src/java.base/windows/native/libnet/SocketInputStream.c#L120-L154
[4] https://openjdk.java.net/jeps/353

Re: RFR (XS) 8230415 : Avoid redundant permission checking in FilePermissionCollection and SocketPermissionCollection

[Sean Mullan]

Hi Ivan,

The fix looks good. Good catch.

--Sean

On 8/30/19 7:32 PM, Ivan Gerasimov wrote:

Hello!

In the two implementations of PermissionCollection.implies(Permission), 
all the permissions are traversed, and their corresponding bit mask are 
checked.


For example, here's a snippet from FilePermission.java:

     int desired = fperm.getMask();
     int effective = 0;
     int needed = desired;

     for (Permission perm : perms.values()) {
     FilePermission fp = (FilePermission)perm;
     if (((needed & fp.getMask()) != 0) && 
fp.impliesIgnoreMask(fperm)) {

     effective |= fp.getMask();
     if ((effective & desired) == desired) {
     return true;
     }
     needed = (desired ^ effective);// <<< should be 
(desired & ~effective)

     }
     }

Here, if a permission's mask `fp.getMask()` intersects with `needed`, 
but does not fully cover all the needed bits, the variable `needed` is 
updated as XOR of desired and effective. This can raise a 
not-really-needed bits in the `needed` mask, so that for all subsequent 
permissions from the collection with that unneeded bits in the mask, an 
expensive fp.impliesIgnoreMask(fperm) will be executed.


The fix does not change the behavior, but helps avoid unnecessary calls 
to impliesIgnoreMask().


Would you please help review a trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8230415
WEBREV: http://cr.openjdk.java.net/~igerasim/8230415/00/webrev/

Thanks in advance!

Re: [ipv6]: 8224081: SOCKS v4 doesn't work with IPv6

[Sean Mullan]

On 5/24/19 4:56 PM, Sean Mullan wrote:

On 5/23/19 8:14 PM, Arthur Eubanks wrote:

Ping on a review from security-dev.

On Fri, May 17, 2019 at 9:53 AM Chris Hegarty 
mailto:chris.hega...@oracle.com>> wrote:


    Arthur,


    On 17 May 2019, at 17:50, Arthur Eubanks mailto:aeuba...@google.com>> wrote:

    Looks good.

    Trivially, maybe amend the comment to be more explicit

       86       // SOCKS V4 ( requires IPv4 )

    -Chris.

    Done
    http://cr.openjdk.java.net/~aeubanks/8224081/webrev.02/

    I will wait for another review from security-dev.


    You have my Review ( conditional on a Reviewer for the test in the
    security area ).


It seems ok but given that this area is a bit unpredictable I would 
recommend you be available/online to monitor CI results after you push 
the fix in case something breaks.


I should add that I would recommend not pushing this until Tuesday 
after the US Memorial Day weekend as many folks probably won't be online.


--Sean

Re: [ipv6]: 8224081: SOCKS v4 doesn't work with IPv6

[Sean Mullan]

On 5/23/19 8:14 PM, Arthur Eubanks wrote:

Ping on a review from security-dev.

On Fri, May 17, 2019 at 9:53 AM Chris Hegarty > wrote:


Arthur,


On 17 May 2019, at 17:50, Arthur Eubanks mailto:aeuba...@google.com>> wrote:

Looks good.

Trivially, maybe amend the comment to be more explicit

   86       // SOCKS V4 ( requires IPv4 )

-Chris.

Done
http://cr.openjdk.java.net/~aeubanks/8224081/webrev.02/

I will wait for another review from security-dev.


You have my Review ( conditional on a Reviewer for the test in the
security area ).


It seems ok but given that this area is a bit unpredictable I would 
recommend you be available/online to monitor CI results after you push 
the fix in case something breaks.


--Sean

Re: [RFR] 8224635: Revert 8224256 and add back java/security/SecureClassLoader/DefineClass.java test

[Sean Mullan]

Looks fine although you should update the 8224635 title to match the 
changeset.


Also, I assume you will file a followon bug for the original issue 
(JDK-8224256). I think a new bug must be filed instead of re-opening 
8224256 because a changeset has already been pushed for it.


--Sean

On 5/23/19 12:08 PM, Arthur Eubanks wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8224635
webrev: http://cr.openjdk.java.net/~aeubanks/8224635/webrev.00/index.html

Test java/security/SecureClassLoader/DefineClass.java is failing after 
JDK-8224256

This change reverts the changes in JDK-8224256 and adds back in the test.

Re: [ipv6] RFR: 8224256: test/jdk/java/security/SecureClassLoader/DefineClass.java hardcodes 127.0.0.1

[Sean Mullan]

On 5/22/19 3:33 PM, Arthur Eubanks wrote:



On Wed, May 22, 2019 at 12:12 PM Sean Mullan <mailto:sean.mul...@oracle.com>> wrote:


On 5/22/19 1:28 PM, Arthur Eubanks wrote:
 > On Wed, May 22, 2019 at 7:13 AM Daniel Fuchs
mailto:daniel.fu...@oracle.com>
 > <mailto:daniel.fu...@oracle.com
<mailto:daniel.fu...@oracle.com>>> wrote:
 >
 >     Hi Arthur,
 >
 >         18 // For IPSupport
 >         19 grant {
 >         20     permission java.net.SocketPermission "localhost:0",
 >     "listen,resolve";
 >         21     permission java.util.PropertyPermission
 >     "java.net.preferIPv4Stack", "read";
 >         22 };
 >
 >     It might be better if these permissions were granted to the
 >     library only.
 >
 > Done.

Have you tested that with jtreg? I believe it may not work because of
the way the SecurityManager is enabled inside the test (rather than
using the jtreg java.security.policy option). You may find that you
also
need to grant those permissions to jtreg.jar since it is higher in the
call stack. If that is the case, you are probably better off granting
the permissions to all code, or restructuring the test to use the jtreg
java.security.policy option, where jtreg installs its own
SecurityManager to grant itself the proper permissions. However, that
will require some code changes and granting some additional permissions
to the test that are needed (for adding a security provider, etc)
before
it currently enables a SM. And that is probably more than you want
to do
for this fix.

--Sean

Tried it directly with jtreg
$ ~/jtreg/build/images/jtreg/bin/jtreg -jdk:./images/jdk/ 
../test/jdk/java/security/SecureClassLoader/DefineClass.java
and it passes. Verified that removing the newly added permissions makes 
it fail again.
There was some discussion on IPSupport and SecurityManagers when 
IPSupport was first introduced: 
https://markmail.org/message/vvemfm367ja3qllj


Ok, the fix looks good then to me.

--Sean

Re: [ipv6] RFR: 8224256: test/jdk/java/security/SecureClassLoader/DefineClass.java hardcodes 127.0.0.1

[Sean Mullan]

On 5/22/19 1:28 PM, Arthur Eubanks wrote:
On Wed, May 22, 2019 at 7:13 AM Daniel Fuchs > wrote:


Hi Arthur,

    18 // For IPSupport
    19 grant {
    20     permission java.net.SocketPermission "localhost:0",
"listen,resolve";
    21     permission java.util.PropertyPermission
"java.net.preferIPv4Stack", "read";
    22 };

It might be better if these permissions were granted to the
library only.

Done.


Have you tested that with jtreg? I believe it may not work because of 
the way the SecurityManager is enabled inside the test (rather than 
using the jtreg java.security.policy option). You may find that you also 
need to grant those permissions to jtreg.jar since it is higher in the 
call stack. If that is the case, you are probably better off granting 
the permissions to all code, or restructuring the test to use the jtreg 
java.security.policy option, where jtreg installs its own 
SecurityManager to grant itself the proper permissions. However, that 
will require some code changes and granting some additional permissions 
to the test that are needed (for adding a security provider, etc) before 
it currently enables a SM. And that is probably more than you want to do 
for this fix.


--Sean

Re: RFR [13] 8220719: Allow other named NetPermissions to be used

[Sean Mullan]

Looks good to me.

--Sean

On 3/15/19 10:30 AM, Chris Hegarty wrote:


This is a review request to resolve an asymmetry that I noticed when
investigating another issue. The NetPermission specification should be
relaxed a little to allow for other target names to be used, similar to
8077055. Really 8077055 should probably have covered NetPermission too,
but was just not noticed at the time.

---

Problem

The current specification of java.net.NetPermission states: "The
following table lists all the possible NetPermission target names ...",
which implies that unless the permission target is listed in the table,
it cannot be used.

However, there is no such enforcement in the implementation, and it is
useful and somewhat common for applications / libraries to create their
own permission target names.

Solution

Relax the specification so that non-listed permissions are allowable.
The table will remain, but will only document Java SE permissions.

Specification

--- a/src/java.base/share/classes/java/net/NetPermission.java
+++ b/src/java.base/share/classes/java/net/NetPermission.java
@@ -43,7 +43,7 @@
   * signify a wildcard match. For example: "foo.*" and "*" signify a 
wildcard

   * match, while "*foo" and "a*b" do not.
   * 
- * The following table lists all the possible NetPermission target names,
+ * The following table lists the standard NetPermission target names,
   * and for each provides a description of what the permission allows
   * and a discussion of the risks of granting code the permission.
   *

Bug
   https://bugs.openjdk.java.net/browse/JDK-8220719
CSR
   https://bugs.openjdk.java.net/browse/JDK-8220720

-Chris.

Re: Disable TLS 1.3 backward compatibility mode?

[Sean Mullan]

-bcc net-dev

Copying security-dev as TLS 1.3 topics are more appropriate for that 
mailing list.


--Sean

On 3/10/19 3:24 PM, ra...@web.de wrote:

Dear,
the Java TLS 1.3 implementation supports middlebox compatibility (e.g. 
sends a non-empty session id and a ChangeCipherSpec message). Out of 
interest: Is it possible to disable this compatibility mode?

Regards, Ralph

Re: RFR [13] JDK-8215430: Remove the internal package com.sun.net.ssl

[Sean Mullan]

Ok, looks good.

--Sean

On 2/25/19 10:53 PM, Xuelei Fan wrote:



It makes sense to me to leave the AbstractDelegateHttpsURLConnection 
methods as public.  We may need to re-org the code later, but it is out 
of the scope of this update.


Here is the new webrev (only AbstractDelegateHttpsURLConnection updated):
     http://cr.openjdk.java.net/~xuelei/8215430/webrev.01/

Thanks,
Xuelei


On 2/25/2019 1:55 PM, Sean Mullan wrote:
(I'd suggest cross-posting to net-dev since some classes in the 
networking area are also changed).


- AbstractDelegateHttpsURLConnection

It might be less risky to leave the methods as public just in case 
some code out there is using them (even though they are not supposed to).


The rest of this looks ok, pretty straightforward.

--Sean

On 2/21/19 10:45 PM, Xuelei Fan wrote:

Hi,

Could I get the following update reviewed:
    http://cr.openjdk.java.net/~xuelei/8215430/webrev.00/

The internal package com.sun.net.ssl had been deprecated since JDK 
1.4, and was not exported in the java.base module since JDK 9. It 
should be safe to remove them in JDK 13.


For more details, please refer to CSR:
 https://bugs.openjdk.java.net/browse/JDK-8218932

Thanks,
Xuelei

Re: RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

[Sean Mullan]

I don't think you really need to run the test with the othervm flag, 
unless you are concerned other tests may be setting this property and 
(incorrectly) not running in a separate VM, which would be a bug in my 
opinion. Well, then maybe you should run it in othervm just in case. 
Otherwise, looks ok to me.


--Sean

On 1/23/19 11:05 AM, Langer, Christoph wrote:

Hi,

please review a small test update.

Bug: https://bugs.openjdk.java.net/browse/JDK-8217657

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/

I’d like to move the test for the correct default value of security 
property “jdk.includeInExceptions” into an own testcase in the 
jdk.security area. This seems a bit more natural than to hide this check 
in a java/net specific test and will help finding/maintaining tests when 
the (default-)value for that property changes. For instance new values 
get added or other OpenJDK builds have different defaults (e.g. SAPMachine).


Thanks

Christoph

Re: A new proposal to add methods to HttpsURLConnection to access SSLSession

[Sean Mullan]

On 11/8/18 4:06 PM, Xuelei Fan wrote:
To make sure the SecureCacheResponse class work, two new tests are added 
(DefaultCacheResponse for default implementation, DummyCacheResponse for 
a test implementation):

     http://cr.openjdk.java.net/~xuelei/8212261/webrev.04/


I think you mean http://cr.openjdk.java.net/~xuelei/8212261/webrev.05/

Looks good.

--Sean



Thanks,
Xuelei

On 11/8/2018 7:03 AM, Sean Mullan wrote:

On 11/7/18 7:22 PM, Xuelei Fan wrote:

On 11/7/2018 1:30 PM, Sean Mullan wrote:

   https://bugs.openjdk.java.net/browse/JDK-8213161
   http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/


I didn't see a test for SecureCacheResponse - is it possible?

JDK does not have the reference implementation of SecureCacheResponse.


Ah, I see. I am sure there is a good reason, but why doesn't it have 
an implementation?



You could also add a test case for IllegalStateExc.


Added in the same test file.

lines 108-113 of HttpsSession.java should be indented 4 more spaces. 
Otherwise looks ok to me.



Updated.

New webrev:
http://cr.openjdk.java.net/~xuelei/8212261/webrev.04/


Looks good.

--Sean

Re: A new proposal to add methods to HttpsURLConnection to access SSLSession

[Sean Mullan]

On 11/7/18 7:22 PM, Xuelei Fan wrote:

On 11/7/2018 1:30 PM, Sean Mullan wrote:

   https://bugs.openjdk.java.net/browse/JDK-8213161
   http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/


I didn't see a test for SecureCacheResponse - is it possible?

JDK does not have the reference implementation of SecureCacheResponse.


Ah, I see. I am sure there is a good reason, but why doesn't it have an 
implementation?



You could also add a test case for IllegalStateExc.


Added in the same test file.

lines 108-113 of HttpsSession.java should be indented 4 more spaces. 
Otherwise looks ok to me.



Updated.

New webrev:
http://cr.openjdk.java.net/~xuelei/8212261/webrev.04/


Looks good.

--Sean

Re: A new proposal to add methods to HttpsURLConnection to access SSLSession

[Sean Mullan]

On 11/5/18 1:52 PM, Xuelei Fan wrote:

Hi Chris and Sean,

There are a few feedback for the CSR approval.  I updated to use 
Optional for the returned value.  For more details, please 
refer to:

   https://bugs.openjdk.java.net/browse/JDK-8213161
   http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/


I didn't see a test for SecureCacheResponse - is it possible? You could 
also add a test case for IllegalStateExc.


lines 108-113 of HttpsSession.java should be indented 4 more spaces. 
Otherwise looks ok to me.


--Sean




Please let me know if you are OK with this change.

Thanks,
Xuelei

On 11/2/2018 11:42 AM, Xuelei Fan wrote:

Thanks for the review and suggestions, Chris and Sean.

I just finalized the CSR.

Thanks,
Xuelei

On 11/2/2018 5:58 AM, Chris Hegarty wrote:

Thanks for the updates Xuelei, some minor comments inline..

On 1 Nov 2018, at 23:42, Xuelei Fan <mailto:xuelei@oracle.com>> wrote:


On 11/1/2018 11:24 AM, Sean Mullan wrote:

On 10/31/18 11:52 AM, Chris Hegarty wrote:

Xuelei,

On 30/10/18 20:55, Xuelei Fan wrote:

Hi,

For the current HttpsURLConnection, there is not much security 
parameters exposed in the public APIs.  An application may need 
richer information for the underlying TLS connections, for 
example the negotiated TLS protocol version.


Please let me know if you have concerns to add a new method 
HttpsURLConnection.getSSLSession() and deprecate the duplicated 
methods, by the end of Nov. 2, 2018.


Here is the proposal:
https://bugs.openjdk.java.net/browse/JDK-8213161
Are there any security issues associated with returning the 
SSLSession, since it is mutable?
It should be fine.  The update APIs of the session (invalidating, 
bind values) does not impact the connection.


Alternatively, as is done in the new HTTP Client, an immutable
SSLSession instance can be returned.

+ *   SHOULD override this method with appropriate 
implementation.

s/appropriate/an appropriate/
I would probably not capitalize "SHOULD" and just say "should". 
"SHOULD" is more common in RFCs. I don't see that much in javadocs.
+ * @implNote The JDK Reference Implementation supports this 
operation.
+ *   As an application may have to use this operation 
for more
+ *   security parameters, it is recommended to support 
this

+ *   operation in all implementations.
I think it should be obvious that the JDK implementation would 
override this method so not sure that first sentence is necessary. 
The other sentence seems like it could be combined with the 
previous sentence, ex:
"Subclasses should override this method with an appropriate 
implementation since an application may need to access additional 
parameters associated with the SSL session."

Updated accordingly, in the CSR and webrev:
https://bugs.openjdk.java.net/browse/JDK-8213161


The CSR looks good. I made a few minor edits to the verbiage
and added myself as reviewer.

The title will need to be updated to reflect the addition of the
new method in SecureCacheResponse. Maybe:

"Add SSLSession accessors to HttpsURLConnection and
SecureCacheResponse"

-Chris.

Re: A new proposal to add methods to HttpsURLConnection to access SSLSession

[Sean Mullan]

On 10/31/18 11:52 AM, Chris Hegarty wrote:

Xuelei,

On 30/10/18 20:55, Xuelei Fan wrote:

Hi,

For the current HttpsURLConnection, there is not much security 
parameters exposed in the public APIs.  An application may need richer 
information for the underlying TLS connections, for example the 
negotiated TLS protocol version.


Please let me know if you have concerns to add a new method 
HttpsURLConnection.getSSLSession() and deprecate the duplicated 
methods, by the end of Nov. 2, 2018.


Here is the proposal:
 https://bugs.openjdk.java.net/browse/JDK-8213161


Are there any security issues associated with returning the SSLSession, 
since it is mutable?


+ *   SHOULD override this method with appropriate 
implementation.


s/appropriate/an appropriate/

I would probably not capitalize "SHOULD" and just say "should". "SHOULD" 
is more common in RFCs. I don't see that much in javadocs.


+ * @implNote The JDK Reference Implementation supports this operation.
+ *   As an application may have to use this operation for more
+ *   security parameters, it is recommended to support this
+ *   operation in all implementations.

I think it should be obvious that the JDK implementation would override 
this method so not sure that first sentence is necessary. The other 
sentence seems like it could be combined with the previous sentence, ex:


"Subclasses should override this method with an appropriate 
implementation since an application may need to access additional 
parameters associated with the SSL session."



--Sean

Re: HttpURLConnection throws SunCertPathBuilderException in jdk11

[Sean Mullan]

This should be fixed in JDK 11 b23. Please try again. See 
https://bugs.openjdk.java.net/browse/JDK-8199779 for more info.


--Sean

On 6/25/18 12:28 AM, Jaikiran Pai wrote:
I couldn't locate this bug in the JIRA nor the bugs.java.net, to see if 
it's acknowledged as an issue. So FWIW - I can reproduce this even on 
MacOS (so it isn't just specific to Windows OS). This is the code:


import java.net.URL;
import java.io.InputStream;

public class CertTest {
     public static void main(final String[] args) throws Exception {
         final URL targetURL = new URL("https://api.vk.com/;);
         try (final InputStream is = 
targetURL.openConnection().getInputStream()) {

             is.read();
         }
     }
}


-Jaikiran


On 16/06/18 12:51 AM, Andrey Turbanov wrote:

Thank you for response.
I submitted bug to bugtracker. Iinternal review ID : 9055666
Didn't find a way to attach files there, but program example is short 
and can be easily run by anyone.



Andrey Turbanov.

2018-06-15 16:58 GMT+03:00 Sean Mullan <mailto:sean.mul...@oracle.com>>:


The 2nd (good) logfile looks like it is from a completely
different program - are you sure you are using the same code?

If it is, please rerun again and also add -Djavax.net.debug=all to
the command-line which should give a bit more debug info as to
where the issue is occurring in the TLS handshake.

I would also recommend filing a bug and attaching the logfiles so
that this is tracked and evaluated more formally:
https://bugreport.java.com/bugreport/
<https://bugreport.java.com/bugreport/>

If this is indeed a regression, it's important that we get to the
bottom of it.

Thanks,
Sean


On 6/12/18 11:10 AM, Андрей Турбанов wrote:

2 log files attached.

Андрей Турбанов

2018-06-12 15:40 GMT+03:00 Sean Mullan mailto:sean.mul...@oracle.com> <mailto:sean.mul...@oracle.com
<mailto:sean.mul...@oracle.com>>>:


    Please add -Djava.security.debug=certpath to the java
command line
    and attach the log file. Preferably, attach 2 log files,
one for a
    good run and one for a bad run. This should help show what the
    problem is.

    --Sean

    On 6/11/18 7:59 PM, Андрей Турбанов wrote:

        Hello.
        I tried to use early jdk11 build
(http://jdk.java.net/11/) -
        Oracle JDK build for Windows.
        I got exception when my program tries to connect (via
        HttpURLConnection) to https://api.vk.com/

   
sun.security.provider.certpath.SunCertPathBuilderException:

        unable to find valid certification path to requested
target
          at
   
sun.security.provider.certpath.SunCertPathBuilder.build(SunCertPathBuilder.java:141)

        ~[?:?]
          at
   
sun.security.provider.certpath.SunCertPathBuilder.engineBuild(SunCertPathBuilder.java:126)

        ~[?:?]
          at
   
java.security.cert.CertPathBuilder.build(CertPathBuilder.java:297)

        ~[?:?]
          at
   
sun.security.validator.PKIXValidator.doBuild(PKIXValidator.java:380)

        ~[?:?]
          at
   
sun.security.validator.PKIXValidator.engineValidate(PKIXValidator.java:290)

        ~[?:?]
          at
   
sun.security.validator.Validator.validate(Validator.java:264)

~[?:?]
          at
   
sun.security.ssl.X509TrustManagerImpl.validate(X509TrustManagerImpl.java:343)

        ~[?:?]

        Same code works well with JDK 10.
        Does JDK11 have different set of SSL certificates? Is
there any
        way to allow connection to vk.com <http://vk.com>
<http://vk.com> <http://vk.com>?

        Andrey Turbanov

Re: java.net.Socket should report the attempted address and port

[Sean Mullan]

Hi Michael,

I agree with Alan and Peter that the name should more clearly identify 
the security implications of setting it.


Alternatively, if you think you may build on this you might want to add 
support for a multi-valued property, like 
jdk.net.includeInExceptions=hostInfo,...


--Sean

On 6/14/18 1:41 PM, Michael McMahon wrote:

Hi Alan,

Thanks for looking at it.

On 14/06/2018, 18:10, Alan Bateman wrote:

On 06/06/2018 08:45, Michael McMahon wrote:

Hi all,

Finally to return to this topic. We have looked at a few different 
approaches
and it seems the best way is to define a security property that can 
be set
in the java.security configuration file, but which can be overridden 
as a
system property. The current behavior will remain the default, but 
setting

the property will add addressing information to exception texts.
The change applies to all TCP socket types in java.net and java.nio.
Webrev at: 
http://cr.openjdk.java.net/~michaelm/8204233/webrev.1/index.html
This looks quite good and the ability to use a system property to 
override the java.security file is useful for ad hoc enabling. The 
property name can probably be improved The jdk.net prefix looks right 
but jdk.net.enhanceExceptionText isn't very clear, esp. when used on 
the command line. It feels it should something like 
jdk.net.includeHostInfoInExceptions or something that makes it clear 
that it adds host information to socket exceptions.


That seems too specific to me. My feeling was that other exceptions 
might be enhanced in the same way and would
hang off the same property name. If we use a name that is specific to 
hostinfo, then we would need a new property

for other information types.

I see Jaikiran Pai spotted the close was accidentally removed from 
AbstractPlainSocketImpl so I assume you'll fix that.



Yes, that was fixed and the webrev updated in place.

Aside from AsynchronousCloseException, are there are other IOException 
classes that don't have the 1-arg String constructor. Just wondering 
if it would be better to special case that to not use SocketExceptions 
or alternative not rely on catching NoSuchMethodException.


The problem was I wrote it first checking types statically, and there 
were a lot of different exception types,
which is ugly enough to begin with but I also overlooked those NIO types 
completely.
It was just difficult to write a test that generated all the possible 
exceptions. So, my concern was overlooking
any future change in that area. Or are you suggesting we just not 
implement this for the async socket channels?


Thanks,

Michael

-Alan

Re: HttpURLConnection throws SunCertPathBuilderException in jdk11

[Sean Mullan]

The 2nd (good) logfile looks like it is from a completely different 
program - are you sure you are using the same code?


If it is, please rerun again and also add -Djavax.net.debug=all to the 
command-line which should give a bit more debug info as to where the 
issue is occurring in the TLS handshake.


I would also recommend filing a bug and attaching the logfiles so that 
this is tracked and evaluated more formally: 
https://bugreport.java.com/bugreport/


If this is indeed a regression, it's important that we get to the bottom 
of it.


Thanks,
Sean


On 6/12/18 11:10 AM, Андрей Турбанов wrote:

2 log files attached.

Андрей Турбанов

2018-06-12 15:40 GMT+03:00 Sean Mullan <mailto:sean.mul...@oracle.com>>:


Please add -Djava.security.debug=certpath to the java command line
and attach the log file. Preferably, attach 2 log files, one for a
good run and one for a bad run. This should help show what the
problem is.

--Sean

On 6/11/18 7:59 PM, Андрей Турбанов wrote:

Hello.
I tried to use early jdk11 build (http://jdk.java.net/11/) -
Oracle JDK build for Windows.
I got exception when my program tries to connect (via
HttpURLConnection) to https://api.vk.com/

sun.security.provider.certpath.SunCertPathBuilderException:
unable to find valid certification path to requested target
      at

sun.security.provider.certpath.SunCertPathBuilder.build(SunCertPathBuilder.java:141)
~[?:?]
      at

sun.security.provider.certpath.SunCertPathBuilder.engineBuild(SunCertPathBuilder.java:126)
~[?:?]
      at
java.security.cert.CertPathBuilder.build(CertPathBuilder.java:297)
~[?:?]
      at
sun.security.validator.PKIXValidator.doBuild(PKIXValidator.java:380)
~[?:?]
      at

sun.security.validator.PKIXValidator.engineValidate(PKIXValidator.java:290)
~[?:?]
      at
sun.security.validator.Validator.validate(Validator.java:264) ~[?:?]
      at

sun.security.ssl.X509TrustManagerImpl.validate(X509TrustManagerImpl.java:343)
~[?:?]

Same code works well with JDK 10.
Does JDK11 have different set of SSL certificates? Is there any
way to allow connection to vk.com <http://vk.com> <http://vk.com>?

Andrey Turbanov

Re: HttpURLConnection throws SunCertPathBuilderException in jdk11

[Sean Mullan]

Please add -Djava.security.debug=certpath to the java command line and 
attach the log file. Preferably, attach 2 log files, one for a good run 
and one for a bad run. This should help show what the problem is.


--Sean

On 6/11/18 7:59 PM, Андрей Турбанов wrote:

Hello.
I tried to use early jdk11 build (http://jdk.java.net/11/) - Oracle JDK 
build for Windows.
I got exception when my program tries to connect (via HttpURLConnection) 
to https://api.vk.com/


sun.security.provider.certpath.SunCertPathBuilderException: unable to 
find valid certification path to requested target
     at 
sun.security.provider.certpath.SunCertPathBuilder.build(SunCertPathBuilder.java:141) 
~[?:?]
     at 
sun.security.provider.certpath.SunCertPathBuilder.engineBuild(SunCertPathBuilder.java:126) 
~[?:?]
     at 
java.security.cert.CertPathBuilder.build(CertPathBuilder.java:297) ~[?:?]
     at 
sun.security.validator.PKIXValidator.doBuild(PKIXValidator.java:380) ~[?:?]
     at 
sun.security.validator.PKIXValidator.engineValidate(PKIXValidator.java:290) 
~[?:?]

     at sun.security.validator.Validator.validate(Validator.java:264) ~[?:?]
     at 
sun.security.ssl.X509TrustManagerImpl.validate(X509TrustManagerImpl.java:343) 
~[?:?]


Same code works well with JDK 10.
Does JDK11 have different set of SSL certificates? Is there any way to 
allow connection to vk.com ?


Andrey Turbanov

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

[Sean Mullan]

SSLParameters.java

649 applicationProtocols = protocols.clone();

You should clone the parameters before checking if they are valid. Move 
this to line 642, and check the validity of the cloned array. Also, use 
a temporary variable for the clone, so as not to pollute the 
applicationProtocols field until you confirm the values are valid, and 
then assign it to applicationProtocols.


ExtensionType.java

47 new ArrayList(15);

nit - you can use diamond operator above

--Sean

On 11/29/2015 07:08 PM, Vincent Ryan wrote:

Hello,

Following on from Brad’s recent email, here is the full webrev of the
API and the implementation classes for ALPN:
http://cr.openjdk.java.net/~vinnie/8144093/webrev.00/

In adds the implementation classes (sun/security/ssl) to the public API
classes (javax/net/ssl) which have already been agreed.
Some basic tests (test/javax/net/ssl) are also included.

Please send any code review comments by close-of-business on Tuesday 1
December.
Thanks.

Re: [9] RFR: 8074531: Remove javax.security.cert.X509Certificate usage in internal networking packages

[Sean Mullan]

Hi Jason,

* HttpsURLConnection.java, HttpsURLConnectionOldImpl.java

This looks fine but as a next-step, we may be able to completely remove 
these classes, which have been deprecated for a long time and are 
implementation-specific. Can you check with Brad/Xuelei and open a 
separate issue to remove them if so?


Otherwise, looks good.

--Sean

On 03/18/2015 05:52 PM, Jason Uh wrote:

Please review this change, which removes methods in internal net
packages that use the deprecated javax.security.cert.X509Certificate type.

webrev: http://cr.openjdk.java.net/~juh/8074531/00/
bug: https://bugs.openjdk.java.net/browse/JDK-8074531

Thanks,
Jason

hg: jdk8/tl/jdk: 8031825: OCSP client can't find responder cert if it uses a different subject key id algorithm than responderID

[sean . mullan]

Changeset: 57c26829deb6
Author:mullan
Date:  2014-01-22 19:06 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/57c26829deb6

8031825: OCSP client can't find responder cert if it uses a different subject 
key id algorithm than responderID
Reviewed-by: vinnie, xuelei

! src/share/classes/sun/security/provider/certpath/OCSPResponse.java

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: aef6c726810e
Author:mullan
Date:  2013-12-23 14:03 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/aef6c726810e

8030813: Signed applet fails to load when CRLs are stored in an LDAP directory
Summary: Skip JNDI application resource lookup to avoid recursive JAR validation
Reviewed-by: vinnie, herrick

! src/share/classes/com/sun/naming/internal/ResourceManager.java
! src/share/classes/sun/security/provider/certpath/ldap/LDAPCertStore.java

Changeset: f3c714eeef6c
Author:mullan
Date:  2013-12-23 14:05 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f3c714eeef6c

Merge

- src/share/classes/sun/util/resources/pt/LocaleNames_pt_BR.properties
- test/sun/security/ssl/javax/net/ssl/SSLContextVersion.java

Re: RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

[Sean Mullan]

Hi Volker,

The security changes look fine. I'm not crazy that we now have to 
maintain one additional java.security file which is the exact same as 
java.security-linux, but this is really an existing issue with 
duplicated content across the java.security files which I will try to 
fix early in JDK 9: https://bugs.openjdk.java.net/browse/JDK-6997010


Thanks,
Sean

On 11/20/2013 01:26 PM, Volker Simonis wrote:

Hi,

this is the second review round for 8024854: Basic changes and files to
build the class library on
AIXhttps://bugs.openjdk.java.net/browse/JDK-8024854.
The previous reviews can be found at the end of this mail in the references
section.

I've tried to address all the comments and suggestions from the first round
and to further streamline the patch (it perfectly builds on Linux/x86_64,
Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). The biggest change
compared to the first review round is the new aix/ subdirectory which
I've now created under jdk/src and which contains AIX-only code.

The webrev is against our
http://hg.openjdk.java.net/ppc-aix-port/stagerepository which has been
recently synchronised with the jdk8 master:

http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/

Below (and in the webrev) you can find some comments which explain the
changes to each file. In order to be able to push this big change, I need
the approval of reviewers from the lib, net, svc, 2d, awt and sec groups.
So please don't hesitate to jump at it - there are enough changes for
everybody:)

Thank you and best regards,
Volker

*References:*

The following reviews have been posted so far (thanks Iris for collecting
them :)

- Alan Bateman (lib): Initial comments (16 Sep [2])
- Chris Hegarty (lib/net): Initial comments (20 Sep [3])
- Michael McMahon (net): Initial comments (20 Sept [4])
- Steffan Larsen (svc): APPROVED (20 Sept [5])
- Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments (15
Oct [7])
- Sean Mullan (sec): Initial comments (26 Sept [8])

[2]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html
[3]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html
[4]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html
[5]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html
[6]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html
[7]: http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html
[8]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html


*Detailed change description:*

The new jdk/src/aix subdirectory contains the following new and
AIX-specific files for now:

  src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
  src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
  src/aix/classes/sun/nio/ch/AixPollPort.java
  src/aix/classes/sun/nio/fs/AixFileStore.java
  src/aix/classes/sun/nio/fs/AixFileSystem.java
  src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
  src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
  src/aix/classes/sun/tools/attach/AixAttachProvider.java
  src/aix/classes/sun/tools/attach/AixVirtualMachine.java
  src/aix/native/java/net/aix_close.c
  src/aix/native/sun/nio/ch/AixPollPort.c
  src/aix/native/sun/nio/fs/AixNativeDispatcher.c
  src/aix/native/sun/tools/attach/AixVirtualMachine.c
  src/aix/porting/porting_aix.c
  src/aix/porting/porting_aix.h

src/aix/porting/porting_aix.c
src/aix/porting/porting_aix.h

- Added these two files for AIX relevant utility code.
- Currently these files only contain an implementation of dladdr which
is not available on AIX.
- In the first review round the existing dladdr users in the code either
called the version from the HotSpot on AIX (
src/solaris/native/sun/awt/awt_LoadLibrary.c) or had a private copy of
the whole implementation (src/solaris/demo/jvmti/hprof/hprof_md.c). This
is now not necessary any more.

  The new file layout required some small changes to the makefiles to make
them aware of the new directory locations:

makefiles/CompileDemos.gmk

- Add an extra argument to SetupJVMTIDemo which can be used to pass
additional source locations.
- Currently this is only used on AIX for the AIX porting utilities which
are required by hprof.

makefiles/lib/Awt2dLibraries.gmk
makefiles/lib/ServiceabilityLibraries.gmk

- On AIX add the sources of the AIX porting utilities to the build. They
are required by src/solaris/native/sun/awt/awt_LoadLibrary.c and
src/solaris/demo/jvmti/hprof/hprof_md.c because dladdr is not available
on AIX.

makefiles/lib/NioLibraries.gmk

- Make the AIX-build aware of the new NIO source locations under
src/aix/native/sun/nio/.

makefiles/lib/NetworkingLibraries.gmk

- Make the AIX-build aware of the new aix_close.c source location under
src/aix/native/java/net/.

src/share/bin/jli_util.h

hg: jdk8/tl/jdk: 3 new changesets

[sean . mullan]

Changeset: 5f4aecd73caa
Author:mullan
Date:  2013-10-22 08:03 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5f4aecd73caa

8021191: Add isAuthorized check to limited doPrivileged methods
Reviewed-by: weijun, xuelei

! src/share/classes/java/security/AccessControlContext.java
! src/share/classes/java/security/AccessController.java

Changeset: 948b390b6952
Author:mullan
Date:  2013-10-22 08:17 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/948b390b6952

Merge

- make/sun/awt/FILES_c_macosx.gmk
- make/sun/awt/FILES_export_macosx.gmk
- makefiles/GendataBreakIterator.gmk
- makefiles/GendataFontConfig.gmk
- makefiles/GendataHtml32dtd.gmk
- makefiles/GendataTZDB.gmk
- makefiles/GendataTimeZone.gmk
- makefiles/GenerateJavaSources.gmk
- makefiles/GensrcBuffer.gmk
- makefiles/GensrcCLDR.gmk
- makefiles/GensrcCharacterData.gmk
- makefiles/GensrcCharsetCoder.gmk
- makefiles/GensrcCharsetMapping.gmk
- makefiles/GensrcExceptions.gmk
- makefiles/GensrcIcons.gmk
- makefiles/GensrcJDWP.gmk
- makefiles/GensrcJObjC.gmk
- makefiles/GensrcLocaleDataMetaInfo.gmk
- makefiles/GensrcMisc.gmk
- makefiles/GensrcProperties.gmk
- makefiles/GensrcSwing.gmk
- makefiles/GensrcX11Wrappers.gmk
- src/macosx/classes/com/apple/resources/MacOSXResourceBundle.java
- src/macosx/native/com/apple/resources/MacOSXResourceBundle.m
- src/share/classes/java/lang/invoke/MagicLambdaImpl.java
- src/share/classes/java/net/HttpURLPermission.java
- src/solaris/doc/sun/man/man1/ja/javaws.1
- src/solaris/doc/sun/man/man1/javaws.1
- test/java/net/HttpURLPermission/HttpURLPermissionTest.java
- test/java/net/HttpURLPermission/URLTest.java
- test/java/net/HttpURLPermission/policy.1
- test/java/net/HttpURLPermission/policy.2
- test/java/net/HttpURLPermission/policy.3

Changeset: 3ea9af449b36
Author:mullan
Date:  2013-10-22 09:06 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3ea9af449b36

Merge

- test/java/net/NetworkInterface/MemLeakTest.java

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: fc7a6fa3589a
Author:ascarpino
Date:  2013-10-22 19:37 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fc7a6fa3589a

8025763: Provider does not override new Hashtable methods
Reviewed-by: mullan

! src/share/classes/java/security/Provider.java

Changeset: b065de1700d3
Author:mullan
Date:  2013-10-22 19:43 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b065de1700d3

Merge

- src/share/demo/jfc/Notepad/resources/Notepad_fr.properties
- src/share/demo/jfc/Notepad/resources/Notepad_sv.properties

hg: jdk8/tl/jdk: 3 new changesets

[sean . mullan]

Changeset: 5d866df64ae3
Author:mullan
Date:  2013-10-17 10:18 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5d866df64ae3

8026346: test/java/lang/SecurityManager/CheckPackageAccess.java failing
Reviewed-by: vinnie

! src/share/lib/security/java.security-macosx
! test/java/lang/SecurityManager/CheckPackageAccess.java

Changeset: 04e8b8fc6906
Author:mullan
Date:  2013-10-17 10:37 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/04e8b8fc6906

Merge

- make/sun/awt/FILES_c_macosx.gmk
- make/sun/awt/FILES_export_macosx.gmk
- src/macosx/classes/com/apple/resources/MacOSXResourceBundle.java
- src/macosx/native/com/apple/resources/MacOSXResourceBundle.m
- src/share/classes/java/net/HttpURLPermission.java
- test/java/net/HttpURLPermission/HttpURLPermissionTest.java
- test/java/net/HttpURLPermission/URLTest.java
- test/java/net/HttpURLPermission/policy.1
- test/java/net/HttpURLPermission/policy.2
- test/java/net/HttpURLPermission/policy.3

Changeset: 85a73ad28c53
Author:mullan
Date:  2013-10-17 11:34 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/85a73ad28c53

Merge

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 4ad76262bac8
Author:mullan
Date:  2013-10-11 08:43 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4ad76262bac8

8007292: Add JavaFX internal packages to package.access
Summary: build hooks to allow closed restricted packages to be added to 
java.security file
Reviewed-by: erikj, dholmes, tbell

! make/java/security/Makefile
! make/tools/Makefile
+ make/tools/addtorestrictedpkgs/Makefile
+ make/tools/src/build/tools/addtorestrictedpkgs/AddToRestrictedPkgs.java
! makefiles/CopyFiles.gmk
! makefiles/Tools.gmk
! test/java/lang/SecurityManager/CheckPackageAccess.java

Changeset: 76df579c0b93
Author:mullan
Date:  2013-10-11 09:17 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/76df579c0b93

Merge

! makefiles/Tools.gmk
- src/macosx/classes/sun/lwawt/SelectionClearListener.java
- src/macosx/classes/sun/lwawt/macosx/CMouseInfoPeer.java
- src/share/classes/com/sun/jdi/connect/package.html
- src/share/classes/com/sun/jdi/connect/spi/package.html
- src/share/classes/com/sun/jdi/event/package.html
- src/share/classes/com/sun/jdi/package.html
- src/share/classes/com/sun/jdi/request/package.html
- src/share/classes/com/sun/management/package.html
- src/share/classes/com/sun/tools/attach/package.html
- src/share/classes/com/sun/tools/attach/spi/package.html
- src/share/classes/com/sun/tools/jconsole/package.html
- src/share/classes/java/lang/invoke/InvokeGeneric.java
- src/share/classes/java/time/chrono/ChronoDateImpl.java
- test/com/oracle/security/ucrypto/TestAES.java
- test/com/oracle/security/ucrypto/TestDigest.java
- test/com/oracle/security/ucrypto/TestRSA.java
- test/com/oracle/security/ucrypto/UcryptoTest.java
- test/com/sun/jdi/Solaris32AndSolaris64Test.sh
- 
test/java/nio/channels/spi/SelectorProvider/inheritedChannel/lib/solaris-i586/libLauncher.so
- 
test/java/nio/channels/spi/SelectorProvider/inheritedChannel/lib/solaris-sparc/libLauncher.so
- test/java/time/tck/java/time/chrono/TCKChronologySerialization.java

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 0aba8b6232af
Author:mullan
Date:  2013-09-06 12:04 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0aba8b6232af

8023362: Don't allow soft-fail behavior if OCSP responder returns unauthorized
Reviewed-by: vinnie, xuelei

! src/share/classes/java/security/cert/PKIXRevocationChecker.java
! src/share/classes/sun/security/provider/certpath/OCSPResponse.java
+ test/java/security/cert/PKIXRevocationChecker/OcspUnauthorized.java

Changeset: f23a84a1cf8e
Author:mullan
Date:  2013-09-06 12:10 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f23a84a1cf8e

Merge

- src/share/classes/java/util/stream/CloseableStream.java
- src/share/classes/java/util/stream/DelegatingStream.java
- test/java/util/Map/TreeBinSplitBackToEntries.java
- test/sun/tools/jconsole/ImmutableResourceTest.java
- test/sun/tools/jconsole/ImmutableResourceTest.sh

hg: jdk8/tl/jdk: 8023769: JDK-8016850 broke the old build

[sean . mullan]

Changeset: 134283a88499
Author:mullan
Date:  2013-08-27 10:46 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/134283a88499

8023769: JDK-8016850 broke the old build
Summary: remove files that were moved/removed from 
com/sun/security/auth/FILES_java.gmk
Reviewed-by: chegar, xuelei

! make/com/sun/security/auth/FILES_java.gmk

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 6a1bfcde4d4d
Author:mullan
Date:  2013-08-27 12:04 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6a1bfcde4d4d

8019830: Add com.sun.media.sound to the list of restricted package
Reviewed-by: vinnie

! src/share/lib/security/java.security-linux
! src/share/lib/security/java.security-macosx
! src/share/lib/security/java.security-solaris
! src/share/lib/security/java.security-windows
! test/java/lang/SecurityManager/CheckPackageAccess.java

Changeset: 3575263eefab
Author:mullan
Date:  2013-08-27 12:27 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3575263eefab

Merge

- src/share/classes/com/sun/security/auth/PolicyParser.java
- src/share/classes/com/sun/security/auth/SubjectCodeSource.java
- src/share/classes/sun/misc/Compare.java
- src/share/classes/sun/misc/Sort.java

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: bce5205dbe84
Author:ascarpino
Date:  2013-08-14 10:50 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bce5205dbe84

8022669: OAEPParameterSpec does not work if MGF1ParameterSpec is set to SHA2 
algorithms
Reviewed-by: mullan

! src/share/classes/sun/security/rsa/RSAPadding.java
! test/com/sun/crypto/provider/Cipher/RSA/TestOAEPPadding.java

Changeset: b40d246d4d81
Author:ascarpino
Date:  2013-08-16 12:31 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b40d246d4d81

8022896: test/com/sun/crypto/provider/Cipher/RSA/TestOAEPPadding.java fails
Reviewed-by: mullan

! test/ProblemList.txt

hg: jdk8/tl/jdk: 8016850: JCK javax.security.auth.Policy tests fail when run in Profiles mode

[sean . mullan]

Changeset: f120e2c4b4b1
Author:mullan
Date:  2013-08-19 17:17 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f120e2c4b4b1

8016850: JCK javax.security.auth.Policy tests fail when run in Profiles mode
Summary: Move default javax.security.auth.Policy implementation to compact1 
profile
Reviewed-by: vinnie

! src/share/classes/com/sun/security/auth/PolicyFile.java
- src/share/classes/com/sun/security/auth/PolicyParser.java
- src/share/classes/com/sun/security/auth/SubjectCodeSource.java
! src/share/classes/javax/security/auth/Policy.java
+ src/share/classes/sun/security/provider/AuthPolicyFile.java
+ src/share/classes/sun/security/provider/SubjectCodeSource.java

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 1f4af3e0447e
Author:mullan
Date:  2013-08-06 08:31 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1f4af3e0447e

8022120: JCK test api/javax_xml/crypto/dsig/TransformService/index_ParamMethods 
fails
Summary: TransformService.init and marshalParams must throw 
NullPointerException when parent parameter is null
Reviewed-by: xuelei

! src/share/classes/org/jcp/xml/dsig/internal/dom/ApacheCanonicalizer.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/ApacheTransform.java
+ test/javax/xml/crypto/dsig/TransformService/NullParent.java

Changeset: ba634b53f53a
Author:mullan
Date:  2013-08-06 08:34 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ba634b53f53a

Merge

hg: jdk8/tl/jdk: 3 new changesets

[sean . mullan]

Changeset: 42b786f2fb99
Author:mullan
Date:  2013-08-02 08:30 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/42b786f2fb99

8001319: Add SecurityPermission insertProvider target name
Reviewed-by: vinnie

! src/share/classes/java/security/Security.java
! src/share/classes/java/security/SecurityPermission.java
+ test/java/security/Security/AddProvider.java
+ test/java/security/Security/AddProvider.policy.1
+ test/java/security/Security/AddProvider.policy.2
+ test/java/security/Security/AddProvider.policy.3

Changeset: 7bbc6c2351d7
Author:mullan
Date:  2013-08-02 08:37 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7bbc6c2351d7

Merge

- src/share/classes/java/net/package.html
- src/share/classes/java/util/stream/StreamBuilder.java
- src/share/classes/javax/security/auth/callback/package.html
- src/share/classes/javax/security/auth/kerberos/package.html
- src/share/classes/javax/security/auth/login/package.html
- src/share/classes/javax/security/auth/package.html
- src/share/classes/javax/security/auth/spi/package.html
- src/share/classes/javax/security/auth/x500/package.html
- src/share/classes/javax/security/cert/package.html
- src/share/classes/javax/security/sasl/package.html
- test/java/util/Collections/EmptySortedSet.java

Changeset: 0a778e487a73
Author:mullan
Date:  2013-08-02 09:38 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0a778e487a73

Merge

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: a834ab2c1354
Author:mullan
Date:  2013-07-25 10:58 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a834ab2c1354

8010748: Add PKIXRevocationChecker NO_FALLBACK option and improve SOFT_FAIL 
option
Reviewed-by: vinnie

! src/share/classes/java/security/cert/PKIXRevocationChecker.java
! src/share/classes/sun/security/provider/certpath/OCSP.java
! src/share/classes/sun/security/provider/certpath/OCSPResponse.java
! src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java
! src/share/classes/sun/security/provider/certpath/ReverseState.java
! src/share/classes/sun/security/provider/certpath/RevocationChecker.java
! src/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java
! test/java/security/cert/PKIXRevocationChecker/UnitTest.java

Changeset: 22a391706a0b
Author:mullan
Date:  2013-07-25 11:09 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/22a391706a0b

Merge

- make/sun/xawt/ToBin.java
- makefiles/sun/awt/X11/ToBin.java
- 
src/share/classes/com/sun/org/apache/xml/internal/security/resource/log4j.properties
- 
src/share/classes/com/sun/org/apache/xml/internal/security/transforms/implementations/FuncHereContext.java
- 
src/share/classes/com/sun/org/apache/xml/internal/security/utils/CachedXPathAPIHolder.java
- 
src/share/classes/com/sun/org/apache/xml/internal/security/utils/CachedXPathFuncHereAPI.java
- 
src/share/classes/com/sun/org/apache/xml/internal/security/utils/XPathFuncHereAPI.java
- src/share/classes/java/security/acl/package.html
- src/share/classes/java/security/cert/package.html
- src/share/classes/java/security/interfaces/package.html
- src/share/classes/java/security/package.html
- src/share/classes/java/security/spec/package.html
- src/share/classes/java/util/stream/StreamBuilder.java
- src/share/classes/javax/security/auth/callback/package.html
- src/share/classes/javax/security/auth/kerberos/package.html
- src/share/classes/javax/security/auth/login/package.html
- src/share/classes/javax/security/auth/package.html
- src/share/classes/javax/security/auth/spi/package.html
- src/share/classes/javax/security/auth/x500/package.html
- src/share/classes/javax/security/cert/package.html
- src/share/classes/javax/security/sasl/package.html
- src/share/classes/sun/misc/Hashing.java
- src/share/classes/sun/security/krb5/internal/rcache/CacheTable.java
- src/share/classes/sun/security/krb5/internal/rcache/ReplayCache.java
! src/share/classes/sun/security/provider/certpath/RevocationChecker.java
- src/solaris/classes/sun/awt/X11/XIconInfo.java
- src/solaris/classes/sun/awt/X11/security-icon-bw16.png
- src/solaris/classes/sun/awt/X11/security-icon-bw24.png
- src/solaris/classes/sun/awt/X11/security-icon-bw32.png
- src/solaris/classes/sun/awt/X11/security-icon-bw48.png
- src/solaris/classes/sun/awt/X11/security-icon-interim16.png
- src/solaris/classes/sun/awt/X11/security-icon-interim24.png
- src/solaris/classes/sun/awt/X11/security-icon-interim32.png
- src/solaris/classes/sun/awt/X11/security-icon-interim48.png
- src/solaris/classes/sun/awt/X11/security-icon-yellow16.png
- src/solaris/classes/sun/awt/X11/security-icon-yellow24.png
- src/solaris/classes/sun/awt/X11/security-icon-yellow32.png
- src/solaris/classes/sun/awt/X11/security-icon-yellow48.png
- src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.Fedora.properties
- src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.SuSE.properties
- src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.Ubuntu.properties
- src/solaris/classes/sun/awt/fontconfigs/linux.fontconfig.properties
- test/java/lang/invoke/7196190/MHProxyTest.java
- test/java/util/Collections/EmptySortedSet.java
- test/java/util/Comparators/BasicTest.java
- test/sun/misc/Hashing.java
- test/sun/security/krb5/auto/ReplayCache.java

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 1744a32d3db3
Author:mullan
Date:  2013-07-25 20:12 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1744a32d3db3

8012288: XML DSig API allows wrong tag names and extra elements in SignedInfo
Reviewed-by: xuelei

! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMKeyValue.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMManifest.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMReference.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMRetrievalMethod.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureProperties.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignedInfo.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMUtils.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMX509IssuerSerial.java
! src/share/classes/org/jcp/xml/dsig/internal/dom/DOMXMLSignature.java

Changeset: 4100ab44ba4f
Author:mullan
Date:  2013-07-25 20:30 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4100ab44ba4f

Merge

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 028ef97797c1
Author:mullan
Date:  2013-07-05 15:54 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/028ef97797c1

8011547: Update XML Signature implementation to Apache Santuario 1.5.4
Reviewed-by: xuelei

! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/Algorithm.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/JCEMapper.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/MessageDigestAlgorithm.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/SignatureAlgorithm.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/SignatureAlgorithmSpi.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/IntegrityHmac.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureBaseRSA.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureDSA.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureECDSA.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/CanonicalizationException.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/Canonicalizer.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/CanonicalizerSpi.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/InvalidCanonicalizerException.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/helper/AttrCompare.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/helper/C14nHelper.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer11.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer11_OmitComments.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer11_WithComments.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer20010315.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer20010315Excl.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer20010315ExclOmitComments.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer20010315ExclWithComments.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer20010315OmitComments.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/Canonicalizer20010315WithComments.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/CanonicalizerBase.java
+ 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/CanonicalizerPhysical.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/NameSpaceSymbTable.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/c14n/implementations/UtfHelpper.java
+ 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/AbstractSerializer.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/AgreementMethod.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/CipherData.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/CipherReference.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/CipherValue.java
+ 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/DocumentSerializer.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/EncryptedData.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/EncryptedKey.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/EncryptedType.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/EncryptionMethod.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/EncryptionProperties.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/EncryptionProperty.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/Reference.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/ReferenceList.java
+ 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/Serializer.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/Transforms.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/XMLCipher.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/XMLCipherInput.java
! 
src/share/classes/com/sun/org/apache/xml/internal/security/encryption/XMLCipherParameters.java
! 

hg: jdk8/tl/jdk: 8014307: Memory leak ... security/jgss/wrapper/GSSLibStub.c

[sean . mullan]

Changeset: f695f447f6b7
Author:jzavgren
Date:  2013-06-14 09:13 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f695f447f6b7

8014307: Memory leak ... security/jgss/wrapper/GSSLibStub.c
Summary: I modified the native procedure: 
Java_sun_security_jgss_wrapper_GSSLibStub_initContext() so that allocated 
memory is freed when errors occur.
Reviewed-by: chegar, valeriep

! src/share/native/sun/security/jgss/wrapper/GSSLibStub.c

Re: RFR: 8012261: update policytool to support java.net.HttpURLPermission

[Sean Mullan]

Looks fine to me.

--Sean

On 05/16/2013 10:17 PM, Weijun Wang wrote:

Hi All

Please take a look at

 http://cr.openjdk.java.net/~weijun/8012261/webrev.00/

which supports the new HttpURLPermission type introduced in

 8010464: Evolve java networking same origin policy
 http://hg.openjdk.java.net/jdk8/tl/jdk/rev/93a268759ec3

Noreg-trivial.

Thanks
Max

Re: 8008662: Add @jdk.Supported to JDK-specific/supported API

[Sean Mullan]

The security related ones look ok to me.

--Sean

On 02/21/2013 01:46 PM, Alan Bateman wrote:


Joe Darcy recently added @jdk.Supported [1] to make it possible to
identify JDK-specific APIs.

I'd like to add this to a number of APIs in the com.sun namespace to
make it obvious these are supported.  Specifically I'm proposing to
add it to:

- Java Debug Interface (com.sun.jdi)
- Attach API (com.sun.tools.attach)
- SCTP API (com.sun.nio.sctp)
- HTTP server API (com.sun.net.httpserver)
- Management extensions (com.sun.management)
- JDK-specific API to JAAS (com.sun.security.auth)
- JDK-specific JGSS API (com.sun.security.jgss)

The javadoc for all of these is generated as part of the regular JDK
docs build and so shouldn't be controversial. There are a number of
other candidates in com.sun with murkier status that I've stayed clear
of for now.

The webrev with the changes is here:

http://cr.openjdk.java.net/~alanb/8008662/webrev/

In a couple of cases the package description is legacy package.html so
I've had to move/convert them to package-info.java.

In all but one case I've added the annotation to the package-info, the
one exception is com.sun.management where there is at least one type
that is documented as not supported. Joe Darcy might have suggestions
on that.

Otherwise this is mostly mechanical and the patch file is easier to
review that the webrev.

-Alan

[1] http://hg.openjdk.java.net/jdk8/tl/langtools/rev/55cca2f38ee6

hg: jdk8/tl/jdk: 8008107: [parfait] Use after free of pointer in jdk/src/share/native/sun/security/pkcs11/wrapper/p11_convert.c

[sean . mullan]

Changeset: 267bca6af07e
Author:jzavgren
Date:  2013-02-19 15:31 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/267bca6af07e

8008107: [parfait] Use after free of pointer in 
jdk/src/share/native/sun/security/pkcs11/wrapper/p11_convert.c
Reviewed-by: mullan, chegar

! src/share/native/sun/security/pkcs11/wrapper/p11_convert.c

hg: jdk8/tl/jdk: 8006813: Compilation error in PKCS12KeyStore.java

[sean . mullan]

Changeset: 89f37f7188df
Author:mullan
Date:  2013-01-23 20:46 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/89f37f7188df

8006813: Compilation error in PKCS12KeyStore.java
Reviewed-by: valeriep

! src/share/classes/sun/security/pkcs12/PKCS12KeyStore.java

hg: jdk8/tl/jdk: 8005389: Backout fix for JDK-6500133

[sean . mullan]

Changeset: c7d54f93d3e5
Author:juh
Date:  2013-01-16 09:51 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c7d54f93d3e5

8005389: Backout fix for JDK-6500133
Reviewed-by: mullan

! src/share/classes/sun/security/x509/URIName.java
! test/sun/security/x509/URIName/Parse.java

hg: jdk8/tl/jdk: 8005939: sun/security/x509/{X509CRLImplX509CertImpl}/Verify.java fail in confusing way when some providers not present

[sean . mullan]

Changeset: f7f77bdf248b
Author:juh
Date:  2013-01-16 13:35 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f7f77bdf248b

8005939: sun/security/x509/{X509CRLImplX509CertImpl}/Verify.java fail in 
confusing way when some providers not present
Reviewed-by: mullan, weijun

! test/sun/security/x509/X509CRLImpl/Verify.java
! test/sun/security/x509/X509CertImpl/Verify.java

hg: jdk8/tl/jdk: 3 new changesets

[sean . mullan]

Changeset: 86828e84654f
Author:mullan
Date:  2013-01-08 19:00 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/86828e84654f

7019834: Eliminate dependency from PolicyFile to 
com.sun.security.auth.PrincipalComparator
Summary: Add new java.security.Principal.implies method
Reviewed-by: alanb

! src/share/classes/java/security/Principal.java
! src/share/classes/sun/security/provider/PolicyFile.java
! src/share/classes/sun/security/provider/PolicyParser.java
! src/share/classes/sun/security/tools/policytool/PolicyTool.java
+ test/java/security/Principal/Implies.java
! test/sun/security/provider/PolicyFile/Comparator.java

Changeset: bf6d0bca5ea7
Author:mullan
Date:  2013-01-08 19:02 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bf6d0bca5ea7

Merge

- make/jdk/asm/Makefile
- src/share/classes/sun/awt/TextureSizeConstraining.java
- src/share/lib/security/java.security
- test/java/rmi/server/Unmarshal/checkUnmarshalOnStopThread/CheckUnmarshall.java

Changeset: f0ed9ef84637
Author:mullan
Date:  2013-01-09 08:59 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f0ed9ef84637

Merge

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 4d28776d7007
Author:mullan
Date:  2012-12-26 10:07 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4d28776d7007

8005117: Eliminate dependency from ConfigSpiFile to 
com.sun.security.auth.login.ConfigFile
Reviewed-by: alanb, mchung, weijun

! src/share/classes/com/sun/security/auth/login/ConfigFile.java
! src/share/classes/sun/security/provider/ConfigSpiFile.java

Changeset: d9cab18f326a
Author:mullan
Date:  2012-12-26 10:08 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d9cab18f326a

Merge

- make/jdk/asm/Makefile

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: c97618a3c8c2
Author:juh
Date:  2012-12-13 09:35 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c97618a3c8c2

7193792: sun/security/pkcs11/ec/TestECDSA.java failing intermittently
Reviewed-by: vinnie, wetmore

! test/ProblemList.txt
! test/sun/security/pkcs11/ec/TestECDSA.java

Changeset: 7b697da6626a
Author:mullan
Date:  2012-12-13 09:37 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7b697da6626a

Merge