Re: RFR: JDK-8308398 Move SunEC crypto provider into java.base
On 17/06/2023 11:13 pm, Alan Bateman wrote: On Tue, 13 Jun 2023 20:36:28 GMT, Anthony Scarpino wrote: This moves the SunEC JCE Provider (Elliptic Curve) into java.base. EC has always been separate from the base module/pkg because of its dependence on a native library. That library was removed in JDK 16. The proposed changes look okay, meaning it should be okay to have the SunEC provider in java.base. However, the motivation isn't clear as there isn't an issue with JCE providers in java.base using native code. I know there were non-technical issues with libsunec in the past but that would haven't prevent the SunEC code form being compiled into java.base. I assume the main implications of the change is that 3rd party JCE providers signed with an EC certificate can now be deployed on the module path. Another way to solve that issue is that delay verification of signed JARs until the boot layer is created - if we did that, would you still want to move the SunEC provider into java.base? Curious, the provider mechanism provides a level of indirection, aka service, a boundary or separation. How are module boundaries defined? Regards, Peter. - PR Comment: https://git.openjdk.org/jdk/pull/14457#issuecomment-1595748679
[jdk21] RFR: 8310549: avoid potential leaks in KeystoreImpl.m related to JNU_CHECK_EXCEPTION early returns
8310549: avoid potential leaks in KeystoreImpl.m related to JNU_CHECK_EXCEPTION early returns - Commit messages: - Backport 7da3f1999fc5d1b9162443d97dfae7fe7b04dfc5 Changes: https://git.openjdk.org/jdk21/pull/86/files Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=86&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310549 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk21/pull/86.diff Fetch: git fetch https://git.openjdk.org/jdk21.git pull/86/head:pull/86 PR: https://git.openjdk.org/jdk21/pull/86
Re: RFR: 8311043: Remove trailing blank lines in source files
On Wed, 28 Jun 2023 16:54:51 GMT, Leo Korinth wrote: > Remove trailing "blank" lines in source files. > > I like to use global-whitespace-cleanup-mode, but I can not use it if the > files are "dirty" to begin with. This fix will make more files "clean". I > also considered adding a check for this in jcheck for Skara, however it seems > jcheck code handling hunks does not track end-of-files. Thus I will only > clean the files. > > The fix removes trailing lines matching ^[[:space:]]*$ in > > - *.java > - *.cpp > - *.hpp > - *.c > - *.h > > I have applied the following bash script to each file: > > file="$1" > > while [[ $(tail -n 1 "$file") =~ ^[[:space:]]*$ ]]; do > truncate -s -1 "$file" > done > > `git diff --ignore-space-change --ignore-blank-lines master` displays no > changes > `git diff --ignore-blank-lines master` displays one change Ending the file with a blank line? I would not have expected that at all. I expect a single EOL at the end of the file; from a visual POV, the last line of non-blank text ends with an EOL. No more, no less. - PR Comment: https://git.openjdk.org/jdk/pull/14698#issuecomment-1614806396
RFR: 8311170: Simplify and modernize equals and hashCode in security area
Please review this PR to use modern APIs and language features to simplify
`equals` and `hashCode` in security area.
I understand that security area is sensitive and a non-expert, such as myself,
should tread carefully; so below are my notes to assist the review.
* Unlike `hashCode`, non-secure `equals` implementations are typically
short-circuit. But because of "timing attacks", we seem to have specialized
implementations, such as `java.security.MessageDigest.isEqual(byte[], byte[])`
and a more general `sun.security.util.ByteArrays.isEqual(byte[], int, int,
byte[], int, int)`. So while reviewing this PR, take an opportunity to audit
the affected `equals` implementations: perhaps some of them need to become
secure, not modern. I have no domain knowledge to tell those cases apart, I
only note that those cases exist.
* This PR sacrifices compatibility for pragmatism: it changes some `hashCode`
implementations to produce different values than before to allow more
utilization of methods from `Objects` and `Arrays`. To my mind, those changes
are **benign**. If you disagree, I'd be happy to discuss that and/or retract
the concerning part of the change.
* BitArray could be a topic of its own, but I'll do my best to be concise.
* Truth to be told, BitArray's `equals` and `hashCode` are not used
anywhere in source, and `equals` is only used in one test. For that reason, I
refrained from reimplementing internals of `BitArray` using more general
`java.util.BitSet`: too much effort and risk for almost nothing.
* Speaking of `BitSet`-powered `BitArray`. Such an implementation is not
for the faint of heart: there's too much impedance mismatch between data
structures that those classes use to store bits. That said, for the sake of
testing that it is possible and that I understand the `BitArray` correctly, I
actually implemented it using `BitSet`. While that implementation is **NOT**
part of this PR, you can have a look at it
[here](https://cr.openjdk.org/~prappo/8311170/BitArray.java).
* One suggestion to consider is to change this somewhat arcane piece in
java.security.UnresolvedPermission.equals:
// check certs
if (this.certs == null && that.certs != null ||
this.certs != null && that.certs == null ||
this.certs != null &&
this.certs.length != that.certs.length) {
return false;
}
int i,j;
boolean match;
for (i = 0; this.certs != null && i < this.certs.length; i++) {
match = false;
for (j = 0; j < that.certs.length; j++) {
if (this.certs[i].equals(that.certs[j])) {
match = true;
break;
}
}
if (!match) return false;
}
for (i = 0; that.certs != null && i < that.certs.length; i++) {
match = false;
for (j = 0; j < this.certs.length; j++) {
if (that.certs[i].equals(this.certs[j])) {
match = true;
break;
}
}
if (!match) return false;
}
return true;
to this:
// check certs
if (this.certs == null && that.certs != null ||
this.certs != null && that.certs == null)
return false;
if (this.certs == null) {
assert that.certs == null;
return false;
}
return Set.of(this.certs).equals(Set.of(that.certs));
-
Commit messages:
- Initial commit
Changes: https://git.openjdk.org/jdk/pull/14738/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14738&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8311170
Stats: 127 lines in 6 files changed: 16 ins; 73 del; 38 mod
Patch: https://git.openjdk.org/jdk/pull/14738.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14738/head:pull/14738
PR: https://git.openjdk.org/jdk/pull/14738
Re: RFR: 8311170: Simplify and modernize equals and hashCode in security area [v2]
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCode`, non-secure `equals` implementations are typically
> short-circuit. But because of "timing attacks", we seem to have specialized
> implementations, such as `java.security.MessageDigest.isEqual(byte[],
> byte[])` and a more general `sun.security.util.ByteArrays.isEqual(byte[],
> int, int, byte[], int, int)`. So while reviewing this PR, take an opportunity
> to audit the affected `equals` implementations: perhaps some of them need to
> become secure, not modern. I have no domain knowledge to tell those cases
> apart, I only note that those cases exist.
>
> * This PR sacrifices compatibility for pragmatism: it changes some `hashCode`
> implementations to produce different values than before to allow more
> utilization of methods from `Objects` and `Arrays`. To my mind, those changes
> are **benign**. If you disagree, I'd be happy to discuss that and/or retract
> the concerning part of the change.
>
> * BitArray could be a topic of its own, but I'll do my best to be concise.
>
> * Truth to be told, BitArray's `equals` and `hashCode` are not used
> anywhere in source, and `equals` is only used in one test. For that reason, I
> refrained from reimplementing internals of `BitArray` using more general
> `java.util.BitSet`: too much effort and risk for almost nothing.
> * Speaking of `BitSet`-powered `BitArray`. Such an implementation is not
> for the faint of heart: there's too much impedance mismatch between data
> structures that those classes use to store bits. That said, for the sake of
> testing that it is possible and that I understand the `BitArray` correctly, I
> actually implemented it using `BitSet`. While that implementation is **NOT**
> part of this PR, you can have a look at it
> [here](https://cr.openjdk.org/~prappo/8311170/BitArray.java).
>
> * One suggestion to consider is to change this somewhat arcane piece in
> java.security.UnresolvedPermission.equals:
>
> // check certs
> if (this.certs == null && that.certs != null ||
> this.certs != null && that.certs == null ||
> this.certs != null &&
> this.certs.length != that.certs.length) {
> return false;
> }
>
> int i,j;
> boolean match;
>
> for (i = 0; this.certs != nu...
Pavel Rappo has updated the pull request incrementally with one additional
commit since the last revision:
More cases
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14738/files
- new: https://git.openjdk.org/jdk/pull/14738/files/36d8347e..0727c264
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=14738&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=14738&range=00-01
Stats: 93 lines in 6 files changed: 8 ins; 62 del; 23 mod
Patch: https://git.openjdk.org/jdk/pull/14738.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14738/head:pull/14738
PR: https://git.openjdk.org/jdk/pull/14738
