Re: [10] RFR : 8186628 : SSL session cache can cause a scalability bottleneck
Hi Ivan, I understand the desire of performance improvement. But I don't think avoiding the use of cache is the price we want to pay for. Besides, avoiding using of session cache is not something improving the performance in general, it is typically something impacting the performance, a lot sometimes. Xuelei On 11/20/2017 5:36 PM, Ivan Gerasimov wrote: Gentle ping. If people agree on the approach, I'll go ahead and file a CCC request for the new recognized system property. With kind regards, Ivan On 11/7/17 6:24 PM, Ivan Gerasimov wrote: Hello everybody! The class sun.security.ssl.SSLSessionContextImpl maintains caches for the sessions reuse. Access to the cache from threads is serialized. It was reported that under heavy load the time of waiting for the turn to access the synchronized methods outweighs the time of creating a new session. It is proposed to introduce a flag that will allow to avoid using the cache altogether. Would you please help review the proposed fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8186628 WEBREV: http://cr.openjdk.java.net/~igerasim/8186628/00/webrev/
Re: [10] RFR : 8186628 : SSL session cache can cause a scalability bottleneck
Gentle ping. If people agree on the approach, I'll go ahead and file a CCC request for the new recognized system property. With kind regards, Ivan On 11/7/17 6:24 PM, Ivan Gerasimov wrote: Hello everybody! The class sun.security.ssl.SSLSessionContextImpl maintains caches for the sessions reuse. Access to the cache from threads is serialized. It was reported that under heavy load the time of waiting for the turn to access the synchronized methods outweighs the time of creating a new session. It is proposed to introduce a flag that will allow to avoid using the cache altogether. Would you please help review the proposed fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8186628 WEBREV: http://cr.openjdk.java.net/~igerasim/8186628/00/webrev/ -- With kind regards, Ivan Gerasimov
Re: Eliminating the security overhead when not running with a security manager
One thing that springs to mind. Some allowance would have to be made for domain combiners and JAAS Subject propagation: this mechanism also uses access control contexts, to its own great detriment. I would say that the JAAS domain combiner strategy should be dropped in favor of a simple thread local holding the Subject, regardless of what else is decided. I do personally believe that using a security manager as some kind of proof against untrusted Java code is a mistake. However I think one important use case of security managers is where the Java code being run actually _is_ trusted, but you want to avoid "weird machine" effects where server code could be exploited (by way of malformed requests or whatever) to perform actions that are unexpected but also technically allowed (for example, if your file management abstraction is only ever intended to modify "/app/user-storage/-" then you don't want it to be able to access "/app-server/secret-passwords", and if that happens, you maybe want to start yelling a lot). In this case it's an extra layer of protection. I don't see this use case going away or being invalidated in any way; an application server request _is_ code, in a way, that must be interpreted and "executed" after a fashion in order to do something useful for the user. In terms of approach: back when the discussion around StackWalker design was still going on, it seemed to me that the performance costs of the security manager could be mostly shifted to the security manager itself (thus solving the problem in a different way), if it would walk the stack to do access checking directly, like this: • An "acc" field on Thread is used to hold the inherited access control context • AccessController.doPrivileged(task) becomes empty other than to clear the acc field and delegate to the called block (and restore the acc field after) • AccessController.doPrivileged(task, acc) becomes empty other than to set the acc field to the given ACC argument and delegate to the called block (and restore the acc field after) • AccessController.checkPermission() iterates the call stack until it hits one-frame-lower-than-doPrivileged or the bottom of the stack, checking access along the way for each protection domain (with some simple duplicate elimination as it walks), and once it hits the end, it calls checkPermission on the inherited acc field in Thread (if any). • Creating a new thread still captures the caller acc but then copies it into the new thread's inherited acc field • Construction/compilation of the ACC could possibly be moved to userspace, which will reduce any memory overhead of having it baked in to the native JVM code when it's not actually used • The doPrivileged(action, acc, perms) case could be handled with another Thread field which could be checked before or after the primary check (in this case all the doPrivileged methods would have to stash and set this field as well, and restore it after) • A "shared secret" is added to let AccessController access the two Thread cache field(s) This way the cost of doPrivileged is greatly minimized (two Thread field reads and writes before the block, and two Thread field writes after the block, assuming the aforementioned second field for array permissions) without affecting security manager functionality should it happen to be enabled in software. Also it's easy to improve the security manager debug output, such that you could get a report on _all_ classes or modules or codeSources or whatever which would do not grant a given permission (instead of just the first one), which is greatly useful in reducing iterations in the design and debug stages. When the security manager is actually enabled, only testing would reveal the comparative performance of a stack walker based approach versus the current native ACC optimization code (maybe C2 can work some magic here). There's probably some kind of disgusting weirdness around method handles and reflection things on the call stack that would have to be examined as well. Finally, it will (as always) be a challenge to craft a Stream-consuming Function that doesn't allocate scads of objects when actually iterating the thread stack. Maybe we could allow an Iterator-based StackWalker method. Maybe I'll keep dreaming. Anyway I never got a chance to prototype this, but it might be a fun option worth exploring. I found the idea of moving this stuff all to user space to be very appealing (due in no small part to the idea that it could potentially be examined and analyzed by a much larger audience, being Java code). It also hints at the possibility of a fully "user space" replacement of the security manager concept (much of the remaining cost lives in the structure of AccessControlContext, which is based on an array of ProtectionDomain objects; this is definitely non-ideal and could possibly be hidden behind a smarter abstraction). I really firmly believe that domain combiners should be ultimately eliminated
Re: KDF API review, round 2
On 11/20/2017 12:34 PM, Michael StJohns wrote: On 11/20/2017 1:10 PM, Jamil Nimeh wrote: You're missing the point that setParameter() provides information used in all future calls to the signature generation, while init() provides data specifically for a given key stream production. In Signature() you call .setParameter() to set up the PSS parameters (or use the defaults). Each subsequent call to initSign or initVerify uses those PSS parameters. The equivalent part of .init() in KeyDerivation is actually the calls to .update() in signature as they provide the specific information for the production of the output key stream. In fact, setting up an HMAC signature instance and passing it the mixin data as part of a .update() is a way of producing the key stream round. So equivalences: KeyDerivation.getInstance(PRF) == Signature.getInstance(HMAC) KeyDerivation.setParameters() == Signature.setParameters() KeyDerivation.init(key, List) == concatenation of the results of multiple calls (each key stream round based on the needed output length) to [Signature.initSign(Key) followed by Signature.update(converttobytearray(List)) followed by Signature.sign()] to produce the key stream KeyDerivation.deriveKey() == various calls to key or object factories with parts of the key stream (signature). Are you expecting that setParameters is called once per instantiation? If so, then the parameters that would go into setParameter (an APS I assume) could just as easily go into the getInstance call. It also removes the chance that someone would call it twice. That was my original proposal. .setParameter() was an alternative that matched the Signature pattern. Yes, I recall that. Since it's a once-per instance call let me come up with a rev with it in the getInstance. There's precedent for getInstance with APS, too. It's just not any of the keyed forms, and the rationale for that was to combine instantiation and initialization (CertStore is an example). It's not a great comparison to KDF for a myriad of reasons most of which you've talked about...but it at least shows that we have added params to getInstance calls in the past. This seems like one place where we're not going to come up with an answer that pleases everyone. If you're expecting someone to call setParameter more than once, then I would expect an init must follow. So why not place it in a form of init that allows you to change that particular set of params? Either way it seems like we could coalesce this method into one of the calls that sandwich it in your proposed model. I don't expect them to call it more than once. The original (now deprecated) .setParameter (String, Object) method in Signature indicated it could be called only once and would throw an error if called again - I'm not sure why that wasn't brought forward to the Signature.setParameter(AlgorithmParameterSpec) method. In any event, I'd rather do the parameter setting in the getInstance call than as a separate .setParameters call if it can be done without exploding the interface. Hmm.. how does that map to the Spi? Does the KeyDerivation.getInstance() code instantiate the object, call a setParameter() method on the SPI and then return the new object? Or what? It may make more sense to just add the parameter related methods to both the KeyDerivationSpi and the KeyDerivation classes and leave the getInstance() method alone I'm sort of a don't care as long as I have a way of tweaking the KDF before run the first derivation. That's a good question, and one that I've been turning around in my head and don't (yet) have a great answer for, but we'll get there. So my original prototype was based off KeyAgreement.java and the order obtaining the spi depends on the form of getInstance. If it's a simple string-based algorithm form, then the provider is actually selected during the init method. In the other two forms where a provider is specified as either a String or a Provider, the spi is obtained through the Provider object and therefore no init-time selection is needed. We may need to have provider selection done a bit earlier since we're not only having to deal with the KDF itself, but a flavor of the KDF with the underlying PRF. I need to find out a little bit of the history on why the provider selection happens during init for some of these APIs. IIRC there was a reason to have delayed provider selection, but I don't have the history on that one.
Re: Eliminating the security overhead when not running with asecuritymanager
Yes Alan correct: I wrongly asumed ist a separate stack. So it is a non-issue. I tried it out and having a doPriv(lambda) block shortens the possible overall thread dept by about 200 calls (with my simple stack usage). BTW: the main thread does behave a bit less predictable: (run results at end of file) https://github.com/ecki/JavaSystemTest/blob/master/src/main/java/net/eckenfels/test/javasystemtest/StackDepth.java#L86 Greetings Bernd -- http://bernd.eckenfels.net Von: Alan Bateman Gesendet: Montag, 20. November 2017 22:02 An: Bernd Eckenfels; OpenJDK Dev list Betreff: Re: Eliminating the security overhead when not running with asecuritymanager On 20/11/2017 20:15, Bernd Eckenfels wrote: : One thing which might be a problem: when doPrivileged does no longer execute the Code in a seperate stack this has implications to the runtime. The stacks will get deeper (and might even overflow (more often)). So maybe this „no seperate stack“ function should be opt-out (if implemented). Are you assuming that the execution of the privileged action is done on a different thread stack? The effect of doPrivileged is to set the top of the privileged stack on the current thread stack, it gets restored when the action completes. So I think there isn't anything to be concerned about here. -Alan
Re: KDF API review, round 2
On 11/20/2017 1:10 PM, Jamil Nimeh wrote: You're missing the point that setParameter() provides information used in all future calls to the signature generation, while init() provides data specifically for a given key stream production. In Signature() you call .setParameter() to set up the PSS parameters (or use the defaults). Each subsequent call to initSign or initVerify uses those PSS parameters. The equivalent part of .init() in KeyDerivation is actually the calls to .update() in signature as they provide the specific information for the production of the output key stream. In fact, setting up an HMAC signature instance and passing it the mixin data as part of a .update() is a way of producing the key stream round. So equivalences: KeyDerivation.getInstance(PRF) == Signature.getInstance(HMAC) KeyDerivation.setParameters() == Signature.setParameters() KeyDerivation.init(key, List) == concatenation of the results of multiple calls (each key stream round based on the needed output length) to [Signature.initSign(Key) followed by Signature.update(converttobytearray(List)) followed by Signature.sign()] to produce the key stream KeyDerivation.deriveKey() == various calls to key or object factories with parts of the key stream (signature). Are you expecting that setParameters is called once per instantiation? If so, then the parameters that would go into setParameter (an APS I assume) could just as easily go into the getInstance call. It also removes the chance that someone would call it twice. That was my original proposal. .setParameter() was an alternative that matched the Signature pattern. If you're expecting someone to call setParameter more than once, then I would expect an init must follow. So why not place it in a form of init that allows you to change that particular set of params? Either way it seems like we could coalesce this method into one of the calls that sandwich it in your proposed model. I don't expect them to call it more than once. The original (now deprecated) .setParameter (String, Object) method in Signature indicated it could be called only once and would throw an error if called again - I'm not sure why that wasn't brought forward to the Signature.setParameter(AlgorithmParameterSpec) method. In any event, I'd rather do the parameter setting in the getInstance call than as a separate .setParameters call if it can be done without exploding the interface. Hmm.. how does that map to the Spi? Does the KeyDerivation.getInstance() code instantiate the object, call a setParameter() method on the SPI and then return the new object? Or what? It may make more sense to just add the parameter related methods to both the KeyDerivationSpi and the KeyDerivation classes and leave the getInstance() method alone I'm sort of a don't care as long as I have a way of tweaking the KDF before run the first derivation. Mike
Re: Eliminating the security overhead when not running with a securitymanager
Hello Alan, not sure if it is good or bad to make the security manager even less Mainstream (especially after there was a Project to reduce the Impact of security Manager). However, not diving into this discussion here :) One thing which might be a problem: when doPrivileged does no longer execute the Code in a seperate stack this has implications to the runtime. The stacks will get deeper (and might even overflow (more often)). So maybe this „no seperate stack“ function should be opt-out (if implemented). Gruss Bernd -- http://bernd.eckenfels.net Von: Alan Bateman Gesendet: Montag, 20. November 2017 15:47 An: OpenJDK Dev list Betreff: Eliminating the security overhead when not running with a securitymanager One of the long standing issues with the security manager support is that the overhead when there is no security manager is non-zero. Two specific examples are (i) the overhead of defineClass (assuming the defining loader is a SecureClassLoader) as it involves a callback to get the permissions for the protection domain and (ii) the overhead of AccessController.doPrivileged in order to execute an action on a privileged stack. The bigger question is of course whether it is interesting to run with a security manager in 2017. It's clearly still important for environments that download potentially malicious code to execute in a sandox but that isn't most applications these days. We have seen a few cases where applications set a security manager in order to enforce some policy, like preventing plugins calling System.exit but these are cases that would be better served with other solutions. I would like to explore changes to the API and implementation that would allow us to eliminate some of the overhead when not running with a security manager. Long term then it would be interesting to explore degrading and eventually dropping the security manager but that is beyond the scope of what I would like to discuss here. Sean Mullan and Jeff Nisewanger ran an interesting BOF at JavaOne 2017 on this topic and I believe are planning to do a survey at some point to understand the current usage of the security manager. For now I would like to explore what it would take to eliminate some of the overhead. A big challenge is the System.setSecurityManager API as allows a security manager to be set in a running VM. This means that classes loaded before a security manager is set may be involved in permission checks that happen after a security manager is set. Similarly, privileged actions may have started before the security manager is set. The complications around this go away if there was some way to know that a security manager can never be turned on in a running system. To get started, I've put a prototype of an initial proposal on the cr site [1]. The summary of the proposal is: 1. A "disallow security manager" mode that is opt-in to disallow setSecurityManager being used to set a security manager in a running VM. Opt-in means it has not impact on existing code. 2. Deprecates setSecurityManager to discourage further usage and allow for the possibility of making "disallow security manager" the default in the future. If that were to become the default then it would have no impact on deployments that set the security manager on the command line at startup. 3. When running in "disallow security manager" mode, AccessController.doPrivileged invokes actions directly and SecureClassLoader.defineClass skips the callback to get permissions for the protection domain. This part helps startup and runtime performance. It's important not to get hung up on the details in the webrev, the important thing is understand if the security folks on this mailing list are open to a run mode that prevents a security manager being set in a running system. -Alan. [1] http://cr.openjdk.java.net/~alanb/8191053/webrev/
Re: KDF API review, round 2
On 11/20/2017 11:17 AM, Michael StJohns wrote: One additional topic for discussion: Late in the week we talked about the current state of the API internally and one item to revisit is where the DerivationParameterSpec objects are passed. It was brought up by a couple people that it would be better to provide the DPS objects pertaining to keys at the time they are called for through deriveKey() and deriveKeys() (and possibly deriveData). Originally we had them all grouped in a List in the init method. One reason for needing it up there was to know the total length of material to generate. If we can provide the total length through the AlgorithmParameterSpec passed in via init() then things like: Key deriveKey(DerivationParameterSpec param); List deriveKeys(List params); become possible. To my eyes at least it does make it more clear what DPS you're processing since they're provided at derive time, rather than the caller having to keep track in their heads where in the DPS list they might be with each successive deriveKey or deriveKeys calls. And I think we could do away with deriveKeys(int), too. See above - the key stream is logically produced in its entirety before any assignment of that stream is made to any cryptographic objects because the mixins (except for the round differentiator) are the same for each key stream production round. Simply passing in the total length may not give you the right result if the KDF requires a per component length (and it should to defeat (5) or it should only produce a single key). In general, if the KDF needs any information up front to produce the output bits, then that information can be supplied in init using the APS. In your example attack for (5), an implementation that prevents this attack would probably take a length and label (e.g. "key1", "key2", "iv1", "iv2") for each derived value. Then the HSM would need to enforce that any value derived with a "key*" label could not be extracted. If that entire sequence of lengths and labels needs to be known up front, then it can be supplied in the APS. In this organization, the only additional information that the DPS passed to deriveKey() needs to provide is the encryption algorithm name and parameters (though taking a length and checking it is probably a good idea).
Re: RFR - 8189646: sun/security/ssl/SSLSocketImpl/SSLSocketCloseHang.java failed with "java.net.SocketTimeoutException: Read timed out"
This should help harden the test. Looks fine to me. Regards, Sean. On 20/11/17 16:16, Rob McKenna wrote: Hi folks, This test appears to be timing out. The main culprit looks to be the fact that the 100ms sleep isn't enough for the server to respond (on a busy test machine) so that timeout has been bumped and some new logging has been added to make it obvious whats happening should the problem occur again. webrev: http://cr.openjdk.java.net/~robm/8189646/webrev.01/ bug: https://bugs.openjdk.java.net/browse/JDK-8189646 Thanks, -Rob
Re: KDF API review, round 2
Apologies in advance for top posting and a need to be a little pedantic about KDFs. I'll have some comments inline below as well. KDF's aren't well understood but people think they are. The key stream generation part is pretty straightforward (keyed PRBG), but the interaction of how the key stream is generated and how the key stream is assigned to actual cryptographic objects is not. Here's why: 1) KDF's are repeatable. Given the exact same inputs (key, mixin data) they produce the same key stream. 2) Any change in the inputs changes ALL of the key stream. 3) Unless the overall length property is included, then changing the length of the key stream will not change the prefix (e.g. if the original call was for 10 bytes and a second call was for 20, the first 10 bytes of both calls will produce the exact same key stream data) 4) The general format of each round of key stream generation is something like PRF (master key, mixins), where mixins are the concatenation of at least a label and context and a value to differentiate each round (a counter or the previous rounds output for example). Including L in the mixin prevents the property described in (3) above. Including a length for each subcomponent as a mixin prevents the property described in (5) below. 5) Unless the length for each derived object is included in the mix in, then it is possible to move the assignment of key stream bytes between objects. For example, both TLS (1.2 and before) and IPSEC use KDFs that generate non-secret IV material along with secret session key material. This is less important for software only KDFs as both the secret key material and the IV material are both in the JVM memory domain. This is very important if you're trying to keep your secret key material secure in an HSM. Example: a given TLS session may need 2 256 bit AES keys and 2 128 bit IVs. That is a requirement for 96 bytes of key stream (if I've got my calculation correct). We have the HSM produce this (see the PKCS11 calling sequence for example) and we get out the IVs. An attacker who has access to the HSM (which may or may not be on the same machine as the TLS instantiation) can call the derivation function with new output parameters (but with the same master key and mixins) which specifies only IV material and have the function output the same key stream bytes that were previously assigned to the secret key material in the IV output. A very easy key extraction attack. This is why TLS1.3 only does single outputs per KDF call and makes the length of that output a mandatory mixin. An HSM can also look at the labels and make a determination as to whether an object need be protected (key material) or in the clear (iv). Given (3) and (5) I believe that both L and l[i] (subcomponent length) may need to be provided for BEFORE any key material is produced which argues for input during initialization phase. On 11/20/2017 5:12 AM, Jamil Nimeh wrote: On 11/19/2017 12:45 PM, Michael StJohns wrote: On 11/17/2017 1:07 PM, Adam Petcher wrote: On 11/17/2017 10:04 AM, Michael StJohns wrote: On 11/16/2017 2:15 PM, Adam Petcher wrote: So it seems like they could all be supplied to init. Alternatively, algorithm names could specify more concrete algorithms that include the mode/PRF/etc. Can you provide more information to explain why these existing patterns won't work in this case? What I need to do is provide a lifecycle diagram, but its hard to do in text. But basically, the .getInstance() followed by .setParameters() builds a concrete engine while the .init() initializes that engine with a key and the derivation parameters. Think about a TLS 1.2 instance - the PRF is selected once, but the KDF may be used multiple times. This is the information I was missing. There are two sets of parameters, and the first set should be fixed, but the second set should be changed on each init. I considered the mode/PRF/etc stuff but that works for things like Cipher and Signature because most of those have exactly the same pattern. For the KDF pattern we;ve got fully specified KDFs (e.g. TLS 1.1 and before, IPSEC), almost fully specified KDFs (TLS 1.2 and HDKF needs a PRF) and then the SP800 style KDFs which are defined to be *very* flexible. So translating that into a naming convention is going to be restrictive and may not cover all of the possible approaches. I'd rather do it as an algorithmparameter instead. With a given KDF implementation having a default if nothing is specified during instantiation. I agree that this is challenging because there is so much variety in KDFs. But I don't think that SP 800-108 is a good example of something that should be exposed as an algorithm in JCA, because it is too broad. SP 800-108 is more of a toolbox that can be used to construct KDFs. Particular specializations of SP 800-108 are widely used, and they will get names that can be used in
RFR - 8189646: sun/security/ssl/SSLSocketImpl/SSLSocketCloseHang.java failed with "java.net.SocketTimeoutException: Read timed out"
Hi folks, This test appears to be timing out. The main culprit looks to be the fact that the 100ms sleep isn't enough for the server to respond (on a busy test machine) so that timeout has been bumped and some new logging has been added to make it obvious whats happening should the problem occur again. webrev: http://cr.openjdk.java.net/~robm/8189646/webrev.01/ bug: https://bugs.openjdk.java.net/browse/JDK-8189646 Thanks, -Rob
Re: Draft design for Key Derivation API
On 11/19/2017 3:15 PM, Michael StJohns wrote: That behavior all sounds reasonable, I just have doubts that this belongs in the spec. Are you expecting KeyDerivation to contain the logic in your last paragraph? Something like this: KDFs are somewhat problematic in that *_they may not necessarily be producing objects from their own provider_*. This unfortunately isn't obvious, but let me try and explain. Your response didn't contain a direct answer to my question above. If I am interpreting your response correctly, then your answer is "Yes, and we may need some additional information in DerivationParameterSpec (or elsewhere) that controls this logic." Though I'm not sure I am interpreting this correctly, so please let me know. To be clear: I don't object to including the method that returns an Object produced by a KDF. I'm specifically asking about the requirement that this class of objects has a (byte[] int) constructor, and how that constructor is expected to be used.
Re: KDF API review, round 2
On 11/20/2017 5:12 AM, Jamil Nimeh wrote: On 11/19/2017 12:45 PM, Michael StJohns wrote: On 11/17/2017 1:07 PM, Adam Petcher wrote: I agree that this is challenging because there is so much variety in KDFs. But I don't think that SP 800-108 is a good example of something that should be exposed as an algorithm in JCA, because it is too broad. SP 800-108 is more of a toolbox that can be used to construct KDFs. Particular specializations of SP 800-108 are widely used, and they will get names that can be used in getInstance. For example, HKDF-Expand is a particular specialization of SP 800-108. So I think the existing pattern of using algorithm names to specify concrete algorithms should work just as well in this API as it does in the rest of JCA. Of course, more flexibility in the API is a nice feature, but supporting this level of generality may be out of scope for this effort. The more I think about it the more I think you're mostly right. But let's split this slightly as almost every KDF allows for the specification of the PRF. So / as the standard naming convention. Or TLS13/HMAC-SHA256 and HKDF/HMAC-SHA256 (which are different because of the mandatory inclusion of "L" in the derivation parameters and each component object for TLS13) This approach seems fine to me. We would probably want to allow any algorithm name after the / (rather than limiting it to PRFs), because JCA doesn't have a notion of PRF, and because some KDFs take other kinds of functions (e.g. PBKDF1 uses a bare hash function). Still - let's include the .setParameters() call as a failsafe as looking forward I can see the need for flexibility rearing its ugly head (e.g. adding PSS parameters to RSA signatures way late in the game.) and it does match the pattern for Signature so its not a new concept. A given provider need not support the call, but its there if needed. Signature appears to have setParameter because the initSign and initVerify didn't have APS parameters in their method signatures. Since we're talking about providing APS objects through both getInstance() for those locked to the algorithm and init() for things like salts, info, etc. that can be changed on successive inits it seems like we're covered without the need for a setParameter method. My argument is that providing APS in getInstance doesn't appear to be necessary. Of course, if you want to tackle this, that's fine with me. But I think it complicates the API and I expect it will lead to other API/design problems that will need to be sorted out. I agree that setParameter() in Signature appears to be there to solve a different problem. This API doesn't have that problem because the init method takes an APS. One additional topic for discussion: Late in the week we talked about the current state of the API internally and one item to revisit is where the DerivationParameterSpec objects are passed. It was brought up by a couple people that it would be better to provide the DPS objects pertaining to keys at the time they are called for through deriveKey() and deriveKeys() (and possibly deriveData). Originally we had them all grouped in a List in the init method. One reason for needing it up there was to know the total length of material to generate. If we can provide the total length through the AlgorithmParameterSpec passed in via init() then things like: Key deriveKey(DerivationParameterSpec param); List deriveKeys(List params); become possible. To my eyes at least it does make it more clear what DPS you're processing since they're provided at derive time, rather than the caller having to keep track in their heads where in the DPS list they might be with each successive deriveKey or deriveKeys calls. And I think we could do away with deriveKeys(int), too. I like this change. It simplifies the API, and forcing the JCA client to be explicit and supply the output length in the APS is a good thing.
Eliminating the security overhead when not running with a security manager
One of the long standing issues with the security manager support is that the overhead when there is no security manager is non-zero. Two specific examples are (i) the overhead of defineClass (assuming the defining loader is a SecureClassLoader) as it involves a callback to get the permissions for the protection domain and (ii) the overhead of AccessController.doPrivileged in order to execute an action on a privileged stack. The bigger question is of course whether it is interesting to run with a security manager in 2017. It's clearly still important for environments that download potentially malicious code to execute in a sandox but that isn't most applications these days. We have seen a few cases where applications set a security manager in order to enforce some policy, like preventing plugins calling System.exit but these are cases that would be better served with other solutions. I would like to explore changes to the API and implementation that would allow us to eliminate some of the overhead when not running with a security manager. Long term then it would be interesting to explore degrading and eventually dropping the security manager but that is beyond the scope of what I would like to discuss here. Sean Mullan and Jeff Nisewanger ran an interesting BOF at JavaOne 2017 on this topic and I believe are planning to do a survey at some point to understand the current usage of the security manager. For now I would like to explore what it would take to eliminate some of the overhead. A big challenge is the System.setSecurityManager API as allows a security manager to be set in a running VM. This means that classes loaded before a security manager is set may be involved in permission checks that happen after a security manager is set. Similarly, privileged actions may have started before the security manager is set. The complications around this go away if there was some way to know that a security manager can never be turned on in a running system. To get started, I've put a prototype of an initial proposal on the cr site [1]. The summary of the proposal is: 1. A "disallow security manager" mode that is opt-in to disallow setSecurityManager being used to set a security manager in a running VM. Opt-in means it has not impact on existing code. 2. Deprecates setSecurityManager to discourage further usage and allow for the possibility of making "disallow security manager" the default in the future. If that were to become the default then it would have no impact on deployments that set the security manager on the command line at startup. 3. When running in "disallow security manager" mode, AccessController.doPrivileged invokes actions directly and SecureClassLoader.defineClass skips the callback to get permissions for the protection domain. This part helps startup and runtime performance. It's important not to get hung up on the details in the webrev, the important thing is understand if the security folks on this mailing list are open to a run mode that prevents a security manager being set in a running system. -Alan. [1] http://cr.openjdk.java.net/~alanb/8191053/webrev/
Re: KDF API review, round 2
On 11/19/2017 12:45 PM, Michael StJohns wrote: On 11/17/2017 1:07 PM, Adam Petcher wrote: On 11/17/2017 10:04 AM, Michael StJohns wrote: On 11/16/2017 2:15 PM, Adam Petcher wrote: So it seems like they could all be supplied to init. Alternatively, algorithm names could specify more concrete algorithms that include the mode/PRF/etc. Can you provide more information to explain why these existing patterns won't work in this case? What I need to do is provide a lifecycle diagram, but its hard to do in text. But basically, the .getInstance() followed by .setParameters() builds a concrete engine while the .init() initializes that engine with a key and the derivation parameters. Think about a TLS 1.2 instance - the PRF is selected once, but the KDF may be used multiple times. This is the information I was missing. There are two sets of parameters, and the first set should be fixed, but the second set should be changed on each init. I considered the mode/PRF/etc stuff but that works for things like Cipher and Signature because most of those have exactly the same pattern. For the KDF pattern we;ve got fully specified KDFs (e.g. TLS 1.1 and before, IPSEC), almost fully specified KDFs (TLS 1.2 and HDKF needs a PRF) and then the SP800 style KDFs which are defined to be *very* flexible. So translating that into a naming convention is going to be restrictive and may not cover all of the possible approaches. I'd rather do it as an algorithmparameter instead. With a given KDF implementation having a default if nothing is specified during instantiation. I agree that this is challenging because there is so much variety in KDFs. But I don't think that SP 800-108 is a good example of something that should be exposed as an algorithm in JCA, because it is too broad. SP 800-108 is more of a toolbox that can be used to construct KDFs. Particular specializations of SP 800-108 are widely used, and they will get names that can be used in getInstance. For example, HKDF-Expand is a particular specialization of SP 800-108. So I think the existing pattern of using algorithm names to specify concrete algorithms should work just as well in this API as it does in the rest of JCA. Of course, more flexibility in the API is a nice feature, but supporting this level of generality may be out of scope for this effort. The more I think about it the more I think you're mostly right. But let's split this slightly as almost every KDF allows for the specification of the PRF. So / as the standard naming convention. Or TLS13/HMAC-SHA256 and HKDF/HMAC-SHA256 (which are different because of the mandatory inclusion of "L" in the derivation parameters and each component object for TLS13) Still - let's include the .setParameters() call as a failsafe as looking forward I can see the need for flexibility rearing its ugly head (e.g. adding PSS parameters to RSA signatures way late in the game.) and it does match the pattern for Signature so its not a new concept. A given provider need not support the call, but its there if needed. Signature appears to have setParameter because the initSign and initVerify didn't have APS parameters in their method signatures. Since we're talking about providing APS objects through both getInstance() for those locked to the algorithm and init() for things like salts, info, etc. that can be changed on successive inits it seems like we're covered without the need for a setParameter method. One additional topic for discussion: Late in the week we talked about the current state of the API internally and one item to revisit is where the DerivationParameterSpec objects are passed. It was brought up by a couple people that it would be better to provide the DPS objects pertaining to keys at the time they are called for through deriveKey() and deriveKeys() (and possibly deriveData). Originally we had them all grouped in a List in the init method. One reason for needing it up there was to know the total length of material to generate. If we can provide the total length through the AlgorithmParameterSpec passed in via init() then things like: Key deriveKey(DerivationParameterSpec param); List deriveKeys(List params); become possible. To my eyes at least it does make it more clear what DPS you're processing since they're provided at derive time, rather than the caller having to keep track in their heads where in the DPS list they might be with each successive deriveKey or deriveKeys calls. And I think we could do away with deriveKeys(int), too. --Jamil
