Re: [Qemu-devel] [PATCH v23 06/11] libcacard: initial commit

[Alon Levy <alevy@redhat.com>]

On Mon, Mar 28, 2011 at 02:35:23PM +0200, Jes Sorensen wrote:
> On 03/23/11 14:19, Alon Levy wrote:
> > From: Robert Relyea <rrelyea@redhat.com>
> > 
> > libcacard emulates a Common Access Card (CAC) which is a standard
> > for smartcards. It is used by the emulated ccid card introduced in
> > a following patch. Docs are available in docs/libcacard.txt
> > 
> > Signed-off-by: Alon Levy <alevy@redhat.com>
> 
> A couple of minor nits.
> 
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 744e1d3..f513ffa 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -352,6 +352,11 @@ user-obj-y += qemu-timer-common.o
> >  endif
> >  endif
> >  
> > +######################################################################
> > +# smartcard
> > +
> > +libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o card_7816.o
> > +
> >  vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> >  
> >  vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> > diff --git a/Makefile.target b/Makefile.target
> > index 62b102a..7f163e3 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -353,6 +353,8 @@ obj-y += $(addprefix $(HWDIR)/, $(hw-obj-y))
> >  
> >  endif # CONFIG_SOFTMMU
> >  
> > +obj-y += $(addprefix ../libcacard/, $(libcacard-$(CONFIG_SMARTCARD_NSS)))
> > +
> >  obj-y += $(addprefix ../, $(trace-obj-y))
> >  obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o
> 
> This is a bit backwards, normally we do
> foobar-$(CONFIG_FOOBAR) = foo.o bar.o
> 
> and then later obj-y = $(foobar-y)
> 
> > diff --git a/libcacard/cac.c b/libcacard/cac.c
> > new file mode 100644
> > index 0000000..7a910d8
> > --- /dev/null
> > +++ b/libcacard/cac.c
> > @@ -0,0 +1,406 @@
> > +/*
> > + * implement the applets for the CAC card.
> > + *
> > + * This code is licensed under the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + *
> > + */
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +#include "qemu-common.h"
> 
> stdlib.h and string.h are both included by qemu-common.h
> 
> > diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
> > new file mode 100644
> > index 0000000..4c10cae
> > --- /dev/null
> > +++ b/libcacard/card_7816.c
> > @@ -0,0 +1,764 @@
> > +/*
> > + * Implement the 7816 portion of the card spec
> > + *
> > + * This code is licensed under the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + */
> > +
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include "qemu-common.h"
> 
> same here
> 
> > diff --git a/libcacard/event.c b/libcacard/event.c
> > new file mode 100644
> > index 0000000..12722cc
> > --- /dev/null
> > +++ b/libcacard/event.c
> > @@ -0,0 +1,108 @@
> > +/*
> > + * event queue implementation.
> > + *
> > + * This code is licensed under the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + */
> > +
> > +#include <stdlib.h>
> > +
> > +#include "qemu-thread.h"
> > +#include "qemu-common.h"
> 
> again here
> 
> 
> > diff --git a/libcacard/vcard.c b/libcacard/vcard.c
> > new file mode 100644
> > index 0000000..d7828a2
> > --- /dev/null
> > +++ b/libcacard/vcard.c
> > @@ -0,0 +1,341 @@
> > +/*
> > + * implement the Java card standard.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + */
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +#include "qemu-common.h"
> 
> and here
> 
> > diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
> > new file mode 100644
> > index 0000000..d3ab7ea
> > --- /dev/null
> > +++ b/libcacard/vcard_emul_nss.c
> > @@ -0,0 +1,1159 @@
> > +/*
> > + * This is the actual card emulator.
> > + *
> > + * These functions can be implemented in different ways on different platforms
> > + * using the underlying system primitives. For Linux it uses NSS, though direct
> > + * to PKCS #11, openssl+pkcs11, or even gnu crypto libraries+pkcs #11 could be
> > + * used. On Windows CAPI could be used.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + */
> > +
> > +/*
> > + * system headers
> > + */
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +/*
> > + * NSS headers
> > + */
> > +#include <nss.h>
> > +#include <pk11pub.h>
> > +#include <cert.h>
> > +#include <key.h>
> > +#include <secmod.h>
> > +#include <prthread.h>
> > +#include <secerr.h>
> > +
> > +#include "qemu-common.h"
> 
> again here
> 
> prthread.h do you have a check for it in configure?  I have to admit I
> really would prefer QEMU not relying on the NSPR stuff, but I don't know
> if it can be avoided with the ccid code?
> 

No, unless you mean I should rewrite the emulation to not use NSS, I don't know
how. Or are you saying NSS can be used without using NSPR? I admited and will
repeat that I have not authored this code (that's why I have Robert Relyea as
the author of this patch), so I'm not familiar with NSS/NSPR except superficially.

> > diff --git a/libcacard/vreader.c b/libcacard/vreader.c
> > new file mode 100644
> > index 0000000..0b67c6c
> > --- /dev/null
> > +++ b/libcacard/vreader.c
> > @@ -0,0 +1,519 @@
> > +/*
> > + * emulate the reader
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + */
> > +
> > +/*
> > + * System includes
> > + */
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +#include "qemu-thread.h"
> > +#include "qemu-common.h"
> 
> and a last one....

Are these a problem enough that you will without an ack? with respect to your
previous acks do you want me to only send this patch again, and Anthony should
merge the acked patches?

> 
> Cheers,
> Jes