Patchwork [12/20] libcacard: fixes from review (v15->v16)

login
register
mail settings
Submitter Alon Levy
Date Feb. 2, 2011, 8:28 p.m.
Message ID <1296678500-19497-13-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/81533/
State New
Headers show

Comments

Alon Levy - Feb. 2, 2011, 8:28 p.m.
Build:
 * don't erase self with distclean
 * fix make clean after make distclean
 * Makefile: make vscclient link quiet

Behavioral:
 * vcard_emul_nss: load coolkey in more situations
 * vscclient:
  * use hton,ntoh
  * send init on connect, only start vevent thread on response
  * read payload after header check, before type switch
  * remove Reconnect
  * update for vscard_common changes, empty Flush implementation

Style/Whitespace:
 * fix wrong variable usage
 * remove unused variable
 * use only C style comments
  * add copyright header
  * fix tabulation

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 Makefile                   |    4 +-
 libcacard/Makefile         |    8 +-
 libcacard/mutex.h          |    2 +-
 libcacard/vcard_emul.h     |    2 +-
 libcacard/vcard_emul_nss.c |   39 ++++++--
 libcacard/vscclient.c      |  219 +++++++++++++++++++++++++-------------------
 6 files changed, 164 insertions(+), 110 deletions(-)

Patch

diff --git a/Makefile b/Makefile
index e1c4aa5..9286eb1 100644
--- a/Makefile
+++ b/Makefile
@@ -173,7 +173,7 @@  check-qlist: check-qlist.o qlist.o qint.o $(CHECK_PROG_DEPS)
 check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS)
 check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o $(CHECK_PROG_DEPS)
 
-QEMULIBS=libhw32 libhw64 libuser libdis libdis-user libcacard
+QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
 
 clean:
 # avoid old build problems by removing potentially incorrect old files
@@ -186,7 +186,7 @@  clean:
 	rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
 	rm -f trace-dtrace.h trace-dtrace.h-timestamp
 	$(MAKE) -C tests clean
-	for d in $(ALL_SUBDIRS) $(QEMULIBS); do \
+	for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \
 	if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
 	rm -f $$d/qemu-options.def; \
         done
diff --git a/libcacard/Makefile b/libcacard/Makefile
index b146779..554c6ea 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -1,11 +1,11 @@ 
-include ../config-host.mak
-include $(SRC_PATH)/Makefile.objs
-include $(SRC_PATH)/rules.mak
+-include ../config-host.mak
+-include $(SRC_PATH)/Makefile.objs
+-include $(SRC_PATH)/rules.mak
 
 $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/libcacard)
 
 vscclient: $(libcacard-y) vscclient.o
-	gcc $(libcacard_libs) -o $@ $^
+	$(call quiet-command,$(CC) $(libcacard_libs) -o $@ $^,"  LINK  $(TARGET_DIR)$@")
 
 all: vscclient
 
diff --git a/libcacard/mutex.h b/libcacard/mutex.h
index db44814..ffbdb31 100644
--- a/libcacard/mutex.h
+++ b/libcacard/mutex.h
@@ -56,4 +56,4 @@  typedef int thread_status_t;
 #define THREAD_SUCCESS(status)  ((status) == 0)
 #endif
 
-#endif // _H_MUTEX
+#endif /* _H_MUTEX */
diff --git a/libcacard/vcard_emul.h b/libcacard/vcard_emul.h
index 5df01a8..05fb57e 100644
--- a/libcacard/vcard_emul.h
+++ b/libcacard/vcard_emul.h
@@ -20,7 +20,7 @@ 
 typedef enum {
     VCARD_EMUL_OK =0,
     VCARD_EMUL_FAIL,
-    // return values by vcard_emul_init
+    /* return values by vcard_emul_init */
     VCARD_EMUL_INIT_ALREADY_INITED,
 } VCardEmulError;
 
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 33404c2..6887cf6 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -29,7 +29,6 @@ 
 #include <stdlib.h>
 #include <string.h>
 
-
 struct VCardKeyStruct {
     CERTCertificate *cert;
     PK11SlotInfo *slot;
@@ -718,6 +717,30 @@  vcard_emul_force_card_insert(VReader *vreader)
     return VCARD_EMUL_OK;
 }
 
+
+static PRBool
+module_has_removable_hw_slots(SECMODModule *mod)
+{
+    int i;
+    PRBool ret = PR_FALSE;
+    SECMODListLock *moduleLock = SECMOD_GetDefaultModuleListLock();
+
+    if (!moduleLock) {
+        PORT_SetError(SEC_ERROR_NOT_INITIALIZED);
+        return ret;
+    }
+    SECMOD_GetReadLock(moduleLock);
+    for (i=0; i < mod->slotCount; i++) {
+        PK11SlotInfo *slot = mod->slots[i];
+        if (PK11_IsRemovable(slot) && PK11_IsHW(slot)) {
+            ret = PR_TRUE;
+            break;
+        }
+    }
+    SECMOD_ReleaseReadLock(moduleLock);
+    return ret;
+}
+
 /* Previously we returned FAIL if no readers found. This makes
  * no sense when using hardware, since there may be no readers connected
  * at the time vcard_emul_init is called, but they will be properly
@@ -730,7 +753,7 @@  VCardEmulError
 vcard_emul_init(const VCardEmulOptions *options)
 {
     SECStatus rv;
-    PRBool ret, has_readers=PR_FALSE, need_module;
+    PRBool ret, has_readers=PR_FALSE, need_coolkey_module;
     VReader *vreader;
     VReaderEmul *vreader_emul;
     SECMODListLock *module_lock;
@@ -826,18 +849,18 @@  vcard_emul_init(const VCardEmulOptions *options)
     /* make sure we have some PKCS #11 module loaded */
     module_lock = SECMOD_GetDefaultModuleListLock();
     module_list = SECMOD_GetDefaultModuleList();
-    need_module = !has_readers;
+    need_coolkey_module = !has_readers;
     SECMOD_GetReadLock(module_lock);
     for (mlp = module_list; mlp; mlp = mlp->next) {
         SECMODModule * module = mlp->module;
-        if (SECMOD_HasRemovableSlots(module)) {
-            need_module = PR_FALSE;
+        if (module_has_removable_hw_slots(module)) {
+            need_coolkey_module = PR_FALSE;
             break;
         }
     }
     SECMOD_ReleaseReadLock(module_lock);
 
-    if (need_module) {
+    if (need_coolkey_module) {
         SECMODModule *module;
         module = SECMOD_LoadUserModule(
                     (char*)"library=libcoolkeypk11.so name=Coolkey",
@@ -882,9 +905,7 @@  vcard_emul_init(const VCardEmulOptions *options)
             has_emul_slots = PR_TRUE;
 
             if (PK11_IsPresent(slot)) {
-                VCardEmulType type;
                 VCard *vcard;
-                type = vcard_emul_get_type(vreader);
                 vcard = vcard_emul_mirror_card(vreader);
                 vreader_insert_card(vreader, vcard);
                 vcard_emul_init_series(vreader, vcard);
@@ -1087,7 +1108,7 @@  vcard_emul_options(const char *args)
             vreaderOpt->name = copy_string(name, name_length);
             vreaderOpt->vname = copy_string(vname, vname_length);
             vreaderOpt->card_type = type;
-            vreaderOpt->type_params = copy_string(name, name_length);
+            vreaderOpt->type_params = copy_string(type_params, type_params_length);
             count = count_tokens(args,',',')');
             vreaderOpt->cert_count = count;
             vreaderOpt->cert_name = (char **)malloc(count*sizeof(char *));
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 4dd8a35..f29fd6b 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -1,3 +1,14 @@ 
+/*
+ * Tester for VSCARD protocol, client side.
+ *
+ * Can be used with ccid-card-passthru.
+ *
+ * Copyright (c) 2011 Red Hat.
+ * Written by Alon Levy.
+ *
+ * This code is licenced under the GNU LGPL, version 2 or later.
+ */
+
 #include <sys/types.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -8,6 +19,7 @@ 
 #include <sys/socket.h>
 #include <netdb.h>
 #include <netinet/in.h>
+#include <arpa/inet.h>
 
 #include "vscard_common.h"
 
@@ -47,17 +59,6 @@  print_usage (void) {
     vcard_emul_usage();
 }
 
-static char*
-ip_numeric_to_char(
-    uint32_t ip
-) {
-    char buf[4*4];
-
-    sprintf(buf, "%d.%d.%d.%d", (ip & 0xff000000) >> 24, (ip & 0xff0000) >> 16,
-        (ip & 0xff00) >> 8, ip & 0xff);
-    return strdup(buf);
-}
-
 static mutex_t write_lock;
 
 static int
@@ -77,9 +78,9 @@  send_msg (
                type, reader_id, length, length);
     }
 
-    mhHeader.type = type;
+    mhHeader.type = htonl(type);
     mhHeader.reader_id = 0;
-    mhHeader.length = length;
+    mhHeader.length = htonl(length);
     rv = write (
         sock,
         &mhHeader,
@@ -295,17 +296,27 @@  do_command(void)
                 reader_id = get_id_from_string(&string[7], reader_id);
             }
             reader = vreader_get_reader_by_id(reader_id);
-            error = vcard_emul_force_card_insert(reader);
-            printf("insert %s, returned %d\n", reader ? vreader_get_name(reader)
-                                               : "invalid reader", error);
+            if (reader != NULL) {
+                error = vcard_emul_force_card_insert(reader);
+                printf("insert %s, returned %d\n",
+                       reader ? vreader_get_name(reader)
+                       : "invalid reader", error);
+            } else {
+                printf("no reader by id %d found\n", reader_id);
+            }
         } else if (strncmp(string,"remove",6) == 0) {
             if (string[6] == ' ') {
                 reader_id = get_id_from_string(&string[7], reader_id);
             }
             reader = vreader_get_reader_by_id(reader_id);
-            error = vcard_emul_force_card_remove(reader);
-            printf("remove %s, returned %d\n", reader ? vreader_get_name(reader)
-                                               : "invalid reader", error);
+            if (reader != NULL) {
+                error = vcard_emul_force_card_remove(reader);
+                printf("remove %s, returned %d\n",
+                        reader ? vreader_get_name(reader)
+                        : "invalid reader", error);
+            } else {
+                printf("no reader by id %d found\n", reader_id);
+            }
         } else if (strncmp(string,"select",6) == 0) {
             if (string[6] == ' ') {
                 reader_id = get_id_from_string(&string[7],
@@ -381,13 +392,12 @@  do_command(void)
 
 static int
 connect_to_qemu (
-    const char *ip,
-    uint32_t port
+    const char *host,
+    const char *port
 ) {
     struct addrinfo hints;
     struct addrinfo* server;
     int ret;
-    char port_str[10];
 
     sock = socket (
         AF_INET,
@@ -400,13 +410,12 @@  connect_to_qemu (
     }
 
     memset(&hints, 0, sizeof(struct addrinfo));
-    hints.ai_family = AF_INET;
+    hints.ai_family = AF_UNSPEC;
     hints.ai_socktype = SOCK_STREAM;
     hints.ai_flags = 0;
     hints.ai_protocol = 0;          /* Any protocol */
-    snprintf(port_str, sizeof(port_str) - 1, "%d", port);
 
-    ret = getaddrinfo(ip, port_str, &hints, &server);
+    ret = getaddrinfo(host, port, &hints, &server);
 
     if (ret != 0) {
         printf ("getaddrinfo failed\n");
@@ -429,13 +438,55 @@  connect_to_qemu (
     return sock;
 }
 
+static int on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
+{
+    uint32_t *capabilities = (incoming->capabilities);
+    int num_capabilities = 1 + ((mhHeader->length - sizeof(VSCMsgInit)) / sizeof(uint32_t));
+    int i;
+    int rv;
+    pthread_t thread_id;
+
+    incoming->version = ntohl(incoming->version);
+    if (incoming->version != VSCARD_VERSION) {
+        if (verbose > 0) {
+            printf("warning: host has version %d, we have %d\n",
+                verbose, VSCARD_VERSION);
+        }
+    }
+    if (incoming->magic != VSCARD_MAGIC) {
+        printf("unexpected magic: got %d, expected %d\n",
+            incoming->magic, VSCARD_MAGIC);
+        return -1;
+    }
+    for (i = 0 ; i < num_capabilities; ++i) {
+        capabilities[i] = ntohl(capabilities[i]);
+    }
+    /* Future: check capabilities */
+    /* remove whatever reader might be left in qemu,
+     * in case of an unclean previous exit. */
+    send_msg(
+        VSC_ReaderRemove,
+        VSCARD_MINIMAL_READER_ID,
+        NULL,
+        0
+    );
+    /* launch the event_thread. This will trigger reader adds for all the
+     * existing readers */
+    rv = pthread_create(&thread_id, NULL, event_thread, NULL);
+    if (rv < 0) {
+        perror("pthread_create");
+        return rv;
+    }
+    return 0;
+}
+
 int
 main (
     int argc,
     char *argv[]
 ) {
-    char* qemu_ip;
-    uint16_t qemu_port;
+    char* qemu_host;
+    char* qemu_port;
     VSCMsgHeader mhHeader;
     VSCMsgError *error_msg;
 
@@ -447,7 +498,6 @@  main (
      VReaderStatus reader_status;
     VReader *reader = NULL;
     VCardEmulOptions *command_line_options = NULL;
-    pthread_t thread_id;
     int passthru = 0;
 
     char* cert_names[MAX_CERTS];
@@ -520,18 +570,9 @@  main (
                                           vcard_emul_options(emul_args);
     }
 
-    qemu_ip = strdup(argv[argc - 2]);
-    qemu_port = (uint16_t)atoi(argv[argc -1]);
-    sock = connect_to_qemu(qemu_ip, qemu_port);
-
-    /* remove whatever reader might be left in qemu,
-     * in case of a unclean previous exit. */
-    send_msg(
-        VSC_ReaderRemove,
-        VSCARD_MINIMAL_READER_ID,
-        NULL,
-        0
-    );
+    qemu_host = strdup(argv[argc - 2]);
+    qemu_port = strdup(argv[argc -1]);
+    sock = connect_to_qemu(qemu_host, qemu_port);
 
     MUTEX_INIT(write_lock);
     MUTEX_INIT(pending_reader_lock);
@@ -544,17 +585,17 @@  main (
 #endif
         vcard_emul_init(command_line_options);
 
-    /* launch the event_thread. This will trigger reader adds for all the
-     * existing readers */
-    rv = pthread_create(&thread_id, NULL, event_thread, reader);
-    if (rv < 0) {
-        perror("pthread_create");
-        exit (1);
-    }
-
     printf("> ");
     fflush(stdout);
 
+    /* Send init message, Host responds (and then we send reader attachments) */
+    VSCMsgInit init = {
+        .version=htonl(VSCARD_VERSION),
+        .magic=VSCARD_MAGIC,
+        .capabilities={0}
+    };
+    send_msg(VSC_Init, mhHeader.reader_id, &init, sizeof(init));
+
     do {
         fd_set fds;
 
@@ -590,6 +631,9 @@  main (
             }
             return (8);
         }
+        mhHeader.type = ntohl(mhHeader.type);
+        mhHeader.reader_id = ntohl(mhHeader.reader_id);
+        mhHeader.length = ntohl(mhHeader.length);
         if (verbose) {
             printf ("Header: type=%d, reader_id=%d length=%d (0x%x)\n",
                     mhHeader.type, mhHeader.reader_id, mhHeader.length,
@@ -597,11 +641,21 @@  main (
         }
         switch (mhHeader.type) {
             case VSC_APDU:
+            case VSC_Flush:
+            case VSC_Error:
+            case VSC_Init:
                 rv = read (
                     sock,
                     pbSendBuffer,
                     mhHeader.length
                 );
+                break;
+            default:
+                printf ("Unexpected message of type 0x%X\n", mhHeader.type);
+                return 0;
+        }
+        switch (mhHeader.type) {
+            case VSC_APDU:
                 if (rv < 0) {
                     /* Error */
                     printf ("read error\n");
@@ -621,10 +675,10 @@  main (
                     pbRecvBuffer, &dwRecvLength);
                 if (reader_status == VREADER_OK) {
                     mhHeader.length = dwRecvLength;
-                if (verbose) {
-                    printf (" send response: ");
-                    print_byte_array (pbRecvBuffer, mhHeader.length);
-                }
+                    if (verbose) {
+                        printf (" send response: ");
+                        print_byte_array (pbRecvBuffer, mhHeader.length);
+                    }
                     send_msg (
                         VSC_APDU,
                         mhHeader.reader_id,
@@ -632,7 +686,7 @@  main (
                         dwRecvLength
                     );
                 } else {
-                       rv = reader_status; /* warning: not meaningful */
+                    rv = reader_status; /* warning: not meaningful */
                     send_msg (
                         VSC_Error,
                         mhHeader.reader_id,
@@ -644,49 +698,23 @@  main (
                 reader = NULL; /* we've freed it, don't use it by accident
                                   again */
                 break;
-            case VSC_Reconnect:
-                {
-                    VSCMsgReconnect reconnect;
-
-                    if (read(sock, (char*)&reconnect, mhHeader.length) < 0) {
-                        printf ("read error\n");
-                        close (sock);
-                        return (8);
-                    }
-                    if (reconnect.ip != 0) {
-                        reconnect.ip = ntohl(reconnect.ip);
-                        free(qemu_ip);
-                        qemu_ip = ip_numeric_to_char(reconnect.ip);
-                        qemu_port = reconnect.port;
-                    } else {
-                        printf("info: reconnect with no target ip:port: "
-                               "bumping port by one and reconnecting\n");
-                        qemu_port = qemu_port + 1;
-                    }
-                    /* sent when qemu is migrating, we need to close the socket
-                     * and reconnect. */
-                    close(sock);
-                    printf("reconnecting to %s:%d\n", qemu_ip, qemu_port);
-                    sock = connect_to_qemu(qemu_ip, qemu_port);
-                }
-                break;
-            case VSC_ReaderAddResponse:
-               MUTEX_LOCK(pending_reader_lock);
-                if (pending_reader) {
-                    vreader_set_id(pending_reader, mhHeader.reader_id);
-                    vreader_free(pending_reader);
-                    pending_reader = NULL;
-                    CONDITION_NOTIFY(pending_reader_condition);
-                }
-                MUTEX_UNLOCK(pending_reader_lock);
+            case VSC_Flush:
+                /* TODO: actually flush */
+                send_msg(VSC_FlushComplete, mhHeader.reader_id, NULL, 0);
                 break;
             case VSC_Error:
-                rv = read (
-                    sock,
-                    pbSendBuffer,
-                    mhHeader.length
-                );
                 error_msg = (VSCMsgError *) pbSendBuffer;
+                if (error_msg->code == VSC_SUCCESS) {
+                    MUTEX_LOCK(pending_reader_lock);
+                    if (pending_reader) {
+                        vreader_set_id(pending_reader, mhHeader.reader_id);
+                        vreader_free(pending_reader);
+                        pending_reader = NULL;
+                        CONDITION_NOTIFY(pending_reader_condition);
+                    }
+                    MUTEX_UNLOCK(pending_reader_lock);
+                    break;
+                }
                 printf("error: qemu refused to add reader\n");
                 if (error_msg->code == VSC_CANNOT_ADD_MORE_READERS) {
                     /* clear pending reader, qemu can't handle any more */
@@ -699,6 +727,11 @@  main (
                     MUTEX_UNLOCK(pending_reader_lock);
                 }
                 break;
+            case VSC_Init:
+                if (on_host_init(&mhHeader, (VSCMsgInit*)pbSendBuffer) < 0) {
+                    return -1;
+                }
+                break;
             default:
                 printf ("Default\n");
                 return 0;