RFR: 8272385: Enforce ECPrivateKey d value to be in the range [1, n-1] for SunEC provider
This fix adds an EC private key range check for the scalar value to be within the range [1, n-1] (n being the order of the generator) for the SunEC ECDSA Signature algorithms and ECDH KeyAgreement algorithms. While the SunEC KeyGenerator for EC keys will not generate private keys that sit outside the accepted range, it is possible to create and attempt to use ECPrivateKey objects that violate this range through a KeyFactory. JBS: https://bugs.openjdk.java.net/browse/JDK-8272385 - Commit messages: - Merge - 8272385: Enforce ECPrivateKey d value to be in the range [1, n-1] for SunEC provider Changes: https://git.openjdk.java.net/jdk/pull/5324/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5324&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272385 Stats: 144 lines in 4 files changed: 143 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5324.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5324/head:pull/5324 PR: https://git.openjdk.java.net/jdk/pull/5324
Re: RFR: 8271745: Correct block size for KW, KWP mode and use fixed IV for KWP mode for SunJCE
On Tue, 31 Aug 2021 12:05:48 GMT, Sean Mullan wrote: >> Line 186 is correct in that the underlying Cipher block size must be >> 128-bit. However, the KW/KWP processing affected the input size requirement >> into 8-byte blocks, thus the overall cipher block size is now 8 instead of >> 16 bytes. > > So the block size is always 8 even when there is no padding? Yes, with KW (no pad), data must be in multiples of 8. When data does not meet this size requirement, an external padding scheme such as PKCS5/7 padding is needed to pad the data to multiples of 8 in order for KW mode to process the data. As for KWP mode, it internally pads the data to multiples of 8 before starting the internal processing. Thus, no external padding is needed. Are you asking if 8 should be returned for KWP mode due to its internal padding? KWP is like a variant of KW, so it seems to me that it should return the same block size as KW. - PR: https://git.openjdk.java.net/jdk/pull/5236
Re: [External] : Re: JDK-8129988 introduces a new behavior when reading the javax.net.ssl.trustStore property.
It looks like an unintended behavior change to me. It looks reasonable to change the behavior back. Xuelei > On Aug 25, 2021, at 2:59 AM, Volker Simonis wrote: > > Hi, > > I'd like to resurrect this old discussion which seems to have got lost. > > David has analyzed and described the behavioral differences introduced > by JDK-8219988 around the handling of the "javax.net.ssl.trustStore" > property pretty well in his initial mail (see below). > > The main difference is that before JDK-8219988, if > "javax.net.ssl.trustStore" was specified but did not exist, an empty > KeyStore was used. After "javax.net.ssl.trustStore", if > "javax.net.ssl.trustStore" was specified but did not exist, the system > will fall back to the default "lib/security/cacerts" store (for more > details see below). > > The only documentation of the "javax.net.ssl.trustStore" property I > could find is Oracle's "Java Secure Socket Extension (JSSE) Reference > Guide" which clearly describes the pre-JDK-8219988 behavior: > > "If the javax.net.ssl.trustStore property is defined but the specified > file does not exist, then a default TrustManager using an empty > keystore is created." > > for jdk8 [1] as well as for 9 [2] (which introduced JDK-8219988), 11 > [3] and 16 [4] > > Because the new behavior has been around since jdk 9 I tend to > recommend that we change the documentation of post jdk-8 releases to > match the implementation. However, for jdk-8 this is a breaking, > backwards incompatible change so I think it would be reasonable to > change the implementation back to conform to the documentation and the > old behavior. > > What do others think? Are there any reasons why this behavioral > changes have been made in the first place? > > Thank you and best regards, > Volker > > [1] > https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html#CustomizingStores > [2] > https://docs.oracle.com/javase/9/security/java-secure-socket-extension-jsse-reference-guide.htm#JSSEC-GUID-7932AB21-2FED-402E-A806-3088402BAEA6 > [3] > https://docs.oracle.com/en/java/javase/11/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-32CF3420-56E8-4BC5-8D3B-1F6B4692A290 > [4] > https://docs.oracle.com/en/java/javase/11/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-32CF3420-56E8-4BC5-8D3B-1F6B4692A290 > > On Mon, Aug 12, 2019 at 4:10 PM Andrew John Hughes > wrote: >> >> Forwarding to security-dev as this was backported from later JDK versions: >> >> On 09/08/2019 20:52, Alvarez, David wrote: >>> Hello, >>> >>> We have detected that JDK-8219988 [1], that has been included in OpenJDK >>> 8u222 >>> included a non-documented change in the behavior of the >>> javax.net.ssl.trustStore property. In previous versions, should this >>> property >>> point to a non-existent file, an empty KeyStore would be used instead. [2] >>> >>> In newer versions, if the value of the property contains an invalid URL, the >>> code will instead fall back and use the lib/security/cacerts file. [3] >>> >>> However, there are two things about that change that caught our attention: >>> - Whenever there is no javax.net.ssl.trustStore property set, the code will >>> first look for lib/security/jssecacerts. If that file does not exist, then >>> it >>> will look for lib/security/cacerts. However, when the property is set to an >>> invalid file, the fallback mechanism jumps directly to lib/security/cacerts, >>> never checking lib/security/jssecacerts. >>> >>> - This fallback mechanism for invalid javax.net.ssl.trustStore values >>> reuses the >>> value of javax.net.ssl.trustStorePassword. If that property is set in >>> conjunction with an invalid value in javax.net.ssl.trustStore the specified >>> password will be used when attempting to read the lib/security/cacerts >>> store. >>> It seems unclear why this path of action is chosen here. >>> >>> If the lib/security/cacerts has no password (and as far as I'm aware, that >>> is >>> the case in the majority of OpenJDK distributions, if not all), the >>> operation >>> will raise an exception. This exception mentions that 'Password verification >>> failed', hiding the underlying cause (the property pointing to a bad >>> store).[4] >>> >>> Although the new behavior isn't necessarily wrong, I think there is room for >>> Improvement. Suggestions: >>> >>> - Make sure lib/security/jssecacerts is checked during the fallback process. >>> - Ignore the value of javax.net.ssl.trustStorePassword when we fallback to >>> use >>> the bundled jssecacerts or cacerts file. >>> - Change the exception message to avoid confusion. >>> >>> I would like to have your opinion on this. >>> >>> Thanks, >>> >>> David >>> >>> -- >>> [1] https://bugs.openjdk.java.net/browse/JDK-8129988 >>> [2] >>> http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/ac2ef877d3e8/src/share/classes/sun/security/ssl/TrustManagerFactoryImpl.java#l132 >>> [3] >>> http://hg.openjdk.java.net/jdk8u/j
Re: RFR: 8272805: Avoid looking up standard charsets [v4]
> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(up to x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> This change includes:
> * demo/utils
> * jdk.xx packages
> * Some places were missed in the previous changes. I have found it by
> tracing the calls to the Charset.forName() by executing tier1,2,3 and desktop
> tests.
>
> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>
> Code excluded in this fix: the Xerces library(should be fixed upstream),
> J2DBench(should be compatible to 1.4), some code in the network(the change
> there are not straightforward, will do it later).
>
> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
Sergey Bylokhov has updated the pull request with a new target base due to a
merge or a rebase. The incremental webrev excludes the unrelated changes
brought in by the merge/rebase. The pull request contains 16 additional commits
since the last revision:
- Merge branch 'master' into standard-encodings-in-non-public-modules
- Merge branch 'master' into standard-encodings-in-non-public-modules
- Update the usage of Files.readAllLines()
- Update datatransfer
- Merge branch 'master' into standard-encodings-in-non-public-modules
- Merge branch 'master' into standard-encodings-in-non-public-modules
- Fix related imports
- Merge branch 'master' into standard-encodings-in-non-public-modules
- Cleanup UnsupportedEncodingException
- Update PacketStream.java
- ... and 6 more: https://git.openjdk.java.net/jdk/compare/7289121a...7fe0327e
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5210/files
- new: https://git.openjdk.java.net/jdk/pull/5210/files/79829ec8..7fe0327e
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5210&range=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5210&range=02-03
Stats: 949 lines in 30 files changed: 678 ins; 166 del; 105 mod
Patch: https://git.openjdk.java.net/jdk/pull/5210.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5210/head:pull/5210
PR: https://git.openjdk.java.net/jdk/pull/5210
Integrated: 8262186: Call X509KeyManager.chooseClientAlias once for all key types
On Wed, 25 Aug 2021 19:00:06 GMT, Weijun Wang wrote: > This code change collects all key types and runs `chooseClientAlias` only > once. This pull request has now been integrated. Changeset: 3d657eb0 Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/3d657eb0a626e33995af5d5ddf12b26d06317962 Stats: 376 lines in 4 files changed: 225 ins; 80 del; 71 mod 8262186: Call X509KeyManager.chooseClientAlias once for all key types Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/5257
Re: RFR: 8271745: Correct block size for KW, KWP mode and use fixed IV for KWP mode for SunJCE
On Mon, 30 Aug 2021 19:13:04 GMT, Sean Mullan wrote: > > > Has bug been filed against NSS for ignoring the IV? I've just filed https://bugzilla.mozilla.org/show_bug.cgi?id=1728419 - PR: https://git.openjdk.java.net/jdk/pull/5236
Re: RFR: 8271745: Correct block size for KW, KWP mode and use fixed IV for KWP mode for SunJCE
On Tue, 24 Aug 2021 01:33:42 GMT, Valerie Peng wrote: > Could someone help review this straight forward change? During the > interoperability testing with PKCS11 KW/KWP support, it is noticed that > SunJCE provider used the wrong block size (AES: 16) when padding is needed > for KW mode. With KW, KWP modes, data block size is multiples of 8-byte, so > the padding should pad data to multiples of 8 bytes instead of 16. In > addition, although PKCS#11 v3.0 states the IV for KWP mode is 4-byte, NSS's > implementation would silently ignore the specified IVs. Thus, for max > interoperability, it seems safer to change SunJCE provider to always use the > same default IV and disallow custom IVs for KWP mode, at least for now. > Regression test is enhanced to test more scenarios. > > Thanks, > Valerie Thanks Sean and Xuelei for the review and feedbacks! If there are additional comments, please let me know. Otherwise, I will proceed with integration tomorrow... - PR: https://git.openjdk.java.net/jdk/pull/5236
RFR: 8269039: Disable SHA-1 Signed JARs
This change will disable JARs signed with algorithms using SHA-1 by default, and treat them as unsigned. This applies to the algorithms used to digest, sign, and optionally timestamp the JAR. It also applies to the signature and digest algorithms of the certificates in the certificate chain of the code signer and the Timestamp Authority, and any CRLs or OCSP responses that are used to verify if those certificates have been revoked. The specific details are more fully described in the CSR: https://bugs.openjdk.java.net/browse/JDK-8272155. Some additional notes about the fix: - This change was previously backed out of JDK 17 and delayed because of performance regressions. The overall performance is still to be verified, but the primary bottlenecks were addressed as follows: - `sun.security.util.DisabledAlgorithmConstraints` no longer depends on `java.text.SimpleDateFormat` to format date fields which is expensive. - the `jdkCA` constraint has been removed as this caused the `cacerts` keystore to be loaded. Applications using SHA-1 JARs signed by certificates that chain back to private CAs and are impacted by the restrictions can, at their own risk, adjust the properties and add back in the `jdkCA` constraint. - `jarsigner` has been enhanced to more accurately warn about algorithms that are disabled based on the constraints specified in the security properties. Previously it had used a simpler scheme which did not take into account constraints such as `Usage` or `DenyAfter`. Similar changes should also be made to `keytool` but that will be addressed in a separate issue. - Some SHA-1 JARs used by tests where it does not affect the results have been re-signed with SHA-2 algorithms. - Commit messages: - Fix trailing whitespace. - Initial revision. Changes: https://git.openjdk.java.net/jdk/pull/5320/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5320&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269039 Stats: 643 lines in 27 files changed: 301 ins; 214 del; 128 mod Patch: https://git.openjdk.java.net/jdk/pull/5320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5320/head:pull/5320 PR: https://git.openjdk.java.net/jdk/pull/5320
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow [v2]
On Tue, 31 Aug 2021 02:08:48 GMT, Weijun Wang wrote: >> This change modifies the default value of the `java.security.manager` system >> property from "allow" to "disallow". This means unless it's explicitly set >> to "allow", any call to `System.setSecurityManager()` would throw an UOE. >> >> The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are >> updated to confirm this behavior change. Two other tests are updated because >> they were added after JDK-8267184 and do not have >> `-Djava.security.manager=allow` on its `@run` line even it they need to >> install a `SecurityManager` at runtime. >> >> Please note that this code change requires jtreg to be upgraded to 6.1, >> where a security manager [will not be >> set](https://bugs.openjdk.java.net/browse/CODETOOLS-7902990). > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > sections etc Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow [v2]
On Tue, 31 Aug 2021 02:08:48 GMT, Weijun Wang wrote: >> This change modifies the default value of the `java.security.manager` system >> property from "allow" to "disallow". This means unless it's explicitly set >> to "allow", any call to `System.setSecurityManager()` would throw an UOE. >> >> The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are >> updated to confirm this behavior change. Two other tests are updated because >> they were added after JDK-8267184 and do not have >> `-Djava.security.manager=allow` on its `@run` line even it they need to >> install a `SecurityManager` at runtime. >> >> Please note that this code change requires jtreg to be upgraded to 6.1, >> where a security manager [will not be >> set](https://bugs.openjdk.java.net/browse/CODETOOLS-7902990). > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > sections etc Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8271745: Correct block size for KW, KWP mode and use fixed IV for KWP mode for SunJCE
On Tue, 31 Aug 2021 00:09:10 GMT, Valerie Peng wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/KeyWrapCipher.java line
>> 237:
>>
>>> 235: @Override
>>> 236: protected int engineGetBlockSize() {
>>> 237: return 8;
>>
>> Line 186 still says:
>>
>> `* symmetric cipher whose block size must be 128-bit`
>>
>> Should that also be updated?
>
> Line 186 is correct in that the underlying Cipher block size must be 128-bit.
> However, the KW/KWP processing affected the input size requirement into
> 8-byte blocks, thus the overall cipher block size is now 8 instead of 16
> bytes.
So the block size is always 8 even when there is no padding?
-
PR: https://git.openjdk.java.net/jdk/pull/5236
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow [v2]
On Tue, 31 Aug 2021 02:08:48 GMT, Weijun Wang wrote: >> This change modifies the default value of the `java.security.manager` system >> property from "allow" to "disallow". This means unless it's explicitly set >> to "allow", any call to `System.setSecurityManager()` would throw an UOE. >> >> The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are >> updated to confirm this behavior change. Two other tests are updated because >> they were added after JDK-8267184 and do not have >> `-Djava.security.manager=allow` on its `@run` line even it they need to >> install a `SecurityManager` at runtime. >> >> Please note that this code change requires jtreg to be upgraded to 6.1, >> where a security manager [will not be >> set](https://bugs.openjdk.java.net/browse/CODETOOLS-7902990). > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > sections etc Marked as reviewed by mullan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5204
