RFR 8163251 : Hard coded loop limit prevents reading of smart card data greater than 8k

[Valerie Peng]

Ignore my earlier comment about fixing the check.

Took a second look at the check and noticed that it's for throwing the 
exception.

Your current webrev is fine.

Regards,
Valerie


On 2/12/2020 8:23 PM, Valerie Peng wrote:
>
> Hi, Ivan,
>
> I share your thought/confusion on the current impl as I am also not 
> familiar with ISO/IEC 7816-4.
>
> Based on my reading of this standard and the CardImpl code, it looks 
> like the while-true loop is for retrieving additional response data. 
> Per the standard and javadoc, max length of the response data is 
> 65536. Given that SW2 is only one byte, it would probably only return 
> at most 256 byte response data at a time. Thus, the iteration count 
> would be at most 256.
>
> So far we are on the same page.
>
> With the latest webrev (webrev.01), it seems that the loop will only 
> be run 255 instead of 256 times as k is incremented before comparison. 
> Thus, I think we should fix the check.
>
> Valerie
>
> On 2/11/2020 12:18 PM, Ivan Gerasimov wrote:
>> Hi Valerie!
>>
>> To be honest, the all these limitations are not quite clear to me.
>>
>> If the command is using an extended Le word to specify the expected 
>> length of the response data, then this length can be at most 65536.
>>
>> If a short Le was used, then the length can be at most 256.
>>
>> However, if we received 0x61 in the SW1, that means that more data 
>> bytes are available and they can be retrieved by issuing another 
>> transmit call with a short Le.  This next call can again potentially 
>> result in 0x61 in SW1, and so on.
>>
>> In the standard, I cannot see any explicit limitations on the number 
>> of retries.  So, I see it as it might be possible to retrieve more 
>> data than 65536 bytes.
>>
>> On the other hand, in the specification for CommandAPDU [1] we have 
>> hardcoded limit for the maximum response length, which is 65536.  So, 
>> even if it were possible to retrieve larger data, there's no point to 
>> try, as the current API prohibits it.
>>
>> Assuming that the 0x61 response can only be received when a short Le 
>> is used, the maximum RESPONSE_ITERATOR should be set to 256, and an 
>> exception should be thrown once that number is exceeded.
>>
>> I've updated the webrev in-place accordingly.
>>
>> [0] http://cr.openjdk.java.net/~igerasim/8163251/01/webrev/
>>
>> [1] 
>> https://docs.oracle.com/en/java/javase/13/docs/api/java.smartcardio/javax/smartcardio/CommandAPDU.html#%3Cinit%3E(int,int,int,int,int)
>>
>> With kind regards,
>>
>> Ivan
>>
>>
>> On 2/10/20 6:40 PM, Valerie Peng wrote:
>>> Hi Ivan,
>>>
>>> You removed the "=", so the actual iteration count is reduced by one.
>>>
>>> Should the iteration count be 256 or 257? If the actual count should 
>>> be 257, then may be the check needs to be changed to k++ from ++k?
>>>
>>> Valerie
>>>
>>> On 2/10/2020 5:07 PM, Ivan Gerasimov wrote:
>>>> Thank you Michael!
>>>>
>>>> It's a good point about maximum length.
>>>>
>>>> Here's the updated webrev with the new System property dropped and 
>>>> the increased number of iterations:
>>>>
>>>> http://cr.openjdk.java.net/~igerasim/8163251/01/webrev/
>>>>
>>>> With kind regards,
>>>> Ivan
>>>>
>>>>
>>>> On 2/10/20 4:18 PM, Michael StJohns wrote:
>>>>> On 2/10/2020 6:49 PM, Ivan Gerasimov wrote:
>>>>>> Hello!
>>>>>>
>>>>>> Current implementation of the method 
>>>>>> javax.smartcardio.CardChannel.transmit() has an internal 
>>>>>> limitation on the maximum allowed amount of the transmitted data.
>>>>>>
>>>>>> This limitation is dictated by the maximum number of iterations 
>>>>>> to transmit data from a card:  Each iteration can transmit up to 
>>>>>> 256 bytes of data, and we have a hardcoded limit of 32 iterations.
>>>>>>
>>>>>> Over time, we've received requests to increase this limit, as 
>>>>>> there are occasions when the effective limit of 8k is not enough.
>>>>>>
>>>>>> Would you please help review a proposal:  First, it is proposed 
>>>>>> to increase the default limit of iteration to 128 (so that up to 
>>>>>> 32k of data can be transmitted); Second, the limit of iterations 
>>>>>> is made configurable via a System property. This limit can be 
>>>>>> increased up to 4096 (so that the total amount of transmitted 
>>>>>> data can be made up to 1m).
>>>>>>
>>>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8163251
>>>>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8163251/00/webrev/
>>>>>>
>>>>>> If there is an agreement on the proposal, I'll file a CSR to 
>>>>>> document a new System property.
>>>>>>
>>>>>> Thanks in advance!
>>>>>>
>>>>> Given that the maximum length for an extended APDU is 64K (65536) 
>>>>> (hmm +7 for the header and +2 for LE), and for its return is 64K + 
>>>>> 2 bytes,  I'm not quite sure why you'd up the number to 4096/1M - 
>>>>> I'd set the default and fixed value to 257 (64K) and leave it at 
>>>>> that.
>>>>>
>>>>> Mike
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20200212/83d94cfe/attachment.htm>