Code review request, 7200295 CertificateRequest message is wrapping when using large numbers of Certs

[Brad Wetmore]

On 9/24/2012 7:01 PM, Xuelei Fan wrote:
> On 9/25/2012 9:23 AM, Brad Wetmore wrote:
>> Are there situations where we might overflow the int?
>>
> Yes, it is possible for many integer add operations. As 2^32 is a lot
> bigger than 2^24 (the biggest number TLS protocol allows), I'm not
> worried too much about int32 overflow.

Ah yes...

> Integer overflow checking would make the code ugly.

Agreed!

>> For example, in CertificateRequest.messageLength()
>>
>>          for (int i = 0; i < authorities.length; i++) {
>>              len += authorities[i].length();
>>          }
>>
>> What if len overflows?
>>
>> Also, all of these field's callers are overflow-1?
>>
> I'm not sure I get your point. In RFC5246, exception session ID, other
> variable length is one of 2^8-1, 2^16-1 or 2^24 -1.

I was just wondering if there were any fields that were 2^8/2^16/2^24 
that would fail with this check.  I hadn't looked through the whole RFC, 
just asking if you had checked to avoid an "off-by-one" error.

I have a recollection that we recently did add some checks for making 
sure that we did not overflow other vector sizes.

Brad


>>
>>
>>
>>
>> On 9/23/2012 7:42 PM, Xuelei Fan wrote:
>>> Hi,
>>>
>>> Please review the update to check output filed length overflow in TLS
>>> handshaking.
>>>
>>> bug   : http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7200295
>>> webrev: http://cr.openjdk.java.net/~xuelei/7200295/webrev.00/
>>>
>>> The cause of the bug is that for 8, 16, 24 bits length-variable fields,
>>> before put the bytes into the fields, we do not check that the length of
>>> the bytes is less than the capabilities of the field.
>>>
>>> Thanks,
>>> Xuelei
>>>
>