Le 13/04/2011 16:14, Douglas E. Engert a écrit :
On 4/13/2011 8:20 AM, Viktor TARASOV wrote:
Hi,

the number of APDUs used by PIV's driver to get the object data can be reduced.

Actually before getting object data, PIV driver tries to get it's size.
For that it reads the first 8 bytes of the object data. Usually card responds 
with '61xx' return code,
that's why one (useless) 'GET RESPONSE' APDU is sended by APDU transmit level 
(in apdu.c).

So, there are two not-necessary APDUs for each getting of the object data .
The intent was to get the length, then allocate a buffer and don't read the
objects unless needed. Unfortunately, the way PKCS#11 applications work,
one ends up reading all the certificates anyway.
I argued with NIST over the years that they needed a directory. The answer
was always, that you could get the length of the object from the object.

So as you point out the method of reading to get the length, does not
work very well. NIST 800-73 lists some "Max Bytes" for the sizes of the
objects, but I have seen objects which are are larger and 800-73-3 now
says these are recommended.

It looks like your buffer is only 8K, and this could cause a problem,
the "Cardholder Facial Image" object, "Max Bytes" is 12704.

So any buffer should be 64K to be on the safe side to avoid ever getting
the "Need to increase the size of internal receive buffer" message.

  >
In proposed patch all object data are read at once into the statically 
allocated buffer.
Its not static, it looks like it is on the stack. Maybe it should be allocated,
as part of the piv_private_data.
Have you any objections?
As long as the buffer is 64K and not on the stack...

There is another patch version where
dynamic 64k buffer used to receive the totality of the object data.


Kind wishes,
Viktor.




_______________________________________________
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel


--
Viktor Tarasov  <viktor.tara...@opentrust.com>

Index: src/libopensc/card-piv.c
===================================================================
--- src/libopensc/card-piv.c    (révision 5338)
+++ src/libopensc/card-piv.c    (copie de travail)
@@ -531,7 +531,7 @@
                /* if using internal buffer, alloc new one */
                if (rbuf == rbufinitbuf) {
                        *recvbuf = malloc(rbuflen); 
-                               sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "DEE 
got buffer %p len %d",*recvbuf,  rbuflen);
+                       sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "DEE got 
buffer %p len %d",*recvbuf,  rbuflen);
                        if (*recvbuf == NULL) {
                                r = SC_ERROR_OUT_OF_MEMORY;
                                goto err;
@@ -870,10 +870,14 @@
        int r = 0;
        u8 tagbuf[8];
        size_t tag_len;
+       u8 *rbuf = NULL;
+       size_t rbuf_len = 0;
        
        SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE);
-       sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "#%d \n", enumtag);
+       if (!buf || !buf_len)
+               SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, 
SC_ERROR_INVALID_ARGUMENTS);
 
+       sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "get object #%d data", 
enumtag);
        /* assert(enumtag >= 0 && enumtag < PIV_OBJ_LAST_ENUM); */
        
        tag_len = piv_objects[enumtag].tag_len;
@@ -883,49 +887,34 @@
        memcpy(p, piv_objects[enumtag].tag_value, tag_len);
        p += tag_len;
 
-       if (*buf_len == 1 && *buf == NULL) { /* we need to get the length */
-               u8 rbufinitbuf[8]; /* tag of 53 with 82 xx xx  will fit in 4 */
-               u8 *rbuf;
-               size_t rbuflen;
-               size_t bodylen;
-               unsigned int cla_out, tag_out;
-               const u8 *body;
+       rbuf = malloc(0x10000);
+       if (!rbuf)
+               SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, 
SC_ERROR_OUT_OF_MEMORY);
+       rbuf_len = 0x10000;
 
-               sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL,"get len of #%d", 
enumtag);
-               rbuf = rbufinitbuf;
-               rbuflen = sizeof(rbufinitbuf);
-               r = piv_general_io(card, 0xCB, 0x3F, 0xFF, tagbuf,  p - tagbuf,
-                               &rbuf, &rbuflen);
-               if (r > 0) {
-                       body = rbuf;
-                       if (sc_asn1_read_tag(&body, 0xffff, &cla_out, &tag_out, 
&bodylen) !=  SC_SUCCESS) {
-                               sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "***** 
received buffer tag MISSING ");
-                               r = SC_ERROR_FILE_NOT_FOUND;
-                               goto err;
-                       }
-                   *buf_len = r;
-               } else if ( r == 0) { 
-                       r = SC_ERROR_FILE_NOT_FOUND;
-                       goto err;
-               } else { 
-                       goto err;
-               }
+       r = piv_general_io(card, 0xCB, 0x3F, 0xFF, tagbuf,  p - tagbuf, &rbuf, 
&rbuf_len);
+       if (r < 0)   {
+               free(rbuf);
+               SC_TEST_RET(card->ctx, SC_LOG_DEBUG_NORMAL, r, "general IO 
failed");
        }
-sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL,"get buffer for #%d len %d", enumtag, 
*buf_len);
-       if (*buf == NULL && *buf_len > 0) {
-               *buf = malloc(*buf_len);
-               if (*buf == NULL ) {
-                       r = SC_ERROR_OUT_OF_MEMORY;
-                       goto err;
+
+       if (*buf == NULL)   {
+               *buf = malloc(rbuf_len);
+               if (*buf == NULL )   {
+                       free(rbuf);
+                       SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, 
SC_ERROR_OUT_OF_MEMORY);
                }
+
+               *buf_len = rbuf_len;
        }
 
-       r = piv_general_io(card, 0xCB, 0x3F, 0xFF, tagbuf,  p - tagbuf, 
-               buf, buf_len);
+       if (*buf_len > rbuf_len)
+               *buf_len = rbuf_len;
 
-err:
+       memcpy(*buf, rbuf, *buf_len);
+       free(rbuf);
 
-       SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, r);
+       SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, *buf_len);
 }
 
 static int piv_get_cached_data(sc_card_t * card, int enumtag,
@@ -935,7 +924,7 @@
        piv_private_data_t * priv = PIV_DATA(card);
        int r;
        u8 *rbuf = NULL;
-       size_t rbuflen;
+       size_t rbuflen = 0;
        
        SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE);
        sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "#%d", enumtag);
@@ -979,7 +968,6 @@
 
        /* Not cached, try to get it, piv_get_data will allocate a buf */ 
        sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL,"get #%d",  enumtag);
-       rbuflen = 1; 
        r = piv_get_data(card, enumtag, &rbuf, &rbuflen);
        if (r > 0) {
                priv->obj_cache[enumtag].flags |= PIV_OBJ_CACHE_VALID;
_______________________________________________
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel

Reply via email to