improved HMAC key derivation

sjlombardo · sjlombardo · commit 9c0dd8d44967 · 2012-01-03T17:26:44.000-05:00
- ensure that the HMAC key derivation salt is different from
  that used to generate the encryption key
- allow the number of pbkdf2 iterations used for HMAC key
  derivation to be configured at runtime via pragma
diff --git a/src/crypto.c b/src/crypto.c
@@ -50,6 +50,18 @@ int codec_set_kdf_iter(sqlite3* db, int nDb, int kdf_iter, int for_ctx) {
   return SQLITE_ERROR;
 }
 
+int codec_set_hmac_kdf_iter(sqlite3* db, int nDb, int kdf_iter, int for_ctx) {
+  struct Db *pDb = &db->aDb[nDb];
+  CODEC_TRACE(("codec_set_kdf_iter: entered db=%d nDb=%d kdf_iter=%d for_ctx=%d\n", db, nDb, kdf_iter, for_ctx));
+
+  if(pDb->pBt) {
+    codec_ctx *ctx;
+    sqlite3pager_get_codec(pDb->pBt->pBt->pPager, (void **) &ctx);
+    return sqlcipher_codec_ctx_set_hmac_kdf_iter(ctx, kdf_iter, for_ctx);
+  }
+  return SQLITE_ERROR;
+}
+
 static int codec_set_btree_to_codec_pagesize(sqlite3 *db, Db *pDb, codec_ctx *ctx) {
   int rc, page_sz, reserve_sz; 
 
diff --git a/src/crypto.h b/src/crypto.h
@@ -56,6 +56,25 @@
 #define DEFAULT_USE_HMAC 1
 #endif
 
+/* by default, sqlcipher will use an equal number of rounds to generate
+   the HMAC key as it will to generate the encryption key */
+#ifndef HMAC_PBKDF2_ITER
+#define HMAC_PBKDF2_ITER PBKDF2_ITER
+#endif
+
+/* this if a fixed random array that will be xor'd with the database salt to ensure that the
+   salt passed to the HMAC key derivation function is not the same as that used to derive
+   the encryption key. This can be overridden at compile time but it will make the resulting
+   binary incompatible with the default builds when using HMAC. A future version of SQLcipher
+   will likely allow this to be defined at runtime via pragma */ 
+#ifndef HMAC_FIXED_SALT
+#define HMAC_FIXED_SALT {42,172,104,131,19,119,84,255,184,238,54,135,186,222,53,250}
+#endif
+
+#ifndef HMAC_FIXED_SALT_SZ
+#define HMAC_FIXED_SALT_SZ 16
+#endif
+
 #ifdef CODEC_DEBUG
 #define CODEC_TRACE(X)  {printf X;fflush(stdout);}
 #else
@@ -125,6 +144,8 @@ int sqlcipher_codec_ctx_get_reservesize(codec_ctx *);
 int sqlcipher_codec_ctx_set_kdf_iter(codec_ctx *, int, int);
 void* sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx);
 
+int sqlcipher_codec_ctx_set_hmac_kdf_iter(codec_ctx *, int, int);
+
 int sqlcipher_codec_ctx_set_cipher(codec_ctx *, const char *, int);
 
 void* sqlcipher_codec_ctx_get_data(codec_ctx *);
diff --git a/src/crypto_impl.c b/src/crypto_impl.c
@@ -47,6 +47,7 @@
 #endif
 #endif
 
+static unsigned char cipher_hmac_fixed_salt[HMAC_FIXED_SALT_SZ] = HMAC_FIXED_SALT;
 
 /* the default implementation of SQLCipher uses a cipher_ctx
    to keep track of read / write state separately. The following
@@ -57,6 +58,7 @@ typedef struct {
   EVP_CIPHER_CTX ectx;
   HMAC_CTX hctx;
   int kdf_iter;
+  int hmac_kdf_iter;
   int key_sz;
   int iv_sz;
   int block_sz;
@@ -74,7 +76,7 @@ int sqlcipher_cipher_ctx_cmp(cipher_ctx *, cipher_ctx *);
 int sqlcipher_cipher_ctx_copy(cipher_ctx *, cipher_ctx *);
 int sqlcipher_cipher_ctx_init(cipher_ctx **);
 int sqlcipher_cipher_ctx_set_pass(cipher_ctx *, const void *, int);
-int  sqlcipher_cipher_ctx_key_derive(codec_ctx *, cipher_ctx *);
+int sqlcipher_cipher_ctx_key_derive(codec_ctx *, cipher_ctx *);
 
 /* prototype for pager HMAC function */
 int sqlcipher_page_hmac(cipher_ctx *, Pgno, unsigned char *, int, unsigned char *);
@@ -83,6 +85,7 @@ struct codec_ctx {
   int kdf_salt_sz;
   int page_sz;
   unsigned char *kdf_salt;
+  unsigned char *hmac_kdf_salt;
   unsigned char *buffer;
   Btree *pBt;
   cipher_ctx *read_ctx;
@@ -202,6 +205,7 @@ int sqlcipher_cipher_ctx_cmp(cipher_ctx *c1, cipher_ctx *c2) {
     c1->evp_cipher == c2->evp_cipher
     && c1->iv_sz == c2->iv_sz
     && c1->kdf_iter == c2->kdf_iter
+    && c1->hmac_kdf_iter == c2->hmac_kdf_iter
     && c1->key_sz == c2->key_sz
     && c1->pass_sz == c2->pass_sz
     && (
@@ -309,6 +313,21 @@ int sqlcipher_codec_ctx_set_kdf_iter(codec_ctx *ctx, int kdf_iter, int for_ctx)
   return SQLITE_OK;
 }
 
+int sqlcipher_codec_ctx_set_hmac_kdf_iter(codec_ctx *ctx, int hmac_kdf_iter, int for_ctx) {
+  cipher_ctx *c_ctx = for_ctx ? ctx->write_ctx : ctx->read_ctx;
+  int rc;
+
+  c_ctx->hmac_kdf_iter = hmac_kdf_iter;
+  c_ctx->derive_key = 1;
+
+  if(for_ctx == 2)
+    if((rc = sqlcipher_cipher_ctx_copy( for_ctx ? ctx->read_ctx : ctx->write_ctx, c_ctx)) != SQLITE_OK)
+      return rc; 
+
+  return SQLITE_OK;
+}
+
+
 int sqlcipher_codec_ctx_set_use_hmac(codec_ctx *ctx, int use) {
   int reserve = EVP_MAX_IV_LENGTH; /* base reserve size will be IV only */ 
 
@@ -386,6 +405,13 @@ int sqlcipher_codec_ctx_init(codec_ctx **iCtx, Db *pDb, Pager *pPager, sqlite3_f
   ctx->kdf_salt = sqlcipher_malloc(ctx->kdf_salt_sz);
   if(ctx->kdf_salt == NULL) return SQLITE_NOMEM;
 
+  /* allocate space for separate hmac salt data. We want the
+     HMAC derivation salt to be different than the encryption
+     key derivation salt */
+  ctx->hmac_kdf_salt = sqlcipher_malloc(ctx->kdf_salt_sz);
+  if(ctx->hmac_kdf_salt == NULL) return SQLITE_NOMEM;
+
+
   /*
      Always overwrite page size and set to the default because the first page of the database
      in encrypted and thus sqlite can't effectively determine the pagesize. this causes an issue in 
@@ -403,6 +429,7 @@ int sqlcipher_codec_ctx_init(codec_ctx **iCtx, Db *pDb, Pager *pPager, sqlite3_f
 
   if((rc = sqlcipher_codec_ctx_set_cipher(ctx, CIPHER, 0)) != SQLITE_OK) return rc;
   if((rc = sqlcipher_codec_ctx_set_kdf_iter(ctx, PBKDF2_ITER, 0)) != SQLITE_OK) return rc;
+  if((rc = sqlcipher_codec_ctx_set_hmac_kdf_iter(ctx, HMAC_PBKDF2_ITER, 0)) != SQLITE_OK) return rc;
   if((rc = sqlcipher_codec_ctx_set_pass(ctx, zKey, nKey, 0)) != SQLITE_OK) return rc;
 
   /* Use HMAC signatures by default. Note that codec_set_use_hmac will implicity call
@@ -422,6 +449,7 @@ void sqlcipher_codec_ctx_free(codec_ctx **iCtx) {
   codec_ctx *ctx = *iCtx;
   CODEC_TRACE(("codec_ctx_free: entered iCtx=%d\n", iCtx));
   sqlcipher_free(ctx->kdf_salt, ctx->kdf_salt_sz);
+  sqlcipher_free(ctx->hmac_kdf_salt, ctx->kdf_salt_sz);
   sqlcipher_free(ctx->buffer, 0);
   sqlcipher_cipher_ctx_free(&ctx->read_ctx);
   sqlcipher_cipher_ctx_free(&ctx->write_ctx);
@@ -532,9 +560,11 @@ int sqlcipher_page_cipher(codec_ctx *ctx, int for_ctx, Pgno pgno, int mode, int
   */
 int sqlcipher_cipher_ctx_key_derive(codec_ctx *ctx, cipher_ctx *c_ctx) {
   CODEC_TRACE(("codec_key_derive: entered c_ctx->pass=%s, c_ctx->pass_sz=%d \
-                ctx->kdf_salt=%d ctx->kdf_salt_sz=%d c_ctx->kdf_iter=%d c_ctx->key_sz=%d\n", 
-                c_ctx->pass, c_ctx->pass_sz, ctx->kdf_salt, ctx->kdf_salt_sz, 
-                c_ctx->kdf_iter, c_ctx->key_sz));
+                ctx->kdf_salt=%d ctx->kdf_salt_sz=%d c_ctx->kdf_iter=%d \
+                ctx->hmac_kdf_salt=%d, c_ctx->hmac_kdf_iter=%d c_ctx->key_sz=%d\n", 
+                c_ctx->pass, c_ctx->pass_sz, ctx->kdf_salt, ctx->kdf_salt_sz, c_ctx->kdf_iter, 
+                ctx->hmac_kdf_salt, c_ctx->hmac_kdf_iter, c_ctx->key_sz)); 
+                
 
   if(c_ctx->pass && c_ctx->pass_sz) { // if pass is not null
     if (c_ctx->pass_sz == ((c_ctx->key_sz*2)+3) && sqlite3StrNICmp(c_ctx->pass ,"x'", 2) == 0) { 
@@ -554,10 +584,23 @@ int sqlcipher_cipher_ctx_key_derive(codec_ctx *ctx, cipher_ctx *c_ctx) {
        key for HMAC. In this case, we use the output of the previous KDF as the input to 
        this KDF run. This ensures a distinct but predictable HMAC key. */
     if(c_ctx->use_hmac) {
-      CODEC_TRACE(("codec_key_derive: deriving hmac key using PBKDF2\n")); 
+      int i;
+
+      /* start by copying the kdf key into the hmac salt slot
+         then XOR it with the fixed hmac salt defined at compile time
+         this ensures that the salt passed in to derive the hmac key, while 
+         easy to derive and publically known, is not the same as the salt used 
+         to generate the encryption key */ 
+      memcpy(ctx->hmac_kdf_salt, ctx->kdf_salt, ctx->kdf_salt_sz);
+      for(i = 0; i < HMAC_FIXED_SALT_SZ && i < ctx->kdf_salt_sz; i++) {
+        ctx->hmac_kdf_salt[i] = ctx->hmac_kdf_salt[i] ^ cipher_hmac_fixed_salt[i];
+      } 
+
+      CODEC_TRACE(("codec_key_derive: deriving hmac key from encryption key using PBKDF2 with %d iterations\n", 
+        HMAC_PBKDF2_ITER)); 
       PKCS5_PBKDF2_HMAC_SHA1( (const char*)c_ctx->key, c_ctx->key_sz, 
-                              ctx->kdf_salt, ctx->kdf_salt_sz, 
-                              c_ctx->kdf_iter, c_ctx->key_sz, c_ctx->hmac_key); 
+                              ctx->hmac_kdf_salt, ctx->kdf_salt_sz, 
+                              c_ctx->hmac_kdf_iter, c_ctx->key_sz, c_ctx->hmac_key); 
     }
 
     c_ctx->derive_key = 0;
diff --git a/src/pragma.c b/src/pragma.c
@@ -1503,6 +1503,10 @@ void sqlite3Pragma(
     extern int codec_set_kdf_iter(sqlite3*, int, int, int);
     codec_set_kdf_iter(db, iDb, atoi(zRight), 2); // change of RW PBKDF2 iteration
   }else
+  if( sqlite3StrICmp(zLeft, "hmac_kdf_iter")==0 && zRight ){
+    extern int codec_set_hmac_kdf_iter(sqlite3*, int, int, int);
+    codec_set_hmac_kdf_iter(db, iDb, atoi(zRight), 2); // change of RW PBKDF2 iteration
+  }else
   if( sqlite3StrICmp(zLeft, "rekey_kdf_iter")==0 && zRight ){
     extern int codec_set_kdf_iter(sqlite3*, int, int, int); 
     codec_set_kdf_iter(db, iDb, atoi(zRight), 1); // change # if W iterations
diff --git a/test/crypto.test b/test/crypto.test
@@ -925,4 +925,54 @@ do_test multiple-key-calls-safe-3 {
 db close
 file delete -force test.db
 
+# 1. create a database with a custom hmac kdf iteration count, 
+# 2. create table and insert operations should work
+# 3. close database, open it again with the same
+#    key and  hmac kdf iteration count
+# 4. verify that the table is readable
+#    and the data just inserted is visible
+do_test custom-hmac-kdf-iter {
+  sqlite_orig db test.db
+
+  execsql {
+    PRAGMA key = 'testkey';
+    PRAGMA hmac_kdf_iter = 10;
+    CREATE table t1(a,b);
+    BEGIN;
+  }
+
+  for {set i 1} {$i<=1000} {incr i} {
+    set r [expr {int(rand()*500000)}]
+    execsql "INSERT INTO t1 VALUES($i,'value $r');" 
+  }
+
+  execsql {
+    COMMIT;
+  } 
+
+  db close
+  sqlite_orig db test.db
+
+  execsql {
+    PRAGMA key = 'testkey';
+    PRAGMA hmac_kdf_iter = 10;
+    SELECT count(*) FROM t1;
+  }
+
+} {1000}
+db close
+
+# open the database with the default hmac
+# kdf iteration count
+# to verify that it is not readable 
+do_test custom-hmac-kdf-iter-must-match {
+  sqlite_orig db test.db
+  catchsql {
+    PRAGMA key = 'testkey';
+    SELECT name FROM sqlite_master WHERE type='table';
+  }
+} {1 {file is encrypted or is not a database}}
+db close
+file delete -force test.db
+
 finish_test
