Re: RFR 7191662: JCE providers should be located via ServiceLoader,
On 25/05/15 09:42, Erik Joelsson wrote: On 2015-05-22 18:53, Mandy Chung wrote: On 05/22/2015 08:09 AM, Alan Bateman wrote: On 22/05/2015 13:55, Chris Hegarty wrote: : I think it could be done either way. Valerie - have you considered not pushing the services configuration files with this change? With the change then the java.security configuration is still class names, not provider names, so the fallback should just work. This is what we've done in a few other areas (like JNDI for example). I wasn't aware of the other areas that move to service provider but remain being loaded with the fallback Class.forName. URL protocol handlers, and JDNI initial context, fall back to Class.forName for built-in handlers/implementations, and do not expose the implementation as services. I would prefer java.security should convert to use the provider names as an example and also exercise the code path using service I think URL protocol handlers works well as an example too. It is a minimal new service type, easy to understand. You can write your own Foo handler in a few lines of code. providers. If this causes much work to workaround it temporarily, I won't object the security providers are not truly service providers (no META-INF/services and java.security lists class name instead) Another option to workaround this: we only need to merge the service config files for generating the image. Can we have do the concatenation of jdk/modules/*/META-INF/services file and output to supports/image_gensrc before the images target and have the image builder to exclude all jdk/modules/*/META-INF/services files and take the supports/image_gensrc instead? This will remove the process-provider logic from Gensrc-*.gmk files. Would this be a better alternative? Maybe, I'm not sure. I still think solving this in java code in the ImageBuilder is the right thing to do. If it is agreed that these files are needed, then I can look at expanding the ImageBuilder to do concatenate them. -Chris. /Erik
Re: [8u] request for review: 8062552 Support keystore type detection for JKS and PKCS12 keystores
Unfortunately we cannot modify a Java SE API in an update release so there is no opportunity to backport the keystore probe mechanism to JDK 8u. On 23 May 2015, at 22:57, Thomas Lußnig open...@suche.org wrote: On 23.05.2015 10:59, Vincent Ryan wrote: The aim of this enhancement is to address a specific compatibility risk for JKS and not to offer a general purpose mechanism for loading any keystore type. In general, the keystore type should match the keystore file format. In JDK 9 there is a new probe mechanism for keystores that is more similar to what you are proposing. The advantage of that mechanism is that the keystore type will exactly match the keystore file format. When there is already an new probe mechanism for keystore detetion, why do not backport/use it ? Why build this limited version for one single usecase instead of using the more gerneral solution ? On 23 May 2015, at 09:42, Thomas Lußnig open...@suche.org wrote: Hi, 1) Would it not be an good idea to check the first bytes of the message so that the dual class already know what type the stream is and there is no unnecessary instanciation of exceptions and engine class? 2) If we add an smart keystore why we limit it to two types? I do not see any reason why it should not be possible to add other store types to: - JCEKS - PKCS11 It could be extended via securit property java.security.smartKeystore.N.type = PKCS11 java.security.smartKeystore.N.magic = HexSequence (Optional for Performance) java.security.smartKeystore.N.engineClass = CanonicalEngine Class Name This would be only an small code change but an usefull improvement. Gruß Thomas On 22.05.2015 22:01, Sean Mullan wrote: Looks fine to me. --Sean On 05/22/2015 03:10 PM, Vincent Ryan wrote: Thanks Thomas and Sean for your review comments. KeyStoreDelegator matches the JDK 9 version. I’ve moved it to the sun.security.package and modified it as suggested. I also made JavaKeyStore package-private but DualFormatJKS needs to remain public. The cert in trusted.pem is an arbitrary X.509 cert and I’ve added a comment in the TestKeystoreCompat test. A new webrev is available at: http://cr.openjdk.java.net/~vinnie/8062552/webrev.02/
Re: RFR 7191662: JCE providers should be located via ServiceLoader,
On 2015-05-22 18:53, Mandy Chung wrote: On 05/22/2015 08:09 AM, Alan Bateman wrote: On 22/05/2015 13:55, Chris Hegarty wrote: : I think it could be done either way. Valerie - have you considered not pushing the services configuration files with this change? With the change then the java.security configuration is still class names, not provider names, so the fallback should just work. This is what we've done in a few other areas (like JNDI for example). I wasn't aware of the other areas that move to service provider but remain being loaded with the fallback Class.forName. I would prefer java.security should convert to use the provider names as an example and also exercise the code path using service providers. If this causes much work to workaround it temporarily, I won't object the security providers are not truly service providers (no META-INF/services and java.security lists class name instead) Another option to workaround this: we only need to merge the service config files for generating the image. Can we have do the concatenation of jdk/modules/*/META-INF/services file and output to supports/image_gensrc before the images target and have the image builder to exclude all jdk/modules/*/META-INF/services files and take the supports/image_gensrc instead? This will remove the process-provider logic from Gensrc-*.gmk files. Would this be a better alternative? Maybe, I'm not sure. I still think solving this in java code in the ImageBuilder is the right thing to do. /Erik
Re: RFR 7191662: JCE providers should be located via ServiceLoader,
On 25/05/2015 09:53, Chris Hegarty wrote: If it is agreed that these files are needed, then I can look at expanding the ImageBuilder to do concatenate them. I agree with Mandy's point that java.security should be change to list the provider name rather than the class name. If that happens then it means that the service configuration files will be required. I don't have a strong view on whether the concatenation is done via make files or the image builder as it's all just temporary and will go away once resources are keyed by a module. One thing about rev'ing the image builder is that we should probably let the jimage refresh get into jdk9/dev first. I don't think we want to delay that due to merge conflicts. -Alan
Re: TLS ALPN Proposal
Hi Brad,
A couple of initial comments/questions.
1) Certificate selection is one feature envisaged by ALPN. ie a client
or a server
ought to be able to choose a different certificate depending on the
application name
that gets negotiated. Is that possible with this API?
2) The notion of client preference needs to be made explicit. This could
just be a matter
of javadoc given that ListString is ordered. So, it could be
enough to say the same
order is used in the protocol.
3) It's a shame that the RFC didn't mandate UTF8 encoded byte sequences
for the
protocol name, because it's theoretically possible that non UTF8
byte sequences
could get registered, but that's not a concern for HTTP/2 at least.
Thanks,
Michael
On 22/05/15 01:55, Bradford Wetmore wrote:
This is a fork of the previous thread:
Subject: TLS Handshake Message Proposal
(Was: Re: JEP 244: TLS Application-Layer Protocol
Negotiation Extension)
I broke this thread off to keep this proposal discussion together, but
if you're interested in the history, please see the previous thread.
This approach combines different suggestions from security-dev in a
new way, and simplifies the ALPN selection process quite a bit. I
think it addresses most of the concerns about selection that were raised.
This approach can be completely separated from the Handshake mechanism
I discussed in the previous message, so I'm thinking of this approach
for JDK 9 and targeting the more general handshake approach for JDK 10.
Here's the summary of API additions:
SSLParameters:
--
Client-side: gets/sets the list of protocol names to send.
Server-side: gets/sets a ApplicationProtocolSelector which is a
user-defined callback mechanism.
ApplicationProtocolSelector:
Server-side: callback methods for SSLSocket/SSLEngine which
are provided with handshake information for making
the selection
ExtendedSSLSession:
---
gets the negotiated protocol String
Specifics below. The javadoc obviously needs work.
class SSLParameters {
...deleted...
/**
* Gets the list of application protocols that will sent by
* the client.
*
* The list could be empty, in which case no protocols will be
* sent.
*
* @returns a list of application protocol names.
*/
public ListString getApplicationProtocols() { };
/**
* Sets the list of application protocols that will sent by
* the client.
*
* protocols could be empty, in which case no protocols will be
* sent.
*
* @param protocols a list of application protocol names
* @throws IllegalArgumentException if protocols is null.
*/
public void setApplicationProtocols(ListString protocols);
/**
* Gets the current server-side application layer protocol selector.
*
* @param the selector mechanism.
* @return the current application protocol selector, or null if
* there is not one.
*/
public ApplicationProtocolSelector
getApplicationProtocolSelector() {};
/**
* Sets the server-side application layer protocol selector.
*
* @param the selector mechanism, or null if protocol selection
* should not be done.
*/
public void setApplicationProtocolSelector(
ApplicationProtocolSelector selector) {};
}
/*
* A callback class on the server side that is responsible for
* choosing which Application Protocol should be used in a SSL/TLS
* connection.
*/
public abstract class ApplicationProtocolSelector {
/*
* SSLSocket callback to choose which Application Protocol value
* to return to a SSL/TLS client
*
* @param sslSocket the SSLSocket for this connection
* @param protocols the list of protocols advertised by the client
* @param protocolVersion the negotiated protocol version
* @param ciphersuite the negotiated ciphersuite
* @return a non-empty protocol String to send to the client,
* or null if no protocol value (i.e. extension) should be sent.
* @throws SSLProtocolException if the connection should be aborted
* because the the server supports none of the protocols that
* the client advertised.
*/
public String select(SSLSocket sslSocket, String[] protocols,
String protocolVersion, String ciphersuite)
throws SSLProtocolException;
/*
* SSLEngine callback to choose which Application Protocol to return
* to a SSL/TLS client.
*
* @param sslEngine the SSLEngine for this connection
* @param protocols the list of protocols advertised by the client
* @param protocolVersion the negotiated protocol version
* @param ciphersuite the negotiated ciphersuite
* @return a non-empty protocol String to send to the client,
* or null if no
Re: TLS ALPN Proposal
Hi, On Mon, May 25, 2015 at 12:08 PM, Michael McMahon michael.x.mcma...@oracle.com wrote: Hi Brad, A couple of initial comments/questions. 1) Certificate selection is one feature envisaged by ALPN. ie a client or a server ought to be able to choose a different certificate depending on the application name that gets negotiated. Is that possible with this API? Interesting. I can definitely see choosing the ALPN protocol based on the SNI name sent by the client. For example, a server able to speak http/1.1 and h2 receiving a request for http1.domain.com wants to force http/1.1. This would be possible, IIUC, using sslEngine.getHandshakeSession().getRequestedServerNames() in the ApplicationProtocolSelector implementation. I see less common choosing the certificate given the application protocol, but I understand it's mentioned in RFC 7301. ALPN definitely needs the cipher to be negotiated to support HTTP/2, so I hope it's not a chicken-egg problem. -- Simone Bordet http://bordet.blogspot.com --- Finally, no matter how good the architecture and design are, to deliver bug-free software with optimal performance and reliability, the implementation technique must be flawless. Victoria Livschitz
Re: TLS ALPN Proposal
On 25/05/15 12:34, Simone Bordet wrote: Hi, On Mon, May 25, 2015 at 12:08 PM, Michael McMahon michael.x.mcma...@oracle.com wrote: Hi Brad, A couple of initial comments/questions. 1) Certificate selection is one feature envisaged by ALPN. ie a client or a server ought to be able to choose a different certificate depending on the application name that gets negotiated. Is that possible with this API? Interesting. I can definitely see choosing the ALPN protocol based on the SNI name sent by the client. For example, a server able to speak http/1.1 and h2 receiving a request for http1.domain.com wants to force http/1.1. This would be possible, IIUC, using sslEngine.getHandshakeSession().getRequestedServerNames() in the ApplicationProtocolSelector implementation. Perhaps, though it seems there are specific ALPNs for HTTP/1.1 (http/1.1) and for HTTP/2 (h2). So, I think you would use ALPN itself to do that negotiation. An incoming TLS connection without the ALPN extension is just assumed to be HTTP/1.1 I see less common choosing the certificate given the application protocol, but I understand it's mentioned in RFC 7301. There aren't very many different applications envisaged yet. There are a couple of NAT related protocols registered. But, actually having thought about it again, client certificate selection happens in the X509ExtendedKeyManager class and that implementation could presumably call sslEngine.getHandshakeSession().getApplicationProtocol() and select the client cert using that information. It doesn't do that now obviously but I think it could in future if necessary. ALPN definitely needs the cipher to be negotiated to support HTTP/2, so I hope it's not a chicken-egg problem. I've been assuming that (by default) we just need to avoid using the black-listed ciphers and make sure to propose at least the one mandatory cipher; then we shouldn't have any problem. HTTP/2 apps can still create their own SSLContexts and configure them any way they want. - Michael.
RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently
Hi All Please review a code change at http://cr.openjdk.java.net/~weijun/8080911/webrev.00/ I've limit the synchronized block to Config creation only and therefore won't deadlock with EType's class initialization. Noreg-hard. The EType call is at class initialization and only run once in a VM session, which is extremely difficult to catch. Thanks Max
RFR: 8048830 - Implement tests for new functionality provided in JEP 166
Hello, Please review tests for JEP 166: Tests check for default key store format i.e., PKCS 12, import a trusted cert into PKCS 12 key store and export cert and print it. Tests validate whether exception is thrown when key entry with invalid cert chain is imported, Read and write key entry data to key store. Tests also validate metadata store and load functionality. Bug: https://bugs.openjdk.java.net/browse/JDK-8048830 Webrev: http://cr.openjdk.java.net/~asmotrak/bhanu/8048830/webrev.00/ Thanks, Bhanu
[9] RFR: 8078823: javax/net/ssl/ciphersuites/DisabledAlgorithms.java fails intermittently
Hello, Please review this fix for javax/net/ssl/ciphersuites/DisabledAlgorithms.java test. It fails very rarely with SocketException. The test runs clients in main thread, but a server runs in a separate thread. In checkFailure() method, clients expect a SSLHandshakeException, and when it occurs, they stop the server by calling SSLServer.stop() method which makes the server close its server socket. The server usually throws an expected SSLHandshakeException, then server socket is closed. But it seems that sometimes the server closes the server socket before handshake failure is processed. As a result, IOException happens instead of SSLHandshakeException. The server should stop if any exception occurs, and clients shouldn't stop the server by themselves. Bug: https://bugs.openjdk.java.net/browse/JDK-8078823 Webrev: http://cr.openjdk.java.net/~asmotrak/8078823/webrev.00/ Artem
Re: TLS ALPN Proposal
Hi, On Mon, May 25, 2015 at 3:57 PM, Michael McMahon michael.x.mcma...@oracle.com wrote: Perhaps, though it seems there are specific ALPNs for HTTP/1.1 (http/1.1) and for HTTP/2 (h2). So, I think you would use ALPN itself to do that negotiation. An incoming TLS connection without the ALPN extension is just assumed to be HTTP/1.1 Sure, but I can see a client looping over a list of domain names (e.g. to check whether the sites support HTTP/2) and therefore not having any knowledge of whether it should or not add the ALPN extension. This client will always add it, but the server must force http/1.1 for http1.domain.com. There aren't very many different applications envisaged yet. There are a couple of NAT related protocols registered. But, actually having thought about it again, client certificate selection happens in the X509ExtendedKeyManager class and that implementation could presumably call sslEngine.getHandshakeSession().getApplicationProtocol() and select the client cert using that information. It doesn't do that now obviously but I think it could in future if necessary. Sure, unless the protocol is not available because ALPN code has not run yet. For what I understand, currently cipher selection and certificate selection happen at the same time, please correct me if I am wrong. If that's the case, then we have a chicken-egg problem: you can't call ALPN code before the cipher is selected, but you need ALPN to select the certificate. If those two steps can be split, then ALPN code could be put in between and all is solved. -- Simone Bordet http://bordet.blogspot.com --- Finally, no matter how good the architecture and design are, to deliver bug-free software with optimal performance and reliability, the implementation technique must be flawless. Victoria Livschitz
Re: RFR 7191662: JCE providers should be located via ServiceLoader,
On May 25, 2015, at 3:00 AM, Alan Bateman alan.bate...@oracle.com wrote: On 25/05/2015 09:53, Chris Hegarty wrote: If it is agreed that these files are needed, then I can look at expanding the ImageBuilder to do concatenate them. I agree with Mandy's point that java.security should be change to list the provider name rather than the class name. If that happens then it means that the service configuration files will be required. I don't have a strong view on whether the concatenation is done via make files or the image builder as it's all just temporary and will go away once resources are keyed by a module. One thing about rev'ing the image builder is that we should probably let the jimage refresh get into jdk9/dev first. I don't think we want to delay that due to merge conflicts. Right this is all temporary. One benefit of having the concatenation done by image builder will get the makefile cleaned up and get ready for the resources keyed for a module as long as the work required is small. Good point about image refresh and any image builder change should come after jimage refresh to avoid causing any merge conflict. Depending on when the image refresh and this security provider change are ready to push to jdk9/dev, I can work with Valerie to determine whether to wait or phase it. thanks Mandy
Run-time configurable sandboxes
Hello! I am a security-conscious Java developer and am interested in using the JVMs built-in security features to run code in separated and run-time configured sandboxes. I'm writing to the list to explain some of the issues I've come up against and am hoping to either elicit suggestions or at least provoke some discussion about how the JVM might better support this. I've been working on a small experimental system for sandboxing due to dissatisfaction with the existing sandboxing packages. The existing sandboxing packages appear to be overly complicated, fragile, and unmaintained. They almost all implement a complicated and error-prone custom security manager and seem to more or less ignore everything else the JVM has in terms of security features. I'm hoping that I can do better! My own use case will be running code that is sandbox-aware and that only uses a few classes from java.lang and talks to an API that I provide to each sandbox. I would expect to restrict arbitrary file I/O (with sandboxed code persisting state via provided key/value interface), restrict network I/O, restrict access to native code, restrict access to reflection, restrict thread creation, and restrict exiting the VM. About the only thing I cannot protect against is heap exhaustion (but the JVM does a decent job of enforcing a global limit anyway, so it's not as if malicious code would end up killing the user's machine or running afoul of operating system limits). It seems that others have somewhat similar use cases, often using some sort of sandbox to provide security to embedded languages that have been compiled to JVM bytecode at run-time. I won't bore anyone here with the details of how the JVM applies security policy because I'd assume everyone on this list already understands it. My basic approach has been to use a custom implementation of java.security.Policy[0] and a custom classloader. The program creates one classloader C per application sandbox S and assigns all classes loaded by C a protection domain P. My assumption is that for a particular sandbox, we no longer care about fine-grained per-CodeSource control of classes inside the sandbox as we're more likely to be applying a coarse sandbox-wide set of restrictions. This then means that that the custom Policy implementation can assign permissions on a per-sandbox basis by simply checking the CodeSource URL and returning any permissions defined for that URL. As a concrete example, I create a sandbox that I then assign a URL of http://sandbox.io7m.com/1. The classloader for that sandbox assigns every loaded class a CodeSource with location http://sandbox.io7m.com/1. Now, whenever the AccessController consults the policy's checkPermission function, the policy simply uses the set of permissions defined for http://sandbox.io7m.com/1. As an aside, I do use a custom SecurityManager but only to add a couple of extra checks for Thread and ThreadGroup creation/modification, because the default SecurityManager is not strict enough. This appears to work well. I've been unable to subvert the sandbox and am reasonably confident in its security simply due to the fact that it does absolutely nothing clever whatsoever and uses the basic provided JVM security features to achieve it. The code is less than 150 lines and is not exciting in any way. The bulk of a real implementation would be providing a pleasant API and a nice way to configure policies at run-time. My main gripes: 1. The ClassLoader and SecureClassLoader classes are not very nice. It seems that I cannot take an existing classloader and preserve all of the semantics with regards to mapping names to byte arrays (such as looking through the classpath for class files, contacting remote servers for classes, etc) if I want to maintain my own control over the resulting ProtectionDomains of those classes. It is likely that ProtectionDomains and CodeSources were never intended to be used in the slightly abusive way I'm using them in the above system. I'm guessing also that the implementations carry a ton of historical baggage and would likely not have their interfaces presented in the way they currently are if they were written/designed today! There is a tempting package-private method in java.lang.Class called setProtectionDomain that I'm not allowed to call. Having access to this would allow me use any existing class loader and simply overwrite the protection domains of the resulting classes without having to modify any code. 2. I feel like I should not have to do any of the things I have done! I realize this sounds silly, but if it were possible to label classes with a simple immutable opaque tag indicating their confinement, and the Policy could refer to this tag... I'd already be done. I would assume that setting a confinement label on a class would require security checks and that it could only be set once. This seems almost too good/simple an approach to be true - would it require an unlimited amount of
Re: TLS ALPN Proposal
On 5/22/2015 8:28 PM, Weijun Wang wrote: On 5/23/2015 9:13 AM, Bradford Wetmore wrote: Weijun wrote: But in the RFC the name is in uppercase and chars in string are all lowercases. ...deleted... - Compare with equalsIgnoreCase() Not following here, the spec is specific about the over-the-wire byte values, and http/1.1 != Http/1.1. Because the spec says o Identification Sequence: The precise set of octet values that identifies the protocol. This could be the UTF-8 encoding [RFC3629] of the protocol name. and the name is uppercase. What if someone really sends HTTP/1.1.getBytes(UTF-8)? I'm sorry, but I'm still not understanding your point. Looking at an existing ALPN directory entry: Protocol: HTTP/1.1 Identification Sequence: 0x68 0x74 0x74 0x70 0x2f 0x31 0x2e 0x31 (http/1.1) Reference: [RFC7230] The name of the Protocol is HTTP/1.1, but the Identification Sequence is 0x68 0x74 0x74 0x70 0x2f 0x31 0x2e 0x31 (http/1.1). I am proposing that the ListString be the values of the Identification Sequence, not the IANA Protocol Names. Is your opinion that the ALPN API String Protocol be the Protocol: and that we should internally map from HTTP/1.1 to http/1.1 before sending? Or that Identification Sequence HTTP/1.1 SHOULD BE treated the same as http/1.1? I think that's what you're saying, since I think you want to compare it using equalsIgnoreCase(). That will make future ALPN protocol name addition challenging. What if someone really sends HTTP/1.1.getBytes(UTF-8)? In my proposal, then they should send HTTP/1.1 instead of http/1.1. I'm really sorry if I'm still missing something. Brad
Re: RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently
synchronized on class looks a little bit unsafe to me. As singleton is a static variable, creating the instance during initialization looks safer. - private static Config singleton = null; + private static Config singleton = new Config(); Xuelei On 5/25/2015 10:16 PM, Weijun Wang wrote: Hi All Please review a code change at http://cr.openjdk.java.net/~weijun/8080911/webrev.00/ I've limit the synchronized block to Config creation only and therefore won't deadlock with EType's class initialization. Noreg-hard. The EType call is at class initialization and only run once in a VM session, which is extremely difficult to catch. Thanks Max
Re: RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently
On 5/26/2015 7:59 AM, Xuelei Fan wrote: synchronized on class looks a little bit unsafe to me. Why? Isn't it the same as making a static method synchronized? [1] As singleton is a static variable, creating the instance during initialization looks safer. - private static Config singleton = null; + private static Config singleton = new Config(); This line might throw an exception. Also, do you intend to make getInstance() simply returning singleton? This means if the first call to new Config() throws an exception, getInstance() will not try to reconstruct it. This might not be common in production, but I don't want to make any behavior change. --Max [1] https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.4.3.6 Xuelei On 5/25/2015 10:16 PM, Weijun Wang wrote: Hi All Please review a code change at http://cr.openjdk.java.net/~weijun/8080911/webrev.00/ I've limit the synchronized block to Config creation only and therefore won't deadlock with EType's class initialization. Noreg-hard. The EType call is at class initialization and only run once in a VM session, which is extremely difficult to catch. Thanks Max
Re: [9] RFR: 8078823: javax/net/ssl/ciphersuites/DisabledAlgorithms.java fails intermittently
Looks fine to me. Thanks, Xuelei On 5/25/2015 11:58 PM, Artem Smotrakov wrote: Hello, Please review this fix for javax/net/ssl/ciphersuites/DisabledAlgorithms.java test. It fails very rarely with SocketException. The test runs clients in main thread, but a server runs in a separate thread. In checkFailure() method, clients expect a SSLHandshakeException, and when it occurs, they stop the server by calling SSLServer.stop() method which makes the server close its server socket. The server usually throws an expected SSLHandshakeException, then server socket is closed. But it seems that sometimes the server closes the server socket before handshake failure is processed. As a result, IOException happens instead of SSLHandshakeException. The server should stop if any exception occurs, and clients shouldn't stop the server by themselves. Bug: https://bugs.openjdk.java.net/browse/JDK-8078823 Webrev: http://cr.openjdk.java.net/~asmotrak/8078823/webrev.00/ Artem
Re: RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently
On 5/26/2015 9:06 AM, Weijun Wang wrote: On 5/26/2015 7:59 AM, Xuelei Fan wrote: synchronized on class looks a little bit unsafe to me. Why? Isn't it the same as making a static method synchronized? [1] Other code may be also able to lock on class. Code 1: lock on MyClass.class Code 2: lock on MyClass.class Code 1 and 2 know nothing about each other. The behavior may be weird. I don't think it is a good practice. As singleton is a static variable, creating the instance during initialization looks safer. - private static Config singleton = null; + private static Config singleton = new Config(); This line might throw an exception. Also, do you intend to make getInstance() simply returning singleton? This means if the first call to new Config() throws an exception, getInstance() will not try to reconstruct it. This might not be common in production, but I don't want to make any behavior change. I see you point. Maybe, you can lock on a object. Xuelei --Max [1] https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.4.3.6 Xuelei On 5/25/2015 10:16 PM, Weijun Wang wrote: Hi All Please review a code change at http://cr.openjdk.java.net/~weijun/8080911/webrev.00/ I've limit the synchronized block to Config creation only and therefore won't deadlock with EType's class initialization. Noreg-hard. The EType call is at class initialization and only run once in a VM session, which is extremely difficult to catch. Thanks Max
Re: TLS ALPN Proposal
Darn those Chicken/Eggs [1]! Yes, you are correct. The steps for the current server code: 1. The ClientHello is parsed, and the SNI matcher callback is called. It does not return which value was matched in the ServerHello, just whether a SNI name was matched or not: The extension_data field of this extension SHALL be empty. 2. Begin generating the ServerHello, choose the Protocol Version. 3. Iterate through the list of client's ciphersuites and call the Server's KeyManager (KM) callback until the KM returns key material we can use. A return string selects the proposed ciphersuite. So we currently don't know the selected ciphersuite until the KM has been called (possibly multiple times). If we choose the ALPN before the ciphersuite, the ciphersuite selection may end up being inappropriate (HTTP/2 blacklist). If we choose the ciphersuite first, then the ALPN value wasn't used to drive the certificate selection. Two suggestions in preferred order below. In each of these cases, unfortunately there is currently no indication of the proposed Ciphersuite, so we need to modify the behavior of getHandshakeSession().getCipherSuite() to fill in the proposed CipherSuite before the call to the KM. This seems ok with the current wording, but we'd need to make that explicit. This value will change for each ciphersuite/KM choice attempt. Each suggestion below is followed by our previously proposed ALPN callback to make the actual ALPN value selection: 1a. Add a parallel method to ExtendedSSLSession: public ListString getRequestedApplicationProtocolNames(); along with the previously proposed selected name: public String getApplicationProtocol() (I'll be changing these names. I'm open to suggestions). When the KM is called, the TLS protocol (e.g. TLSv1.2) has already been selected. Both of the major selection parameters (protocol/proposed ciphersuite) are now available, and applications have access to the ordered ALPN list to see what the client's requested values were. -or- 1b. Keep API as is, and make two callbacks. This first is an advisory value, the TLS protocol version and proposed ciphersuite will be available in getHandshakeSession(). The second callback sets the final value that will be sent. I think 1.a is my preference. To answer some of the other questions. On 5/25/2015 3:08 AM, Michael McMahon wrote: 2) The notion of client preference needs to be made explicit. This could just be a matter of javadoc given that ListString is ordered. So, it could be enough to say the same order is used in the protocol. Yes, I'll add that. 3) It's a shame that the RFC didn't mandate UTF8 encoded byte sequences for the protocol name, because it's theoretically possible that non UTF8 byte sequences could get registered, but that's not a concern for HTTP/2 at least. No. Not sure what we can do about that, short of going back to the byte[] option. Given that IANA operates mainly in English, I would expect the namespaces will probably be ASCII, but that is just conjecture. This would be possible, IIUC, using sslEngine.getHandshakeSession().getRequestedServerNames() in the ApplicationProtocolSelector implementation. Yes. but I understand it's mentioned in RFC 7301. Yes, see the last sentence section 1. Brad [1] https://www.youtube.com/watch?v=ixgf5SlvOB4feature=youtu.bet=27
Re: RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently
On 5/26/2015 9:22 AM, Xuelei Fan wrote: On 5/26/2015 9:06 AM, Weijun Wang wrote: On 5/26/2015 7:59 AM, Xuelei Fan wrote: synchronized on class looks a little bit unsafe to me. Why? Isn't it the same as making a static method synchronized? [1] Other code may be also able to lock on class. Code 1: lock on MyClass.class Code 2: lock on MyClass.class Code 1 and 2 know nothing about each other. The behavior may be weird. I don't think it is a good practice. So you are not a fan of all synchronized methods? :-) I thought it's quite common to use synchronized static methods to prevent simultaneous access to a static field. While a method in another class can lock on thisClass.class (in this case the class is internal), I don't see why it wants to do this. This is not casual, it's just evil. As singleton is a static variable, creating the instance during initialization looks safer. - private static Config singleton = null; + private static Config singleton = new Config(); This line might throw an exception. Also, do you intend to make getInstance() simply returning singleton? This means if the first call to new Config() throws an exception, getInstance() will not try to reconstruct it. This might not be common in production, but I don't want to make any behavior change. I see you point. Maybe, you can lock on a object. A private static final Object? Yes I can, but is it worth doing? Thanks Max Xuelei --Max [1] https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.4.3.6 Xuelei On 5/25/2015 10:16 PM, Weijun Wang wrote: Hi All Please review a code change at http://cr.openjdk.java.net/~weijun/8080911/webrev.00/ I've limit the synchronized block to Config creation only and therefore won't deadlock with EType's class initialization. Noreg-hard. The EType call is at class initialization and only run once in a VM session, which is extremely difficult to catch. Thanks Max
Re: RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently
I do not like class level synchronization because it may not work as expected, especially if the synchronization can be used by other codes. However, your update does not change this behavior. The fix looks fine to me. Please go ahead if you don't want to use object level synchronization. Xuelei On 5/26/2015 10:40 AM, Weijun Wang wrote: On 5/26/2015 9:22 AM, Xuelei Fan wrote: On 5/26/2015 9:06 AM, Weijun Wang wrote: On 5/26/2015 7:59 AM, Xuelei Fan wrote: synchronized on class looks a little bit unsafe to me. Why? Isn't it the same as making a static method synchronized? [1] Other code may be also able to lock on class. Code 1: lock on MyClass.class Code 2: lock on MyClass.class Code 1 and 2 know nothing about each other. The behavior may be weird. I don't think it is a good practice. So you are not a fan of all synchronized methods? :-) I thought it's quite common to use synchronized static methods to prevent simultaneous access to a static field. While a method in another class can lock on thisClass.class (in this case the class is internal), I don't see why it wants to do this. This is not casual, it's just evil. As singleton is a static variable, creating the instance during initialization looks safer. - private static Config singleton = null; + private static Config singleton = new Config(); This line might throw an exception. Also, do you intend to make getInstance() simply returning singleton? This means if the first call to new Config() throws an exception, getInstance() will not try to reconstruct it. This might not be common in production, but I don't want to make any behavior change. I see you point. Maybe, you can lock on a object. A private static final Object? Yes I can, but is it worth doing? Thanks Max Xuelei --Max [1] https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.4.3.6 Xuelei On 5/25/2015 10:16 PM, Weijun Wang wrote: Hi All Please review a code change at http://cr.openjdk.java.net/~weijun/8080911/webrev.00/ I've limit the synchronized block to Config creation only and therefore won't deadlock with EType's class initialization. Noreg-hard. The EType call is at class initialization and only run once in a VM session, which is extremely difficult to catch. Thanks Max
Re: RFR 8038089: TLS optional support for Kerberos cipher suites needs to be re-examine
This is the latest webrev of this bug http://cr.openjdk.java.net/~weijun/8038089/webrev.06/ No significant change from the previous one, mainly rebase. There are some issues which need changes inside JSSE. I'd like to file another bug for them. 1. JsseJce.java still uses core reflection to detect whether Kerberos support is available. It cannot call ClientKeyExchangeService.find() because there is a circular initialization problem between it and CipherSuite. 2. CipherSuite.java still contains hard coded krb5-related KeyExchange and CipherSuite values. These should be moved into plugin. Finally, a lot of you speak out that RFC 2712 is dead and we needn't support them. Thanks for the advice. However, this code change is mainly a refactoring of existing codes because in jdk9 we will have to separate TLS and Kerberos into different modules, and we cannot simply drop the feature. Thanks Max On 9/16/2014 9:31 AM, Wang Weijun wrote: Hi Xuelei Please review the latest code change at http://cr.openjdk.java.net/~weijun/8038089/webrev.04/ Compared with webrev.03, only the way the provider is loaded is changed, which is the static block on lines 50-71 of Krb5Helper.java. Thanks Max
