Hi Audrey,
Thanks for the comment, we can apply your suggestion in the next update
of KnownOIDs class.
Valerie
On 6/6/2020 9:03 AM, Andrey Turbanov wrote:
List.of(KnownOIDs.values()).forEach(o -> {
register(o);
});
I wonder if this 'forEach' is better than plain old 'for' in some ways?
>List.of(KnownOIDs.values()).forEach(o -> {
>register(o);
>});
I wonder if this 'forEach' is better than plain old 'for' in some ways?
```
for (KnownOIDs oid : KnownOIDs.values()) {
register(oid);
}
```
I think a variant with plain 'for' generates a bit less garbage.
Andrey Turbanov
Still looks fine to me.
Thanks,
Max
> On May 19, 2020, at 5:36 AM, Valerie Peng wrote:
>
> Updated again due to the merge with Tony's EdDSA change:
>
> http://cr.openjdk.java.net/~valeriep/8242151/webrev.06
>
> Added Ed25519 and Ed448 to KnownOIDs, and rest are just adjustments
>
Updated again due to the merge with Tony's EdDSA change:
http://cr.openjdk.java.net/~valeriep/8242151/webrev.06
Added Ed25519 and Ed448 to KnownOIDs, and rest are just adjustments
accordingly.
Touched:
src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdDSAParameters.java
Great, thanks much for the thorough review~
Valerie
On 5/15/2020 8:57 PM, Weijun Wang wrote:
Well done. Everything looks fine to me.
--Max
On May 16, 2020, at 5:47 AM, Valerie Peng wrote:
Hi Max,
I have updated the webrev
(http://cr.openjdk.java.net/~valeriep/8242151/webrev.05/) to
Well done. Everything looks fine to me.
--Max
> On May 16, 2020, at 5:47 AM, Valerie Peng wrote:
>
> Hi Max,
>
> I have updated the webrev
> (http://cr.openjdk.java.net/~valeriep/8242151/webrev.05/) to address your
> suggestion below. Touched classes are NamedCurve, CurveDB,
>
Hi Max,
I have updated the webrev
(http://cr.openjdk.java.net/~valeriep/8242151/webrev.05/) to address
your suggestion below. Touched classes are NamedCurve, CurveDB,
ConstraintsParameters, and SunEC. The result of using the single method
looks pretty good - clean and shorter code. :)
> On May 14, 2020, at 3:17 AM, Valerie Peng wrote:
>
> Hi Max,
>
> Thanks for review~
>
> Webrev is updated accordingly for your comments:
> http://cr.openjdk.java.net/~valeriep/8242151/webrev.04/
>
> Please find replies inline below:
>
> On 5/11/2020 9:24 PM, Weijun Wang wrote:
>> Some
Hi Max,
Thanks for review~
Webrev is updated accordingly for your comments:
http://cr.openjdk.java.net/~valeriep/8242151/webrev.04/
Please find replies inline below:
On 5/11/2020 9:24 PM, Weijun Wang wrote:
Some files have trailing spaces.
Yes, I generally check for white spaces before
Some files have trailing spaces.
KnownOIDs.java:
- Is there an order for the fields? I see they are grouped but now always
ordered.
- Unused "import java.security.Provider;"
- 1.3.14.3.2.29 used to be an alias for SHA1withRSA, now it's not. Is this
intended?
- s/_/-/ in process() is now
Thanks for filing the bug for PBES2Parameters class.
Webrev for 8242151 is updated at:
http://cr.openjdk.java.net/~valeriep/8242151/webrev.03/
It addresses:
- added KnownOIDs for CurveDB class
- updated the KDF parsing code in PBES2Parameters class to match
existing behavior
- removed the
>
> It seems that existing impl of PBES2Parameters class only enforces that the
> KDF algo is one of the HmacSHAxxx. But it does not throw exception if the
> instance is requested with "PBEWithHmacSHA256AndAES_256" and then initialized
> with DER encoding containing
Hi Max,
Thanks for the review, I find your comments very useful.
Please find responses inline.
On 5/6/2020 5:48 AM, Weijun Wang wrote:
- PBES2Parameters.java:
In parseKDF(), any OID can be accepted as kdfAlgo, wonder if it would lead to
unexpected result:
jshell> var a =
Sure, I can include this in. Will address in webrev.03.
Thanks,
Valerie
On 5/5/2020 3:33 AM, Weijun Wang wrote:
I am playing with keytool + BouncyCastle and generate a key pair using the sigalg
"SHA3-256WITHECDSA", and `keytool -list` cannot show the signature name.
So I tried 2 changes in
- PBES2Parameters.java:
In parseKDF(), any OID can be accepted as kdfAlgo, wonder if it would lead to
unexpected result:
jshell> var a = AlgorithmParameters.getInstance("PBEWithHmacSHA256AndAES_256")
jshell> a.init(new PBEParameterSpec("hello".getBytes(), 100, new
Hi Max,
Webrev has been updated,
http://cr.openjdk.java.net/~valeriep/8242151/webrev.02/.
Major changes are:
- Moved oidTable caching from AlgorithmId class to ObjectIdentifier
class. Made ObjectIdentifier constructor private and callers have to use
the of(String) method which always check
Hi Max,
Thanks for the comments, they are very helpful. I should have webrev.02
ready in a bit. Please find comments below in line.
On 5/1/2020 2:45 AM, Weijun Wang wrote:
ObjectIdentifier.java
-
Have you thought about storing the ObjectIdentifier object somewhere?
I am playing with keytool + BouncyCastle and generate a key pair using the
sigalg "SHA3-256WITHECDSA", and `keytool -list` cannot show the signature name.
So I tried 2 changes in AlgorithmId.java:
1. add both the name->oid and oid->name mapping inside collectOIDAliases() for
3rd party
Do you want to add OIDs in CurveDB into KnownOIDs as well?
Thanks,
Max
Hmm, I took a shot at keytool/Main.java and used
KnownOIDs.findMatch(...) to construct the oid.
They will be included in webrev.02.
Thanks,
Valerie
On 5/1/2020 3:29 PM, Valerie Peng wrote:
These two BASE ones are simply used to get rid of the hardcoded oid
string code in
These two BASE ones are simply used to get rid of the hardcoded oid
string code in keytool/Main.java.
I can remove them (in webrev.02) and maybe you can update
keytool/Main.java later to use the right KnownOIDs enum for oid
construction? There are a few places in keytool/Main.java which
One more thing:
In KnownOIDs.java, I found these 2 lines:
PKIX_KP_BASE("1.3.6.1.5.5.7.3."),
PKIX_OCSP_BASE("1.3.6.1.5.5.7.48."),
IMHO, they should not belong here, at least, we should remove the dot at the
end and make them real OIDs.
I was testing the ObjectIdentifier generation and
ObjectIdentifier.java
-
Have you thought about storing the ObjectIdentifier object somewhere?
ObjectIdentifier.of() creates a new object each time and the conversion of
string to byte[] might be a performance issue. We used to have a lot of
ObjectIdentifier objects in
> On Apr 30, 2020, at 9:55 PM, Weijun Wang wrote:
>
> Sorry, it's been a busy day on something else and I haven't looked at your
> webrev.01 yet. Will look at it tomorrow.
>
>> On Apr 30, 2020, at 8:15 AM, Valerie Peng wrote:
>>
>>
>> If you look at the original SunJCE impl, it also did
Sorry, it's been a busy day on something else and I haven't looked at your
webrev.01 yet. Will look at it tomorrow.
> On Apr 30, 2020, at 8:15 AM, Valerie Peng wrote:
>
>
> If you look at the original SunJCE impl, it also did not register oid for RC4
> cipher. So, that's why I didn't include
If you look at the original SunJCE impl, it also did not register oid
for RC4 cipher. So, that's why I didn't include RC4 oid in
SecurityProviderConstants in the aliases for RC4.
If I recall correctly, "RC4" is the algorithm name, however, due to some
patent(?) concern, non-RSA vendors
Hi Max,
I believe most of your comments (except the two below) in this email
should already be addressed in webrev.01.
I caught them by looking at the "skipped" debug output. Can we add some logic
to detect this? For example, for those nonpreferred OIDs, use a special constructor?
Hmm, ok,
toString() returns the standard name which the enum corresponds to.
Yes, I agree that it's better to use a dedicated method name for this.
Will address this in webrev.02.
Valerie
On 4/28/2020 3:37 AM, Weijun Wang wrote:
Is OidString::toString really used anywhere? If so, I suggest we
Hi Max,
Here is the updated webrev
http://cr.openjdk.java.net/~valeriep/8242151/webrev.01/
which should address your earlier comments (see below)
- OidString renamed to KnownOIDs and updated the aliasing handling code
in both KnownOIDs and SecurityProviderConstants classes
- Removed
Is OidString::toString really used anywhere? If so, I suggest we create a
special method for it so it's easy to use an IDE to find out all usages.
Finding toString returns usages of Object::toString and it's everywhere.
--Max
> On Apr 24, 2020, at 7:11 AM, Valerie Peng wrote:
>
> Hi Max,
>
I found two algorithm names in a very twisted relation, in
SecurityProviderConstants.java:
store("ARCFOUR", "RC4");
and in OidString.java:
RC4("1.2.840.113549.3.4", "ARCFOUR")
So each is the other's alias, and because of this, Cipher.ARCFOUR does not have
OID aliases.
I can see
Where is the following OID used
RSAEncryption("1.2.840.113549.1.1.1", "RSA"), // in RSA Cipher
I only found in RSAUtil.java:
case RSA:
oid = AlgorithmId.RSAEncryption_oid;
break;
What if we do not give it a different stdName? Or,
> On Apr 28, 2020, at 6:19 AM, Valerie Peng wrote:
>
> Hi Max,
>
> Thanks for reviewing this~ Please find my replies inline.
>
> On 4/25/2020 3:28 AM, Weijun Wang wrote:
>> OidString.java
>> ==
>>
>> 1. ExtendedKeyUsage names: used to be "serverAuth", now "ServerAuth". First
> On Apr 28, 2020, at 8:08 AM, Valerie Peng wrote:
>
> Yes, it can be quite a confusing scenario. Some algorithms have multiple oids
> (say X) and multiple algorithm names (say Y). We need to be able to handle
> all of them (X+Y). However, the separation of alias definition for OidString
>
Yes, it can be quite a confusing scenario. Some algorithms have multiple
oids (say X) and multiple algorithm names (say Y). We need to be able to
handle all of them (X+Y). However, the separation of alias definition
for OidString and SecurityProviderConstants are intentional and I have
them
Hi Max,
Thanks for reviewing this~ Please find my replies inline.
On 4/25/2020 3:28 AM, Weijun Wang wrote:
OidString.java
==
1. ExtendedKeyUsage names: used to be "serverAuth", now "ServerAuth". First
letter capitalized, is this necessary?
Yes, I made a change here. Using
I see this change in Provider$Service::toString output of SHA1withRSA:
SunRsaSign: Signature.SHA1withRSA -> sun.security.rsa.RSASignature$SHA1withRSA
- aliases: [1.2.840.113549.1.1.5, 1.3.14.3.2.29, OID.1.2.840.113549.1.1.5]
+ aliases: [1.2.840.113549.1.1.5, OID.1.2.840.113549.1.1.5]
And I am confused by aliases in SecurityProviderConstants, together with the
duplicated stdName in OidString, it seems to show that one OID could have
multiple names, and one name could have multiple OIDs. Can we consolidate
aliases inside OidString also?
Also, looking at the store() calls in
OidString.java
==
1. ExtendedKeyUsage names: used to be "serverAuth", now "ServerAuth". First
letter capitalized, is this necessary?
2. Can we move name2oidStr() from OidString to AlgorithmId? The computeOidTable
process looks like an alien.
3. Two questions on the following
Hi Max,
Would you have time to review this change? The current webrev attempts
to cover all security classes where hard-coded oid strings and
consolidate these known oid string values into a single enum type. The
changes are quite extensive, I can trim back and only cover the provider
40 matches
Mail list logo
