<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Thank you Mike for the additional analysis!</p>
    <p>I would like to avoid any significant modifications to the code
      at this moment, as I have only limited resources to test them.</p>
    <p>The last proposed patch [1] takes a conservative approach and
      just increases the limit of the iterations.</p>
    <p>So, this is highly unlikely to break any existing code.</p>
    <p>Given that this patch was confirmed to resolve a problem in at
      least one existing scenario, I'd like to go ahead with this fix
      unless there are strong objections.</p>
    <p>I agree that it may make sense to revisit this code and make sure
      it conform to the standards and real hardware, but I think it
      should be done separately from this issue.</p>
    <p>Another reason to keep the fix as simple as possible is that
      we're planning for backporting the fix to the earlier releases of
      the JDK, so I'd rather keep it low-risk.<br>
    </p>
    <p>With kind regards,</p>
    <p>Ivan<br>
    </p>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2/12/20 9:43 PM, Michael StJohns
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:5bed26e1-be4e-da41-0d7a-059ca33e3deb@comcast.net">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">Hi - <br>
      </div>
      <br>
      <p>I needed to go take a quick look at 7816-3 to figure out what's
        what.</p>
      <p>Basically, this code is a bit problematic.  <br>
      </p>
      <p>1) Since this code doesn't support extended length APDUs for
        T=0, you should never have to do multiple calls to get the
        responses - e.g. the max response handled here should be 256
        bytes.  But this is the only case in which the GetResponse code
        should actually be active???  T=1 uses low level mapping of
        I-Blocks rather than an APDU mapping.  And looking at pcsc.c -
        you've got 8192 as the maximum size for a per-call return from
        SCardTransmit, and I'm not sure what SCardTransmit will do - I'm
        wondering if its returning the 6Cxx error code.<br>
      </p>
      <p>2) The iteration count should probably be set from the Le value
        in the transmitted CommandAPDU rather than set to a fixed
        value.  Or even better, don't use an iteration count, but simply
        count down from the specified Le and stop when negative.</p>
      <p>I'm very confused now.  <br>
      </p>
      <p>What protocol is the customer using?   What's the actual APDU
        they're sending to get the data? (Mostly curious about Lc and Le
        here).<br>
      </p>
      <p>If you changed the iteration loop to parse the Le field and use
        that to count down received data, would that break things?<br>
      </p>
      <p>Mike<br>
      </p>
      <p><br>
      </p>
      <div class="moz-cite-prefix">On 2/12/2020 11:23 PM, Valerie Peng
        wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:3c87015d-3d01-2450-7691-42cc4f35ed26@oracle.com">
        <meta http-equiv="Content-Type" content="text/html;
          charset=UTF-8">
        <p>Hi, Ivan,</p>
        <p><span style="left: 729.868px; top: 57.2085px; font-size:
            18.4px; font-family: sans-serif; transform:
            scaleX(1.00278);">I share your thought/confusion on the
            current impl as </span>I am also not familiar with <span
            style="left: 729.868px; top: 57.2085px; font-size: 18.4px;
            font-family: sans-serif; transform: scaleX(1.00278);">ISO/IEC
            7816-4. <br>
          </span></p>
        <p><span style="left: 729.868px; top: 57.2085px; font-size:
            18.4px; font-family: sans-serif; transform:
            scaleX(1.00278);">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.<br>
          </span></p>
        <span style="left: 729.868px; top: 57.2085px; font-size: 18.4px;
          font-family: sans-serif; transform: scaleX(1.00278);"></span>
        <p>So far we are on the same page. <br>
        </p>
        <p>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.<br>
        </p>
        <p>Valerie<br>
        </p>
        <div class="moz-cite-prefix">On 2/11/2020 12:18 PM, Ivan
          Gerasimov wrote:<br>
        </div>
        <blockquote type="cite"
          cite="mid:e1380ad6-c05c-9a40-8dd1-76049d3ed221@oracle.com">Hi
          Valerie! <br>
          <br>
          To be honest, the all these limitations are not quite clear to
          me. <br>
          <br>
          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. <br>
          <br>
          If a short Le was used, then the length can be at most 256. <br>
          <br>
          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. <br>
          <br>
          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. <br>
          <br>
          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. <br>
          <br>
          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. <br>
          <br>
          I've updated the webrev in-place accordingly. <br>
          <br>
          [0] <a class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/~igerasim/8163251/01/webrev/"
            moz-do-not-send="true">http://cr.openjdk.java.net/~igerasim/8163251/01/webrev/</a>
          <br>
          <br>
          [1] <a class="moz-txt-link-freetext"
href="https://docs.oracle.com/en/java/javase/13/docs/api/java.smartcardio/javax/smartcardio/CommandAPDU.html#%3Cinit%3E(int,int,int,int,int)"
            moz-do-not-send="true">https://docs.oracle.com/en/java/javase/13/docs/api/java.smartcardio/javax/smartcardio/CommandAPDU.html#%3Cinit%3E(int,int,int,int,int)</a><br>
          <br>
          With kind regards, <br>
          <br>
          Ivan <br>
          <br>
          <br>
          On 2/10/20 6:40 PM, Valerie Peng wrote: <br>
          <blockquote type="cite">Hi Ivan, <br>
            <br>
            You removed the "=", so the actual iteration count is
            reduced by one. <br>
            <br>
            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? <br>
            <br>
            Valerie <br>
            <br>
            On 2/10/2020 5:07 PM, Ivan Gerasimov wrote: <br>
            <blockquote type="cite">Thank you Michael! <br>
              <br>
              It's a good point about maximum length. <br>
              <br>
              Here's the updated webrev with the new System property
              dropped and the increased number of iterations: <br>
              <br>
              <a class="moz-txt-link-freetext"
                href="http://cr.openjdk.java.net/~igerasim/8163251/01/webrev/"
                moz-do-not-send="true">http://cr.openjdk.java.net/~igerasim/8163251/01/webrev/</a>
              <br>
              <br>
              With kind regards, <br>
              Ivan <br>
              <br>
              <br>
              On 2/10/20 4:18 PM, Michael StJohns wrote: <br>
              <blockquote type="cite">On 2/10/2020 6:49 PM, Ivan
                Gerasimov wrote: <br>
                <blockquote type="cite">Hello! <br>
                  <br>
                  Current implementation of the method
                  javax.smartcardio.CardChannel.transmit() has an
                  internal limitation on the maximum allowed amount of
                  the transmitted data. <br>
                  <br>
                  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. <br>
                  <br>
                  Over time, we've received requests to increase this
                  limit, as there are occasions when the effective limit
                  of 8k is not enough. <br>
                  <br>
                  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). <br>
                  <br>
                  BUGURL: <a class="moz-txt-link-freetext"
                    href="https://bugs.openjdk.java.net/browse/JDK-8163251"
                    moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8163251</a>
                  <br>
                  WEBREV: <a class="moz-txt-link-freetext"
                    href="http://cr.openjdk.java.net/~igerasim/8163251/00/webrev/"
                    moz-do-not-send="true">http://cr.openjdk.java.net/~igerasim/8163251/00/webrev/</a>
                  <br>
                  <br>
                  If there is an agreement on the proposal, I'll file a
                  CSR to document a new System property. <br>
                  <br>
                  Thanks in advance! <br>
                  <br>
                </blockquote>
                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. <br>
                <br>
                Mike <br>
                <br>
                <br>
                <br>
                <br>
                <br>
              </blockquote>
            </blockquote>
          </blockquote>
        </blockquote>
      </blockquote>
      <p>l<br>
      </p>
    </blockquote>
    <pre class="moz-signature" cols="72">-- 
With kind regards,
Ivan Gerasimov</pre>
  </body>
</html>