Re: Low level hooks in JDK for instrumentation of permission checks.
Hi Sean, Sorry I've confused you. What I should have said is a ProtectionDomain with a null CodeSource. What I mean to ask is, where ProtectionDomain is created with a null CodeSource, in Class::getProtectionDomain() can we have CodeSource's that represents system modules instead of null? A CodeSource with URL's like jrt:/jdk.* or jrt:/java.* for system modules? Hopefully my comments below will make a little more sense now. Regards, Peter. On 10/06/2021 1:07 am, Sean Mullan wrote: On 6/8/21 9:35 PM, Peter Firmstone wrote: I would also like to request that all JDK modules be given ProtectionDomain's following SecurityManager deprecation. Currently some modules have null ProtectionDomain's to show they have AllPermission. However we don't grant AllPermission to code in practise, we like to grant certain Permission's to Principal's, not code, where the Principal is the source of data, indicating the user has been authenticated and we only grant what's necessary and no more. As described in JEP 411, there are no plans to deprecate ProtectionDomain at this time. --Sean
Re: RFR: 8209092: Remove outdated wording from RC5ParameterSpec [v3]
> The RC5ParameterSpec class description contains the following sentence: "This > class can be used to initialize a Cipher object that implements the RC5 > algorithm as supplied by RSA Security LLC, or any parties authorized by RSA > Security." > > The part "as supplied by RSA Security LLC, or any parties authorized by RSA > Security." should be removed. We don't generally include information about > 3rd-party JCA providers in the standard javadocs. Also the "authorized" part > was probably referring to the RC5 patent but that has since expired. Jack Hartstein has updated the pull request incrementally with one additional commit since the last revision: remove extra reference to RFC 2040 - Changes: - all: https://git.openjdk.java.net/jdk/pull/4443/files - new: https://git.openjdk.java.net/jdk/pull/4443/files/bf6b9096..1d20ebc9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4443=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4443=01-02 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4443.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4443/head:pull/4443 PR: https://git.openjdk.java.net/jdk/pull/4443
Re: RFR: 8209092: Remove outdated wording from RC5ParameterSpec [v2]
On Wed, 9 Jun 2021 20:55:42 GMT, Jack Hartstein
wrote:
>> src/java.base/share/classes/javax/crypto/spec/RC5ParameterSpec.java line 39:
>>
>>> 37: *
>>> 38: * This class can be used to initialize a {@code Cipher} object that
>>> 39: * implements the RC5 algorithm as specified in >> href="https://datatracker.ietf.org/doc/html/rfc2040;>RFC 2040.
>>
>> Try to keep lines to <= 80 chars. Maybe break after "in"
>
> Fixed. I'll check on the CSR.
This class already references RFC 2040 (see line 32) so on that basis a CSR for
this specific change is probably not required.
But the "RC5" algorithm is listed as a standard algorithm in the Java Security
Standard Algorithm Names specification but the definition does not reference
RFC 2040:
https://docs.oracle.com/en/java/javase/16/docs/specs/security/standard-names.html#cipher-algorithm-names
Instead it has this description: "Variable-key-size encryption algorithms
developed by Ron Rivest for RSA Data Security, Inc.". That definition is
somewhat dated and I think we should just replace this with "The RC5 algorithm
as specified in RFC 2040" to match what you have in the javadoc. But that type
of change probably requires a CSR since it would be modifying that
specification.
I think you could either tackle that change as part of this, or file a
follow-on issue to update the RC5 algorithm in the Standard Algorithm Names
specification.
-
PR: https://git.openjdk.java.net/jdk/pull/4443
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed [v2]
On Wed, 9 Jun 2021 20:56:32 GMT, Xue-Lei Andrew Fan wrote:
>> Evan Whelan has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Added new line at end of file
>> - Cleaned up test case
>
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1332:
>
>> 1330: // ignore the exception
>> 1331: if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
>> 1332: SSLLogger.warning("output stream close failed.
>> Debug info only. Exception details:", ioe);
>
> I may look at this bug report different. It is a problem that the user does
> not understand the debug log properly. Debug log is for debug information
> only, and the debug log level indicates the level of the message.
>
> It looks like there is too much duplicated information. A log message has
> already indicated that the message is debug information only. Otherwise,
> exception should has been thrown in application level. The adding of
> "Exception details:" adds unnecessary dependency of the exception logging
> format.
>
> It may be fine to keep it unchanged, as if the users understand the logging
> message and logging levels. This kind of information normally means there is
> something that an application should take care of. That why we use a warning
> level log, rather than a fine level log message.
>
> It we really want an update, may be we could have a documentation enhancement
> instead.
>
> Similar comments for other update.
Sorry that I did not have my comment earlier, and while I was typing the
comment the update was integrated. Please just ignore this comment.
-
PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: 8209092: Remove outdated wording from RC5ParameterSpec [v2]
> The RC5ParameterSpec class description contains the following sentence: "This > class can be used to initialize a Cipher object that implements the RC5 > algorithm as supplied by RSA Security LLC, or any parties authorized by RSA > Security." > > The part "as supplied by RSA Security LLC, or any parties authorized by RSA > Security." should be removed. We don't generally include information about > 3rd-party JCA providers in the standard javadocs. Also the "authorized" part > was probably referring to the RC5 patent but that has since expired. Jack Hartstein has updated the pull request incrementally with one additional commit since the last revision: added line break - Changes: - all: https://git.openjdk.java.net/jdk/pull/4443/files - new: https://git.openjdk.java.net/jdk/pull/4443/files/ee75475e..bf6b9096 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4443=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4443=00-01 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4443.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4443/head:pull/4443 PR: https://git.openjdk.java.net/jdk/pull/4443
Re: RFR: 8209092: Remove outdated wording from RC5ParameterSpec [v2]
On Wed, 9 Jun 2021 20:48:29 GMT, Bradford Wetmore wrote:
>> Jack Hartstein has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> added line break
>
> src/java.base/share/classes/javax/crypto/spec/RC5ParameterSpec.java line 39:
>
>> 37: *
>> 38: * This class can be used to initialize a {@code Cipher} object that
>> 39: * implements the RC5 algorithm as specified in > href="https://datatracker.ietf.org/doc/html/rfc2040;>RFC 2040.
>
> Try to keep lines to <= 80 chars. Maybe break after "in"
Fixed. I'll check on the CSR.
-
PR: https://git.openjdk.java.net/jdk/pull/4443
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed [v2]
On Wed, 9 Jun 2021 17:46:42 GMT, Evan Whelan wrote:
>> Hi,
>>
>> Please review my fix for JDK-8255148 which clarifies when an exception
>> contains debug information only.
>>
>> Regards,
>> Evan
>
> Evan Whelan has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Added new line at end of file
> - Cleaned up test case
src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1332:
> 1330: // ignore the exception
> 1331: if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
> 1332: SSLLogger.warning("output stream close failed.
> Debug info only. Exception details:", ioe);
I may look at this bug report different. It is a problem that the user does
not understand the debug log properly. Debug log is for debug information
only, and the debug log level indicates the level of the message.
It looks like there is too much duplicated information. A log message has
already indicated that the message is debug information only. Otherwise,
exception should has been thrown in application level. The adding of
"Exception details:" adds unnecessary dependency of the exception logging
format.
It may be fine to keep it unchanged, as if the users understand the logging
message and logging levels. This kind of information normally means there is
something that an application should take care of. That why we use a warning
level log, rather than a fine level log message.
It we really want an update, may be we could have a documentation enhancement
instead.
Similar comments for other update.
-
PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: 8209092: Remove outdated wording from RC5ParameterSpec
On Wed, 9 Jun 2021 20:19:12 GMT, Jack Hartstein
wrote:
> The RC5ParameterSpec class description contains the following sentence: "This
> class can be used to initialize a Cipher object that implements the RC5
> algorithm as supplied by RSA Security LLC, or any parties authorized by RSA
> Security."
>
> The part "as supplied by RSA Security LLC, or any parties authorized by RSA
> Security." should be removed. We don't generally include information about
> 3rd-party JCA providers in the standard javadocs. Also the "authorized" part
> was probably referring to the RC5 patent but that has since expired.
I was also wondering about the CSR, check with the CSR folks.
src/java.base/share/classes/javax/crypto/spec/RC5ParameterSpec.java line 39:
> 37: *
> 38: * This class can be used to initialize a {@code Cipher} object that
> 39: * implements the RC5 algorithm as specified in href="https://datatracker.ietf.org/doc/html/rfc2040;>RFC 2040.
Try to keep lines to <= 80 chars. Maybe break after "in"
-
Marked as reviewed by wetmore (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4443
Integrated: 8255148: Confusing log output: SSLSocket duplex close failed
On Fri, 4 Jun 2021 10:44:32 GMT, Evan Whelan wrote: > Hi, > > Please review my fix for JDK-8255148 which clarifies when an exception > contains debug information only. > > Regards, > Evan This pull request has now been integrated. Changeset: 408e0a9c Author:Evan Whelan Committer: Sean Mullan URL: https://git.openjdk.java.net/jdk/commit/408e0a9c696888d41809e35bf252869f09f735db Stats: 106 lines in 2 files changed: 102 ins; 0 del; 4 mod 8255148: Confusing log output: SSLSocket duplex close failed Reviewed-by: mullan - PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: 8209092: Remove outdated wording from RC5ParameterSpec
On Wed, 9 Jun 2021 20:19:12 GMT, Jack Hartstein wrote: > The RC5ParameterSpec class description contains the following sentence: "This > class can be used to initialize a Cipher object that implements the RC5 > algorithm as supplied by RSA Security LLC, or any parties authorized by RSA > Security." > > The part "as supplied by RSA Security LLC, or any parties authorized by RSA > Security." should be removed. We don't generally include information about > 3rd-party JCA providers in the standard javadocs. Also the "authorized" part > was probably referring to the RC5 patent but that has since expired. The update looks good to me. A CSR might be required as it is a spec update, although it is just a trivial update. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4443
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed [v2]
On Wed, 9 Jun 2021 17:46:42 GMT, Evan Whelan wrote: >> Hi, >> >> Please review my fix for JDK-8255148 which clarifies when an exception >> contains debug information only. >> >> Regards, >> Evan > > Evan Whelan has updated the pull request incrementally with two additional > commits since the last revision: > > - Added new line at end of file > - Cleaned up test case Marked as reviewed by mullan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed [v2]
On Wed, 9 Jun 2021 20:27:41 GMT, Sean Mullan wrote: >> Hi @seanjmullan @coffeys >> >> I also don't have a logging level preference, so if there's merit to >> changing it, I'll be happy to do so. >> I've updated the test case and verified it passes on all platforms. >> >> Looking forward to any further feedback :) > > Ok, It sounds like whether it should be a different logging level (fine) or > not can be handled separately as part of a more global effort across other > SSL logging messages. So I am ok with the change as-is. Thanks Sean! - PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed [v2]
On Wed, 9 Jun 2021 17:43:45 GMT, Evan Whelan wrote:
>> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 590:
>>
>>> 588: // ignore the exception
>>> 589: if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
>>> 590: SSLLogger.warning("SSLSocket duplex close failed.
>>> Debug info only. Exception details:", ioe);
>>
>> If this is a debug message, shouldn't we just use `SSLLogger.fine()` instead
>> of `SSLLogger.warning()`, with the same message "SSLSocket duplex close
>> failed"? @coffeys what do you think?
>
> Hi @seanjmullan @coffeys
>
> I also don't have a logging level preference, so if there's merit to changing
> it, I'll be happy to do so.
> I've updated the test case and verified it passes on all platforms.
>
> Looking forward to any further feedback :)
Ok, It sounds like whether it should be a different logging level (fine) or not
can be handled separately as part of a more global effort across other SSL
logging messages. So I am ok with the change as-is.
-
PR: https://git.openjdk.java.net/jdk/pull/4354
RFR: 8209092: Remove outdated wording from RC5ParameterSpec
The RC5ParameterSpec class description contains the following sentence: "This class can be used to initialize a Cipher object that implements the RC5 algorithm as supplied by RSA Security LLC, or any parties authorized by RSA Security." The part "as supplied by RSA Security LLC, or any parties authorized by RSA Security." should be removed. We don't generally include information about 3rd-party JCA providers in the standard javadocs. Also the "authorized" part was probably referring to the RC5 patent but that has since expired. - Commit messages: - fixed whitespace error in file header - 8209092: fixed RC5 specifications and updated copyright date Changes: https://git.openjdk.java.net/jdk/pull/4443/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4443=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8209092 Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4443.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4443/head:pull/4443 PR: https://git.openjdk.java.net/jdk/pull/4443
Re: RFR: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions [v2]
On Fri, 4 Jun 2021 23:49:31 GMT, Smita Kamath wrote: >> I would like to submit AES-GCM optimization for x86_64 architectures >> supporting AVX3+VAES (Evex encoded AES). This optimization interleaves AES >> and GHASH operations. >> Performance gain of ~1.5x - 2x for message sizes 8k and above. > > Smita Kamath has updated the pull request incrementally with one additional > commit since the last revision: > > 8267125:Updated intrinsic signature to remove copies of counter, state and > subkeyHtbl With JDK-827 integrated, I'll provide you a merged copy of your java side changes. - PR: https://git.openjdk.java.net/jdk/pull/4019
Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance
On Wed, 9 Jun 2021 07:53:43 GMT, Dongbo He wrote:
> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm
> has been disabled. It is less efficient when there are more disabled elements
> in the list, we can use Set instead of List to speed up the search.
>
> Patch contains a benchmark that can be run with `make test
> TEST="micro:java.security.AlgorithmConstraintsPermits"`.
> Baseline results before patch:
>
> Benchmark(algorithm) Mode CntScore
> Error Units
> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 21.687 ?
> 1.118 ns/op
> AlgorithmConstraintsPermits.permits DES avgt5 324.216 ?
> 6.233 ns/op
> AlgorithmConstraintsPermits.permits NULL avgt5 709.462 ?
> 51.259 ns/op
> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 687.497 ?
> 170.181 ns/op
>
> Benchmark results after patch:
>
> Benchmark(algorithm) Mode CntScoreError
> Units
> AlgorithmConstraintsPermits.permitsSSLv3 avgt5 46.407 ? 1.057
> ns/op
> AlgorithmConstraintsPermits.permits DES avgt5 65.722 ? 0.578
> ns/op
> AlgorithmConstraintsPermits.permits NULL avgt5 43.988 ? 1.264
> ns/op
> AlgorithmConstraintsPermits.permits TLS1.3 avgt5 399.546 ? 11.194
> ns/op
>
> SSLv3, DES, NULL are the first, middle and last element in
> `jdk.tls.disabledAlgorithms` from `conf/security/java.security`.
>
> Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 keep-alive)+Jmeter:
> Before patch:
>
> summary + 50349 in 00:00:30 = 1678.4/s Avg: 238 Min: 188 Max: 298 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 135183 in 00:01:22 = 1654.5/s Avg: 226 Min:16 Max: 1281 Err:
> 0 (0.00%)
> summary + 50240 in 00:00:30 = 1674.1/s Avg: 238 Min: 200 Max: 308 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 185423 in 00:01:52 = 1659.7/s Avg: 229 Min:16 Max: 1281 Err:
> 0 (0.00%)
> summary + 50351 in 00:00:30 = 1678.4/s Avg: 238 Min: 191 Max: 306 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 235774 in 00:02:22 = 1663.7/s Avg: 231 Min:16 Max: 1281 Err:
> 0 (0.00%)
> summary + 50461 in 00:00:30 = 1681.9/s Avg: 237 Min: 174 Max: 303 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
>
> After patch:
>
> summary + 59003 in 00:00:30 = 1966.6/s Avg: 203 Min: 158 Max: 272 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 146675 in 00:01:18 = 1884.6/s Avg: 198 Min:26 Max: 697 Err:
> 0 (0.00%)
> summary + 58965 in 00:00:30 = 1965.9/s Avg: 203 Min: 166 Max: 257 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 205640 in 00:01:48 = 1907.2/s Avg: 199 Min:26 Max: 697 Err:
> 0 (0.00%)
> summary + 59104 in 00:00:30 = 1969.1/s Avg: 203 Min: 157 Max: 266 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 264744 in 00:02:18 = 1920.7/s Avg: 200 Min:26 Max: 697 Err:
> 0 (0.00%)
> summary + 59323 in 00:00:30 = 1977.6/s Avg: 202 Min: 158 Max: 256 Err:
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
>
>
> Testing: tier1, tier2
src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java
line 130:
> 128: AlgorithmDecomposer decomposer) {
> 129: super(decomposer);
> 130: List disabledAlgorithmsList =
> getAlgorithms(propertyName);
Is it doable to have the getAlgorithms() method return a Set?
-
PR: https://git.openjdk.java.net/jdk/pull/4424
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed [v2]
On Fri, 4 Jun 2021 14:08:58 GMT, Sean Mullan wrote:
>> Evan Whelan has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Added new line at end of file
>> - Cleaned up test case
>
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 590:
>
>> 588: // ignore the exception
>> 589: if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
>> 590: SSLLogger.warning("SSLSocket duplex close failed. Debug
>> info only. Exception details:", ioe);
>
> If this is a debug message, shouldn't we just use `SSLLogger.fine()` instead
> of `SSLLogger.warning()`, with the same message "SSLSocket duplex close
> failed"? @coffeys what do you think?
Hi @seanjmullan @coffeys
I also don't have a logging level preference, so if there's merit to changing
it, I'll be happy to do so.
I've updated the test case and verified it passes on all platforms.
Looking forward to any further feedback :)
-
PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed [v2]
> Hi, > > Please review my fix for JDK-8255148 which clarifies when an exception > contains debug information only. > > Regards, > Evan Evan Whelan has updated the pull request incrementally with two additional commits since the last revision: - Added new line at end of file - Cleaned up test case - Changes: - all: https://git.openjdk.java.net/jdk/pull/4354/files - new: https://git.openjdk.java.net/jdk/pull/4354/files/8bf9b687..c4ad5abb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4354=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4354=00-01 Stats: 20 lines in 1 file changed: 5 ins; 7 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/4354.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4354/head:pull/4354 PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: 8255148: Confusing log output: SSLSocket duplex close failed
On Fri, 4 Jun 2021 14:08:58 GMT, Sean Mullan wrote:
>> Hi,
>>
>> Please review my fix for JDK-8255148 which clarifies when an exception
>> contains debug information only.
>>
>> Regards,
>> Evan
>
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 590:
>
>> 588: // ignore the exception
>> 589: if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
>> 590: SSLLogger.warning("SSLSocket duplex close failed. Debug
>> info only. Exception details:", ioe);
>
> If this is a debug message, shouldn't we just use `SSLLogger.fine()` instead
> of `SSLLogger.warning()`, with the same message "SSLSocket duplex close
> failed"? @coffeys what do you think?
@seanjmullan - It's the exception stacktrace printed after the message that's
causing issue for some. Some are reading it as a JDK functionality issue (when
it's not)
I've no strong preference whether this should be printed at warning() or fine()
level. That granularity is largely broken since JDK 11 - people need to use
-Djavax.net.debug=all to get any reasonable amount of data. It would be good to
see https://bugs.openjdk.java.net/browse/JDK-8044609 worked on.
I think the patch looks ok as ie. Evan has fixed up some issues with the test -
we should expect a new version shortly.
-
PR: https://git.openjdk.java.net/jdk/pull/4354
Re: RFR: JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests
On Wed, 9 Jun 2021 14:42:23 GMT, Mahendra Chhipa
wrote:
> …HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests
test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 116:
> 114: String reqbody = "";
> 115: try(InputStream inputStream = req.getRequestBody()) {
> 116: if(inputStream != null) {
I don't think inputStream can be null, ever.
test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 117:
> 115: try(InputStream inputStream = req.getRequestBody()) {
> 116: if(inputStream != null) {
> 117: reqbody = new String(inputStream.readAllBytes());
Please double check that client and server use the same charset.
test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 138:
> 136: break;
> 137: case 2: /* test 3 */
> 138: reqbody = new
> String(req.getRequestBody().readAllBytes());
Please double check that client and server use the same charset.
test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 150:
> 148: req.getResponseHeaders().set("Connection", "close");
> 149: req.sendResponseHeaders(200, 0);
> 150: try (PrintWriter pw = new
> PrintWriter(req.getResponseBody())) {
It's dangerous to rely on the default charset when using PrintWriter.
I'd suggest to change the client and server logic to use UTF-8 explicitly
everywhere (when creating new strings, when obtaining string bytes, when using
print writers or print streams, etc...)
test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 161:
> 159: case 4: /* test 7 */
> 160: case 5: /* test 8 */
> 161: reqbody = new
> String(req.getRequestBody().readAllBytes());
Please double check that client and server use the same charset, or better,
explicitly use UTF-8 everywhere.
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/B6216082.java line 218:
> 216: // if the bug exsits, it'll send 2 GET commands
> 217: // check 2nd GET here
> 218: String duplicatedGet =
> trans.getRequestHeaders().getFirst(null);
I'm not sure you will be testing the same thing here. You probably can't use
the com.sun.net.httpserver.HttpServer for this.
-
PR: https://git.openjdk.java.net/jdk/pull/4432
Re: Low level hooks in JDK for instrumentation of permission checks.
On 6/8/21 9:35 PM, Peter Firmstone wrote: I would also like to request that all JDK modules be given ProtectionDomain's following SecurityManager deprecation. Currently some modules have null ProtectionDomain's to show they have AllPermission. However we don't grant AllPermission to code in practise, we like to grant certain Permission's to Principal's, not code, where the Principal is the source of data, indicating the user has been authenticated and we only grant what's necessary and no more. As described in JEP 411, there are no plans to deprecate ProtectionDomain at this time. --Sean
RFR: JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, …
…HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests - Commit messages: - JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests Changes: https://git.openjdk.java.net/jdk/pull/4432/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4432=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268464 Stats: 1689 lines in 7 files changed: 118 ins; 1477 del; 94 mod Patch: https://git.openjdk.java.net/jdk/pull/4432.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4432/head:pull/4432 PR: https://git.openjdk.java.net/jdk/pull/4432
Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance
Nice, the benchmark results look impressive. I'll take a closer look at the patch a little later. --Sean On 6/9/21 4:03 AM, Dongbo He wrote: Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm has been disabled. It is less efficient when there are more disabled elements in the list, we can use Set instead of List to speed up the search. Patch contains a benchmark that can be run with `make test TEST="micro:java.security.AlgorithmConstraintsPermits"`. Baseline results before patch: Benchmark(algorithm) Mode CntScore Error Units AlgorithmConstraintsPermits.permitsSSLv3 avgt5 21.687 ? 1.118 ns/op AlgorithmConstraintsPermits.permits DES avgt5 324.216 ? 6.233 ns/op AlgorithmConstraintsPermits.permits NULL avgt5 709.462 ? 51.259 ns/op AlgorithmConstraintsPermits.permits TLS1.3 avgt5 687.497 ? 170.181 ns/op Benchmark results after patch: Benchmark(algorithm) Mode CntScoreError Units AlgorithmConstraintsPermits.permitsSSLv3 avgt5 46.407 ? 1.057 ns/op AlgorithmConstraintsPermits.permits DES avgt5 65.722 ? 0.578 ns/op AlgorithmConstraintsPermits.permits NULL avgt5 43.988 ? 1.264 ns/op AlgorithmConstraintsPermits.permits TLS1.3 avgt5 399.546 ? 11.194 ns/op SSLv3, DES, NULL are the first, middle and last element in `jdk.tls.disabledAlgorithms` from `conf/security/java.security`. Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 keep-alive)+Jmeter: Before patch: summary + 50349 in 00:00:30 = 1678.4/s Avg: 238 Min: 188 Max: 298 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 135183 in 00:01:22 = 1654.5/s Avg: 226 Min:16 Max: 1281 Err: 0 (0.00%) summary + 50240 in 00:00:30 = 1674.1/s Avg: 238 Min: 200 Max: 308 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 185423 in 00:01:52 = 1659.7/s Avg: 229 Min:16 Max: 1281 Err: 0 (0.00%) summary + 50351 in 00:00:30 = 1678.4/s Avg: 238 Min: 191 Max: 306 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 235774 in 00:02:22 = 1663.7/s Avg: 231 Min:16 Max: 1281 Err: 0 (0.00%) summary + 50461 in 00:00:30 = 1681.9/s Avg: 237 Min: 174 Max: 303 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 After patch: summary + 59003 in 00:00:30 = 1966.6/s Avg: 203 Min: 158 Max: 272 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 146675 in 00:01:18 = 1884.6/s Avg: 198 Min:26 Max: 697 Err: 0 (0.00%) summary + 58965 in 00:00:30 = 1965.9/s Avg: 203 Min: 166 Max: 257 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 205640 in 00:01:48 = 1907.2/s Avg: 199 Min:26 Max: 697 Err: 0 (0.00%) summary + 59104 in 00:00:30 = 1969.1/s Avg: 203 Min: 157 Max: 266 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 264744 in 00:02:18 = 1920.7/s Avg: 200 Min:26 Max: 697 Err: 0 (0.00%) summary + 59323 in 00:00:30 = 1977.6/s Avg: 202 Min: 158 Max: 256 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 - Commit messages: - 8268427: Improve AlgorithmConstraints:checkAlgorithm performance Changes: https://git.openjdk.java.net/jdk/pull/4424/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4424=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268427 Stats: 114 lines in 4 files changed: 85 ins; 11 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/4424.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4424/head:pull/4424 PR: https://git.openjdk.java.net/jdk/pull/4424
RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance
Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm has been disabled. It is less efficient when there are more disabled elements in the list, we can use Set instead of List to speed up the search. Patch contains a benchmark that can be run with `make test TEST="micro:java.security.AlgorithmConstraintsPermits"`. Baseline results before patch: Benchmark(algorithm) Mode CntScore Error Units AlgorithmConstraintsPermits.permitsSSLv3 avgt5 21.687 ? 1.118 ns/op AlgorithmConstraintsPermits.permits DES avgt5 324.216 ? 6.233 ns/op AlgorithmConstraintsPermits.permits NULL avgt5 709.462 ? 51.259 ns/op AlgorithmConstraintsPermits.permits TLS1.3 avgt5 687.497 ? 170.181 ns/op Benchmark results after patch: Benchmark(algorithm) Mode CntScoreError Units AlgorithmConstraintsPermits.permitsSSLv3 avgt5 46.407 ? 1.057 ns/op AlgorithmConstraintsPermits.permits DES avgt5 65.722 ? 0.578 ns/op AlgorithmConstraintsPermits.permits NULL avgt5 43.988 ? 1.264 ns/op AlgorithmConstraintsPermits.permits TLS1.3 avgt5 399.546 ? 11.194 ns/op SSLv3, DES, NULL are the first, middle and last element in `jdk.tls.disabledAlgorithms` from `conf/security/java.security`. Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 keep-alive)+Jmeter: Before patch: summary + 50349 in 00:00:30 = 1678.4/s Avg: 238 Min: 188 Max: 298 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 135183 in 00:01:22 = 1654.5/s Avg: 226 Min:16 Max: 1281 Err: 0 (0.00%) summary + 50240 in 00:00:30 = 1674.1/s Avg: 238 Min: 200 Max: 308 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 185423 in 00:01:52 = 1659.7/s Avg: 229 Min:16 Max: 1281 Err: 0 (0.00%) summary + 50351 in 00:00:30 = 1678.4/s Avg: 238 Min: 191 Max: 306 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 235774 in 00:02:22 = 1663.7/s Avg: 231 Min:16 Max: 1281 Err: 0 (0.00%) summary + 50461 in 00:00:30 = 1681.9/s Avg: 237 Min: 174 Max: 303 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 After patch: summary + 59003 in 00:00:30 = 1966.6/s Avg: 203 Min: 158 Max: 272 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 146675 in 00:01:18 = 1884.6/s Avg: 198 Min:26 Max: 697 Err: 0 (0.00%) summary + 58965 in 00:00:30 = 1965.9/s Avg: 203 Min: 166 Max: 257 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 205640 in 00:01:48 = 1907.2/s Avg: 199 Min:26 Max: 697 Err: 0 (0.00%) summary + 59104 in 00:00:30 = 1969.1/s Avg: 203 Min: 157 Max: 266 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 summary = 264744 in 00:02:18 = 1920.7/s Avg: 200 Min:26 Max: 697 Err: 0 (0.00%) summary + 59323 in 00:00:30 = 1977.6/s Avg: 202 Min: 158 Max: 256 Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0 - Commit messages: - 8268427: Improve AlgorithmConstraints:checkAlgorithm performance Changes: https://git.openjdk.java.net/jdk/pull/4424/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4424=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268427 Stats: 114 lines in 4 files changed: 85 ins; 11 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/4424.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4424/head:pull/4424 PR: https://git.openjdk.java.net/jdk/pull/4424
