Re: JDK 8 Code Review Request: 6500133/6931888: CertificateParsingException for CDP
Thanks, Sean. New webrev updated with your suggestions: http://cr.openjdk.java.net/~juh/6500133/webrev.01/ Jason On 08/15/2012 10:38 AM, Sean Mullan wrote: This looks good to me. Couple of comments: 111: Can you add a comment, something like "Try parsing the URI again after encoding/escaping any illegal characters". 113-4: When this code was written there probably wasn't yet an IOException(String, Throwable) ctor. Now there is, so you can change this to: throw new IOException("invalid URI name:" + name, use2); There are also a couple other places in URIName where you can replace the same code using initCause with the IOExc ctor above. That's a low-risk refactoring you can include in this change. --Sean On 08/14/2012 11:51 PM, Jason Uh wrote: Hi all, This change fixes -- 6500133: CertificateParsingException for CRL Distribution Point with blank; and 6931888: Inconsistent behavior for invalid URI name in cert file CRs: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6500133 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6931888 They are effectively duplicates, both regarding an exception thrown when parsing CRL Distribution Point URIs with invalid characters, like a space or backslash. This change uses sun.net.www.ParseUtil.encodePath(String) to re-encode bad URIs. Webrev: http://cr.openjdk.java.net/~juh/6500133/webrev.00/ Thanks, Jason
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
On 8/15/2012 3:26 AM, Xuelei Fan wrote: More comments about whether we are able to override the default value. On 8/15/2012 10:45 AM, Xuelei Fan wrote: Thought more about the design, I would have to say that we cannot return the default value in sslParameters.getServerNames(). Otherwise, the following two block of codes look very weird to me: // case one: 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); 2 sslParameters.clearServerName("host_name"); 3 Map names = sslParameters.getServerNames(); 4 sslSocket.setSSLParameters(sslParameters); 5 sslParameters = sslSocket.getSSLParameters(); 6 names = sslParameters.getServerNames(); In line 3, the returned map does not contain "host_name" entry. But in line 6, it may be expected that no "host_name" in the returned map. But if we want to return default values, line 6 do need to return a map containing "host_name". The behavior is pretty confusing. We may want to try avoid the confusion. I'm not following your confusion, it seemed pretty straightforward to me, it works much like CipherSuites. We have a set of ciphersuites which are enabled by default. We can turn some off by using SSLParameters. Compatibility is my concerns here. When the SSLParameters class was was introduced in JDK 6, set/getCipherSuites() and set/getProtocols were new, so there was no compatibility issue for these two pairs methods at that time, as they were not used in old applications. In JDK 7, we introduced two new methods, set/getAlgorithmConstraints(). We were luck that we cannot allow the override of default constraints because of security consideration, I didn't quite follow this sentence. We do allow the override, right? Looking at SSLSocketImpl.setSSLParameters(): algorithmConstraints = params.getAlgorithmConstraints(); and the concept of algorithm constraints was new in JDK 7. So we needed not to consider the compatibility issue too much between JDK 6 and 7 for this pair of methods. However, things get changed for the Server Name Inidication (SNI), because in JDK 7 we have already support default SNI extension implicit. We have to consider the compatibility issues more between JDK 7 and JDK 8, we need to make sure the behaviors are consistent whenever we call the set/getServerName(). If we allow override of default value, the application may run into corner trap as the description in my previous mail. Not sure this is a problem, possibly I didn't explain my thought fully. That's the background of my thoughts. Hope it helps you understand the current design. Here's what I was trying to propose: SSLParameters: // Local storage like the other variables private Map sniNames = null; ...deleted... /** * Set the SNI Names. * * A null parameter means don't set anything. Nothing is set. * * disallow invalid String->null entries. * * valid String->String entries will be added to SNI extension * if there is a recognized Key type. */ public void setServerNames(Map map) /** * Returns a copy of the array of servernames or null if * none have been set. */ public Map getServerNames() SSLSocketImpl: synchronized public void setSSLParameters(SSLParameters params) { super.setSSLParameters(); ...deleted... Map sniNames = params.getServerNames(); if (sniNames != null) { sniNameStorage = sniNames.clone(); } synchronized public SSLParameters getSSLParameters() { SSLParameters params = super.getSSLParameters(); ...deleted... // sniNameStorage currently has one entry: // "host_name", "www.example.com" params.setServerNames = sniNameStorage.clone(); So looking at the two use cases you pointed out: 1. SSLParameters sslp = new SSLParameters() getServernames() would return null. When SSLSocketImpl.setSSLParameters is called, the null indicates there is nothing to set, and the default value remains. If app wants to set something here, it can, which would override the existing default. 2. SSLParameters sslp = SSLSocket.getSSLParameters() The SSLParameters will be populated with the SNI map value stored in SSLSocketImpl/SSLEngineImpl, and apps can do whatever they choose with the Map. When setSSLParameters is called, the potentially new values are just pushed back to the SNI. In this case, the application has full access to the default Map and can see what will be sent by default. In both cases, the default remains unless the application takes steps to change it. For applications, this is the same call model as setCipherSuites()/setProtocols(), but the magic happens at different levels. Am I missing something? > because in JDK 7 we have already support default SNI extension > implicit. We are just providing API access to that functionality. Brad Thanks, Xuelei Expanding a bit on your example here, I'll describe what
hg: jdk8/tl/jdk: 6931128: (spec) File attribute tests fail when run as root.
Changeset: da14e2c90bcb Author:robm Date: 2012-08-15 22:46 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/da14e2c90bcb 6931128: (spec) File attribute tests fail when run as root. Reviewed-by: alanb ! src/share/classes/java/io/File.java ! test/java/io/File/Basic.java ! test/java/io/File/SetAccess.java ! test/java/io/File/SetReadOnly.java ! test/java/io/File/SymLinks.java + test/java/io/File/Util.java
hg: jdk8/tl/langtools: 7191449: update copyright year to match last edit in jdk8 langtools repository
Changeset: 9d47f4850714 Author:jjh Date: 2012-08-15 13:48 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/9d47f4850714 7191449: update copyright year to match last edit in jdk8 langtools repository Reviewed-by: jjh Contributed-by: steve.si...@oracle.com ! make/jprt.properties ! make/tools/anttasks/CompilePropertiesTask.java ! make/tools/anttasks/GenStubsTask.java ! make/tools/anttasks/SelectToolTask.java ! make/tools/compileproperties/CompileProperties.java ! make/tools/genstubs/GenStubs.java ! src/share/classes/com/sun/tools/javac/code/Source.java ! src/share/classes/com/sun/tools/javac/code/Type.java ! src/share/classes/com/sun/tools/javac/comp/AttrContext.java ! src/share/classes/com/sun/tools/javac/comp/TransTypes.java ! src/share/classes/com/sun/tools/javac/file/ZipFileIndex.java ! src/share/classes/com/sun/tools/javac/main/Main.java ! src/share/classes/com/sun/tools/javac/util/RichDiagnosticFormatter.java ! test/tools/javac/ProtectedInnerClass/ProtectedInnerClass.sh ! test/tools/javac/api/7086261/T7086261.java ! test/tools/javac/api/T6397104.java ! test/tools/javac/diags/CheckExamples.java ! test/tools/javac/diags/MessageInfo.java ! test/tools/javac/diags/RunExamples.java ! test/tools/javac/diags/examples/ApplicableMethodFound1.java ! test/tools/javac/diags/examples/IllegalDot.java ! test/tools/javac/diags/examples/InconvertibleTypes.java ! test/tools/javac/diags/examples/KindnameConstructor.java ! test/tools/javac/diags/examples/NotApplicableMethodFound.java ! test/tools/javac/diags/examples/PossibleLossPrecision.java ! test/tools/javac/diags/examples/ResourceNotApplicableToType.java ! test/tools/javac/diags/examples/VarargsArgumentMismatch.java ! test/tools/javac/diags/examples/VerboseResolveMulti1.java ! test/tools/javac/diags/examples/WhereCaptured.java ! test/tools/javac/diags/examples/WhereCaptured1.java ! test/tools/javac/diags/examples/WhereIntersection.java ! test/tools/javac/diags/examples/WhereTypeVar.java ! test/tools/javac/generics/typevars/T7148242.java ! test/tools/javac/newlines/Newlines.sh ! test/tools/javac/parser/T4881269.java ! test/tools/javac/processing/TestWarnErrorCount.java
Re: JDK 8 Code Review Request: 6500133/6931888: CertificateParsingException for CDP
This looks good to me. Couple of comments: 111: Can you add a comment, something like "Try parsing the URI again after encoding/escaping any illegal characters". 113-4: When this code was written there probably wasn't yet an IOException(String, Throwable) ctor. Now there is, so you can change this to: throw new IOException("invalid URI name:" + name, use2); There are also a couple other places in URIName where you can replace the same code using initCause with the IOExc ctor above. That's a low-risk refactoring you can include in this change. --Sean On 08/14/2012 11:51 PM, Jason Uh wrote: Hi all, This change fixes -- 6500133: CertificateParsingException for CRL Distribution Point with blank; and 6931888: Inconsistent behavior for invalid URI name in cert file CRs: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6500133 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6931888 They are effectively duplicates, both regarding an exception thrown when parsing CRL Distribution Point URIs with invalid characters, like a space or backslash. This change uses sun.net.www.ParseUtil.encodePath(String) to re-encode bad URIs. Webrev: http://cr.openjdk.java.net/~juh/6500133/webrev.00/ Thanks, Jason
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
On Aug 15, 2012, at 11:05 PM, Weijun Wang wrote: >>> 4. I just noticed that the type can be also "sni-". What if someone call setServerName("sni-0", "...")? Is it the same as calling setServerName(SNI_HOST_NAME, "...")? >>> Unspecified behavior. Maybe not, because of the value encoding method. >>> We may throw exception in Oracle provider. Other providers may want to >>> support "sni-32" or "principal" for its private server name type. >> >> I'm not sure. Suppose tomorrow the type nick_name(1) is supported and a >> customer begins using "sni-1". The day after tomorrow, Java is updated >> and we have a string "nick_name" for it. I guess you will make "sni-1" >> and "nick_home" equivalent so that the customer's app will still work? >> If so, why not start doing it today for "sni-0" and "host_name"? > > In fact, I don't quite understand how this is used. On the client side, when > the developer knows nick_name(1) is not yet supported by the current JDK, he > will use the "sni-1" type so that the SNI will be correctly encoded. Then, > JDK is updated and the name type is now "nick_name", but I believe the old > app should still work, so "sni-1" should be still accepted and be equivalent > to "nick_name". > > On the server side, the old JDK and new JDK would return different values for > getServerNames(). It's likely that the user app will be broken if it cannot > successfully predict the "nick_name" name and already support it. > > Therefore, seriously, I suggest we change Map to > Map. Since we already defined SSLParamters constant for the > types, the user won't feel uncomfortable. > We have to consider two things, the type and the encoding method of the value. For oracle provider, we will not support unknown type in SSLParameters because we don't know the encoding method. We support unknown type in session but we require the value is encoded in utf-8. I do not prefer integer for the type because from the integer type I cannot tell whether the type is unknown or not, and then cannot make sure what is the proper encoding according. For example, the old application does not know "nick-name", so the type integer is 1 and the value encoded as UTF-8, but what if the encoding is not UTF-8 in the formal spec? We will not be able to decode the value properly. Xuelei > Thanks > Max >
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
4. I just noticed that the type can be also "sni-". What if someone call setServerName("sni-0", "...")? Is it the same as calling setServerName(SNI_HOST_NAME, "...")? Unspecified behavior. Maybe not, because of the value encoding method. We may throw exception in Oracle provider. Other providers may want to support "sni-32" or "principal" for its private server name type. I'm not sure. Suppose tomorrow the type nick_name(1) is supported and a customer begins using "sni-1". The day after tomorrow, Java is updated and we have a string "nick_name" for it. I guess you will make "sni-1" and "nick_home" equivalent so that the customer's app will still work? If so, why not start doing it today for "sni-0" and "host_name"? In fact, I don't quite understand how this is used. On the client side, when the developer knows nick_name(1) is not yet supported by the current JDK, he will use the "sni-1" type so that the SNI will be correctly encoded. Then, JDK is updated and the name type is now "nick_name", but I believe the old app should still work, so "sni-1" should be still accepted and be equivalent to "nick_name". On the server side, the old JDK and new JDK would return different values for getServerNames(). It's likely that the user app will be broken if it cannot successfully predict the "nick_name" name and already support it. Therefore, seriously, I suggest we change Map to Map. Since we already defined SSLParamters constant for the types, the user won't feel uncomfortable. Thanks Max
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
On 08/15/2012 10:34 PM, Xuelei Fan wrote: On 8/15/2012 10:06 PM, Weijun Wang wrote: Some suggestions to SSLParameters: 1. Since you now decide to exclude the default SNI value from getServerNames(), maybe you can emphasis this by using "user-provided server name" in the spec. I have a paragraph in setServerName: * Note that the returned Map of * {@link #setServerName(String, String)} does not contain the provider * default server name types and values. Did you suggest to replace the "desired" with "user-provided" or "user-specified"? Yes. 2. At disableServerName(), maybe you can explicitly point out that "A disabled server name type cannot be used as a part of the server name indication in SSL/TLS handshaking, even if the provider has a default value for this server name type". OK. 3. isDisabledServerName, maybe isServerNameDisabled? Neither sounds perfect. I'd like to collect more votes on the name. ;-) 4. I just noticed that the type can be also "sni-". What if someone call setServerName("sni-0", "...")? Is it the same as calling setServerName(SNI_HOST_NAME, "...")? Unspecified behavior. Maybe not, because of the value encoding method. We may throw exception in Oracle provider. Other providers may want to support "sni-32" or "principal" for its private server name type. I'm not sure. Suppose tomorrow the type nick_name(1) is supported and a customer begins using "sni-1". The day after tomorrow, Java is updated and we have a string "nick_name" for it. I guess you will make "sni-1" and "nick_home" equivalent so that the customer's app will still work? If so, why not start doing it today for "sni-0" and "host_name"? Thanks Max Thanks for the quick response! Xuelei Thanks Max On 08/15/2012 09:30 PM, Xuelei Fan wrote: This paragraph was removed in the latest update. Please ignore the webrev_spec.04 and previous webrevs, and review webrev_spec.05. http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/ Thanks, Xuelei On 8/15/2012 3:24 PM, Bradford Wetmore wrote: How would the ServerHello be generated until you call this method and then start the handshaking on the returned SSLSocket? One can use SSLEngine.wrap()/unwrap() to generate handshaking messages from socket I/O stream. // accept a socket // read/write the I/O with SSLEngine // call this method Switching between SSLSocket/SSLEngine like that would require intentional misuse of the API. You could make the same claim for regular handshaking. Brad Xuelei You said in a previous internal conversation that SSLExplore would not generate any ServerHello, so this can't be what you mean. Are you talking about layering another SSLSocket over a already layered SSLSocket? 2. "consumed network data is resumable" wasn't clear either. To me this should mean that you can obtain the data which has already been read from "s". Yes, need wordsmithing here. 3. "Otherwise, the behavior of the return socket is not defined" lost me. Does this mean that that SSLParameters and assorted settings are not otherwise defined? See above example. I think I need the above answered before I can comment further here. I think you could delete this paragraph. From your second email: Thought more about the design, I would have to say that we cannot return the default value in sslParameters.getServerNames(). Otherwise, the following two block of codes look very weird to me: // case one: 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); 2 sslParameters.clearServerName("host_name"); 3 Map names = sslParameters.getServerNames(); 4 sslSocket.setSSLParameters(sslParameters); 5 sslParameters = sslSocket.getSSLParameters(); 6 names = sslParameters.getServerNames(); In line 3, the returned map does not contain "host_name" entry. But in line 6, it may be expected that no "host_name" in the returned map. But if we want to return default values, line 6 do need to return a map containing "host_name". The behavior is pretty confusing. We may want to try avoid the confusion. I'm not following your confusion, it seemed pretty straightforward to me, it works much like CipherSuites. We have a set of ciphersuites which are enabled by default. We can turn some off by using SSLParameters. Expanding a bit on your example here, I'll describe what I think would happen internally/externally: 1SSLSocket sslSocket = mySSLSocketFactory.createSocket( "www.example.com", 443); mySSLSocketFactory sets any initial parameters as usual. SSLSocketImpl knows it's connecting to www.example.com and automatically stores "host_name" -> "www.example.com" in its local host data (map or separate variables). 2 SSLparameters sslParameters = sslSocket.getSSLParameters(); SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the hostmap to the one value "host_name" -> "www.example.com" If the application want to get the "default values", they just pull th
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
On 08/14/2012 10:45 PM, Xuelei Fan wrote: I only reply on the items that I may need more review. On 8/15/2012 7:54 AM, Brad Wetmore wrote: SSLParameters.java == 76: Not sure why you want/need a LinkedHashMap with only one currently defined NameType. Even if there were multiple types, I don't think that SNI requires an ordering. You also mention this in setAccessibleServerName, but not sure what purpose this serves. I'm not strongly against it, just wondering. I am also not sure about the strong desire that the SNI should be ordered. But Weijun prefers it to be ordered because he think the SNI in RFC6066 is defined as an ordered sequence. struct { ServerName server_name_list<1..2^16-1> } ServerNameList; I've gone through RFC6066 pretty carefully, and I'm not seeing any indication that this should be ordered. In RFC 2246, if there is an ordering required, such as cipher_suites/compression/certs/cert_requests, it's specifically called out. For any other "lists", it is not specified. Section 7.4.1.2 The CipherSuite list, passed from the client to the server in the client hello message, contains the combinations of cryptographic algorithms supported by the client in order of the client's preference (favorite choice first). ...deleted... The client hello includes a list of compression algorithms supported by the client, ordered according to the client's preference. ...deleted... cipher_suites This is a list of the cryptographic options supported by the client, with the client's first preference first. ...deleted... compression_methods This is a list of the compression methods supported by the client, sorted by client preference. Section 7.4.2 certificate_list This is a sequence (chain) of X.509v3 certificates. The sender's certificate must come first in the list. Section 7.4.4 certificate_types This field is a list of the types of certificates requested, sorted in order of the server's preference. Weijun, did you see something else in your read of the spec that indicates an ordering? If not, maybe we should not put in the order wording now. If it turns out we do need it, we can always add that wording later in a later release, but it will be impossible to remove it if we add it now. I think you are right. If Weijun has no other concerns, I will remove related description. Doesn't a "List" imply it is ordered? I guess I'm ok with not putting any specific wording in right now, since there's likely only going to be one entry anyway, but if subsequent standard name types are defined later, order might become important for some reason. --Sean
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
On 8/15/2012 10:06 PM, Weijun Wang wrote: > Some suggestions to SSLParameters: > > 1. Since you now decide to exclude the default SNI value from > getServerNames(), maybe you can emphasis this by using "user-provided > server name" in the spec. > I have a paragraph in setServerName: * Note that the returned Map of * {@link #setServerName(String, String)} does not contain the provider * default server name types and values. Did you suggest to replace the "desired" with "user-provided" or "user-specified"? > 2. At disableServerName(), maybe you can explicitly point out that "A > disabled server name type cannot be used as a part of the server name > indication in SSL/TLS handshaking, even if the provider has a default > value for this server name type". > OK. > 3. isDisabledServerName, maybe isServerNameDisabled? Neither sounds > perfect. > I'd like to collect more votes on the name. ;-) > 4. I just noticed that the type can be also "sni-". What if someone > call setServerName("sni-0", "...")? Is it the same as calling > setServerName(SNI_HOST_NAME, "...")? > Unspecified behavior. Maybe not, because of the value encoding method. We may throw exception in Oracle provider. Other providers may want to support "sni-32" or "principal" for its private server name type. Thanks for the quick response! Xuelei > Thanks > Max > > > On 08/15/2012 09:30 PM, Xuelei Fan wrote: >> This paragraph was removed in the latest update. Please ignore the >> webrev_spec.04 and previous webrevs, and review webrev_spec.05. >> >> http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/ >> >> Thanks, >> Xuelei >> >> On 8/15/2012 3:24 PM, Bradford Wetmore wrote: >>> >>> > How would the ServerHello be generated until you call this method and > then start the handshaking on the returned SSLSocket? One can use SSLEngine.wrap()/unwrap() to generate handshaking messages from socket I/O stream. // accept a socket // read/write the I/O with SSLEngine // call this method >>> >>> Switching between SSLSocket/SSLEngine like that would require >>> intentional misuse of the API. You could make the same claim for >>> regular handshaking. >>> >>> Brad >>> Xuelei > You said in a > previous internal conversation that SSLExplore would not generate any > ServerHello, so this can't be what you mean. Are you talking about > layering another SSLSocket over a already layered SSLSocket? > >>> 2. "consumed network data is resumable" wasn't clear either. To me >>> this >>> should mean that you can obtain the data which has already been read >>> from "s". >>> >> Yes, need wordsmithing here. >> >>> 3. "Otherwise, the behavior of the return socket is not defined" >>> lost >>> me. Does this mean that that SSLParameters and assorted settings >>> are >>> not otherwise defined? >>> >> See above example. > > I think I need the above answered before I can comment further here. > >>> I think you could delete this paragraph. >>> >>>From your second email: >>> Thought more about the design, I would have to say that we cannot return the default value in sslParameters.getServerNames(). Otherwise, the following two block of codes look very weird to me: // case one: 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); 2 sslParameters.clearServerName("host_name"); 3 Map names = sslParameters.getServerNames(); 4 sslSocket.setSSLParameters(sslParameters); 5 sslParameters = sslSocket.getSSLParameters(); 6 names = sslParameters.getServerNames(); In line 3, the returned map does not contain "host_name" entry. But in line 6, it may be expected that no "host_name" in the returned map. But if we want to return default values, line 6 do need to return a map containing "host_name". The behavior is pretty confusing. We may want to try avoid the confusion. >>> >>> I'm not following your confusion, it seemed pretty >>> straightforward to >>> me, it works much like CipherSuites. We have a set of ciphersuites >>> which are enabled by default. We can turn some off by using >>> SSLParameters. Expanding a bit on your example here, I'll describe >>> what >>> I think would happen internally/externally: >>> >>> 1SSLSocket sslSocket = mySSLSocketFactory.createSocket( >>>"www.example.com", 443); >>> >>> mySSLSocketFactory sets any initial parameters as usual. >>> SSLSocketImpl >>> knows it's connecting to www.example.com and automatically stores >>> "host_name" -> "www.example.com" in its local host data (map or >>> separate >>> variables). >>> >>> 2 SSLparameters sslParameters = sslSocket.getSSLPa
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
Some suggestions to SSLParameters: 1. Since you now decide to exclude the default SNI value from getServerNames(), maybe you can emphasis this by using "user-provided server name" in the spec. 2. At disableServerName(), maybe you can explicitly point out that "A disabled server name type cannot be used as a part of the server name indication in SSL/TLS handshaking, even if the provider has a default value for this server name type". 3. isDisabledServerName, maybe isServerNameDisabled? Neither sounds perfect. 4. I just noticed that the type can be also "sni-". What if someone call setServerName("sni-0", "...")? Is it the same as calling setServerName(SNI_HOST_NAME, "...")? Thanks Max On 08/15/2012 09:30 PM, Xuelei Fan wrote: This paragraph was removed in the latest update. Please ignore the webrev_spec.04 and previous webrevs, and review webrev_spec.05. http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/ Thanks, Xuelei On 8/15/2012 3:24 PM, Bradford Wetmore wrote: How would the ServerHello be generated until you call this method and then start the handshaking on the returned SSLSocket? One can use SSLEngine.wrap()/unwrap() to generate handshaking messages from socket I/O stream. // accept a socket // read/write the I/O with SSLEngine // call this method Switching between SSLSocket/SSLEngine like that would require intentional misuse of the API. You could make the same claim for regular handshaking. Brad Xuelei You said in a previous internal conversation that SSLExplore would not generate any ServerHello, so this can't be what you mean. Are you talking about layering another SSLSocket over a already layered SSLSocket? 2. "consumed network data is resumable" wasn't clear either. To me this should mean that you can obtain the data which has already been read from "s". Yes, need wordsmithing here. 3. "Otherwise, the behavior of the return socket is not defined" lost me. Does this mean that that SSLParameters and assorted settings are not otherwise defined? See above example. I think I need the above answered before I can comment further here. I think you could delete this paragraph. From your second email: Thought more about the design, I would have to say that we cannot return the default value in sslParameters.getServerNames(). Otherwise, the following two block of codes look very weird to me: // case one: 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); 2 sslParameters.clearServerName("host_name"); 3 Map names = sslParameters.getServerNames(); 4 sslSocket.setSSLParameters(sslParameters); 5 sslParameters = sslSocket.getSSLParameters(); 6 names = sslParameters.getServerNames(); In line 3, the returned map does not contain "host_name" entry. But in line 6, it may be expected that no "host_name" in the returned map. But if we want to return default values, line 6 do need to return a map containing "host_name". The behavior is pretty confusing. We may want to try avoid the confusion. I'm not following your confusion, it seemed pretty straightforward to me, it works much like CipherSuites. We have a set of ciphersuites which are enabled by default. We can turn some off by using SSLParameters. Expanding a bit on your example here, I'll describe what I think would happen internally/externally: 1SSLSocket sslSocket = mySSLSocketFactory.createSocket( "www.example.com", 443); mySSLSocketFactory sets any initial parameters as usual. SSLSocketImpl knows it's connecting to www.example.com and automatically stores "host_name" -> "www.example.com" in its local host data (map or separate variables). 2 SSLparameters sslParameters = sslSocket.getSSLParameters(); SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the hostmap to the one value "host_name" -> "www.example.com" If the application want to get the "default values", they just pull them out of the SSLParameters here 3 sslParameters.clearServerName("host_name"); Or sslParameters.setServerName("host_name", null)? User just decided to clear it. Ok, that's what we do. It becomes an empty map in SSLParameters. 4 Map names = sslParameters.getServerNames(); Returns empty Map. As far as good. 5 sslSocket.setSSLParameters(sslParameters); SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this SSLParameters and as a result, clears it's internal "host_name" map to null, and thus won't send anything out since it's empty. We have problems here. We need to support that if an application does not specified host_name value, we should use default values. I. SSLParameters sslParameters = new SSLParameters(); II. sslParameters.setCipherSuites(...); III. SSLSocket sslSocket = sslSocketFactory.createSocket("www.example.com", 443) IV. sslSocket.setSSLParameters(sslParameters); Before line IV and after line II, the sslParameters.getServerNames() are empty. In li
Re: (3rd Round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
On 8/15/2012 9:36 PM, Xuelei Fan wrote: > Updated webrev according to recent feedbacks: > http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/ > http://cr.openjdk.java.net./~xuelei/7068321/README_05.txt > > The major differences: > 1. add constant SSLParameters.SNI_HOST_NAME, and specify the format of > unknown SNI type (sni-) in ExtendedSSLSession. >Then we don't need to document the types and values in Java > Cryptography Architecture Standard Algorithm Name Documentation. > > 2. Other updates according to feedback from Brad, Weijun, and Sean. >Please let me know I missed misunderstood something. > missed or misunderstood ;-) > Thanks, > Xuelei > > On 8/12/2012 8:50 PM, Xuelei Fan wrote: >> Hi, >> >> Please review the spec of JEP 114, TLS Server Name Indication (SNI) >> Extension. >> >> http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/ >> >> Please read the README to help you understanding the the specification: >> >>http://cr.openjdk.java.net./~xuelei/7068321/README_04.txt >> >> The major differences comparing with previous webrev are: >> 1. client mode and server mode will use separated API set. >>For client, the related APIs are: >> setServerName(String type, String value) >> clearServerName(String type) >> disableServerName(String type) >> enableServerName(String type) >> isDisabledServerName(String type) >> getServerNames() >> >>For server side, the related APIs are: >> setServerNamePattern(String type, Pattern pattern) >> clearServerNamePattern(String type) >> getServerNamePatterns() >> >> 2. close the door to use the generated socket in client mode. >> >>SSLSocketFactory.createSocket(Socket s, >>InputStream consumed, boolean autoClose) >> >>The returned socket was set in server mode. >> >> Regards, >> Xuelei >> >
Re: (3rd Round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
Updated webrev according to recent feedbacks: http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/ http://cr.openjdk.java.net./~xuelei/7068321/README_05.txt The major differences: 1. add constant SSLParameters.SNI_HOST_NAME, and specify the format of unknown SNI type (sni-) in ExtendedSSLSession. Then we don't need to document the types and values in Java Cryptography Architecture Standard Algorithm Name Documentation. 2. Other updates according to feedback from Brad, Weijun, and Sean. Please let me know I missed misunderstood something. Thanks, Xuelei On 8/12/2012 8:50 PM, Xuelei Fan wrote: > Hi, > > Please review the spec of JEP 114, TLS Server Name Indication (SNI) > Extension. > > http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/ > > Please read the README to help you understanding the the specification: > >http://cr.openjdk.java.net./~xuelei/7068321/README_04.txt > > The major differences comparing with previous webrev are: > 1. client mode and server mode will use separated API set. >For client, the related APIs are: > setServerName(String type, String value) > clearServerName(String type) > disableServerName(String type) > enableServerName(String type) > isDisabledServerName(String type) > getServerNames() > >For server side, the related APIs are: > setServerNamePattern(String type, Pattern pattern) > clearServerNamePattern(String type) > getServerNamePatterns() > > 2. close the door to use the generated socket in client mode. > >SSLSocketFactory.createSocket(Socket s, >InputStream consumed, boolean autoClose) > >The returned socket was set in server mode. > > Regards, > Xuelei >
hg: jdk8/tl/jdk: 7110151: Use underlying platform's zlib library for Java zlib support
Changeset: 35e024c6a62c Author:andrew Date: 2012-08-15 14:35 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/35e024c6a62c 7110151: Use underlying platform's zlib library for Java zlib support Summary: Make SYSTEM_ZLIB more flexible by using ZLIB_{CFLAGS,LIBS} and building on more than just MACOSX. Reviewed-by: sherman, alanb ! make/com/sun/java/pack/Makefile ! make/common/Program.gmk ! make/common/shared/Defs-linux.gmk ! make/common/shared/Defs-macosx.gmk ! make/common/shared/Defs-solaris.gmk ! make/java/jli/Makefile ! make/java/zip/Makefile ! make/jdk_generic_profile.sh ! make/sun/splashscreen/Makefile ! src/share/native/com/sun/java/util/jar/pack/defines.h ! src/share/native/java/util/zip/Adler32.c ! src/share/native/java/util/zip/CRC32.c ! src/share/native/java/util/zip/Deflater.c ! src/share/native/java/util/zip/Inflater.c ! src/share/native/java/util/zip/zip_util.c
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
This paragraph was removed in the latest update. Please ignore the webrev_spec.04 and previous webrevs, and review webrev_spec.05. http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/ Thanks, Xuelei On 8/15/2012 3:24 PM, Bradford Wetmore wrote: > > >>> How would the ServerHello be generated until you call this method and >>> then start the handshaking on the returned SSLSocket? >> One can use SSLEngine.wrap()/unwrap() to generate handshaking messages >> from socket I/O stream. >> // accept a socket >> // read/write the I/O with SSLEngine >> // call this method > > Switching between SSLSocket/SSLEngine like that would require > intentional misuse of the API. You could make the same claim for > regular handshaking. > > Brad > >> Xuelei >> >>> You said in a >>> previous internal conversation that SSLExplore would not generate any >>> ServerHello, so this can't be what you mean. Are you talking about >>> layering another SSLSocket over a already layered SSLSocket? >>> > 2. "consumed network data is resumable" wasn't clear either. To me > this > should mean that you can obtain the data which has already been read > from "s". > Yes, need wordsmithing here. > 3. "Otherwise, the behavior of the return socket is not defined" lost > me. Does this mean that that SSLParameters and assorted settings are > not otherwise defined? > See above example. >>> >>> I think I need the above answered before I can comment further here. >>> > I think you could delete this paragraph. > > From your second email: > >> Thought more about the design, I would have to say that we cannot >> return >> the default value in sslParameters.getServerNames(). Otherwise, the >> following two block of codes look very weird to me: >>// case one: >> 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); >> 2 sslParameters.clearServerName("host_name"); >> 3 Map names = sslParameters.getServerNames(); >> 4 sslSocket.setSSLParameters(sslParameters); >> 5 sslParameters = sslSocket.getSSLParameters(); >> 6 names = sslParameters.getServerNames(); >> >> In line 3, the returned map does not contain "host_name" entry. >> But in >> line 6, it may be expected that no "host_name" in the returned >> map. But >> if we want to return default values, line 6 do need to return a map >> containing "host_name". The behavior is pretty confusing. We may >> want >> to try avoid the confusion. > > I'm not following your confusion, it seemed pretty straightforward to > me, it works much like CipherSuites. We have a set of ciphersuites > which are enabled by default. We can turn some off by using > SSLParameters. Expanding a bit on your example here, I'll describe > what > I think would happen internally/externally: > > 1SSLSocket sslSocket = mySSLSocketFactory.createSocket( > "www.example.com", 443); > > mySSLSocketFactory sets any initial parameters as usual. > SSLSocketImpl > knows it's connecting to www.example.com and automatically stores > "host_name" -> "www.example.com" in its local host data (map or > separate > variables). > > 2 SSLparameters sslParameters = sslSocket.getSSLParameters(); > > SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the > hostmap to the one value "host_name" -> "www.example.com" > > If the application want to get the "default values", they just pull > them > out of the SSLParameters here > > 3 sslParameters.clearServerName("host_name"); > > Or sslParameters.setServerName("host_name", null)? > > User just decided to clear it. Ok, that's what we do. It becomes an > empty map in SSLParameters. > > 4 Map names = sslParameters.getServerNames(); > > Returns empty Map. > As far as good. > 5 sslSocket.setSSLParameters(sslParameters); > > SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this > SSLParameters and as a result, clears it's internal "host_name" map to > null, and thus won't send anything out since it's empty. > We have problems here. We need to support that if an application does not specified host_name value, we should use default values. I. SSLParameters sslParameters = new SSLParameters(); II. sslParameters.setCipherSuites(...); III. SSLSocket sslSocket = sslSocketFactory.createSocket("www.example.com", 443) IV. sslSocket.setSSLParameters(sslParameters); Before line IV and after line II, the sslParameters.getServerNames() are empty. In line IV, we need to make sure the internal "host_name", "www.example.com" is used as default value, and send it to server in SNI. That's the default behavio
Re: Code review request: 7184246: Simplify Config.get() of krb5
Updated at http://cr.openjdk.java.net/~weijun/7184246/webrev.01 Changes: 1. String values (even if there is only one) for the same key are stored in a vector. Two methods are provided: get(String... keys) returns the last value getAll(String... keys) returns all values concatenated so if you see [realms] R = { kdc = k1 kdc = k2 } then get("realms","R","kdc") returns k2, getAll("realms","R","kdc") returns "k1 k2". 2. SCDynamicStore is updated to be consistent with above. I also break the getConfig() method into 2 so that I can write a test. Thanks Max On 08/02/2012 10:14 PM, Weijun Wang wrote: Hi Valerie Please take a look at this http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7184246 The code changes include: 1. Config.java: a. Retrieve settings using .get(String... keys) now b. Some changes to parsing. The sub-section depth can be at any level. For compatibility reasons, multiple values for the same key are only for [realms] and [capaths] sections. c. Still using Hashtable and Vector because I don't want to make changes to Mac's SCDynamicStoreConfig.m. 2. initStatic() methods in several classes that read krb5.conf settings to static fields 3. All other old calls to getDefault() methods. Thanks Max Original Message 7184246: Simplify Config.get() of krb5 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7184246 === *Description* This is about the internal class sun.security.krb5.Config. If you want to get a value from inside krb5.conf, you can call getDefault(String). This might be good to get a value from the [libdefaults] section. However, the method was designed to be so smart that it can recursively search for key/value pairs no matter where and how deep it is. For example, given a krb5.conf [s1] a=b [s2] c=d [s3] e = { f = g } getDefault("a") = "b", getDefault("c") = "d", and astonishingly, getDefault("f") = "g". I don't think this is a good design, for several reasons: 1. It depends on the order of sections if there are key/value pairs with the same key in different sections. 2. It ignores wrong settings. For example, when doing a cross-realm auth, the Realm.getRealmsList(from,to) is used to get a path which should be defined in [capaths]. However, the method simply crawls recursively into any subsection it found and won't notice the [capaths] being mistakenly typed as [capath] 3. It lacks certain features. Because the function always return a String (same with the getDefault(String,String) method), getDefault("e") can only return a null. Therefore there is no way to find out the existence of the subsection e unless we also know it contains a key f. 4. The current Config class needs to know what subsections contains more subsections, and it hardcodes names like [capaths] and [realms]. In short, it's just too smart and becomes unsafe to use. I suggest removing all this smartness and a user must use the full paths to get a value, say, kdc = config.get("realms", "SUN.COM", "kdc") My proposed spec is: 1. The Config class should understand a krb5.conf without knowing any specific section names. All it maintains is a Value, which can be either of String List TreeMap Here I use TreeMap to preserve the order (might not be necessary). 2. The basic retrieval method will be Value get(String... key) 3. There are simpler methods if you already know what the type in your case is String getAsString(String... key) List getAsStringList(String... key) The compatibility risk will be low, and if there really comes a compatibility issue, most likely it will be because the caller had written his krb5.conf wrong. One of the advantages of the original design is that when a key is provided in both [libdefaults] and a given realm, the method can find it anyway. This will be useful for keys like kdc_timeout, max_retries. However, I think this automatic retrieval is confusing and error-prone, I'd rather manually call the get() method twice.
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
More comments about whether we are able to override the default value. On 8/15/2012 10:45 AM, Xuelei Fan wrote: >>> >> Thought more about the design, I would have to say that we cannot return >>> >> the default value in sslParameters.getServerNames(). Otherwise, the >>> >> following two block of codes look very weird to me: >>> >> // case one: >>> >> 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); >>> >> 2 sslParameters.clearServerName("host_name"); >>> >> 3 Map names = sslParameters.getServerNames(); >>> >> 4 sslSocket.setSSLParameters(sslParameters); >>> >> 5 sslParameters = sslSocket.getSSLParameters(); >>> >> 6 names = sslParameters.getServerNames(); >>> >> >>> >> In line 3, the returned map does not contain "host_name" entry. But in >>> >> line 6, it may be expected that no "host_name" in the returned map. But >>> >> if we want to return default values, line 6 do need to return a map >>> >> containing "host_name". The behavior is pretty confusing. We may want >>> >> to try avoid the confusion. >> > >> > I'm not following your confusion, it seemed pretty straightforward to >> > me, it works much like CipherSuites. We have a set of ciphersuites >> > which are enabled by default. We can turn some off by using >> > SSLParameters. Compatibility is my concerns here. When the SSLParameters class was was introduced in JDK 6, set/getCipherSuites() and set/getProtocols were new, so there was no compatibility issue for these two pairs methods at that time, as they were not used in old applications. In JDK 7, we introduced two new methods, set/getAlgorithmConstraints(). We were luck that we cannot allow the override of default constraints because of security consideration, and the concept of algorithm constraints was new in JDK 7. So we needed not to consider the compatibility issue too much between JDK 6 and 7 for this pair of methods. However, things get changed for the Server Name Inidication (SNI), because in JDK 7 we have already support default SNI extension implicit. We have to consider the compatibility issues more between JDK 7 and JDK 8, we need to make sure the behaviors are consistent whenever we call the set/getServerName(). If we allow override of default value, the application may run into corner trap as the description in my previous mail. That's the background of my thoughts. Hope it helps you understand the current design. Thanks, Xuelei >> > Expanding a bit on your example here, I'll describe what >> > I think would happen internally/externally: >> > >> > 1SSLSocket sslSocket = mySSLSocketFactory.createSocket( >> > "www.example.com", 443); >> > >> > mySSLSocketFactory sets any initial parameters as usual. SSLSocketImpl >> > knows it's connecting to www.example.com and automatically stores >> > "host_name" -> "www.example.com" in its local host data (map or separate >> > variables). >> > >> > 2 SSLparameters sslParameters = sslSocket.getSSLParameters(); >> > >> > SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the >> > hostmap to the one value "host_name" -> "www.example.com" >> > >> > If the application want to get the "default values", they just pull them >> > out of the SSLParameters here >> > >> > 3 sslParameters.clearServerName("host_name"); >> > >> > Or sslParameters.setServerName("host_name", null)? >> > >> > User just decided to clear it. Ok, that's what we do. It becomes an >> > empty map in SSLParameters. >> > >> > 4 Map names = sslParameters.getServerNames(); >> > >> > Returns empty Map. >> > > As far as good. > >> > 5 sslSocket.setSSLParameters(sslParameters); >> > >> > SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this >> > SSLParameters and as a result, clears it's internal "host_name" map to >> > null, and thus won't send anything out since it's empty. >> > > We have problems here. We need to support that if an application does > not specified host_name value, we should use default values. > I. SSLParameters sslParameters = new SSLParameters(); > II. sslParameters.setCipherSuites(...); > III. SSLSocket sslSocket = > sslSocketFactory.createSocket("www.example.com", 443) > IV. sslSocket.setSSLParameters(sslParameters); > > Before line IV and after line II, the sslParameters.getServerNames() are > empty. In line IV, we need to make sure the internal "host_name", > "www.example.com" is used as default value, and send it to server in > SNI. That's the default behaviors in JDK 7. We cannot break it without > strong desires. > > I think it means that we cannot clear the internal "host_name" when the > sslParameters.getServerNames() return empty. > > Does it make sense to you? > > Thanks, > Xuelei > >> > 6 sslParameters = sslSocket.getSSLParameters(); >> > >> > SSLSocketImpl.getSSLParameters() creates a SSLParameters, which sees >> > that there's no name indication, so it creates an empty name map and >> > stores in SSLParameters. >> > >>
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
How would the ServerHello be generated until you call this method and then start the handshaking on the returned SSLSocket? One can use SSLEngine.wrap()/unwrap() to generate handshaking messages from socket I/O stream. // accept a socket // read/write the I/O with SSLEngine // call this method Switching between SSLSocket/SSLEngine like that would require intentional misuse of the API. You could make the same claim for regular handshaking. Brad Xuelei You said in a previous internal conversation that SSLExplore would not generate any ServerHello, so this can't be what you mean. Are you talking about layering another SSLSocket over a already layered SSLSocket? 2. "consumed network data is resumable" wasn't clear either. To me this should mean that you can obtain the data which has already been read from "s". Yes, need wordsmithing here. 3. "Otherwise, the behavior of the return socket is not defined" lost me. Does this mean that that SSLParameters and assorted settings are not otherwise defined? See above example. I think I need the above answered before I can comment further here. I think you could delete this paragraph. From your second email: Thought more about the design, I would have to say that we cannot return the default value in sslParameters.getServerNames(). Otherwise, the following two block of codes look very weird to me: // case one: 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); 2 sslParameters.clearServerName("host_name"); 3 Map names = sslParameters.getServerNames(); 4 sslSocket.setSSLParameters(sslParameters); 5 sslParameters = sslSocket.getSSLParameters(); 6 names = sslParameters.getServerNames(); In line 3, the returned map does not contain "host_name" entry. But in line 6, it may be expected that no "host_name" in the returned map. But if we want to return default values, line 6 do need to return a map containing "host_name". The behavior is pretty confusing. We may want to try avoid the confusion. I'm not following your confusion, it seemed pretty straightforward to me, it works much like CipherSuites. We have a set of ciphersuites which are enabled by default. We can turn some off by using SSLParameters. Expanding a bit on your example here, I'll describe what I think would happen internally/externally: 1SSLSocket sslSocket = mySSLSocketFactory.createSocket( "www.example.com", 443); mySSLSocketFactory sets any initial parameters as usual. SSLSocketImpl knows it's connecting to www.example.com and automatically stores "host_name" -> "www.example.com" in its local host data (map or separate variables). 2 SSLparameters sslParameters = sslSocket.getSSLParameters(); SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the hostmap to the one value "host_name" -> "www.example.com" If the application want to get the "default values", they just pull them out of the SSLParameters here 3 sslParameters.clearServerName("host_name"); Or sslParameters.setServerName("host_name", null)? User just decided to clear it. Ok, that's what we do. It becomes an empty map in SSLParameters. 4 Map names = sslParameters.getServerNames(); Returns empty Map. As far as good. 5 sslSocket.setSSLParameters(sslParameters); SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this SSLParameters and as a result, clears it's internal "host_name" map to null, and thus won't send anything out since it's empty. We have problems here. We need to support that if an application does not specified host_name value, we should use default values. I. SSLParameters sslParameters = new SSLParameters(); II. sslParameters.setCipherSuites(...); III. SSLSocket sslSocket = sslSocketFactory.createSocket("www.example.com", 443) IV. sslSocket.setSSLParameters(sslParameters); Before line IV and after line II, the sslParameters.getServerNames() are empty. In line IV, we need to make sure the internal "host_name", "www.example.com" is used as default value, and send it to server in SNI. That's the default behaviors in JDK 7. We cannot break it without strong desires. I think it means that we cannot clear the internal "host_name" when the sslParameters.getServerNames() return empty. Does it make sense to you? Ok, that's an issue, I was assuming people would generally get a SSLParameters from the SSLSocket/SSLEngine, which would have prepopulated values such as CipherSuites/Protocols and SNI values. Now I hear what you're saying. So in SSLSocket/SSLEngine, we have a very similar situation with the CipherSuites/Protocols. The way we did it there was if SSLParameters.getCipherSuites() is null (that is, has not been set in this instance), we let the default values stand (that is we did not call SSLSocket.setEnabledCipherSuites()). You could do something like that with the SNI info. I've always felt the setServerName/getServerName should take/return the
Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
On 8/15/2012 2:10 PM, Bradford Wetmore wrote: > > > On 8/14/2012 7:45 PM, Xuelei Fan wrote: > > 197: You're not planning to process (e.g. > ServerHandshaker/ClientHandshaker.process_message) the consumed > handshaking bytes immediately during the createSocket call, are > you? You > still need to allow the user time to set the socket > options/SSLParameters/etc. I was expecting in this method you'd just > suck in the consumed bytes into temporary storage, then create/return > the socket, and then when the handshaking is started, you then read > out > from the temporary storage until you run out, then you switch to the > Socket's InputStream. > You're right. It is allowed to set more options in the returned socket before kick off handshake. > 197: This needs some wordsmithing here. This method will produce the > SSLSocket that will be consuming this data, so of course it has to be > called first. > I'm not sure I understand your point. Please comment it again with the revised APIs if you still have concerns. >>> >>> I just didn't understand much of this paragraph. >>> >>> 1. You have to call this method, then set up your parameters, then start >>> your handshaking. So the first half of this sentence doesn't apply. >>> >> Oh, I know your concerns. What I want to express is that before the >> calling to method, the caller should not do real handshaking. The logic >> I concerned looks like: >> // 1. accept a socket >> // 2. read ClientHello and reply ServerHello to output stream. >> // 3. call this method >> SSLSocket sslSocket = (SSLSocket)sslSocketFactory.createSocket( >> socke, inputStream, true); >> >> because the handshaking has started in step 2, then in step 3, we cannot >> get a proper SSLSocket. > > How would the ServerHello be generated until you call this method and > then start the handshaking on the returned SSLSocket? One can use SSLEngine.wrap()/unwrap() to generate handshaking messages from socket I/O stream. // accept a socket // read/write the I/O with SSLEngine // call this method Xuelei > You said in a > previous internal conversation that SSLExplore would not generate any > ServerHello, so this can't be what you mean. Are you talking about > layering another SSLSocket over a already layered SSLSocket? > >>> 2. "consumed network data is resumable" wasn't clear either. To me this >>> should mean that you can obtain the data which has already been read >>> from "s". >>> >> Yes, need wordsmithing here. >> >>> 3. "Otherwise, the behavior of the return socket is not defined" lost >>> me. Does this mean that that SSLParameters and assorted settings are >>> not otherwise defined? >>> >> See above example. > > I think I need the above answered before I can comment further here. > >>> I think you could delete this paragraph. >>> >>> From your second email: >>> Thought more about the design, I would have to say that we cannot return the default value in sslParameters.getServerNames(). Otherwise, the following two block of codes look very weird to me: // case one: 1 SSLparameters sslParameters = sslSocket.getSSLParameters(); 2 sslParameters.clearServerName("host_name"); 3 Map names = sslParameters.getServerNames(); 4 sslSocket.setSSLParameters(sslParameters); 5 sslParameters = sslSocket.getSSLParameters(); 6 names = sslParameters.getServerNames(); In line 3, the returned map does not contain "host_name" entry. But in line 6, it may be expected that no "host_name" in the returned map. But if we want to return default values, line 6 do need to return a map containing "host_name". The behavior is pretty confusing. We may want to try avoid the confusion. >>> >>> I'm not following your confusion, it seemed pretty straightforward to >>> me, it works much like CipherSuites. We have a set of ciphersuites >>> which are enabled by default. We can turn some off by using >>> SSLParameters. Expanding a bit on your example here, I'll describe what >>> I think would happen internally/externally: >>> >>> 1SSLSocket sslSocket = mySSLSocketFactory.createSocket( >>> "www.example.com", 443); >>> >>> mySSLSocketFactory sets any initial parameters as usual. SSLSocketImpl >>> knows it's connecting to www.example.com and automatically stores >>> "host_name" -> "www.example.com" in its local host data (map or separate >>> variables). >>> >>> 2 SSLparameters sslParameters = sslSocket.getSSLParameters(); >>> >>> SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the >>> hostmap to the one value "host_name" -> "www.example.com" >>> >>> If the application want to get the "default values", they just pull them >>> out of the SSLParameters here >>> >>> 3 sslParameters.clearServerName("host_name"); >>> >>> Or sslParameters.setServerName("