Re: RFR: 8235710: Remove the legacy elliptic curves [v3]
> This change removes the native elliptic curves library code; as well as, and > calls to that code, tests, and files > associated with those libraries. The makefiles have been changed to remove > from all source builds of the ec code. The > SunEC system property is removed and java.security configurations changed to > reflect the removed curves. This will > remove the following elliptic curves from SunEC: secp112r1, secp112r2, > secp128r1, secp128r2, secp160k1, secp160r1, > secp160r2, secp192k1, secp192r1, secp224k1, secp224r1, secp256k1, sect113r1, > sect113r2, sect131r1, sect131r2, > sect163k1, sect163r1, sect163r2, sect193r1, sect193r2, sect233k1, sect233r1, > sect239k1, sect283k1, sect283r1, > sect409k1, sect409r1, sect571k1, sect571r1, X9.62 c2tnb191v1, X9.62 > c2tnb191v2, X9.62 c2tnb191v3, X9.62 c2tnb239v1, > X9.62 c2tnb239v2, X9.62 c2tnb239v3, X9.62 c2tnb359v1, X9.62 c2tnb431r1, X9.62 > prime192v2, X9.62 prime192v3, X9.62 > prime239v1, X9.62 prime239v2, X9.62 prime239v3, brainpoolP256r1 > brainpoolP320r1, brainpoolP384r1, brainpoolP512r1 Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision: change exception for ec keyagreement fix supportedcurves in SunEC - Changes: - all: https://git.openjdk.java.net/jdk/pull/289/files - new: https://git.openjdk.java.net/jdk/pull/289/files/8a04ce7a..1f9820ab Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=289=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=289=01-02 Stats: 20 lines in 3 files changed: 4 ins; 10 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/289.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/289/head:pull/289 PR: https://git.openjdk.java.net/jdk/pull/289
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]
On Wed, 23 Sep 2020 20:02:45 GMT, Erik Gahlin wrote: >> Philippe Marschall has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now >> contains one commit: >> 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate > > Marked as reviewed by egahlin (Reviewer). @marschall I will sponsor it after you integrate the latest update. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8235710: Remove the legacy elliptic curves [v2]
On Wed, 23 Sep 2020 17:07:21 GMT, Valerie Peng wrote: >> Anthony Scarpino has updated the pull request incrementally with one >> additional commit since the last revision: >> >> remove JDKOPT_DETECT_INTREE_EC from configure.ac > > src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java line 219: > >> 217: >> 218: Collection supportedCurves; >> 219: supportedCurves = CurveDB.getSupportedCurves(); > > Shouldn't the supportedCurves be the hardcoded 3 curves? Ah yes. Thanks for seeing that. - PR: https://git.openjdk.java.net/jdk/pull/289
Re: RFR: 8235710: Remove the legacy elliptic curves [v2]
On Tue, 22 Sep 2020 13:53:12 GMT, Sean Mullan wrote: >> Anthony Scarpino has updated the pull request incrementally with one >> additional commit since the last revision: >> >> remove JDKOPT_DETECT_INTREE_EC from configure.ac > > src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java line > 180: > >> 178: ((privNC != null) ? privNC.toString() : " >> unknown") + >> 179: ", PublicKey:" + >> 180: ((pubNC != null) ? pubNC.toString() : " >> unknown"))); > > Spacing issues: "PublicKey:" should be "PublicKey: " and " unknown" should be > "unknown". After considering the keys closer, I don't need to specify the key anymore. PrivateKey and PublicKey were removed - PR: https://git.openjdk.java.net/jdk/pull/289
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]
On Wed, 23 Sep 2020 18:41:06 GMT, Philippe Marschall wrote: >> Hello, newbie here >> >> I picked JDK-8138732 to work on because it has a "starter" label and I >> believe I understand what to do. >> >> - I tried to update the copyright year to 2020 in every file. >> - I decided to change `@since` from 9 to 16 since it is a new annotation >> name in a new package. >> - I tried to keep code changes to a minimum, eg not change to imports if >> fully qualified class names are used instead of >> imports. In some cases I did minor reordering of imports to keep them >> sorted alphabetically. >> - All tier1 tests pass. >> - One jpackage/jlink tier2 test fails but I don't believe it is related. >> - Some tier3 Swing tests fail but I don't think they are related. > > Philippe Marschall has updated the pull request with a new target base due to > a merge or a rebase. The pull request now > contains one commit: > 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate Marked as reviewed by egahlin (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On 23.09.20 07:13, David Holmes wrote: On Tue, 22 Sep 2020 08:43:04 GMT, Erik Gahlin wrote: Marked as reviewed by kvn (Reviewer). Have you run the JFR tests in test/jdk/jdk/jfr? @marschall Please do not force-push anything as it breaks the commit history in the PR and renders previous reviews/comments obsolete. There is no way for the reviewers to see what changed between the commit they reviewed and the commit now in the PR. To update to latest changes you should have just merged your branch with your (up-to-date) local master, committed that merge in your local branch and then pushed that commit to your Personal Fork. The skara tooling will flatten the series of commits in a PR into one single well-formed commit that is pushed to the master branch of the repo. My apologies, I was not aware of this, I won't make this mistake in the future again. I don't assume there is a way to fix this now. Philippe
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On 22.09.20 10:45, Erik Gahlin wrote: On Sat, 12 Sep 2020 00:19:00 GMT, Vladimir Kozlov wrote: Philippe Marschall has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate Marked as reviewed by kvn (Reviewer). Have you run the JFR tests in test/jdk/jdk/jfr? I initially did not but now I have. jdk.jfr.event.runtime.TestModuleEvents was failing, I have updated it to what makes sense to me. Cheers Philippe
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]
On Wed, 23 Sep 2020 18:41:06 GMT, Philippe Marschall wrote: >> Hello, newbie here >> >> I picked JDK-8138732 to work on because it has a "starter" label and I >> believe I understand what to do. >> >> - I tried to update the copyright year to 2020 in every file. >> - I decided to change `@since` from 9 to 16 since it is a new annotation >> name in a new package. >> - I tried to keep code changes to a minimum, eg not change to imports if >> fully qualified class names are used instead of >> imports. In some cases I did minor reordering of imports to keep them >> sorted alphabetically. >> - All tier1 tests pass. >> - One jpackage/jlink tier2 test fails but I don't believe it is related. >> - Some tier3 Swing tests fail but I don't think they are related. > > Philippe Marschall has updated the pull request with a new target base due to > a merge or a rebase. The pull request now > contains one commit: > 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate Update seems fine. Thanks for JFR testing and fixing. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8252523: Add ASN1 Formatter to work with HexPrinter [v2]
On Mon, 21 Sep 2020 00:19:53 GMT, Weijun Wang wrote:
>> Roger Riggs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Small cleanups to javadoc and code
>
> test/lib/jdk/test/lib/hexdump/ASN1Formatter.java line 228:
>
>> 226: break;
>> 227:
>> 228: case TAG_OctetString:
>
> I'd rather print nothing for OCTET STRING. My understanding of it is opaque
> octets and not meant to be printable.
Octet Strings can may contain printable strings and it is useful to print them.
A heuristic is used to determine if they are printable and the length is
limited to avoid spewing garbage.
> test/lib/jdk/test/lib/hexdump/ASN1Formatter.java line 362:
>
>> 360: switch (tag & 0xc0) {
>> 361: case TAG_APPLICATION:
>> 362: return "APPLICATION " + cons + (tag & 0x1f);
>
> I am not sure how important it is to print out "cons". Also, the tag here is
> usually shown as "[1]" in ASN.1
> definition. Of course, if you choose this style, you might want to avoid
> using brackets for length.
It provides a more complete info on the tag even though the application may not
have publicly defined its values.
-
PR: https://git.openjdk.java.net/jdk/pull/268
Re: RFR: 8252523: Add ASN1 Formatter to work with HexPrinter [v4]
> # JDK-8252523: Add ASN.1 Formatter to work with test utility HexPrinter > > Debugging functions that utilize ASN.1, DER, and BER encoded streams is > difficult without test utilities to show the contents. > The ASN.1 formatter reads a stream and produces annotated output of the > tags, values, and structures. > When used with the test library jdk.test.lib.hexdump.HexPrinter the > annotations are synchronized > with the hex formatted output. > > Small changes to HexPrinter are included to improve the output readability. > > > Example decoding of a .pem certificate: > SEQUENCE [910] > SEQUENCE [630] > CONTEXT cons 0 [3] > BYTE 2, > BYTE 3, > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > NULL > SEQUENCE [76] > SET [11] > SEQUENCE [9] > OBJECT ID [3] 2.5.4.6 (CountryName) > 'IN' > ... > SET [16] > SEQUENCE [14] > OBJECT ID [3] 2.5.4.3 (CommonName) > Client1 > SEQUENCE [30] > UTCTIME [13] '150526221718Z' > UTCTIME [13] '250523221718Z' > ... > SEQUENCE [290] > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.1 (RSA) > NULL > BIT STRING [271] > CONTEXT cons 3 [123] > SEQUENCE [121] > SEQUENCE [9] > OBJECT ID [3] 2.5.29.19 (BasicConstraints) > OCTET STRING [2] > SEQUENCE [44] > OBJECT ID [9] 2.16.840.1.113730.1.13 > OCTET STRING [31] '..OpenSSL Generated Certificate' > SEQUENCE [29] > OBJECT ID [3] 2.5.29.14 (SubjectKeyID) > OCTET STRING [22] > SEQUENCE [31] > OBJECT ID [3] 2.5.29.35 (AuthorityKeyID) > OCTET STRING [24] > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > NULL > BIT STRING [257] > When used with the HexPrinter test utility, the formatting of the > hexadecimal values is selected with the parameters to HexPrinter. > > : 30 82 03 8e ; SEQUENCE [910] > 0004: 30 82 02 76 ; SEQUENCE [630] > 0008: a0 03 ; CONTEXT cons > 0 [3] > 000a: 02 01 02 ; BYTE 2, > 000d:02 01 03 ; BYTE 3, > 0010: 30 0d ; SEQUENCE [13] > 0012: 06 09 2a 86 48 86 f7 0d 01 01 0b ; OBJECT ID > [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > 001d:05 00; NULL > 001f: 30 ; SEQUENCE [76] > 0020: 4c ; > 0021:31 0b; SET [11] > 0023: 30 09 ; SEQUENCE > [9] > 0025:06 03 55 04 06 ; OBJECT > ID [3] 2.5.4.6 (CountryName) > 002a: 13 02 49 4e ; 'IN' > > ... ... > > 005b: 31 10 ; SET [16] > 005d:30 0e; SEQUENCE > [14] > 005f: 06 ; OBJECT > ID [3] 2.5.4.3 (CommonName) > 0060: 03 55 04 03 ; > 0064: 0c 07 43 6c 69 65 6e 74 31 ; Client1 > 006d:30 1e; SEQUENCE [30] > 006f: 17 ; UTCTIME > [13] '150526221718Z' > 0070: 0d 31 35 30 35 32 36 32 32 31 37 31 38 5a ; > 007e: 17 0d ; UTCTIME > [13] '250523221718Z' > 0080: 32 35 30 35 32 33 32 32 31 37 31 38 5a ; > > ... ... > > 00db: 30 82 01 22; SEQUENCE [290] > 00df: 30 ; SEQUENCE > [13] > 00e0: 0d ; > 00e1:06 09 2a 86 48 86 f7 0d 01 01 01 ; OBJECT ID > [9] 1.2.840.113549.1.1.1 (RSA) > 00ec: 05 00 ; NULL > 00ee: 03 82 ; BIT STRING > [271] > 00f0: 01 0f 00 30 82 01 0a 02 82 01 01 00 d8 70 03 54 ; > > ... > > 01f0: 0a 2d f5 de 59 3e d9 5e 74 93 d2 45 02 03 01 00 ; > 0200: 01 ; > 0201:a3 7b;
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]
> Hello, newbie here > > I picked JDK-8138732 to work on because it has a "starter" label and I > believe I understand what to do. > > - I tried to update the copyright year to 2020 in every file. > - I decided to change `@since` from 9 to 16 since it is a new annotation name > in a new package. > - I tried to keep code changes to a minimum, eg not change to imports if > fully qualified class names are used instead of > imports. In some cases I did minor reordering of imports to keep them > sorted alphabetically. > - All tier1 tests pass. > - One jpackage/jlink tier2 test fails but I don't believe it is related. > - Some tier3 Swing tests fail but I don't think they are related. Philippe Marschall has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate - Changes: https://git.openjdk.java.net/jdk/pull/45/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=45=04 Stats: 749 lines in 65 files changed: 149 ins; 149 del; 451 mod Patch: https://git.openjdk.java.net/jdk/pull/45.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/45/head:pull/45 PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8235710: Remove the legacy elliptic curves [v2]
On Tue, 22 Sep 2020 00:18:07 GMT, Anthony Scarpino wrote: >> This change removes the native elliptic curves library code; as well as, and >> calls to that code, tests, and files >> associated with those libraries. The makefiles have been changed to remove >> from all source builds of the ec code. The >> SunEC system property is removed and java.security configurations changed to >> reflect the removed curves. This will >> remove the following elliptic curves from SunEC: secp112r1, secp112r2, >> secp128r1, secp128r2, secp160k1, secp160r1, >> secp160r2, secp192k1, secp192r1, secp224k1, secp224r1, secp256k1, sect113r1, >> sect113r2, sect131r1, sect131r2, >> sect163k1, sect163r1, sect163r2, sect193r1, sect193r2, sect233k1, sect233r1, >> sect239k1, sect283k1, sect283r1, >> sect409k1, sect409r1, sect571k1, sect571r1, X9.62 c2tnb191v1, X9.62 >> c2tnb191v2, X9.62 c2tnb191v3, X9.62 c2tnb239v1, >> X9.62 c2tnb239v2, X9.62 c2tnb239v3, X9.62 c2tnb359v1, X9.62 c2tnb431r1, >> X9.62 prime192v2, X9.62 prime192v3, X9.62 >> prime239v1, X9.62 prime239v2, X9.62 prime239v3, brainpoolP256r1 >> brainpoolP320r1, brainpoolP384r1, brainpoolP512r1 > > Anthony Scarpino has updated the pull request incrementally with one > additional commit since the last revision: > > remove JDKOPT_DETECT_INTREE_EC from configure.ac src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java line 219: > 217: > 218: Collection supportedCurves; > 219: supportedCurves = CurveDB.getSupportedCurves(); Shouldn't the supportedCurves be the hardcoded 3 curves? - PR: https://git.openjdk.java.net/jdk/pull/289
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException
On Wed, 23 Sep 2020 15:12:58 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for >> https://bugs.openjdk.java.net/browse/JDK-8242882? >> >> As noted in that JBS issue, if the size of the Manifest entry in the jar >> happens to be very large (such that it exceeds >> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can >> lead to a `NegativeArraySizeException`. This >> is due to the: if (len != -1 && len <= 65535) block which evaluates to >> `true` when the size of the manifest entry is >> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the >> code which can lead to the >> `NegativeArraySizeException`. The commit in this PR fixes that issue by >> changing those `if/else` blocks to prevent >> this issue and instead use a code path that leads to the >> `InputStream#readAllBytes()` which internally has the >> necessary checks to throw the expected `OutOfMemoryError`. This commit also >> includes a jtreg test case which >> reproduces the issue and verifies the fix. > > I had created a copy of this branch in my personal fork and included the > `submit.yml` workflow as noted in this recent > mail here[1] to try and run the `tier1` testing for this change. But it looks > like that has failed for unrelated > reasons[2]. So I'll go ahead and trigger the `tier1` tests locally instead. > [1] > https://mail.openjdk.java.net/pipermail/jdk-dev/2020-September/004736.html [2] > https://github.com/jaikiran/jdk/actions/runs/268948812 Hi, Jaikiran. I can sponsor this change. - PR: https://git.openjdk.java.net/jdk/pull/323
RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException
Can I please get a review and a sponsor for a fix for https://bugs.openjdk.java.net/browse/JDK-8242882? As noted in that JBS issue, if the size of the Manifest entry in the jar happens to be very large (such that it exceeds the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can lead to a `NegativeArraySizeException`. This is due to the: if (len != -1 && len <= 65535) block which evaluates to `true` when the size of the manifest entry is larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the code which can lead to the `NegativeArraySizeException`. The commit in this PR fixes that issue by changing those `if/else` blocks to prevent this issue and instead use a code path that leads to the `InputStream#readAllBytes()` which internally has the necessary checks to throw the expected `OutOfMemoryError`. This commit also includes a jtreg test case which reproduces the issue and verifies the fix. - Commit messages: - 8242882: opening jar file with large manifest might throw NegativeArraySizeException Changes: https://git.openjdk.java.net/jdk/pull/323/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=323=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8242882 Stats: 88 lines in 2 files changed: 86 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/323.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/323/head:pull/323 PR: https://git.openjdk.java.net/jdk/pull/323
Re: RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException
On Wed, 23 Sep 2020 15:06:55 GMT, Jaikiran Pai wrote:
> Can I please get a review and a sponsor for a fix for
> https://bugs.openjdk.java.net/browse/JDK-8242882?
>
> As noted in that JBS issue, if the size of the Manifest entry in the jar
> happens to be very large (such that it exceeds
> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can
> lead to a `NegativeArraySizeException`. This
> is due to the: if (len != -1 && len <= 65535) block which evaluates to
> `true` when the size of the manifest entry is
> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the
> code which can lead to the
> `NegativeArraySizeException`. The commit in this PR fixes that issue by
> changing those `if/else` blocks to prevent
> this issue and instead use a code path that leads to the
> `InputStream#readAllBytes()` which internally has the
> necessary checks to throw the expected `OutOfMemoryError`. This commit also
> includes a jtreg test case which
> reproduces the issue and verifies the fix.
I had created a copy of this branch in my personal fork and included the
`submit.yml` workflow as noted in this recent
mail here[1] to try and run the `tier1` testing for this change. But it looks
like that has failed for unrelated
reasons[2]. So I'll go ahead and trigger the `tier1` tests locally instead.
[1] https://mail.openjdk.java.net/pipermail/jdk-dev/2020-September/004736.html
[2] https://github.com/jaikiran/jdk/actions/runs/268948812
test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 60:
> 58: final OutOfMemoryError oome =
> Assert.expectThrows(OutOfMemoryError.class, () -> jar.getManifest());
> 59: // additionally verify that the OOM was for the right/expected
> reason
> 60: if (!"Required array size too large".equals(oome.getMessage())) {
I wasn't too sure if I should add this additional check on the message of the
`OutOfMemoryError`, but decided to do it
anyway, given that from what I remember there were some discussions in the
`core-libs-dev` list a while back on the
exact messages that such OOMs should throw. So I guessed that it might be OK to
rely on those messages in the tests
within this project. However, I am open to removing specific check if it's
considered unnecessary.
-
PR: https://git.openjdk.java.net/jdk/pull/323
RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA
Major points in CSR at https://bugs.openjdk.java.net/browse/JDK-8245274: - new sigalg "RSASSA-PSS", "EdDSA", "Ed25519" and "Ed448" can be used in jarsigner - The ".RSA" and ".EC" block extension types (PKCS #7 SignedData inside a signed JAR) are reused for new signature algorithms - A new JarSigner property "directsign" - Updating the jarsigner tool doc Major code changes: - Always use the signature algorithm directly as SignerInfo::signatureAlgorithm. We used to use the encryption algorithm there like RSA, DSA, and EC. Now it's always SHA1withRSA or RSASSA-PSS. - Move signature related utilities methods from AlgorithmId.java to SignatureUtil.java - Add new SignatureUtil methods fromKey() and fromSignature() to simplify creating Signature and getting its AlgorithmId - Use the new methods in PKCS10, X509CertImpl, and X509CRLImpl signing - Add a new (and intuitive, IMHO) PKCS7::generateNewSignedData capable of all old and new signature algorithms - Mark all -altsign related code deprecated and they can be removed once ContentSigner is removed - Commit messages: - 8242068: Signed JAR support for RSASSA-PSS and EdDSA Changes: https://git.openjdk.java.net/jdk/pull/322/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=322=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8242068 Stats: 1641 lines in 20 files changed: 930 ins; 548 del; 163 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8252377: Incorrect encoding for EC AlgorithmIdentifier
On Wed, 23 Sep 2020 06:07:09 GMT, Hai-May Chao wrote: >> I don't quite understand what the test is for. The bug is about encoding but >> the test seems to be decoding the >> certificates. Does the test fail before this fix and succeed after it? > > This is because the encoding of Algorithm Identifier incorrectly adds two > NULL tags to the parameters field in the > canned certificate. (There are two Algorithm Identifiers in the cert, with > each NULL tag containing two bytes: tag + > length.) I use the length of an encoded certificate > (x509Cert.getEncoded().length) to verify that the certificate > contains an extra 4 bytes to hold the two NULL tags. Therefore, the > certificate without the fix should be 4 bytes (5 > bytes if one byte alignment is applied) longer in length than the certificate > with the fix. But the certificates included in the test are static and if one day we mistakenly add the NULL back it will not be detected by the test. My idea is to generate a cert on the fly inside the test so that can check the `derEncode` method in *this* JDK. As for checking if the NULL is there, you can try looking at the `DerUtils::shouldNotExist` method with "021" and "11" as arguments. - PR: https://git.openjdk.java.net/jdk/pull/312
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
On Tue, 22 Sep 2020 20:19:21 GMT, Alexey Bakhtin wrote: >> Hi, >> >> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support >> for Java GSS/Kerberos. >> Initial review is available at core-devs: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html >> This version removes "tls-unique" CB type from the list of possible channel >> binding types. The only supported type is >> "tls-server-end-point" >> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 >> >> Thank you >> Alexey > > Alexey Bakhtin has updated the pull request incrementally with one additional > commit since the last revision: > > 8245527: version.01 Hi Alexey, The latest changes looks good to me. - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
On Tue, 22 Sep 2020 20:19:21 GMT, Alexey Bakhtin wrote: >> Hi, >> >> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support >> for Java GSS/Kerberos. >> Initial review is available at core-devs: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html >> This version removes "tls-unique" CB type from the list of possible channel >> binding types. The only supported type is >> "tls-server-end-point" >> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 >> >> Thank you >> Alexey > > Alexey Bakhtin has updated the pull request incrementally with one additional > commit since the last revision: > > 8245527: version.01 Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR(S): 8252407: Build failure with gcc-8+ and asan
Hi Kim, Sorry for the delay. This patch removes a redundant string copy in NetworkInterface.c to avoid string-truncation warning. Other warnings we talked before, which are unable to completely fix in different version of gcc, I have to use pragma to suppress them as a workaround. This patch now could compile with gcc-7, gcc-8, gcc-9, gcc-10 both with or without asan. [TESTS] Jtreg: hotspot::hotspot_all_no_apps, jdk::jdk_core and langtools::tier1. No new failure found. http://cr.openjdk.java.net/~qfeng/ericliu/jdk/stringop_trunc/webrev.01/ https://bugs.openjdk.java.net/browse/JDK-8252407 Thanks, Eric
Re: RFR: 8252377: Incorrect encoding for EC AlgorithmIdentifier
On Wed, 23 Sep 2020 02:49:29 GMT, Weijun Wang wrote: >> This change fixes the DER encoding for ECDSA AlgorithmIdentifier to omit the >> parameters field instead of encoding a >> Null tag. > > I don't quite understand what the test is for. The bug is about encoding but > the test seems to be decoding the > certificates. Does the test fail before this fix and succeed after it? This is because the encoding of Algorithm Identifier incorrectly adds two NULL tags to the parameters field in the canned certificate. (There are two Algorithm Identifiers in the cert, with each NULL tag containing two bytes: tag + length.) I use the length of an encoded certificate (x509Cert.getEncoded().length) to verify that the certificate contains an extra 4 bytes to hold the two NULL tags. Therefore, the certificate without the fix should be 4 bytes (5 bytes if one byte alignment is applied) longer in length than the certificate with the fix. - PR: https://git.openjdk.java.net/jdk/pull/312
