Integrated: 8279903: Redundant modulo operation in ECDHKeyAgreement
On Wed, 12 Jan 2022 06:57:45 GMT, John Jiang wrote: > In class `sun.security.ec.ECDHKeyAgreement`, the last `mod()` in the below > line looks redundant, > > BigInteger lhs = y.modPow(BigInteger.valueOf(2), p).mod(p); > > I think this tiny change just be a code cleanup, so no test for it and label > `noreg-cleanup` is added for this JBS issue. This pull request has now been integrated. Changeset: 48519480 Author:John Jiang URL: https://git.openjdk.java.net/jdk/commit/485194805966e8dbb76473fa26276e5ba26d8097 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8279903: Redundant modulo operation in ECDHKeyAgreement Reviewed-by: weijun, xuelei - PR: https://git.openjdk.java.net/jdk/pull/7042
Integrated: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Tue, 11 Jan 2022 20:38:30 GMT, Weijun Wang wrote: > Change the order so parent class is at the left. This pull request has now been integrated. Changeset: cb250298 Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/cb25029885b176be9ebbc84ac1a8ba71be96a6a7 Stats: 308 lines in 15 files changed: 117 ins; 167 del; 24 mod 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards Reviewed-by: xuelei, valeriep - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279801: EC KeyFactory and KeyPairGenerator do not have aliases for OID format
On Wed, 12 Jan 2022 21:22:34 GMT, Valerie Peng wrote:
>> src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java line 322:
>>
>>> 320: */
>>> 321: putService(new ProviderServiceA(this, "KeyPairGenerator",
>>> 322: "EC", "sun.security.ec.ECKeyPairGenerator", ATTRS));
>>
>> How about the EllipticCurve alias? Are we dropping it?
>
> Hmm, I searched various security docs such as SunEC provider and standard
> names, there is no mentioning of EllipticCurve alias. So probably ok to drop
> it.
Er, maybe I can add it into KnownOIDs? `EC("1.2.840.10045.2.1", "EC",
"EllipticCurve")`.
-
PR: https://git.openjdk.java.net/jdk/pull/7036
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards [v2]
On Wed, 12 Jan 2022 22:40:09 GMT, Weijun Wang wrote: >> Change the order so parent class is at the left. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > 8279800: isAssignableFrom checks in > AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards Changes look good. Thanks~ - Marked as reviewed by valeriep (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279801: EC KeyFactory and KeyPairGenerator do not have aliases for OID format
On Tue, 11 Jan 2022 20:34:59 GMT, Weijun Wang wrote: > Add OID aliases for the 2 service. This makes sure KeyFactory can be created > and read an encoded key without knowing what the OID in the encoding is for. Marked as reviewed by valeriep (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7036
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards [v2]
On Wed, 12 Jan 2022 20:49:13 GMT, Xue-Lei Andrew Fan wrote: > > If so, then the `if` block will be true and the spec object is casted to > > your specified class (`AlgorithmParameterSpec.class` or `Object.class`) and > > it always succeeds. > > This is exactly what I want to achieve. > > Unfortunately, there is a returned value that we cannot return an object of > any class. Oh, I missed that the paramSpec should be of class T, which extends AlgorithmParameterSpec. Then, I have no more concerns. - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards [v2]
On Wed, 12 Jan 2022 22:40:09 GMT, Weijun Wang wrote: >> Change the order so parent class is at the left. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > 8279800: isAssignableFrom checks in > AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Tue, 11 Jan 2022 20:38:30 GMT, Weijun Wang wrote: > Change the order so parent class is at the left. New commit pushed. Turns out `PBKDF2HmacSHA1Factory.java` is useless now. The algorithm is now implemented as a sub-class of `PBKDF2Core`. - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards [v2]
> Change the order so parent class is at the left. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards - Changes: - all: https://git.openjdk.java.net/jdk/pull/7037/files - new: https://git.openjdk.java.net/jdk/pull/7037/files/5dccd0ed..a5b20563 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7037=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7037=00-01 Stats: 215 lines in 4 files changed: 33 ins; 167 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/7037.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7037/head:pull/7037 PR: https://git.openjdk.java.net/jdk/pull/7037
RFR: 8278851: Correct signer logic for jars signed with multiple digestalgs
If a JAR is signed with multiple digest algorithms and one of the digest algorithms is disabled, `ManifestEntryVerifier.verify()` was incorrectly returning null indicating that the jar entry has no signers. This fixes the issue such that an entry is considered signed if at least one of the digest algorithms is not disabled and the digest match passes. This makes the fix consistent with how multiple digest algorithms are handled in the Signature File. This also fixes an issue in the `ManifestEntryVerifier.getParams()` method in which it was incorrectly checking the algorithm constraints against all signers of a JAR when it should check them only against the signers of the entry that is being verified. An additional cache has also been added to avoid checking if the digest algorithm is disabled more than once for entries signed by the same set of signers. - Commit messages: - Initial revision. Changes: https://git.openjdk.java.net/jdk/pull/7056/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7056=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278851 Stats: 263 lines in 3 files changed: 213 ins; 20 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/7056.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7056/head:pull/7056 PR: https://git.openjdk.java.net/jdk/pull/7056
Re: RFR: 8279801: EC KeyFactory and KeyPairGenerator do not have aliases for OID format
On Wed, 12 Jan 2022 21:17:41 GMT, Valerie Peng wrote: >> Add OID aliases for the 2 service. This makes sure KeyFactory can be created >> and read an encoded key without knowing what the OID in the encoding is for. > > src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java line 322: > >> 320: */ >> 321: putService(new ProviderServiceA(this, "KeyPairGenerator", >> 322: "EC", "sun.security.ec.ECKeyPairGenerator", ATTRS)); > > How about the EllipticCurve alias? Are we dropping it? Hmm, I searched various security docs such as SunEC provider and standard names, there is no mentioning of EllipticCurve alias. So probably ok to drop it. - PR: https://git.openjdk.java.net/jdk/pull/7036
Re: RFR: 8279801: EC KeyFactory and KeyPairGenerator do not have aliases for OID format
On Tue, 11 Jan 2022 20:34:59 GMT, Weijun Wang wrote: > Add OID aliases for the 2 service. This makes sure KeyFactory can be created > and read an encoded key without knowing what the OID in the encoding is for. src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java line 322: > 320: */ > 321: putService(new ProviderServiceA(this, "KeyPairGenerator", > 322: "EC", "sun.security.ec.ECKeyPairGenerator", ATTRS)); How about the EllipticCurve alias? Are we dropping it? - PR: https://git.openjdk.java.net/jdk/pull/7036
Re: RFR: 8279903: Redundant modulo operation in ECDHKeyAgreement [v2]
On Wed, 12 Jan 2022 07:29:04 GMT, John Jiang wrote: >> In class `sun.security.ec.ECDHKeyAgreement`, the last `mod()` in the below >> line looks redundant, >> >> BigInteger lhs = y.modPow(BigInteger.valueOf(2), p).mod(p); >> >> I think this tiny change just be a code cleanup, so no test for it and label >> `noreg-cleanup` is added for this JBS issue. > > John Jiang has updated the pull request incrementally with one additional > commit since the last revision: > > update copyright notes Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7042
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 20:01:10 GMT, Valerie Peng wrote: >> Right, I suppose so. > > PBEKeyFactory.java, PBKDF2Core.java and PBKDF2HmacSHA1Factory.java also have > isAssignableFrom() calls which seem backward. Perhaps covering them as well? > If so, then the `if` block will be true and the spec object is casted to your > specified class (`AlgorithmParameterSpec.class` or `Object.class`) and it > always succeeds. > > This is exactly what I want to achieve. Unfortunately, there is a returned value that we cannot return an object of any class. - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 19:54:38 GMT, Valerie Peng wrote: >> The check ensures casting always succeeds. The fact that this has not been >> noticed for such a long time means everyone is using the exact subclass type >> when calling the method. > > Right, I suppose so. PBEKeyFactory.java, PBKDF2Core.java and PBKDF2HmacSHA1Factory.java also have isAssignableFrom() calls which seem backward. Perhaps covering them as well? - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 19:45:49 GMT, Weijun Wang wrote:
>> test/jdk/java/security/spec/IsAssignableFromOrder.java line 72:
>>
>>> 70: static void test(String algorithm, AlgorithmParameterSpec spec,
>>> 71: Class... classes) throws
>>> Exception {
>>> 72: var ap1 = AlgorithmParameters.getInstance(algorithm);
>>
>> nit: Print out the algorithm which has been tested?
>
> I can, but if an exception is thrown, I can find out which algorithm has a
> problem by looking at the line number.
Yes, just thought that reporting the algorithm to the test output seems more
informative. Otherwise, always have to dig out the source to find out what has
been covered.
-
PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 19:49:28 GMT, Weijun Wang wrote: >> Interesting... In hindsight, the cast call sort of confirms that the >> intended ordering is the suggested one. > > The check ensures casting always succeeds. The fact that this has not been > noticed for such a long time means everyone is using the exact subclass type > when calling the method. Right, I suppose so. - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 19:31:31 GMT, Valerie Peng wrote: >> If so, then the `if` block will be true and the spec object is casted to >> your specified class (`AlgorithmParameterSpec.class` or `Object.class`) and >> it always succeeds. >> >> This is exactly what I want to achieve. In fact, this bug and the other >> `getInstance(oid)` bug have the same root. I was trying to decode an >> algorithm identifier from its encoding. First, the encoding of the algorithm >> is in OID so `AlgorithmParameters.getInstance()` must support OID. Second, I >> want to get the spec from the parameters without knowing the algorithm name >> and the child `AlgorithmParametersSpec` class type, so >> `AlgorithmParameters::getParameterSpec` must support >> `AlgorithmParameterSpec.class` as the argument. >> >> Otherwise, the program needs to know name and parameter spec type on all >> supported algorithms. > > Interesting... In hindsight, the cast call sort of confirms that the intended > ordering is the suggested one. The check ensures casting always succeeds. The fact that this has not been noticed for such a long time means everyone is using the exact subclass type when calling the method. - PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 19:41:31 GMT, Valerie Peng wrote:
>> Change the order so parent class is at the left.
>
> test/jdk/java/security/spec/IsAssignableFromOrder.java line 72:
>
>> 70: static void test(String algorithm, AlgorithmParameterSpec spec,
>> 71: Class... classes) throws
>> Exception {
>> 72: var ap1 = AlgorithmParameters.getInstance(algorithm);
>
> nit: Print out the algorithm which has been tested?
I can, but if an exception is thrown, I can find out which algorithm has a
problem by looking at the line number.
-
PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 14:50:37 GMT, Weijun Wang wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/BlockCipherParamsCore.java
>> line 111:
>>
>>> 109: T getParameterSpec(Class
>>> paramSpec)
>>> 110: throws InvalidParameterSpecException {
>>> 111: if (paramSpec.isAssignableFrom(IvParameterSpec.class)) {
>>
>> The call to cast() is confusing. But if the paramSpec is
>> AlgorithmParameterSpec.class or Object.class, what's the expected behavior?
>> There are potential casting exception, I guess. Maybe, a exactly class
>> matching could be better.
>
> If so, then the `if` block will be true and the spec object is casted to your
> specified class (`AlgorithmParameterSpec.class` or `Object.class`) and it
> always succeeds.
>
> This is exactly what I want to achieve. In fact, this bug and the other
> `getInstance(oid)` bug have the same root. I was trying to decode an
> algorithm identifier from its encoding. First, the encoding of the algorithm
> is in OID so `AlgorithmParameters.getInstance()` must support OID. Second, I
> want to get the spec from the parameters without knowing the algorithm name
> and the child `AlgorithmParametersSpec` class type, so
> `AlgorithmParameters::getParameterSpec` must support
> `AlgorithmParameterSpec.class` as the argument.
>
> Otherwise, the program needs to know name and parameter spec type on all
> supported algorithms.
Interesting... In hindsight, the cast call sort of confirms that the intended
ordering is the suggested one.
-
PR: https://git.openjdk.java.net/jdk/pull/7037
Re: RFR: 8279903: Redundant modulo operation in ECDHKeyAgreement [v2]
On Wed, 12 Jan 2022 07:29:04 GMT, John Jiang wrote: >> In class `sun.security.ec.ECDHKeyAgreement`, the last `mod()` in the below >> line looks redundant, >> >> BigInteger lhs = y.modPow(BigInteger.valueOf(2), p).mod(p); >> >> I think this tiny change just be a code cleanup, so no test for it and label >> `noreg-cleanup` is added for this JBS issue. > > John Jiang has updated the pull request incrementally with one additional > commit since the last revision: > > update copyright notes Good catch. Looks good to me. Thanks. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7042
Re: RFR: 8279800: isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec appear to be backwards
On Wed, 12 Jan 2022 06:08:29 GMT, Xue-Lei Andrew Fan wrote:
>> Change the order so parent class is at the left.
>
> src/java.base/share/classes/com/sun/crypto/provider/BlockCipherParamsCore.java
> line 111:
>
>> 109: T getParameterSpec(Class
>> paramSpec)
>> 110: throws InvalidParameterSpecException {
>> 111: if (paramSpec.isAssignableFrom(IvParameterSpec.class)) {
>
> The call to cast() is confusing. But if the paramSpec is
> AlgorithmParameterSpec.class or Object.class, what's the expected behavior?
> There are potential casting exception, I guess. Maybe, a exactly class
> matching could be better.
If so, then the `if` block will be true and the spec object is casted to your
specified class (`AlgorithmParameterSpec.class` or `Object.class`) and it
always succeeds.
This is exactly what I want to achieve. In fact, this bug and the other
`getInstance(oid)` bug have the same root. I was trying to decode an algorithm
identifier from its encoding. First, the encoding of the algorithm is in OID so
`AlgorithmParameters.getInstance()` must support OID. Second, I want to get the
spec from the parameters without knowing the algorithm name and the child
`AlgorithmParametersSpec` class type, so
`AlgorithmParameters::getParameterSpec` must support
`AlgorithmParameterSpec.class` as the argument.
Otherwise, the program needs to know name and parameter spec type on all
supported algorithms.
-
PR: https://git.openjdk.java.net/jdk/pull/7037
Re: JEP 411: sandboxing use case
Hi Olivier, After JEP 421 (deprecation of finalizers) and a JEP is assigned to removal of finalizers, it will be possible to instrument java methods and intercept their calls. While finalizers exist, instrumenting constructors would allow finalizer attacks to circumvent the permission check. OpenJDK has no intention of developing a new authorization framework / api. Until such time as finalizers are removed, SM is the only option currently available for authorization within the JVM. You can make a JVM process less privileged, or sandbox a JVM within a VM, but if the JVM (without SM) is able to generate a students result, then student code will also be able to. I did consider briefly the possibility of using two processes, one for the student code (isolated in an unprivileged process or VM) and one for the grading code, but I think the grading jvm would be susceptible to some form of injection attack, as it has to parse untrusted data. Be sure to disable Serialization, as SM doesn't prevent its use and any other forms of potentially dangerous data parsing. -Djdk.serialFilter=!*,\ -Dcom.sun.jndi.ldap.object.trustSerialData=false,\ Regards, Peter. On 12/01/2022 7:49 am, Sean Mullan wrote: On 1/10/22 9:22 AM, Olivier Cailloux wrote: Dear list, I would like to share my use case for currently using the security manager mechanism (SM) in my software. Now that JEP 411 is there, any advice about any currently existing solution for replacement would be welcome, if this is already possible; alternatively, I hope that a replacement for these needs will be available soon. You may want to consider container technologies. This is mentioned in the last paragraph of the Motivation section of JEP 411. --Sean My software grades student work. It download their code from GitHub, compiles it, runs it, and observe the results (similar to running JUnit tests, but on pluggable code). Their code is then graded automatically depending on the expected versus actual results. I currently use SM to prevent student code to alter the system on which the code runs or have external impact. I don’t want them to read files or send network requests (they usually do not need to do anything like this for the exercices assigned to them). I currently use a simple “no priviledged calls at all” configuration, where everything that can be forbidden by SM is forbidden for their code, as they only need to be able to deal with their own objects and classes from the JDK that operate “taint-free” (as Chapman Flak puts it), such as classical List or Set structures. Though I do not currently need such more advanced feature, I considered as a good bonus that SM allowed me, if I wanted to, to give exercices that also deal with file writing (through telling SM that their code can access a restricted set of files). If any replacement solution could also allow this kind of flexibility, that would be nice. I am aware that their code could implement a denial of service; I am okay to live with this risk as any resulting damage would be low (worst case, just restart the computer). But I’d like to reduce the risk that their code would read or modify files or other aspects of the system it is running on, for example, as the resulting damage could be much higher (such as: alter the way the system works so that the grading of other students, graded next, would be modified; read personal files from the account that is running the grading software and posting their content on the internet; inadvertently delete files on the host system…) I implement code isolation so that one student code does not see or interact with the code of other students classical using class loader mechanisms, for which JEP 411 does not create problems. But I ignore how to prevent file writing, socket opening, or similar stuff, using other means than SM. My needs resemble (but are not identical to) the ones exposed by Chapman Flack in “JEP 411: Missing use-case: user functions in an RDBMS”, https://marc.info/?m=162216583127042. I share the concern of this poster (https://marc.info/?m=162221303911911) that it currently seems that I’d have to come up with various, specialized mechanisms to prevent various kinds of operations (file system access, socket access, …), which seems inelegant and error-prone. Even after reading the insightful article of Ron Pressler, Shallow Java Sandboxes (https://inside.java/2021/04/23/security-and-sandboxing-post-securitymanager/), it is unclear to me whether I can get rid of SecurityManager with existing Java 17 technology. Any advice would be welcome. If not possible, please consider this use case when thinking about further progress in replacing the security related APIs. (I am quite worried by the wording of JEP 411 Future Work not mentioning this kind of sandboxing need.) Olivier
