[Qemu-devel] [PATCH 0/2] Try to fix problem with emulated smartcards where invalid PIN succeeds

[Qemu-devel] [PATCH 0/2] Try to fix problem with emulated smartcards where invalid PIN succeeds

[Ray Strode]

I started writing a blog post yesterday about virtualized smartcards here:

https://blogs.gnome.org/halfline/2013/09/08/another-smartcard-post/

and while testing what I was writing I noticed an invalid PIN worked when
it shouldn't have. It turns out that typing a valid PIN once in one program in
the guest, is enough to make all future programs asking for the PIN to succeed
regardless of what gets typed in for the PIN.

I did some digging through the libcacard code, and noticed it uses the
NSS PK11_Authenticate function which calls a function that has this comment above it:

    If we're already logged in and this function is called we
    will still prompt for a password, but we will probably succeed
    no matter what the password was.

Also, PK11_Authenticate short-circuits to an early "return SECSuccess" if the token
is already logged in.

The two patches in this series attempt to correct this problem by calling PK11_Logout.
I'm not 100% certain I've placed the PK11_Logout call in the best place, but it does
seeming to fix the issue.

[Qemu-devel] [PATCH 1/2] libcacard: introduce new vcard_emul_logout

[Ray Strode]

From: Ray Strode <rstrode@redhat.com>

vcard_emul_reset currently only logs NSS out, but there is a TODO
for potentially sending insertion/removal events when powering down
or powering up.

For clarity, this commit moves the current guts of vcard_emul_reset to
a new vcard_emul_logout function which will never send insertion/removal
events. The vcard_emul_reset function now just calls vcard_emul_logout,
but also retains its TODO for watching power state transitions and sending
insertion/removal events.

Signed-off-by: Ray Strode <rstrode@redhat.com>
---
 libcacard/vcard_emul.h     |  1 +
 libcacard/vcard_emul_nss.c | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/libcacard/vcard_emul.h b/libcacard/vcard_emul.h
index 963563f..f09ee98 100644
--- a/libcacard/vcard_emul.h
+++ b/libcacard/vcard_emul.h
@@ -13,53 +13,54 @@
 #ifndef VCARD_EMUL_H
 #define VCARD_EMUL_H 1
 
 #include "card_7816t.h"
 #include "vcard.h"
 #include "vcard_emul_type.h"
 
 /*
  * types
  */
 typedef enum {
     VCARD_EMUL_OK = 0,
     VCARD_EMUL_FAIL,
     /* return values by vcard_emul_init */
     VCARD_EMUL_INIT_ALREADY_INITED,
 } VCardEmulError;
 
 /* options are emul specific. call card_emul_parse_args to change a string
  * To an options struct */
 typedef struct VCardEmulOptionsStruct VCardEmulOptions;
 
 /*
  * Login functions
  */
 /* return the number of login attempts still possible on the card. if unknown,
  * return -1 */
 int vcard_emul_get_login_count(VCard *card);
 /* login into the card, return the 7816 status word (sw2 || sw1) */
 vcard_7816_status_t vcard_emul_login(VCard *card, unsigned char *pin,
                                      int pin_len);
+void vcard_emul_logout(VCard *card);
 
 /*
  * key functions
  */
 /* delete a key */
 void vcard_emul_delete_key(VCardKey *key);
 /* RSA sign/decrypt with the key, signature happens 'in place' */
 vcard_7816_status_t vcard_emul_rsa_op(VCard *card, VCardKey *key,
                                   unsigned char *buffer, int buffer_size);
 
 void vcard_emul_reset(VCard *card, VCardPower power);
 void vcard_emul_get_atr(VCard *card, unsigned char *atr, int *atr_len);
 
 /* Re-insert of a card that has been removed by force removal */
 VCardEmulError vcard_emul_force_card_insert(VReader *vreader);
 /* Force a card removal even if the card is not physically removed */
 VCardEmulError vcard_emul_force_card_remove(VReader *vreader);
 
 VCardEmulOptions *vcard_emul_options(const char *args);
 VCardEmulError vcard_emul_init(const VCardEmulOptions *options);
 void vcard_emul_replay_insertion_events(void);
 void vcard_emul_usage(void);
 #endif
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index fb429b1..c3a26d7 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -374,78 +374,86 @@ vcard_emul_login(VCard *card, unsigned char *pin, int pin_len)
     if (!nss_emul_init) {
         return VCARD7816_STATUS_ERROR_CONDITION_NOT_SATISFIED;
     }
     slot = vcard_emul_card_get_slot(card);
      /* We depend on the PKCS #11 module internal login state here because we
       * create a separate process to handle each guest instance. If we needed
       * to handle multiple guests from one process, then we would need to keep
       * a lot of extra state in our card structure
       * */
     pin_string = g_malloc(pin_len+1);
     memcpy(pin_string, pin, pin_len);
     pin_string[pin_len] = 0;
 
     /* handle CAC expanded pins correctly */
     for (i = pin_len-1; i >= 0 && (pin_string[i] == 0xff); i--) {
         pin_string[i] = 0;
     }
 
     rv = PK11_Authenticate(slot, PR_FALSE, pin_string);
     memset(pin_string, 0, pin_len);  /* don't let the pin hang around in memory
                                         to be snooped */
     g_free(pin_string);
     if (rv == SECSuccess) {
         return VCARD7816_STATUS_SUCCESS;
     }
     /* map the error from port get error */
     return VCARD7816_STATUS_ERROR_CONDITION_NOT_SATISFIED;
 }
 
 void
-vcard_emul_reset(VCard *card, VCardPower power)
+vcard_emul_logout(VCard *card)
 {
     PK11SlotInfo *slot;
 
     if (!nss_emul_init) {
         return;
     }
 
+    slot = vcard_emul_card_get_slot(card);
+    if (PK11_IsLoggedIn(slot,NULL)) {
+        PK11_Logout(slot); /* NOTE: ignoring SECStatus return value */
+    }
+}
+
+void
+vcard_emul_reset(VCard *card, VCardPower power)
+{
     /*
      * if we reset the card (either power on or power off), we lose our login
      * state
      */
+    vcard_emul_logout(card);
+
     /* TODO: we may also need to send insertion/removal events? */
-    slot = vcard_emul_card_get_slot(card);
-    PK11_Logout(slot); /* NOTE: ignoring SECStatus return value */
 }
 
-
 static VReader *
 vcard_emul_find_vreader_from_slot(PK11SlotInfo *slot)
 {
     VReaderList *reader_list = vreader_get_reader_list();
     VReaderListEntry *current_entry = NULL;
 
     if (reader_list == NULL) {
         return NULL;
     }
     for (current_entry = vreader_list_get_first(reader_list); current_entry;
                         current_entry = vreader_list_get_next(current_entry)) {
         VReader *reader = vreader_list_get_reader(current_entry);
         VReaderEmul *reader_emul = vreader_get_private(reader);
         if (reader_emul->slot == slot) {
             return reader;
         }
         vreader_free(reader);
     }
 
     return NULL;
 }
 
 /*
  * create a new reader emul
  */
 static VReaderEmul *
 vreader_emul_new(PK11SlotInfo *slot, VCardEmulType type, const char *params)
 {
     VReaderEmul *new_reader_emul;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] libcacard: Lock NSS cert db when selecting an applet on an emulated card

[Ray Strode]

From: Ray Strode <rstrode@redhat.com>

When a process in a guest uses an emulated smartcard, libcacard passes
the PIN from the guest to the PK11_Authenticate NSS function. The first
time PK11_Authenticate is called the passed in PIN is used to unlock the
certificate database. Subsequent calls to PK11_Authenticate within the
next 60 seconds will transparently succeed, regardless of the passed in
PIN. This is a convenience for applications provided by NSS.

Of course, the guest may have many applications using the one emulated
smart card all driven from the same host qemu process.  That means if a
user enters the right PIN in one program in the guest, and then enters the
wrong PIN in another program in the guest, the wrong PIN will still
succeed (as long as it's within 60 seconds of the right PIN being entered).

This commit forces the NSS certificate database to be locked anytime an
applet is selected on an emulated smartcard by calling vcard_emul_logout.

Signed-off-by: Ray Strode <rstrode@redhat.com>
---
 libcacard/vcard.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libcacard/vcard.c b/libcacard/vcard.c
index 539177b..cf02a25 100644
--- a/libcacard/vcard.c
+++ b/libcacard/vcard.c
@@ -243,60 +243,65 @@ vcard_find_applet(VCard *card, unsigned char *aid, int aid_len)
 {
     VCardApplet *current_applet;
 
     for (current_applet = card->applet_list; current_applet;
                                         current_applet = current_applet->next) {
         if (current_applet->aid_len != aid_len) {
             continue;
         }
         if (memcmp(current_applet->aid, aid, aid_len) == 0) {
             break;
         }
     }
     return current_applet;
 }
 
 unsigned char *
 vcard_applet_get_aid(VCardApplet *applet, int *aid_len)
 {
     if (applet == NULL) {
         return NULL;
     }
     *aid_len = applet->aid_len;
     return applet->aid;
 }
 
 
 void
 vcard_select_applet(VCard *card, int channel, VCardApplet *applet)
 {
     assert(channel < MAX_CHANNEL);
+
+    /* If using an emulated card, make sure to log out of any already logged in
+     * session. */
+    vcard_emul_logout(card);
+
     card->current_applet[channel] = applet;
     /* reset the applet */
     if (applet && applet->reset_applet) {
         applet->reset_applet(card, channel);
     }
 }
 
 VCardAppletPrivate *
 vcard_get_current_applet_private(VCard *card, int channel)
 {
     VCardApplet *applet = card->current_applet[channel];
 
     if (applet == NULL) {
         return NULL;
     }
     return applet->applet_private;
 }
 
 VCardStatus
 vcard_process_applet_apdu(VCard *card, VCardAPDU *apdu,
                           VCardResponse **response)
 {
     if (card->current_applet[apdu->a_channel]) {
         return card->current_applet[apdu->a_channel]->process_apdu(
                                                         card, apdu, response);
     }
     return VCARD_NEXT;
 }
 
 /*
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] Try to fix problem with emulated smartcards where invalid PIN succeeds

[Alon Levy]

> I started writing a blog post yesterday about virtualized smartcards here:
> 
> https://blogs.gnome.org/halfline/2013/09/08/another-smartcard-post/
> 
> and while testing what I was writing I noticed an invalid PIN worked when
> it shouldn't have. It turns out that typing a valid PIN once in one program
> in
> the guest, is enough to make all future programs asking for the PIN to
> succeed
> regardless of what gets typed in for the PIN.
> 
> I did some digging through the libcacard code, and noticed it uses the
> NSS PK11_Authenticate function which calls a function that has this comment
> above it:
> 
>     If we're already logged in and this function is called we
>     will still prompt for a password, but we will probably succeed
>     no matter what the password was.
> 
> Also, PK11_Authenticate short-circuits to an early "return SECSuccess" if the
> token
> is already logged in.
> 
> The two patches in this series attempt to correct this problem by calling
> PK11_Logout.
> I'm not 100% certain I've placed the PK11_Logout call in the best place, but
> it does
> seeming to fix the issue.

Hi Ray,

 Thanks for the patches! It looks good to me but I'll defer to Robert,

Alon

> 
> 
> 

Re: [Qemu-devel] [PATCH 0/2] Try to fix problem with emulated smartcards where invalid PIN succeeds

[Robert Relyea]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On 09/08/2013 01:18 AM, Alon Levy wrote:
>> I started writing a blog post yesterday about virtualized smartcards here:
>>
>> https://blogs.gnome.org/halfline/2013/09/08/another-smartcard-post/
>>
>> and while testing what I was writing I noticed an invalid PIN worked when
>> it shouldn't have. It turns out that typing a valid PIN once in one program
>> in
>> the guest, is enough to make all future programs asking for the PIN to
>> succeed
>> regardless of what gets typed in for the PIN.
>>
>> I did some digging through the libcacard code, and noticed it uses the
>> NSS PK11_Authenticate function which calls a function that has this comment
>> above it:
>>
>>     If we're already logged in and this function is called we
>>     will still prompt for a password, but we will probably succeed
>>     no matter what the password was.
>>
>> Also, PK11_Authenticate short-circuits to an early "return SECSuccess" if the
>> token
>> is already logged in.
>>
>> The two patches in this series attempt to correct this problem by calling
>> PK11_Logout.
>> I'm not 100% certain I've placed the PK11_Logout call in the best place, but
>> it does
>> seeming to fix the issue.
> Hi Ray,
>
>  Thanks for the patches! It looks good to me but I'll defer to Robert,
>
> Alon

ack... The original problem is a little worse than ray says. It's not a
60 second window, it's pretty much anytime until the card is explicitly
logged out. Ray's patch will fix this.

bob
>
>>
>>



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4521 bytes --]

Re: [Qemu-devel] [PATCH 0/2] Try to fix problem with emulated smartcards where invalid PIN succeeds

[Ray Strode]

Hi,

On Mon, Sep 9, 2013 at 2:19 PM, Robert Relyea <rrelyea@redhat.com> wrote:
> ack... The original problem is a little worse than ray says. It's not a
> 60 second window, it's pretty much anytime until the card is explicitly
> logged out. Ray's patch will fix this.
Okay, I'll resend the patch series with an improved commit message for
the second patch and Reviewed-By's for you and alon.

--Ray