Skip to content

Commit

Permalink
Revised: Inform pkcs15 and card drivers of PKCS#11 C_Login(CKU_CONTEX…
Browse files Browse the repository at this point in the history
…T_SPECIFIC)"

Framework-pkcs15.c will now set pin_info->auth_method to SC_AC_CONTEXT_SPECIFIC

iso7816.c iso7816_build_pin_apdu treats this the same as SC_AC_CHV

card-piv.c piv_pin_cmd sets priv->xcontext_specific=1 and calls sc_lock before
the verify command. If the verify fails sc_unlock is called.
Later after the next card command returns, if priv->context_specific==1 piv_check_sw
will call sc_unlock as the application may not have requested the crypto but
some other command.

Some additional calls to sc_lock and sc_unlock have been added to make sure
PIV internal command sequences including the crypto command ('87') and any get
responses are always protected by a lock.

This guarantees the card is locked for verify and the next command
which should be the crypto operation. The PIV card also inforces this restriction
on the card.

This is based on suggestions in:
:https://github.com//pull/1256#issuecomment-361975751

 On branch piv-aid-discovery
 Changes to be committed:
	modified:   src/libopensc/card-piv.c
	modified:   src/libopensc/iso7816.c
	modified:   src/libopensc/types.h
	modified:   src/pkcs11/framework-pkcs15.c
  • Loading branch information
dengert committed Feb 1, 2018
1 parent 1a0e200 commit 906df5f
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 1 deletion.
32 changes: 32 additions & 0 deletions src/libopensc/card-piv.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ typedef struct piv_private_data {
int logged_in;
int pstate;
int pin_cmd_verify;
int context_specific;
int pin_cmd_noparse;
unsigned int pin_cmd_verify_sw1;
unsigned int pin_cmd_verify_sw2;
Expand Down Expand Up @@ -919,6 +920,8 @@ piv_get_data(sc_card_t * card, int enumtag, u8 **buf, size_t *buf_len)
SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE);
sc_log(card->ctx, "#%d", enumtag);

sc_lock(card); /* do check len and get data in same transaction */

/* assert(enumtag >= 0 && enumtag < PIV_OBJ_LAST_ENUM); */

tag_len = piv_objects[enumtag].tag_len;
Expand Down Expand Up @@ -970,6 +973,7 @@ piv_get_data(sc_card_t * card, int enumtag, u8 **buf, size_t *buf_len)
r = piv_general_io(card, 0xCB, 0x3F, 0xFF, tagbuf, p - tagbuf, buf, buf_len);

err:
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, r);
}

Expand Down Expand Up @@ -3218,7 +3222,16 @@ static int piv_check_sw(struct sc_card *card, unsigned int sw1, unsigned int sw2
if (priv->pin_cmd_verify) {
priv->pin_cmd_verify_sw1 = sw1;
priv->pin_cmd_verify_sw2 = sw2;
} else {
/* a command has completed and it is not verify */
/* If we are in a context_specific sequence, unlock */
if (priv->context_specific) {
sc_log(card->ctx,"Clearing CONTEXT_SPECIFIC lock");
priv->context_specific = 0;
sc_unlock(card);
}
}

if (priv->card_issues & CI_VERIFY_630X) {

/* Handle the Yubikey NEO or any other PIV card which returns in response to a verify
Expand Down Expand Up @@ -3360,10 +3373,29 @@ piv_pin_cmd(sc_card_t *card, struct sc_pin_cmd_data *data, int *tries_left)
}
}

/*
* If this was for a CKU_CONTEXT_SPECFIC login, lock the card one more time.
* to avoid any interference from other applications.
* Sc_unlock will be called at a later time after the next card command
* that should be a crypto operation. If its not then it is a error by the
* calling appication.
*/
if (data->cmd == SC_PIN_CMD_VERIFY && data->pin_type == SC_AC_CONTEXT_SPECIFIC) {
priv->context_specific = 1;
sc_log(card->ctx,"Starting CONTEXT_SPECIFIC verify");
sc_lock(card);
}

priv->pin_cmd_verify = 1; /* tell piv_check_sw its a verify to save sw1, sw2 */
r = iso_drv->ops->pin_cmd(card, data, tries_left);
priv->pin_cmd_verify = 0;

/* if verify failed, release the lock */
if (data->cmd == SC_PIN_CMD_VERIFY && r < 0 && priv->context_specific) {
sc_log(card->ctx,"Clearing CONTEXT_SPECIFIC");
priv->context_specific = 0;

This comment has been minimized.

Copy link
@Jakuje

Jakuje Feb 1, 2018

Member

Should not we have here sc_unlock(card); also?

}

/* if access to applet is know to be reset by other driver we select_aid and try again */
if ( priv->card_issues & CI_OTHER_AID_LOSE_STATE && priv->pin_cmd_verify_sw1 == 0x6DU) {
sc_log(card->ctx, "AID may be lost doing piv_find_aid and retry pin_cmd");
Expand Down
1 change: 1 addition & 0 deletions src/libopensc/iso7816.c
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,7 @@ iso7816_build_pin_apdu(struct sc_card *card, struct sc_apdu *apdu,
case SC_AC_CHV:
/* fall through */
case SC_AC_SESSION:
case SC_AC_CONTEXT_SPECIFIC:
break;
default:
return SC_ERROR_INVALID_ARGUMENTS;
Expand Down
1 change: 1 addition & 0 deletions src/libopensc/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ struct sc_crt {
#define SC_AC_SCB 0x00000040 /* IAS/ECC SCB byte. */
#define SC_AC_IDA 0x00000080 /* PKCS#15 authentication ID */
#define SC_AC_SESSION 0x00000100 /* Session PIN */
#define SC_AC_CONTEXT_SPECIFIC 0x00000200 /* Context specific login */

#define SC_AC_UNKNOWN 0xFFFFFFFE
#define SC_AC_NEVER 0xFFFFFFFF
Expand Down
11 changes: 10 additions & 1 deletion src/pkcs11/framework-pkcs15.c
Original file line number Diff line number Diff line change
Expand Up @@ -1617,7 +1617,16 @@ pkcs15_login(struct sc_pkcs11_slot *slot, CK_USER_TYPE userType,
}
}

rc = sc_pkcs15_verify_pin(p15card, auth_object, pPin, ulPinLen);
if (userType == CKU_CONTEXT_SPECIFIC && pin_info) {
int auth_meth_saved = pin_info->auth_method;

sc_log(context, "Setting SC_AC_CONTEXT_SPECIFIC");
pin_info->auth_method = SC_AC_CONTEXT_SPECIFIC;
rc = sc_pkcs15_verify_pin(p15card, auth_object, pPin, ulPinLen);
pin_info->auth_method = auth_meth_saved;
} else
rc = sc_pkcs15_verify_pin(p15card, auth_object, pPin, ulPinLen);

sc_log(context, "PKCS15 verify PIN returned %d", rc);

if (rc != SC_SUCCESS)
Expand Down

0 comments on commit 906df5f

Please sign in to comment.