[Spice-devel] [PATCH libcacard v2 20/35] vcard_emul: New function vcard_emul_read_object()

[Jakub Jelen]

Hello,

On Thu, 2018-08-02 at 15:39 +0200, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 2, 2018 at 11:43 AM, Jakub Jelen <jjelen at redhat.com>
> wrote:
> >  * This function is used to read generic data objects presented by
> >    the underlying card, if available. It can provide some
> > structures
> >    that we are not able to emulate in softeare card.
> > 
> > Signed-off-by: Jakub Jelen <jjelen at redhat.com>
> > Reviewed-by: Robert Relyea <rrelyea at redhat.com>
> > ---
> >  docs/libcacard.txt   |  8 +++++++
> >  src/libcacard.syms   |  1 +
> >  src/vcard_emul.h     |  4 ++++
> >  src/vcard_emul_nss.c | 53
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 66 insertions(+)
> > 
> > diff --git a/docs/libcacard.txt b/docs/libcacard.txt
> > index 324dcb8..f421054 100644
> > --- a/docs/libcacard.txt
> > +++ b/docs/libcacard.txt
> > @@ -354,6 +354,14 @@ and applet.
> > 
> >       This function returns the size of RSA key in bits.
> > 
> > +         unsigned char *vcard_emul_read_object(VCard *card,
> > +                                               const unsigned char
> > *label,
> > +                                               unsigned int
> > *ret_len);
> > +
> > +     This function reads generic data from underlying smart card
> > by the label,
> > +     if avaialble.
> > +
> > +
> >  The sample card type emulator is found in cac.c. It implements the
> > cac specific
> >  applets.  Only those applets needed by the coolkey pkcs#11 driver
> > on the guest
> >  have been implemented. To support the full range CAC middleware, a
> > complete CAC
> > diff --git a/src/libcacard.syms b/src/libcacard.syms
> > index 04c0f89..b073fb8 100644
> > --- a/src/libcacard.syms
> > +++ b/src/libcacard.syms
> > @@ -21,6 +21,7 @@ vcard_emul_rsa_bits
> >  vcard_emul_type_from_string
> >  vcard_emul_type_select
> >  vcard_emul_usage
> > +vcard_emul_read_object
> 
> same remark as vcard_emul_rsa_bits, let's not expot the function
> unless there is a good reason

I am not sure what was the initial intention, but I assume these
functions were meant to be also the API to the card emulators. I am not
sure how good idea it is, but I am also fine with not adding them and
marking the rest of internal deprecated.

> >  vcard_find_applet
> >  vcard_free
> >  vcard_get_atr
> > diff --git a/src/vcard_emul.h b/src/vcard_emul.h
> > index ec64605..cb7fcbb 100644
> > --- a/src/vcard_emul.h
> > +++ b/src/vcard_emul.h
> > @@ -64,4 +64,8 @@ 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);
> > +
> > +unsigned char *vcard_emul_read_object(VCard *card, const char
> > *label,
> > +                                      unsigned int *ret_len);
> > +
> >  #endif
> > diff --git a/src/vcard_emul_nss.c b/src/vcard_emul_nss.c
> > index e213d7f..e02426b 100644
> > --- a/src/vcard_emul_nss.c
> > +++ b/src/vcard_emul_nss.c
> > @@ -1327,6 +1327,59 @@ vcard_emul_options(const char *args)
> >      return opts;
> >  }
> > 
> > +unsigned char *
> > +vcard_emul_read_object(VCard *card, const char *label,
> > +    unsigned int *ret_len)
> > +{
> > +    PK11SlotInfo *slot;
> > +    PK11GenericObject *obj, *firstObj, *myObj = NULL;
> > +    SECItem result;
> > +    SECStatus r;
> > +
> > +    slot = vcard_emul_card_get_slot(card);
> > +
> > +    firstObj = PK11_FindGenericObjects(slot, CKO_DATA);
> > +    fprintf(stderr, "%s: Search for generic objects: got %p",
> > __func__, firstObj);
> 
> this looks like it should be g_debug() or removed
> 
> Can touch on commit

True. This can be removed or changed to g_debug.

> > +    for (obj = firstObj; obj; obj =
> > PK11_GetNextGenericObject(obj)) {
> > +        int found = 0;
> > +        r = PK11_ReadRawAttribute(PK11_TypeGeneric, obj,
> > +            CKA_LABEL, &result);
> > +        if (r != SECSuccess) {
> > +            PK11_DestroyGenericObjects(firstObj);
> > +            return NULL;
> > +        }
> > +
> > +        if (strlen(label) == result.len
> > +            && memcmp(label, result.data, result.len) == 0)
> > +            found = 1;
> > +
> > +        free(result.data);
> > +        result.data = NULL;
> > +
> > +        if (found) {
> > +            if (obj == firstObj)
> > +                firstObj = obj;
> 
> What was the intention here? can I remove those 2 lines?

This was taken from some example code from NSS, but seems I did it
wrong and the last line should say

firstObj = PK11_GetNextGenericObject(obj);

The intention is to clean up the linked list even for this corner case,
where we would lose the pointer to the start of the list if I
understand this API right:


https://dxr.mozilla.org/mozilla-central/source/security/nss/lib/pk11wrap/pk11obj.c#1456

> > +            PK11_UnlinkGenericObject(obj);
> > +            myObj = obj;
> > +            break;
> > +        }
> > +    }
> > +    PK11_DestroyGenericObjects(firstObj);
> > +
> > +    if (!myObj)
> > +        return NULL;
> > +
> > +    r = PK11_ReadRawAttribute(PK11_TypeGeneric, myObj,
> > +        CKA_VALUE, &result);
> > +    PK11_DestroyGenericObject(myObj);
> > +    if (r != SECSuccess)
> > +        return NULL;
> > +
> > +    *ret_len = result.len;
> > +    return result.data;
> > +
> > +}
> > +
> >  void
> >  vcard_emul_usage(void)
> >  {
> > --
> > 2.17.1
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
> 
-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.