Re: RFR: 8210838: Override javax.crypto.Cipher.toString()
Signature's toString looks like
public String toString() {
String initState = "";
switch (state) {
case UNINITIALIZED:
initState = "";
break;
case VERIFY:
initState = "";
break;
case SIGN:
initState = "";
break;
}
return "Signature object: " + getAlgorithm() + initState;
}
Maybe you can add some similar info.
In fact, it looks like you can extract the debug output at the end of each
init() methods into a new toString() method.
Thanks
Max
> On Nov 16, 2018, at 12:35 AM, Seán Coffey wrote:
>
> A simple enhancement to override toString() for javax.crypto.Cipher class
>
> https://bugs.openjdk.java.net/browse/JDK-8210838
> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8210838/webrev/
>
> regards,
> Sean.
>
Re: RFR 8212003: Obsoleting the default keytool -keyalg option
Would like also please review the release note here? https://bugs.openjdk.java.net/browse/JDK-8213965 I had thought about using RN-Deprecated but there is no API here. If you think it's better, I'll use it and also change all "obsolete" into "deprecate" in the description and title of the release note/CSR/bug. Thanks Max > On Nov 15, 2018, at 9:19 AM, Weijun Wang wrote: > > Thanks to Xuelei and Sean. I added your recommended words and proposed the > CSR. > >> On Nov 15, 2018, at 6:16 AM, Sean Mullan wrote: >> >> On 11/14/18 5:07 AM, Weijun Wang wrote: >>> The CSR is re-opened. It is now focusing on -keyalg only. Please take a >>> review: >>> https://bugs.openjdk.java.net/browse/JDK-8212111 >> >> I think the CSR should also include an example of the informational text >> showing what algs and size were used. Looks good otherwise. >> >> --Sean >> >>> Thanks >>> Max On Nov 7, 2018, at 11:51 PM, Weijun Wang wrote: Oops, I take this back. The CSR needs more update. Sorry if you have already start reading it. Thanks Max > On Nov 7, 2018, at 9:27 AM, Weijun Wang wrote: > > After some discussion, we decided to cover -keysize and -sigalg in this > deprecation process too. > > Please review the updated CSR at > > https://bugs.openjdk.java.net/browse/JDK-8212111 > > No webrev available yet. > > Thanks > Max > > >> On Oct 18, 2018, at 10:34 AM, Weijun Wang wrote: >> >> Please review the code change and CSR for >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8212003 >> >> at >> >> webrev: http://cr.openjdk.java.net/~weijun/8212003/webrev.00/ >> CSR: https://bugs.openjdk.java.net/browse/JDK-8212111 >> >> When -keyalg is not provided for -genkeypair or -genseckey, keytool will >> print out a warning. We plan to make this an error in a future release. >> >> A new regression test ObsoleteKeyalg.java added. "-keyalg DSA" or >> "-keyalg DES" added to other tests. >> >> A Mach5 job on tier1 and tier2 running now. >> >> Thanks >> Max >> > >
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
Hi Sean,
Are you OK if we do it later? I'm waiting for the @systemProperty tag,
proposed within JDK-5076751. I will file a bug to use the tag for more
cleanup.
Thanks,
Xuelei
On 11/15/2018 11:55 AM, Sean Mullan wrote:
This is a good opportunity to document the
javax.net.ssl.sessionCacheSize system property in the SSLSessionContext
API (and use the new @systemProperty tag) in an @implNote, for example:
/**
* Returns the size of the cache used for storing
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @implNote The JDK implementation returns the cache size as set by
* the {@code setSessionCacheSize method}, or if not set, the value
* of the {@systemProperty javax.net.ssl.sessionCacheSize} system
* property. If neither is set, it returns a default value of 20480.
*
* @return size of the session cache; zero means there is no size
limit.
* @see #setSessionCacheSize
*/
public int getSessionCacheSize();
On 11/14/18 11:59 AM, Xuelei Fan wrote:
Hi,
Please review this simple update:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.00/
The default value for the maximum number of entries in the SSL session
cache (SSLSessionContext.getSessionCacheSize()) is infinite now. In
the request, the default value is updated to 20480 for performance
consideration.
For the detailed behavior update, please refer to CSR:
https://bugs.openjdk.java.net/browse/JDK-8213577
Thanks,
Xuelei
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
This is a good opportunity to document the
javax.net.ssl.sessionCacheSize system property in the SSLSessionContext
API (and use the new @systemProperty tag) in an @implNote, for example:
/**
* Returns the size of the cache used for storing
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @implNote The JDK implementation returns the cache size as set by
* the {@code setSessionCacheSize method}, or if not set, the value
* of the {@systemProperty javax.net.ssl.sessionCacheSize} system
* property. If neither is set, it returns a default value of 20480.
*
* @return size of the session cache; zero means there is no size
limit.
* @see #setSessionCacheSize
*/
public int getSessionCacheSize();
On 11/14/18 11:59 AM, Xuelei Fan wrote:
Hi,
Please review this simple update:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.00/
The default value for the maximum number of entries in the SSL session
cache (SSLSessionContext.getSessionCacheSize()) is infinite now. In the
request, the default value is updated to 20480 for performance
consideration.
For the detailed behavior update, please refer to CSR:
https://bugs.openjdk.java.net/browse/JDK-8213577
Thanks,
Xuelei
RFR: 8210838: Override javax.crypto.Cipher.toString()
A simple enhancement to override toString() for javax.crypto.Cipher class https://bugs.openjdk.java.net/browse/JDK-8210838 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8210838/webrev/ regards, Sean.
Re: FYI: new javadoc tag to document system properties
In JCE and JSSE, the public APIs definition (javax.net.ssl) and the
internal implementation (sun.security.ssl) are separated. The system
property can be defined in the internal implementation classes. I think
we should add the @systemProperty on the public APIs, right?
The public API class and the implementation class may be not a
one-to-one map. Multiple public APIs may be impacted by the system
property. How to handle such cases?
Thanks,
Xuelei
On 11/14/2018 2:27 PM, Jonathan Gibbons wrote:
This is an FYI to announce some initial, long-overdue support in javadoc
for documenting system properties (JDK-5076751).
Currently, system properties are just documented using ad-hoc narrative
text, which is fine if you know where to look for any given property.
JDK 12 introduces a new inline javadoc tag, {@systemProperty
/property-name/} which can be used to "declare" the name of a system
property. You can use this tag as a drop-in replacement for the
plain-text property name; it will have no overt effect on the narrative
text, but it will cause the property name to be put into the search
index and the A-Z index. Thus, property names will become searchable, to
allow users to easily find the definition of any such system property.
Adding support into javadoc is only part of the story. Now that the
support is in javadoc, we want to update the API documentation, so that
the documentation is updated for all Java SE and JDK system properties.
Where should the tag be used? The tag should be used in the text of the
defining instance of the property. This is where the characteristics of
the system property are described, which may include information like:
"what is the property for", "how and when is it set", "can it be
modified", and so on. For example, it could be used in the docs for
System.getProperties [1] or Networking Properties [2] In general, it
should -not- be used in situations that merely refer to the system
property by name. For example, the docs for Path.toAbsolutePath [3]
make a reference to the system property user.dir, but that is not a
definition of the property.
Going forward, in future releases, we expect to explore some
enhancements to this feature. Here are some of the ideas that have been
suggested:
* Create a "summary page" that lists all the system properties. This
would be somewhat similar to the current top-level "Constant Values"
page.
* Add information regarding the "scope" of the definition: is it
defined by the Java SE specification, or JDK, or JavaFX, etc. This
information could be used to organize the content on the summary page.
* Update @see and {@link} to be able to refer to system properties.
* Allow a short description to be included in the {@systemProperty}
tag. This text could be included in the search index, the A-Z index,
and the summary page.
-- Jon
[1]:
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/System.html#getProperties()
[2]:
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/doc-files/net-properties.html
[3]:
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/file/Path.html#toAbsolutePath()
Re: RFR 8213009: Refactoring existing SunMSCAPI classes
Oops, my copy/paste sequence goes wrong. > On Nov 15, 2018, at 11:38 PM, Weijun Wang wrote: > > Webrev updated at > https://cr.openjdk.java.net/~weijun/8213009/webrev.01/ > > More refactorings: > > - getEncoded and getFormat of CKey removed, implemented in CPublicKey and > CPrivateKey. > > - CPublicKey has child class CRSAPublicKey, CKeyPairGenerator has child class > RSA. > > - CPublicKey and CPrivateKey now have a static of() method that can return a > child instance. > > - CCipher renamed to CRSACipher. I realized there won't be CECCipher. > > Thanks > Max > > >> On Nov 7, 2018, at 12:13 AM, Weijun Wang wrote: >> >> Webrev updated at >> >> https://cr.openjdk.java.net/~weijun/8213009/webrev.00/ >> >> The subtask id is now used. >> >> The previous refactoring has removed the "RSA" algorithm info from some >> keys. This update adds them back. >> >> Thanks >> Max >> >>> On Oct 25, 2018, at 4:38 PM, Weijun Wang wrote: >>> >>> Please review the change at >>> >>> https://cr.openjdk.java.net/~weijun/8026953/webrev.00/ >>> >>> (I will use a sub-task id for this change but currently JBS is down). >>> >>> The major change is renaming classes. Since we are going to support >>> algorithms other than RSA, I've renamed the classes like RSAPrivateKey -> >>> CPrivateKey. Classes that have the same name as JCA classes (like Key, >>> KeyStore) are also renamed (to CKey, CKeyStore) so it's easy to tell them >>> apart. >>> >>> Others are not about renaming but they are also related to supporting other >>> algorithms, and there is no behavior change. They include: >>> >>> - CKey (plus its child classes CPublicKey and CPrivateKey) has a new field >>> "algorithm". This field is used by >>> CKeyStore::generateRSAKeyAndCertificateChain and its value is obtained from >>> the public key algorithm in a cert [1]. >>> >>> - Child class named "RSA" of CKeyPairGenerator. >>> >>> - Child class named "RSA" of CSignature. I also moved some RSA-related >>> methods into this child class as overridden methods. >>> >>> - CKeyStore::setPrivateKey's key parameter has a new type Key, but it still >>> only accepts RSAPrivateCrtKey now. >>> >>> Noreg-cleanup. >>> >>> Thanks >>> Max >>> >>> [1] >>> https://docs.microsoft.com/en-gb/windows/desktop/api/wincrypt/ns-wincrypt-_crypt_algorithm_identifier >> >
Re: RFR 8213009: Refactoring existing SunMSCAPI classes
Webrev updated at
Asserts.assertTrue(pr.getAlgorithm().equals("RSA"));
More refactorings:
- getEncoded and getFormat of CKey removed, implemented in CPublicKey and
CPrivateKey.
- CPublicKey has child class CRSAPublicKey, CKeyPairGenerator has child class
RSA.
- CPublicKey and CPrivateKey now have a static of() method that can return a
child instance.
- CCipher renamed to CRSACipher. I realized there won't be CECCipher.
Thanks
Max
> On Nov 7, 2018, at 12:13 AM, Weijun Wang wrote:
>
> Webrev updated at
>
> https://cr.openjdk.java.net/~weijun/8213009/webrev.00/
>
> The subtask id is now used.
>
> The previous refactoring has removed the "RSA" algorithm info from some keys.
> This update adds them back.
>
> Thanks
> Max
>
>> On Oct 25, 2018, at 4:38 PM, Weijun Wang wrote:
>>
>> Please review the change at
>>
>> https://cr.openjdk.java.net/~weijun/8026953/webrev.00/
>>
>> (I will use a sub-task id for this change but currently JBS is down).
>>
>> The major change is renaming classes. Since we are going to support
>> algorithms other than RSA, I've renamed the classes like RSAPrivateKey ->
>> CPrivateKey. Classes that have the same name as JCA classes (like Key,
>> KeyStore) are also renamed (to CKey, CKeyStore) so it's easy to tell them
>> apart.
>>
>> Others are not about renaming but they are also related to supporting other
>> algorithms, and there is no behavior change. They include:
>>
>> - CKey (plus its child classes CPublicKey and CPrivateKey) has a new field
>> "algorithm". This field is used by
>> CKeyStore::generateRSAKeyAndCertificateChain and its value is obtained from
>> the public key algorithm in a cert [1].
>>
>> - Child class named "RSA" of CKeyPairGenerator.
>>
>> - Child class named "RSA" of CSignature. I also moved some RSA-related
>> methods into this child class as overridden methods.
>>
>> - CKeyStore::setPrivateKey's key parameter has a new type Key, but it still
>> only accepts RSAPrivateCrtKey now.
>>
>> Noreg-cleanup.
>>
>> Thanks
>> Max
>>
>> [1]
>> https://docs.microsoft.com/en-gb/windows/desktop/api/wincrypt/ns-wincrypt-_crypt_algorithm_identifier
>
Re: RFR 8213363: X25519 private key PKCS#8 encoding/decoding is incorrect
Done[1]. Please take a look when you have a chance. [1] https://bugs.openjdk.java.net/browse/JDK-8213946 On 11/14/2018 4:29 PM, Sean Mullan wrote: The fix and the CSR look good. Please also add a release note. --Sean On 11/8/18 11:51 AM, Adam Petcher wrote: Oops. And the JBS ticket: https://bugs.openjdk.java.net/browse/JDK-8213363 On 11/8/2018 11:43 AM, Adam Petcher wrote: webrev: http://cr.openjdk.java.net/~apetcher/8213363/webrev.00/ CSR: https://bugs.openjdk.java.net/browse/JDK-8213493 This change fixes a bug related to the encoded format of X25519/X448 private keys. The new code will not preserve compatibility with the improperly-formatted keys produced by the existing code. I expect the compatibility impact of this change to be minimal, as described in the CSR ticket.
Re: Problems with AES-GCM native acceleration
Hi all, Thanks for the prompt feedback on this stuff, appreciated. 1. Analytic queries are often interactive or one-off. A data scientist would get an on-demand notebook with a Spark cluster (spawned as a K8s pod), and run a number of queries. The cluster will be then closed either explicitly, or after a timeout. This is done both for a better resource utilization, and for security reasons. Re-using JVM for another user/tenant might leak the sensitive data and encryption keys, kept in the JVM memory. I'm not saying its the only way to solve this, there are architectures based on a long running service. But this short-lived approach is real and needs to be addressed. Even if the data scientist keeps the cluster alive for a few hours - having to wait a long time for the results of the first few queries (because the decryption is not warmed up yet) is a problem, since the things are interactive and expected to be done in real time. 2. Analytics and AI workloads work with ~ 64MB blocks; sometimes, they are broken in ~1MB pieces (like in Parquet). Still, taking even the minimal size of 1MB, and waiting the 10,000 rounds to get the decryption acceleration, means we process the first ~10GB at a slow rate. Sounds harsh. Both in absolute numbers, and in comparison to ENcryption, which kicks in after warming up with say 1KB chunks (created by breaking 1MB blocks into many update calls) - meaning ~1,000x faster than DEcryption. 3. Adam has mentioned an approach of "modifying the decryption operation (to decrypt immediately and buffer plaintext)" (in a negative context, though :). To me, it looks like a sound solution. However, I don't know how much effort does it require (?) - but it makes decryption implementation similar to encryption, and solves the problem at hand. Maybe there are other options, though. 4. AOT sounds interesting, I'll check it out. But its experimental for now. Moreover, both AOT and command line options require extra care in production, as correctly pointed out below. They will be a hard sell in real production environments. The same is true (or even worse) for manual warm-up with a repeated decryption of small blocks. This is indeed a benchmarking hack, I don't see it been used in production. Having the decryption optimized in the HotSpot engine would be ideal. Cheers, Gidon. On Thu, Nov 15, 2018 at 3:33 AM Anthony Scarpino < anthony.scarp...@oracle.com> wrote: > I agree with Adam that this is more of a tuning issue and not a problem > with the crypto. Sending multiple updates is a hack. > > I've been aware of this bug for a while and I do not understand why this > is a significant problem. The stackoverflow comments say it takes 50 > seconds to trigger the intrinsic. If this is a long running server > application slowness for the first 50 seconds is trivial. For smaller > operations, those are commonly small transactions, not decrypting a 3GB > file. > > If it cannot be resolved by commandline options and this is occurring in > a real world situation, please explain it fully. If this is only for > benchmarking, then that's not a real world situation. > > Tony > > On 11/14/18 8:41 AM, Adam Petcher wrote: > > I'm adding back in hotspot-dev, because this is a somewhat tricky topic > > related to intrinsics and JIT. Hopefully, a Hotspot expert can correct > > anything that I say below that is wrong, and suggest any solutions that > > I missed. > > > > The AES acceleration is implemented in a HotSpot intrinsic. In order for > > it to kick in, the code must be JIT compiled by the VM. As I understand > > it, this only happens to some particular method after it has been called > > a certain number of times. The rules that determine this number are > > somewhat complicated, but I think you can guarantee JIT in the default > > configuration by calling a method 10,000 times. > > > > The doFinal method calls the update method, so either one should trigger > > the acceleration as long as you call it enough. Breaking the message up > > into smaller chunks and calling update on each one works only because it > > ends up calling the update method more. You should be able to trigger > > the acceleration by calling doFinal more, too. > > > > The reason why the workaround doesn't work with decryption is that the > > decryption routine buffers the ciphertext and then decrypts it all at > > the end. So calling update multiple times and then calling doFinal at > > the end is essentially the same as calling doFinal once with the entire > > ciphertext. > > > > So here are some solutions that you may want to try: > > > > 1) In your benchmark, run at least 10,000 "warmup" iterations of > > whatever you are trying to do at the beginning, without timing it. This > > is a good idea for benchmarks, anyway. If it helps, you can try using > > smaller buffers in your "warmup" phase in order to get it to complete > > faster. > > > > 2) Try -XX:CompileThreshold=(some number smaller than 1) as an > > argumen
