First batch of tweaks after testing with hsmbully.

2 files changed, 144 insertions, 20 deletions

diff --git a/GNUmakefile b/GNUmakefile
index 692ddf1..10299f7 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -28,6 +28,13 @@
 # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF 
 # ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+# Whether to enable threading.  Main reason for being able to turn it
+# off is that gdb on the Novena goes bananas when threading is enabled.
+
+ifndef ENABLE_THREADS
+  ENABLE_THREADS := yes
+endif
+
 LIBHAL_DIR   = ../libhal
 LIBTFM_DIR   = ../libtfm
 SQLITE3_DIR  = ../sqlite3
@@ -35,6 +42,12 @@ SQLITE3_DIR  = ../sqlite3
 CFLAGS	+= -g3 -fPIC -Wall -std=c99 -I${LIBHAL_DIR} -I${SQLITE3_DIR}
 LIBS	:= ${LIBHAL_DIR}/libhal.a ${LIBTFM_DIR}/libtfm.a ${SQLITE3_DIR}/libsqlite3.a
 
+ifeq "${ENABLE_THREADS}" "yes"
+  CFLAGS += -pthread
+else
+  CFLAGS += -DUSE_PTHREADS=0
+endif
+
 all: libpkcs11.so
 
 clean:
@@ -53,7 +66,7 @@ pkcs11.o: pkcs11.c schema.h attributes.h
 	${CC} ${CFLAGS} -c $<
 
 pkcs11.so: pkcs11.o ${LIBS}
-	${CC} -shared -o $@ -Wl,-Bsymbolic-functions -Wl,-Bsymbolic -Wl,-z,noexecstack -g $^
+	${CC} ${CFLAGS} -shared -o $@ -Wl,-Bsymbolic-functions -Wl,-Bsymbolic -Wl,-z,noexecstack -g $^
 
 libpkcs11.so: pkcs11.so
 	objcopy -w -G 'C_*' $< $@
@@ -62,3 +75,29 @@ tags: TAGS
 
 TAGS: *.[ch]
 	etags $^
+
+# Kudge for testing, gussy this up if we decide to keep it
+
+HSMBULLY := $(firstword $(wildcard $(addsuffix /hsmbully,$(subst :, ,.:${PATH}))))
+
+ifneq "${HSMBULLY}" ""
+
+HSMBULLY_OPTIONS := \
+	--pin fnord --so-pin fnord --pkcs11lib $(abspath libpkcs11.so) \
+	--verbose=9 --fast-and-frivolous --skip-fragmentation --skip-keysizing
+
+FNORD	:= x'98034e94dd4fc527fb8a9296eab55677cd4ce40e025d42acae3cfcc7813f3b9d'
+SALT	:= x'000102030405060708090a0b0c0d0e0f'
+PIN_SQL := UPDATE global SET user_pin = ${FNORD}, user_pin_salt = ${SALT}, so_pin = ${FNORD}, so_pin_salt = ${SALT}
+
+export PKCS11_DATABASE=$(abspath .pkcs11.db)
+
+SQLITE3_CMD := $(abspath ../sqlite3/build/sqlite3)
+
+bully: all
+	rm -f ${PKCS11_DATABASE} ${PKCS11_DATABASE}-journal
+	${SQLITE3_CMD} ${PKCS11_DATABASE} <schema.sql
+	${SQLITE3_CMD} -echo ${PKCS11_DATABASE} "${PIN_SQL}"
+	sudo -E ${HSMBULLY} ${HSMBULLY_OPTIONS}
+
+endif
diff --git a/pkcs11.c b/pkcs11.c
index cb80888..f99face 100644
--- a/pkcs11.c
+++ b/pkcs11.c
@@ -107,6 +107,14 @@
 #define DEBUG_SQL       1
 #endif
 
+#ifndef DEBUG_HAL
+#define DEBUG_HAL       0
+#endif
+
+#ifndef DEBUG_PKCS11
+#define DEBUG_PKCS11    1
+#endif
+
 /*
  * Default filename for SQL database lives.  Can be overriden at
  * runtime by setting PKCS11_DATABASE environment variable.
@@ -124,12 +132,27 @@
 #define USE_POSIX 1
 #endif
 
+/*
+ * Whether to use POSIX threads.
+ */
+
+#ifndef USE_PTHREADS
+#define USE_PTHREADS USE_POSIX
+#endif
+
+#if USE_PTHREADS && !USE_POSIX
+#error Can not use POSIX threads without using POSIX
+#endif
+
 #if USE_POSIX
 #include <unistd.h>
-#include <pthread.h>
 #include <errno.h>
 #endif
 
+#if USE_PTHREADS
+#include <pthread.h>
+#endif
+
 
 
 /*
@@ -298,10 +321,6 @@ static pid_t initialized_pid;
  * Error checking for libhal calls.
  */
 
-#ifndef DEBUG_HAL
-#define DEBUG_HAL 0
-#endif
-
 #if DEBUG_HAL
 
 static int _hal_check(const hal_error_t err, const char * const expr, const const * const file, const unsigned line)
@@ -373,7 +392,7 @@ static int _hal_check(const hal_error_t err, const char * const expr, const cons
  * Mutex implementation using POSIX mutexes.
  */
 
-#if USE_POSIX
+#if USE_PTHREADS
 
 static CK_RV posix_mutex_create(CK_VOID_PTR_PTR ppMutex)
 {
@@ -484,7 +503,7 @@ static CK_RV posix_mutex_unlock(CK_VOID_PTR pMutex)
   return rv;
 }
 
-#endif /* USE_POSIX */
+#endif /* USE_PTHREADS */
 
 
 
@@ -1018,7 +1037,10 @@ static CK_RV p11_object_check_rights(const p11_session_t *session,
                                      const CK_OBJECT_HANDLE object_handle,
                                      const p11_object_access_t rights)
 {
-  static const char session_handle_query[] =
+  static const char object_exists_query[] =
+    " SELECT count(*) FROM object WHERE object_handle = ?1";
+
+  static const char session_object_query[] =
     " SELECT session_handle FROM session NATURAL JOIN object WHERE object_handle = ?1";
 
   CK_BBOOL object_is_private;
@@ -1040,7 +1062,7 @@ static CK_RV p11_object_check_rights(const p11_session_t *session,
   }
 
   /*
-   * Private objects don't for sessions in the wrong state.
+   * Private objects don't exist for sessions in the wrong state.
    */
 
   switch (session->state) {
@@ -1052,11 +1074,22 @@ static CK_RV p11_object_check_rights(const p11_session_t *session,
   }
 
   /*
+   * Does the object even exist?
+   */
+
+  if (!sql_check_ok(sql_prepare(&q, object_exists_query))       ||
+      !sql_check_ok(sqlite3_bind_int64(q, 1, object_handle))    ||
+      !sql_check_row(sqlite3_step(q))                           ||
+      !sqlite3_column_int(q, 0))
+    lose(CKR_OBJECT_HANDLE_INVALID);
+
+  /*
    * Session objects are only visible to the session which created them.
    */
 
   if (!is_token_handle(object_handle)                           &&
-      (!sql_check_ok(sql_prepare(&q, session_handle_query))     ||
+      (!sql_check_ok(sql_finalize_and_clear(&q))                ||
+       !sql_check_ok(sql_prepare(&q, session_object_query))     ||
        !sql_check_ok(sqlite3_bind_int64(q, 1, object_handle))   ||
        !sql_check_row(sqlite3_step(q))                          ||
        sqlite3_column_int64(q, 0) != session->handle))
@@ -1632,6 +1665,11 @@ static CK_RV p11_check_keypair_attributes_check_template_1(const CK_ATTRIBUTE_TY
   rv = CKR_OK;
 
  fail:
+#if DEBUG_PKCS11
+  if (rv != CKR_OK)
+    fprintf(stderr, "p11_check_keypair_attributes_check_template_1() rejected attribute 0x%08lx\n",
+            (unsigned long) type);
+#endif
   return rv;
 }
 
@@ -2067,7 +2105,7 @@ CK_RV C_Initialize(CK_VOID_PTR pInitArgs)
     }
 
     else if ((a->flags & CKF_OS_LOCKING_OK) != 0) {
-#if USE_POSIX
+#if USE_PTHREADS
       mutex_cb_create  = posix_mutex_create;
       mutex_cb_destroy = posix_mutex_destroy;
       mutex_cb_lock    = posix_mutex_lock;
@@ -2599,9 +2637,8 @@ CK_RV C_DestroyObject(CK_SESSION_HANDLE hSession,
        !sql_check_ok(sql_finalize_and_clear(&q))))
     lose(CKR_FUNCTION_FAILED);
 
-  if (
-      !sql_check_ok(sql_prepare(&q, delete_object))     ||
-      !sql_check_ok(sqlite3_bind_int64(q, 1, hObject))  ||
+  if (!sql_check_ok(sql_prepare(&q, delete_object))             ||
+      !sql_check_ok(sqlite3_bind_int64(q, 1, hObject))          ||
       !sql_check_done(sqlite3_step(q)))
     lose(CKR_FUNCTION_FAILED);
 
@@ -3256,6 +3293,59 @@ CK_RV C_GenerateRandom(CK_SESSION_HANDLE hSession,
 
 
 /*
+ * hsmbully wants additional methods, no real surprise.
+ */
+
+/*
+ * Supply information about a particular mechanism.  We may want a
+ * more generic structure for this, for the moment, just answer the
+ * questions hsmbully is asking.
+ *
+ * Not really sure whether I should be setting CKF_HW here or not, RSA
+ * is a mix of hardware and software at the moment, but I'm also a
+ * little unclear on what "the device" means in this context, so let's
+ * just say that if it's implemented by libhal or the Verilog hiding
+ * behind libhal, it's implemented in hardware.
+ */
+
+CK_RV C_GetMechanismInfo(CK_SLOT_ID slotID,
+                         CK_MECHANISM_TYPE type,
+                         CK_MECHANISM_INFO_PTR pInfo)
+{
+  /*
+   * No locking here, no obvious need for it.
+   */
+
+  if (pInfo == NULL)
+    return CKR_ARGUMENTS_BAD;
+
+  if (slotID != P11_ONE_AND_ONLY_SLOT)
+    return CKR_SLOT_ID_INVALID;
+
+  switch (type) {
+    
+  case CKM_RSA_PKCS_KEY_PAIR_GEN:
+    pInfo->ulMinKeySize = 1024;
+    pInfo->ulMaxKeySize = 8192;
+    pInfo->flags = CKF_HW | CKF_GENERATE_KEY_PAIR;
+    break;
+
+  case CKM_RSA_PKCS:
+    pInfo->ulMinKeySize = 1024;
+    pInfo->ulMaxKeySize = 8192;
+    pInfo->flags = CKF_HW | CKF_SIGN;
+    break;
+
+  default:
+    return CKR_MECHANISM_INVALID;
+  }
+
+  return CKR_OK;
+}
+
+
+
+/*
  * Stubs for unsupported functions below here.  Per the PKCS #11
  * specification, it's OK to skip implementing almost any function in
  * the API, but if one does so, one must provide a stub which returns
@@ -3287,11 +3377,6 @@ CK_RV C_GetMechanismList(CK_SLOT_ID slotID,
                          CK_ULONG_PTR pulCount)
 { return CKR_FUNCTION_NOT_SUPPORTED; }
 
-CK_RV C_GetMechanismInfo(CK_SLOT_ID slotID,
-                         CK_MECHANISM_TYPE type,
-                         CK_MECHANISM_INFO_PTR pInfo)
-{ return CKR_FUNCTION_NOT_SUPPORTED; }
-
 CK_RV C_InitToken(CK_SLOT_ID slotID,
                   CK_UTF8CHAR_PTR pPin,
                   CK_ULONG ulPinLen,