rework salt initialization to ensure PRAGMA cipher_salt always returns a value

src/crypto.c

@@ -337,9 +337,15 @@ int sqlcipher_codec_pragma(sqlite3* db, int iDb, Parse *pParse, const char *zLef
           sqlite3_free(salt);
         }
       } else {
-          char *salt = (char*) sqlite3_malloc((FILE_HEADER_SZ*2)+1);
-          cipher_bin2hex(sqlcipher_codec_ctx_get_kdf_salt(ctx), FILE_HEADER_SZ, salt);
-          codec_vdbe_return_string(pParse, "cipher_salt", salt, P4_DYNAMIC);
+        void *salt;
+        char *hexsalt = (char*) sqlite3_malloc((FILE_HEADER_SZ*2)+1);
+        if((rc = sqlcipher_codec_ctx_get_kdf_salt(ctx, &salt)) == SQLITE_OK) {
+          cipher_bin2hex(salt, FILE_HEADER_SZ, hexsalt);
+          codec_vdbe_return_string(pParse, "cipher_salt", hexsalt, P4_DYNAMIC);
+        } else {
+          sqlite3_free(hexsalt);
+          sqlcipher_codec_ctx_set_error(ctx, rc);
+        }
       }
     }
   }else
@@ -651,7 +657,6 @@ static void* sqlite3Codec(void *iCtx, void *data, Pgno pgno, int mode) {
   int page_sz = sqlcipher_codec_ctx_get_pagesize(ctx); 
   unsigned char *pData = (unsigned char *) data;
   void *buffer = sqlcipher_codec_ctx_get_data(ctx);
-  void *kdf_salt = sqlcipher_codec_ctx_get_kdf_salt(ctx);
   int plaintext_header_sz = sqlcipher_codec_ctx_get_plaintext_header_size(ctx);
   int cctx = CIPHER_READ_CTX;
 
@@ -687,9 +692,15 @@ static void* sqlite3Codec(void *iCtx, void *data, Pgno pgno, int mode) {
       cctx = CIPHER_WRITE_CTX; 
 
     case CODEC_JOURNAL_OP: /* encrypt journal page, operate on read context use to get the original page data from the database */ 
-      if(pgno == 1) /* copy initial part of file header or salt to buffer */ 
+      if(pgno == 1) { /* copy initial part of file header or salt to buffer */ 
+        void *kdf_salt = NULL; 
+        /* retrieve the kdf salt */
+        if((rc = sqlcipher_codec_ctx_get_kdf_salt(ctx, &kdf_salt)) != SQLITE_OK) {
+          sqlcipher_codec_ctx_set_error(ctx, rc); 
+          return NULL;
+        }
         memcpy(buffer, plaintext_header_sz ? pData : kdf_salt, offset); 
-
+      }
       rc = sqlcipher_page_cipher(ctx, cctx, pgno, CIPHER_ENCRYPT, page_sz - offset, pData + offset, (unsigned char*)buffer + offset);
       if(rc != SQLITE_OK) sqlcipher_codec_ctx_set_error(ctx, rc);
       return buffer; /* return persistent buffer data, pData remains intact */

src/crypto.h

@@ -233,7 +233,7 @@ int sqlcipher_codec_ctx_set_kdf_iter(codec_ctx *, int);
 int sqlcipher_codec_ctx_get_kdf_iter(codec_ctx *ctx);
 
 int sqlcipher_codec_ctx_set_kdf_salt(codec_ctx *ctx, unsigned char *salt, int sz);
-void* sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx);
+int sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx, void **salt);
 
 int sqlcipher_codec_ctx_set_fast_kdf_iter(codec_ctx *, int);
 int sqlcipher_codec_ctx_get_fast_kdf_iter(codec_ctx *);

src/crypto_impl.c

@@ -756,6 +756,23 @@ void* sqlcipher_codec_ctx_get_data(codec_ctx *ctx) {
   return ctx->buffer;
 }
 
+static int sqlcipher_codec_ctx_init_kdf_salt(codec_ctx *ctx) {
+  sqlite3_file *fd = sqlite3PagerFile(ctx->pBt->pBt->pPager);
+
+  if(!ctx->need_kdf_salt) {
+    return SQLITE_OK; /* don't reload salt when not needed */
+  }
+
+  /* read salt from header, if present, otherwise generate a new random salt */
+  CODEC_TRACE("sqlcipher_codec_ctx_init_kdf_salt: obtaining salt\n");
+  if(fd == NULL || fd->pMethods == 0 || sqlite3OsRead(fd, ctx->kdf_salt, ctx->kdf_salt_sz, 0) != SQLITE_OK) {
+    CODEC_TRACE("sqlcipher_codec_ctx_init_kdf_salt: unable to read salt from file header, generating random\n");
+    if(ctx->provider->random(ctx->provider_ctx, ctx->kdf_salt, ctx->kdf_salt_sz) != SQLITE_OK) return SQLITE_ERROR;
+  }
+  ctx->need_kdf_salt = 0;
+  return SQLITE_OK; 
+}
+
 int sqlcipher_codec_ctx_set_kdf_salt(codec_ctx *ctx, unsigned char *salt, int size) {
   if(size >= ctx->kdf_salt_sz) {
     memcpy(ctx->kdf_salt, salt, ctx->kdf_salt_sz);
@@ -765,8 +782,13 @@ int sqlcipher_codec_ctx_set_kdf_salt(codec_ctx *ctx, unsigned char *salt, int si
   return SQLITE_ERROR;
 }
 
-void* sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx) {
-  return ctx->kdf_salt;
+int sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx, void** salt) {
+  int rc = SQLITE_OK;
+  if(ctx->need_kdf_salt) {
+    rc = sqlcipher_codec_ctx_init_kdf_salt(ctx);
+  }
+  *salt = ctx->kdf_salt;
+  return rc;
 }
 
 void sqlcipher_codec_get_keyspec(codec_ctx *ctx, void **zKey, int *nKey) {
@@ -1090,17 +1112,13 @@ static int sqlcipher_cipher_ctx_key_derive(codec_ctx *ctx, cipher_ctx *c_ctx) {
                 ctx->hmac_kdf_salt, c_ctx->fast_kdf_iter, ctx->key_sz); 
                 
   
-  if(c_ctx->pass && c_ctx->pass_sz) { /* if pass is not null */
+  if(c_ctx->pass && c_ctx->pass_sz) {  /* if key material is present on the context for derivation */ 
+   
+    /* if necessary, initialize the salt from the header or random source */
     if(ctx->need_kdf_salt) {
-      sqlite3_file *fd = sqlite3PagerFile(ctx->pBt->pBt->pPager);
-      /* read salt from header, if present, otherwise generate a new random salt */
-      CODEC_TRACE("sqlcipher_cipher_ctx_key_derive: obtaining salt\n");
-      if(fd == NULL || fd->pMethods == 0 || sqlite3OsRead(fd, ctx->kdf_salt, ctx->kdf_salt_sz, 0) != SQLITE_OK) {
-        CODEC_TRACE("sqlcipher_cipher_ctx_key_derive: unable to read salt from file header, generating random\n");
-        if(ctx->provider->random(ctx->provider_ctx, ctx->kdf_salt, ctx->kdf_salt_sz) != SQLITE_OK) return SQLITE_ERROR;
-      }
-      ctx->need_kdf_salt = 0;
+      if((rc = sqlcipher_codec_ctx_init_kdf_salt(ctx)) != SQLITE_OK) return rc;
     }
+ 
     if (c_ctx->pass_sz == ((ctx->key_sz * 2) + 3) && sqlite3StrNICmp((const char *)c_ctx->pass ,"x'", 2) == 0 && cipher_isHex(c_ctx->pass + 2, ctx->key_sz * 2)) { 
       int n = c_ctx->pass_sz - 3; /* adjust for leading x' and tailing ' */
       const unsigned char *z = c_ctx->pass + 2; /* adjust lead offset of x' */

test/sqlcipher-plaintext-header.test

@@ -430,8 +430,9 @@ do_test test-plaintext-header-migrate-journal-wal-string-key-random-salt {
 db close
 file delete -force test.db
 
-# when cipher_salt is the first statement the values are zeros
-# after the databse is first used and key derivation occurs it will change
+# when cipher_salt is the first statement a new salt should be generated
+# and it should match the salt after key derviation occurs. At no point
+# should the salt be zero
 do_test plaintext-header-size-salt-first-op {
   set rc {}
   sqlite_orig db test.db
@@ -447,10 +448,10 @@ do_test plaintext-header-size-salt-first-op {
     PRAGMA cipher_salt;
   }]
 
-  lappend rc $salt1
   lappend rc [string equal $salt1 "00000000000000000000000000000000"] 
   lappend rc [string equal $salt2 "00000000000000000000000000000000"] 
-} {00000000000000000000000000000000 1 0}
+  lappend rc [string equal $salt1 $salt2]
+} {0 0 1}
 db close
 file delete -force test.db