Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.03/
A significant update of CipherBox.java.
We are not able to know whether a cipher for a particular key size is
available or not until the cipher is successfully initialized. For
example, we can get instance for "AES/GCM/NoPadding". But we don't known
whether the instance can work with AES-128 or AES-256 or not unless we
the Cipher.init() is called.
In the past, when a CipherBox is constructed, the cipher is always
initialized. However, for AEAD ciphers, we cannot initialized the cipher
in the constructor. We need an additional method to tell whether a
CipherBox is available or not for AEAD ciphers. The
CipherSuite.BulkCipher.isAvailable() will use this method to test the
availability of a cipher suites.
Thanks,
Xuelei
/*
* Is this cipher available?
*
* This method can only be called by CipherSuite.BulkCipher.isAvailable()
* to test the availability of a cipher suites. Please DON'T use it in
* other places, otherwise, the behavior may be unexpected because we may
* initialize AEAD cipher improperly in the method.
*/
Boolean isAvailable() {
// We won't know whether a cipher for a particular key size is
// available until the cipher is successfully initialized.
//
// We do not initialize AEAD cipher in the constructor. Need to
// initialize the cipher to ensure that the AEAD mode for a
// particular key size is supported.
if (cipherType == AEAD_CIPHER) {
try {
Authenticator authenticator =
new Authenticator(protocolVersion);
byte[] nonce = authenticator.sequenceNumber();
byte[] iv = Arrays.copyOf(fixedIv,
fixedIv.length + nonce.length);
System.arraycopy(nonce, 0, iv, fixedIv.length, nonce.length);
GCMParameterSpec spec = new GCMParameterSpec(tagSize * 8, iv);
cipher.init(mode, key, spec, random);
} catch (Exception e) {
return Boolean.FALSE;
}
} // Otherwise, we have initialized the cipher in the constructor.
return Boolean.TRUE;
}
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
EngineOutputRecord.java
===
294/296: Another great comment. I might suggest reversing the
comments so that the comment about AEAD is in the AEAD arm, and CBC is
outside.
I'm not sure I catch your ideas. ;-) Would you please show me the code?
Just a simple reversal of the lines so that the code you're talking
about is contained in the block that handles it:
if (!writeCipher.isAEADMode()) {
// DON'T encrypt the nonce_explicit for AEAD mode
dstBB.position(dstPos + headerSize);
} // The explicit IV in TLS 1.1 and later can be encrypted.
Hope that's clearer.
Looks like my logic is correct. If the cipher is not AEAD mode, the
explicit IV can be encrypted; (otherwise) if the cipher is AEAD mode,
don't encrypt the nonce_explicit.
if (!writeCipher.isAEADMode()) {
// The explicit IV in TLS 1.1 and later can be encrypted.
dstBB.position(dstPos + headerSize);
} // Otherwise, DON'T encrypt the nonce_explicit for AEAD mode
Good grief. I obviously need more sleep. My apologies. :(
Brad
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
On 1/19/2013 7:55 AM, Brad Wetmore wrote:
> I've pulled out things that need no further discussion.
>
>> I will be doing a putback of the JCE providers, so I can do your SunJCE
>> signed provider putback for you. Let's coordinate when you are ready.
>> I will probably be ready early next week.
>
> I will likely be putting back on Monday, maybe Tuesday. I'll try to
> coordinate with you over the weekend.
>
OK.
I think we are on the same page except the following minor comment
style. Thanks for the code review.
>>> EngineOutputRecord.java
>>> ===
>>> 294/296: Another great comment. I might suggest reversing the
>>> comments so that the comment about AEAD is in the AEAD arm, and CBC is
>>> outside.
>>>
>> I'm not sure I catch your ideas. ;-) Would you please show me the code?
>
> Just a simple reversal of the lines so that the code you're talking
> about is contained in the block that handles it:
>
> if (!writeCipher.isAEADMode()) {
> // DON'T encrypt the nonce_explicit for AEAD mode
> dstBB.position(dstPos + headerSize);
> } // The explicit IV in TLS 1.1 and later can be encrypted.
>
> Hope that's clearer.
>
Looks like my logic is correct. If the cipher is not AEAD mode, the
explicit IV can be encrypted; (otherwise) if the cipher is AEAD mode,
don't encrypt the nonce_explicit.
if (!writeCipher.isAEADMode()) {
// The explicit IV in TLS 1.1 and later can be encrypted.
dstBB.position(dstPos + headerSize);
} // Otherwise, DON'T encrypt the nonce_explicit for AEAD mode
>>> Handshaker.java
>>> ===
>>> 2. If a remote server selects a GCM ciphersuite and a version other
>>> than TLS 1.2, our client will send an alert_illegal_parameter. I
>>> think this is done at ClientHandshaker:461 calling into
>>> Handshaker:isNegotiable(), but the comments in getActiveCipherSuites()
>>> only talk about the initial request, not the CipherSuite we actually
>>> get back from the server. Can you please double check this?
>>>
>> Good catch. We did not check the cipher suite and protocol version
>> together. Need to address this in a new bug so that we can backport the
>> update.
>
> Not following what you mean by "backport the update"? GCM only exists
> in 8 at the moment. Or do you mean check the other TLS 1.2 ciphersuites
> besides GCM?
>
Yes, besides the GCM cipher suites, there are some new cipher suites are
defined since TLS 1.2. I think we'd better check the mismatching for all
such cipher suites, in both JDK 7 and 8.
Thanks for the code review. It helps a lot to me.
Xuelei
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
I've pulled out things that need no further discussion.
> I will be doing a putback of the JCE providers, so I can do your SunJCE
> signed provider putback for you. Let's coordinate when you are ready.
> I will probably be ready early next week.
I will likely be putting back on Monday, maybe Tuesday. I'll try to
coordinate with you over the weekend.
CipherBox.java
==
312/564: I don't remember why we decided to use the RuntimeException
AIOOBE here. Ugh...anyway, can you also use exception chaining to help
debug any reported problems?
We don't have chaining constructor for ArrayIndexOutOfBoundsException class.
Well, that explains it. :) I wonder if there is already a RFE for this?
CipherSuite.java
510: I don't think this statement is true anymore. This is likley a
carryover from the 1.4 days when we used to have a crypto provider in
the JSSE jar file. Not for this review, but maybe we should actually
check that implementations are available? If we remove SunEC and
SunPKCS11 providers from the provider list, could we potentially
disable the EC suites?
Good catch.
EC algorithm will be checked in KeyExchange, rather than BulkCipher. If
EC algorithm is not available, EC cipher suites will be disabled.
I worry about the RC4 cipher. The AES/CBC/128 is the minimal
requirements of a crypto provider, so I don't worry about AES/CBC/128.
However, RC4 is not in the minimal requirements of a crypto provider, so
we should check the availability of RC4 here.
I will file a new for this issue.
Thanks.
522: Why are you also checking for CipherType.AEAD_CIPHER?
AEAD/GCM is not implemented in all providers, for example the SunPKCS11
provider. So we need to check the available of AEAD implementation.
Of course, now I see it. The extra check for AES_256_GCM threw me. If
you're also checking for AEAD, you probably don't need the separate
check for AES_256_GCM. Maybe:
if (this == B_AES_256 ||
(this.cipherType == CipherType.AEAD_CIPHER)) {
is sufficient?
970: "before" -> "while"
It looks strange to me with two whiles:
// ..., we decrease the
// priority of cipher suites in GCM mode for a while while GCM
// technologies become mature in the industry.
I think "before" may be better word here.
Sorry, definitely not two whiles. I think I probably meant to say "as"
but obviously didn't.
Maybe:
// Placeholder for cipher suites in GCM mode.
//
// For better compatibility and interoperability, we decrease the
// priority of cipher suites in GCM mode for a while as GCM
// technologies mature in the industry. Eventually we'll move
// the GCM suites here.
EngineOutputRecord.java
===
294/296: Another great comment. I might suggest reversing the
comments so that the comment about AEAD is in the AEAD arm, and CBC is
outside.
I'm not sure I catch your ideas. ;-) Would you please show me the code?
Just a simple reversal of the lines so that the code you're talking
about is contained in the block that handles it:
if (!writeCipher.isAEADMode()) {
// DON'T encrypt the nonce_explicit for AEAD mode
dstBB.position(dstPos + headerSize);
} // The explicit IV in TLS 1.1 and later can be encrypted.
Hope that's clearer.
306: The original code was bad (double debug != null :) ), and I
realize the original code was lacking in parens "()", but can you
please add parens to indicate exactly what expression order you intend
here. My head is spinning from parsing the various cases: I'm not sure
the logic here is correct. I think we should output if debug is on
and either handshake/record is active or we're outputing a CCS. That
is:
if ((debug ! null) &&
((Debug.isOn("record") || (Debug.isOn("handshake") ||
(contentType() == ct_change_cipher_spec) {
Is my thinking incorrect?
In the old code, there is no dump for "handshake" level log unless it is
a change_cipher_spec message. However, the log is dumped for "record"
level log. I think it was right because the log here is record
information, but not handshake message.
I think the update does not changes the behavior.
You are correct, the actual update does not change the observed
behavior, but I had to write an app to prove it to myself.
However, this code is not following the failfast paradigm of Debug,
though it does give the same answer because of the way the Debug.isOn()
code was written. debug!=null is supposed to be a lightweight check to
see if any Debug options are on, and if so, then do the more heavyweight
checks.
Abstracting a bit, test1 is the "debug!=null" test. The resulting code is:
if (test1() && test2() || test3() && test4()) {
System.out...
}
So if test1 fails, then we jump to the "test3 && test4" case, which then
potentially has to go through the isOn processing twice. Granted,
because of the way isOn was written (if
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.02/
No significant changes in the new webrev.
> I will be doing a putback of the JCE providers, so I can do your SunJCE
> signed provider putback for you. Let's coordinate when you are ready.
> I will probably be ready early next week.
>
Good!
> Minor nits:
> ===
> What is the indentation style you are using? I noticed a few places
> where it's not 4/8 spaces, and it's not lining up with anything on the
> previous line. For example CipherSuite.java:522:544.
>
I usually use 4 spaces for new lines. But if the next line is a
continuation of the previous line, I did not follow the 8 spaces strictly.
> Also, in several of your new methods, you are writing pseudo-javadoc
> style (@return/etc), but you're not starting the comment with "/**".
> This probably doesn't matter since we don't generate javadoc for
> internal classes. Just mentioning since you took the time to format
> it and hoping it would render.
>
I just want to use the tag in order to make the comments clear and easy
understood.
> CipherBox.java
> ==
> 312/564: I don't remember why we decided to use the RuntimeException
> AIOOBE here. Ugh...anyway, can you also use exception chaining to help
> debug any reported problems?
>
We don't have chaining constructor for ArrayIndexOutOfBoundsException class.
>
> CipherSuite.java
>
> 510: I don't think this statement is true anymore. This is likley a
> carryover from the 1.4 days when we used to have a crypto provider in
> the JSSE jar file. Not for this review, but maybe we should actually
> check that implementations are available? If we remove SunEC and
> SunPKCS11 providers from the provider list, could we potentially
> disable the EC suites?
>
Good catch.
EC algorithm will be checked in KeyExchange, rather than BulkCipher. If
EC algorithm is not available, EC cipher suites will be disabled.
I worry about the RC4 cipher. The AES/CBC/128 is the minimal
requirements of a crypto provider, so I don't worry about AES/CBC/128.
However, RC4 is not in the minimal requirements of a crypto provider, so
we should check the availability of RC4 here.
I will file a new for this issue.
> 522: Why are you also checking for CipherType.AEAD_CIPHER?
>
AEAD/GCM is not implemented in all providers, for example the SunPKCS11
provider. So we need to check the available of AEAD implementation.
>
> 970: "before" -> "while"
>
It looks strange to me with two whiles:
// ..., we decrease the
// priority of cipher suites in GCM mode for a while while GCM
// technologies become mature in the industry.
I think "before" may be better word here.
> 1054: From http://www.rfc-editor.org/rfc/rfc6460.txt
>
>A Suite B TLS client configured at a minimum level of security of 128
>bits MUST offer the TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 or the
>TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 cipher suite in the
>ClientHello message. The TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
>cipher suite is preferred; if offered, it MUST appear before the
>TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 cipher suite.
>
> I would have thought what you have is the correct order, but it
> doesn't follow the RFC. Please add a note to remind us, because this
> order will not make sense in 6 months. ;)
>
You are right. The current default cipher suites preference is not
Suite-B/RFC6460 compliant. We need additional work to configure the
cipher suites in order to comply with RFC 6460. We may want to add a
new feature in the future.
> EngineInputRecord.java
> ==
> 191: Your comment is a little confusing here. Does this mean it does
> this internally?
>
Need to remove this line. Removed.
> EngineOutputRecord.java
> ===
> 294/296: Another great comment. I might suggest reversing the
> comments so that the comment about AEAD is in the AEAD arm, and CBC is
> outside.
>
I'm not sure I catch your ideas. ;-) Would you please show me the code?
> 306: The original code was bad (double debug != null :) ), and I
> realize the original code was lacking in parens "()", but can you
> please add parens to indicate exactly what expression order you intend
> here. My head is spinning from parsing the various cases: I'm not sure
> the logic here is correct. I think we should output if debug is on
> and either handshake/record is active or we're outputing a CCS. That
> is:
>
> if ((debug ! null) &&
> ((Debug.isOn("record") || (Debug.isOn("handshake") ||
> (contentType() == ct_change_cipher_spec) {
>
> Is my thinking incorrect?
>
In the old code, there is no dump for "handshake" level log unless it is
a change_cipher_spec message. However, the log is dumped for "record"
level log. I think it was right because the log here is record
information, but not handshake message.
I think the update does not changes the behavior.
>
> Handshaker.java
>
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
Hi Xuelei,
Minor stuff. A couple things to check.
Xuelei wrote:
Hi Valerie, Max or Brad,
Can you review the update for JDK-7030966? It is the JSSE part of JEP 115.
webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/
JEP 115: http://openjdk.java.net/jeps/115
I looked at webrev.01.
In the update, I have not remove the debug synchronization. I will
remove them before pushing the changeset.
I will be doing a putback of the JCE providers, so I can do your SunJCE
signed provider putback for you. Let's coordinate when you are ready.
I will probably be ready early next week.
Minor nits:
===
What is the indentation style you are using? I noticed a few places
where it's not 4/8 spaces, and it's not lining up with anything on the
previous line. For example CipherSuite.java:522:544.
Also, in several of your new methods, you are writing pseudo-javadoc
style (@return/etc), but you're not starting the comment with "/**".
This probably doesn't matter since we don't generate javadoc for
internal classes. Just mentioning since you took the time to format
it and hoping it would render.
TlsKeyMaterialGenerator.java
OK
TlsKeyMaterialParameterSpec.java
OK
TlsKeyMaterialSpec.java
===
OK
P11TlsKeyMaterialGenerator.java
===
OK
CipherBox.java
==
In various places, you mention RFC 5246 (TLS 1.2), but it also applies
to RFC 4346 (TLS 1.1). For example in getExplicitNonceSize(). You
might want to mention both RFCs in these places for completeness and to
avoid confusion.
180: We only need to keep the key around for AEAD, so saving a copy
here for CBC prevents GC of an object we'll never use again. In your
comment of 126, you also mention that it's reserved for AEAD, so I would
not expect to see this key nonnull for anything besides AEAD.
131/205: In a similar vein, tagSize is only used for AEAD, so you
could initialize tagSize to 0 and then move line 205 inside the "if
(AEAD)" arm. Not critical, but it seems incorrect to have a regular
AES/CBC CipherBox with a tagsize of 16.
214/218: Same comment about fixedIV/recordIVSize. Only used in AEAD.
No need to keep around this dummy array or calculate a value we don't
need.
207/215: Some additional commenting of the AEAD vs. CBC Cipher init
logic would be helpful here for people trying to understand the code.
Something like:
AEAD must completely initialize the cipher for each packet, and so
we save initialization parameters for packet processing time.
CBC only requires one initialization during its lifetime (future
packets/IVs set the proper CBC state), so we can initialize now.
298: Exception chaining for debugging? Minor nit: since you're using
the new multi-catch Exceptions here, maybe a name change (ibse) is in
order?
312/564: I don't remember why we decided to use the RuntimeException
AIOOBE here. Ugh...anyway, can you also use exception chaining to help
debug any reported problems?
361: Variable name OutputSize is capitalized.
774/803: Minor nit (take or leave): you might want to change "explicit
nonce" -> "nonce_explicit" for readers are following the RFC. The
others are fine.
790/849: "alsoe" -> "also"
798: "Applys" -> "Applies"
825: Your exception description just mentions nonces, but your check
is for both nonce/tagSize. This description is incomplete.
CipherSuite.java
510: I don't think this statement is true anymore. This is likley a
carryover from the 1.4 days when we used to have a crypto provider in
the JSSE jar file. Not for this review, but maybe we should actually
check that implementations are available? If we remove SunEC and
SunPKCS11 providers from the provider list, could we potentially
disable the EC suites?
522: Why are you also checking for CipherType.AEAD_CIPHER?
577: If you actually pulled out something for b in 538, you're putting
it right back in at 577. You should probably have the second if
completely inside the first and move 577 inside that if.
Boolean b = availableCache.get(cipher);
if (b == null) {
if (cipher.keySize > 128)
...deleted...
if (b == null) {
b = Boolean.FALSE;
...deleted...
}
availableCache.put(cipher, b);
}
return b.booleanValue();
540-550: Question: is this just an optimization to see if it's > 128
before actually trying it at 566? You might mention that.
970: "before" -> "while"
1054: From http://www.rfc-editor.org/rfc/rfc6460.txt
A Suite B TLS client configured at a minimum level of security of 128
bits MUST offer the TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 or the
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 cipher suite in the
ClientHello message. The TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
cipher suite is preferred; if offered, it MUST appear before the
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 ciph
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.01/ The main update in this webrev is to correct the MAC checking in (Engine)InputRecord to handle null cipher (as TLS_RSA_WITH_NULL_MD5). Thanks, Xuelei On 12/17/2012 10:50 AM, Xuelei Fan wrote: > Ping again ... ;-) > > On 12/4/2012 11:07 PM, Xuelei Fan wrote: >> Ping ... >> >> On 11/22/2012 11:00 PM, Xuelei Fan wrote: >>> Hi Valerie, Max or Brad, >>> >>> Can you review the update for JDK-7030966? It is the JSSE part of JEP 115. >>> >>> webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ >>> JEP 115: http://openjdk.java.net/jeps/115 >>> >>> In the update, I have not remove the debug synchronization. I will >>> remove them before pushing the changeset. >>> >>> Thanks, >>> Xuelei >>> >> >
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
Ping again ... ;-) On 12/4/2012 11:07 PM, Xuelei Fan wrote: > Ping ... > > On 11/22/2012 11:00 PM, Xuelei Fan wrote: >> Hi Valerie, Max or Brad, >> >> Can you review the update for JDK-7030966? It is the JSSE part of JEP 115. >> >> webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ >> JEP 115: http://openjdk.java.net/jeps/115 >> >> In the update, I have not remove the debug synchronization. I will >> remove them before pushing the changeset. >> >> Thanks, >> Xuelei >> >
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
Ping ... On 11/22/2012 11:00 PM, Xuelei Fan wrote: > Hi Valerie, Max or Brad, > > Can you review the update for JDK-7030966? It is the JSSE part of JEP 115. > > webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ > JEP 115: http://openjdk.java.net/jeps/115 > > In the update, I have not remove the debug synchronization. I will > remove them before pushing the changeset. > > Thanks, > Xuelei >
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
On 11/23/2012 8:01 PM, Florian Weimer wrote: > On 11/23/2012 12:54 PM, Xuelei Fan wrote: >> On 11/23/2012 7:22 PM, Florian Weimer wrote: >>> On 11/22/2012 04:00 PM, Xuelei Fan wrote: Hi Valerie, Max or Brad, Can you review the update for JDK-7030966? It is the JSSE part of JEP 115. webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ JEP 115: http://openjdk.java.net/jeps/115 In the update, I have not remove the debug synchronization. I will remove them before pushing the changeset. >>> >>> Does this add a new memory allocation to every TLS record which is being >>> processed? > >> Not really. We used to use large memory block (Record.maxRecordSize) >> for every record. The buffer size does not get changed. > > I'm referring to this code in > Authenticator.acquireAuthenticationBytes(). If I'm not mistaken, this > is called for every record. Previously, the array was directly hashed, > and after the refactoring, a copy is made and then hashed. Or am I > missing something? > Sorry for my miss-understanding. You are right here. We can return the array reference without clone. I was too conservative so that I always want to return copied array for safe coding style. It's not big problem to me because the array size is only 13 bytes, the clone should be pretty fast, and the memory footprint should be pretty small. Thanks, Xuelei
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
On 11/23/2012 12:54 PM, Xuelei Fan wrote: On 11/23/2012 7:22 PM, Florian Weimer wrote: On 11/22/2012 04:00 PM, Xuelei Fan wrote: Hi Valerie, Max or Brad, Can you review the update for JDK-7030966? It is the JSSE part of JEP 115. webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ JEP 115: http://openjdk.java.net/jeps/115 In the update, I have not remove the debug synchronization. I will remove them before pushing the changeset. Does this add a new memory allocation to every TLS record which is being processed? Not really. We used to use large memory block (Record.maxRecordSize) for every record. The buffer size does not get changed. I'm referring to this code in Authenticator.acquireAuthenticationBytes(). If I'm not mistaken, this is called for every record. Previously, the array was directly hashed, and after the refactoring, a copy is made and then hashed. Or am I missing something? -- Florian Weimer / Red Hat Product Security Team
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
On 11/23/2012 7:22 PM, Florian Weimer wrote: > On 11/22/2012 04:00 PM, Xuelei Fan wrote: >> Hi Valerie, Max or Brad, >> >> Can you review the update for JDK-7030966? It is the JSSE part of JEP >> 115. >> >> webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ >> JEP 115: http://openjdk.java.net/jeps/115 >> >> In the update, I have not remove the debug synchronization. I will >> remove them before pushing the changeset. > > Does this add a new memory allocation to every TLS record which is being > processed? Not really. We used to use large memory block (Record.maxRecordSize) for every record. The buffer size does not get changed. In the future, we may consider to use size-adjustable small records. > > I believe the comment in Authenticator should read: > > * This interface represents an SSL/TLS message authentication token, > * which encapsulates a sequence number and ensures that attempts to > * delete or reorder messages can be detected. > > Quotes from the RFC should probable marked as such. > Thanks for the word-smithing. It really helps a lot to me. Thanks, Xuelei
Re: Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
On 11/22/2012 04:00 PM, Xuelei Fan wrote: Hi Valerie, Max or Brad, Can you review the update for JDK-7030966? It is the JSSE part of JEP 115. webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ JEP 115: http://openjdk.java.net/jeps/115 In the update, I have not remove the debug synchronization. I will remove them before pushing the changeset. Does this add a new memory allocation to every TLS record which is being processed? I believe the comment in Authenticator should read: * This interface represents an SSL/TLS message authentication token, * which encapsulates a sequence number and ensures that attempts to * delete or reorder messages can be detected. Quotes from the RFC should probable marked as such. -- Florian Weimer / Red Hat Product Security Team
Code Review Request for 7030966, Support AEAD CipherSuites (JSSE part of JEP 115)
Hi Valerie, Max or Brad, Can you review the update for JDK-7030966? It is the JSSE part of JEP 115. webrev: http://cr.openjdk.java.net./~xuelei/7030966/webrev.00/ JEP 115: http://openjdk.java.net/jeps/115 In the update, I have not remove the debug synchronization. I will remove them before pushing the changeset. Thanks, Xuelei
