Support for new MinInt agent card #1092
Conversation
src/libopensc/card-iasecc.c
Outdated
| LOG_TEST_RET(ctx, rv, "Could not select MI's AID"); | ||
| resp_len = sizeof(resp); | ||
| rv = iasecc_select_aid(card, &MIIASECC_AID, resp, &resp_len); | ||
| LOG_TEST_GOTO_ERR(ctx, rv, "Could not select MI's AID"); |
There was a problem hiding this comment.
Please only use the err: label (and goto in general) for cleanup.
| @@ -264,6 +264,9 @@ iasecc_select_mf(struct sc_card *card, struct sc_file **file_out) | |||
| apdu.resplen = sizeof(apdu_resp); | |||
| apdu.resp = apdu_resp; | |||
|
|
|||
| if (card->type == SC_CARD_TYPE_IASECC_MI2) | |||
| apdu.p2 = 0x04; | |||
There was a problem hiding this comment.
why is this change required if the applet only changes the AID?
src/libopensc/card-iasecc.c
Outdated
| @@ -323,6 +326,9 @@ iasecc_select_aid(struct sc_card *card, struct sc_aid *aid, unsigned char *out, | |||
| apdu.resplen = sizeof(apdu_resp); | |||
| apdu.resp = apdu_resp; | |||
|
|
|||
| if (card->type == SC_CARD_TYPE_IASECC_MI2) | |||
| apdu.p2 = 0x04; | |||
There was a problem hiding this comment.
why is this change required if the applet only changes the AID?
src/libopensc/card-iasecc.c
Outdated
|
|
||
| rv = iasecc_mi_match(card); | ||
| LOG_TEST_RET(ctx, rv, "Could not match MI's AID"); | ||
|
|
||
| if(rv) { |
src/libopensc/card-iasecc.c
Outdated
|
|
||
| if(rv) { | ||
| card->type = SC_CARD_TYPE_IASECC_MI2; | ||
| rv = iasecc_init_sagem(card); |
There was a problem hiding this comment.
Could you please unify the usage of iasecc_init_amos, iasecc_init_sagem and your card? Currently, they all do the same, which should be generalized under a different name.
I think the driver is not very consistently written; iasecc_init_oberthur and iasecc_init_gemalto also shares most of the code...
This card uses the same ATR as the existing card, but the applet installed does not have the same AID. This card actually works exactly as the IASECC_SAGEM.
src/libopensc/card-iasecc.c
Outdated
| resp_len = sizeof(resp); | ||
| rv = iasecc_select_aid(card, &MIIASECC_AID, resp, &resp_len); | ||
|
|
||
| rv = iasecc_mi_match(card); |
There was a problem hiding this comment.
By convention, the match function should be used to determine the card's type. I suggest you add iasecc_select_aid(card, &MIIASECC_AID, resp, &resp_len); to iasecc_mi_match() (note that you should check rv when selecting the applet!). (Yes, I know that this has been a mistake of the past, but giving some love to the code today, will benefit all contributors in the future.)
| if (rv) | ||
| card->type = SC_CARD_TYPE_IASECC_MI2; | ||
| else | ||
| LOG_FUNC_RETURN(ctx, SC_SUCCESS); |
There was a problem hiding this comment.
If iasecc_mi_match() doesn't succeed, you should not return with SC_SUCCESS. I think would break all other cards that are initialized with iasecc_init_amos(), right?
| @@ -562,65 +557,26 @@ iasecc_init_amos(struct sc_card *card) | |||
| card->caps |= SC_CARD_CAP_APDU_EXT; | |||
| card->caps |= SC_CARD_CAP_USE_FCI_AC; | |||
|
|
|||
| if (card->type == SC_CARD_TYPE_IASECC_MI) { | |||
There was a problem hiding this comment.
Since the type of card is tested here, there is no issue with other cards.
There was a problem hiding this comment.
thanks for the clearification
| @@ -562,65 +557,26 @@ iasecc_init_amos(struct sc_card *card) | |||
| card->caps |= SC_CARD_CAP_APDU_EXT; | |||
| card->caps |= SC_CARD_CAP_USE_FCI_AC; | |||
|
|
|||
| if (card->type == SC_CARD_TYPE_IASECC_MI) { | |||
There was a problem hiding this comment.
thanks for the clearification
|
Can you check whether the "old" iasecc cards are still functioning as expected? |
|
For the SAGEM_MI2 card, the applet actually differs a bit from the one installed on SAGEM_MI cards as it really requires P2 to be set to 0x04 when selecting the MF; that is why I reintroduced this change. I tested both SAGEM_MI and SAGEM_MI2 cards, as well as the "simple" SAGEM IAS/ECC. All are working fine via pkcs11-tool (provisioning, encryption, signature). I have no AMOS card so I cannot test if it is impacted. |
|
OK, fine. |
* Support for new MinInt agent card This card uses the same ATR as the existing card, but the applet installed does not have the same AID. This card actually works exactly as the IASECC_SAGEM. Unify iasecc_init for AMOS/SAGEM and MI cards
This card uses the same ATR as the existing card, but the applet installed
does not have the same AID. This card actually works exactly as the
IASECC_SAGEM.