[PATCH] scd: better handling of extended APDU
NIIBE Yutaka
gniibe at fsij.org
Tue Apr 14 04:31:37 CEST 2015
Hello,
This is a patch for GnuPG's internal ccid-driver support for better
handling of extended APDU. This is basically the response to
https://bugs.g10code.com/gnupg/issue1947
This patch is tested by the reporter, and confirmed it works well.
I'm going to commit/push this to master.
Please test it if you have readers which supports extended APDU.
Well, extended APDU is a kind of difficult specification in smartcard
and its readers (to be implemented). If possible, I wouldn't like to
implement this feature, but there is an OpenPGPcard implementation
which doesn't provide command-chaining, this is inevitable.
Just reading the specification lightly, it looks like extended APDU is
modern way and good for smartcard. But, observing some reader
implementations with problems, I couldn't say it's good thing.
Following implementation is not general purpose, but intended only
for OpenPGPcard implementations because CCID_MAX_BUF is about 2KB.
To be general purpose, it would be like 64KB, and/or we need to offer
specific API, so that application can specify buffer size for readers.
Currently, in scd/app-openpgp.c, it just has
app->app_local->cardcap.ext_lc_le (if we can use extended APDU), but
the capability of reader matter.
Basically, it means that the application for smartcard and its reader
cannot be layered like:
application
reader
smartcard
I understand that the extended APDU was intended to make things
easier, but it just introduced more problems, in practice.
In this development, I tested VASCO DIGIPASS 920 with OpenPGPcard 2.0
with RSA-2048 keys. VASCO DIGIPASS 920 has extended APDU support, but
dwMaxCCIDMsgLen=271. It supports sending (from host PC to reader)
large APDU chained by smaller CCID message, so, decryption works well.
But I realized that it doesn't work well to receive large APDU from
smartcard (and to send it to host PC by chained smaller CCID message).
So, there is no way with this card reader to get correct output of gpg
--card-status (specifically, reading public key from smartcard fails).
diff --git a/scd/apdu.c b/scd/apdu.c
index 53cc4b9..f6cca8c 100644
--- a/scd/apdu.c
+++ b/scd/apdu.c
@@ -3806,9 +3806,9 @@ send_le (int slot, int class, int ins, int p0, int p1,
apdu[apdulen++] = ins;
apdu[apdulen++] = p0;
apdu[apdulen++] = p1;
- apdu[apdulen++] = 0; /* Z byte: Extended length marker. */
- if (lc >= 0)
+ if (lc > 0)
{
+ apdu[apdulen++] = 0; /* Z byte: Extended length marker. */
apdu[apdulen++] = ((lc >> 8) & 0xff);
apdu[apdulen++] = (lc & 0xff);
memcpy (apdu+apdulen, data, lc);
@@ -3817,6 +3817,8 @@ send_le (int slot, int class, int ins, int p0, int p1,
}
if (le != -1)
{
+ if (lc <= 0)
+ apdu[apdulen++] = 0; /* Z byte: Extended length marker. */
apdu[apdulen++] = ((le >> 8) & 0xff);
apdu[apdulen++] = (le & 0xff);
}
diff --git a/scd/app-common.h b/scd/app-common.h
index 50046a4..379bcd1 100644
--- a/scd/app-common.h
+++ b/scd/app-common.h
@@ -67,10 +67,10 @@ struct app_ctx_s {
size_t serialnolen; /* Length in octets of serialnumber. */
const char *apptype;
unsigned int card_version;
- int did_chv1;
- int force_chv1; /* True if the card does not cache CHV1. */
- int did_chv2;
- int did_chv3;
+ unsigned int did_chv1:1;
+ unsigned int force_chv1:1; /* True if the card does not cache CHV1. */
+ unsigned int did_chv2:1;
+ unsigned int did_chv3:1;
struct app_local_s *app_local; /* Local to the application. */
struct {
void (*deinit) (app_t app);
diff --git a/scd/ccid-driver.c b/scd/ccid-driver.c
index 1926f71..151b371 100644
--- a/scd/ccid-driver.c
+++ b/scd/ccid-driver.c
@@ -96,6 +96,16 @@
#define DRVNAME "ccid-driver: "
+/* Max length of buffer with out CCID message header of 10-byte
+ Sending: 547 for RSA-4096 key import
+ APDU size = 540 (24+4+256+256)
+ commnd + lc + le = 4 + 3 + 0
+ Sending: write data object of cardholder certificate
+ APDU size = 2048
+ commnd + lc + le = 4 + 3 + 0
+ Receiving: 2048 for cardholder certificate
+*/
+#define CCID_MAX_BUF (2048+7+10)
/* Depending on how this source is used we either define our error
output to go to stderr or to the jnlib based logging functions. We
@@ -242,7 +252,7 @@ struct ccid_driver_s
unsigned char t1_nr;
unsigned char nonnull_nad;
int max_ifsd;
- int ifsd;
+ int max_ccid_msglen;
int ifsc;
unsigned char apdu_level:2; /* Reader supports short APDU level
exchange. With a value of 2 short
@@ -711,7 +721,7 @@ prepare_special_transport (ccid_driver_t handle)
handle->nonnull_nad = 0;
handle->auto_ifsd = 0;
handle->max_ifsd = 32;
- handle->ifsd = 0;
+ handle->max_ccid_msglen = CCID_MAX_BUF;
handle->has_pinpad = 0;
handle->apdu_level = 0;
switch (handle->id_product)
@@ -743,7 +753,6 @@ parse_ccid_descriptor (ccid_driver_t handle,
handle->nonnull_nad = 0;
handle->auto_ifsd = 0;
handle->max_ifsd = 32;
- handle->ifsd = 0;
handle->has_pinpad = 0;
handle->apdu_level = 0;
handle->auto_voltage = 0;
@@ -884,6 +893,7 @@ parse_ccid_descriptor (ccid_driver_t handle,
us = convert_le_u32(buf+44);
DEBUGOUT_1 (" dwMaxCCIDMsgLen %5u\n", us);
+ handle->max_ccid_msglen = us;
DEBUGOUT ( " bClassGetResponse ");
if (buf[48] == 0xff)
@@ -2794,109 +2804,101 @@ is_exlen_apdu (const unsigned char *apdu, size_t apdulen)
/* Helper for ccid_transceive used for APDU level exchanges. */
static int
ccid_transceive_apdu_level (ccid_driver_t handle,
- const unsigned char *apdu_buf, size_t apdu_buflen,
+ const unsigned char *apdu_buf, size_t apdu_len,
unsigned char *resp, size_t maxresplen,
size_t *nresp)
{
int rc;
- unsigned char send_buffer[10+261+300], recv_buffer[10+261+300];
- const unsigned char *apdu;
- size_t apdulen;
- unsigned char *msg;
+ unsigned char msg[CCID_MAX_BUF];
+ const unsigned char *apdu_p;
+ size_t apdu_part_len;
size_t msglen;
unsigned char seqno;
int bwi = 4;
+ unsigned char chain = 0;
- msg = send_buffer;
+ if (apdu_len == 0 || apdu_len > sizeof (msg) - 10)
+ return CCID_DRIVER_ERR_INV_VALUE; /* Invalid length. */
- apdu = apdu_buf;
- apdulen = apdu_buflen;
- assert (apdulen);
+ apdu_p = apdu_buf;
+ while (1)
+ {
+ apdu_part_len = apdu_len;
+ if (apdu_part_len > handle->max_ccid_msglen - 10)
+ {
+ apdu_part_len = handle->max_ccid_msglen - 10;
+ chain |= 0x01;
+ }
- /* The maximum length for a short APDU T=1 block is 261. For an
- extended APDU T=1 block the maximum length 65544; however
- extended APDU exchange level is not fully supported yet. */
- if (apdulen > sizeof (send_buffer) - 10)
- return CCID_DRIVER_ERR_INV_VALUE; /* Invalid length. */
+ msg[0] = PC_to_RDR_XfrBlock;
+ msg[5] = 0; /* slot */
+ msg[6] = seqno = handle->seqno++;
+ msg[7] = bwi;
+ msg[8] = chain;
+ msg[9] = 0;
+ memcpy (msg+10, apdu_p, apdu_part_len);
+ set_msg_len (msg, apdu_part_len);
+ msglen = 10 + apdu_part_len;
- msg[0] = PC_to_RDR_XfrBlock;
- msg[5] = 0; /* slot */
- msg[6] = seqno = handle->seqno++;
- msg[7] = bwi; /* bBWI */
- msg[8] = 0; /* RFU */
- msg[9] = 0; /* RFU */
- memcpy (msg+10, apdu, apdulen);
- set_msg_len (msg, apdulen);
- msglen = 10 + apdulen;
+ rc = bulk_out (handle, msg, msglen, 0);
+ if (rc)
+ return rc;
- rc = bulk_out (handle, msg, msglen, 0);
- if (rc)
- return rc;
+ apdu_p += apdu_part_len;
+ apdu_len -= apdu_part_len;
- msg = recv_buffer;
- rc = bulk_in (handle, msg, sizeof recv_buffer, &msglen,
- RDR_to_PC_DataBlock, seqno, 5000, 0);
- if (rc)
- return rc;
+ rc = bulk_in (handle, msg, sizeof msg, &msglen,
+ RDR_to_PC_DataBlock, seqno, 5000, 0);
+ if (rc)
+ return rc;
+
+ if (!(chain & 0x01))
+ break;
- if (msg[9] == 1)
+ chain = 0x02;
+ }
+
+ apdu_len = 0;
+ while (1)
{
- size_t total_msglen = msglen;
+ apdu_part_len = msglen - 10;
+ if (resp && apdu_len + apdu_part_len <= maxresplen)
+ memcpy (resp + apdu_len, msg+10, apdu_part_len);
+ apdu_len += apdu_part_len;
- while (1)
- {
- unsigned char status;
+ if (!(msg[9] & 0x01))
+ break;
- msg = recv_buffer + total_msglen;
+ msg[0] = PC_to_RDR_XfrBlock;
+ msg[5] = 0; /* slot */
+ msg[6] = seqno = handle->seqno++;
+ msg[7] = bwi;
+ msg[8] = 0x10; /* Request next data block */
+ msg[9] = 0;
+ set_msg_len (msg, 0);
+ msglen = 10;
- msg[0] = PC_to_RDR_XfrBlock;
- msg[5] = 0; /* slot */
- msg[6] = seqno = handle->seqno++;
- msg[7] = bwi; /* bBWI */
- msg[8] = 0x10; /* Request next data block */
- msg[9] = 0;
- set_msg_len (msg, 0);
- msglen = 10;
-
- rc = bulk_out (handle, msg, msglen, 0);
- if (rc)
- return rc;
-
- rc = bulk_in (handle, msg, sizeof recv_buffer - total_msglen, &msglen,
- RDR_to_PC_DataBlock, seqno, 5000, 0);
- if (rc)
- return rc;
- status = msg[9];
- memmove (msg, msg+10, msglen - 10);
- total_msglen += msglen - 10;
- if (total_msglen >= sizeof recv_buffer)
- return CCID_DRIVER_ERR_OUT_OF_CORE;
-
- if (status == 0x02)
- break;
- }
+ rc = bulk_out (handle, msg, msglen, 0);
+ if (rc)
+ return rc;
- apdu = recv_buffer + 10;
- apdulen = total_msglen - 10;
- }
- else
- {
- apdu = msg + 10;
- apdulen = msglen - 10;
+ rc = bulk_in (handle, msg, sizeof msg, &msglen,
+ RDR_to_PC_DataBlock, seqno, 5000, 0);
+ if (rc)
+ return rc;
}
if (resp)
{
- if (apdulen > maxresplen)
+ if (apdu_len > maxresplen)
{
DEBUGOUT_2 ("provided buffer too short for received data "
"(%u/%u)\n",
- (unsigned int)apdulen, (unsigned int)maxresplen);
+ (unsigned int)apdu_len, (unsigned int)maxresplen);
return CCID_DRIVER_ERR_INV_VALUE;
}
- memcpy (resp, apdu, apdulen);
- *nresp = apdulen;
+ *nresp = apdu_len;
}
return 0;
--
More information about the Gnupg-devel
mailing list