Skip to content

Commit 1ca606d

Browse files
sjlombardosjlombardo
committed
rework salt initialization to ensure PRAGMA cipher_salt always returns a value
1 parent 4f9202c commit 1ca606d

File tree

4 files changed

+52
-22
lines changed

4 files changed

+52
-22
lines changed

‎src/crypto.c‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -337,9 +337,15 @@ int sqlcipher_codec_pragma(sqlite3* db, int iDb, Parse *pParse, const char *zLef
337337
sqlite3_free(salt);
338338
}
339339
} else {
340-
char *salt = (char*) sqlite3_malloc((FILE_HEADER_SZ*2)+1);
341-
cipher_bin2hex(sqlcipher_codec_ctx_get_kdf_salt(ctx), FILE_HEADER_SZ, salt);
342-
codec_vdbe_return_string(pParse, "cipher_salt", salt, P4_DYNAMIC);
340+
void *salt;
341+
char *hexsalt = (char*) sqlite3_malloc((FILE_HEADER_SZ*2)+1);
342+
if((rc = sqlcipher_codec_ctx_get_kdf_salt(ctx, &salt)) == SQLITE_OK) {
343+
cipher_bin2hex(salt, FILE_HEADER_SZ, hexsalt);
344+
codec_vdbe_return_string(pParse, "cipher_salt", hexsalt, P4_DYNAMIC);
345+
} else {
346+
sqlite3_free(hexsalt);
347+
sqlcipher_codec_ctx_set_error(ctx, rc);
348+
}
343349
}
344350
}
345351
}else
@@ -651,7 +657,6 @@ static void* sqlite3Codec(void *iCtx, void *data, Pgno pgno, int mode) {
651657
int page_sz = sqlcipher_codec_ctx_get_pagesize(ctx);
652658
unsigned char *pData = (unsigned char *) data;
653659
void *buffer = sqlcipher_codec_ctx_get_data(ctx);
654-
void *kdf_salt = sqlcipher_codec_ctx_get_kdf_salt(ctx);
655660
int plaintext_header_sz = sqlcipher_codec_ctx_get_plaintext_header_size(ctx);
656661
int cctx = CIPHER_READ_CTX;
657662

@@ -687,9 +692,15 @@ static void* sqlite3Codec(void *iCtx, void *data, Pgno pgno, int mode) {
687692
cctx = CIPHER_WRITE_CTX;
688693

689694
case CODEC_JOURNAL_OP: /* encrypt journal page, operate on read context use to get the original page data from the database */
690-
if(pgno == 1) /* copy initial part of file header or salt to buffer */
695+
if(pgno == 1) { /* copy initial part of file header or salt to buffer */
696+
void *kdf_salt = NULL;
697+
/* retrieve the kdf salt */
698+
if((rc = sqlcipher_codec_ctx_get_kdf_salt(ctx, &kdf_salt)) != SQLITE_OK) {
699+
sqlcipher_codec_ctx_set_error(ctx, rc);
700+
return NULL;
701+
}
691702
memcpy(buffer, plaintext_header_sz ? pData : kdf_salt, offset);
692-
703+
}
693704
rc = sqlcipher_page_cipher(ctx, cctx, pgno, CIPHER_ENCRYPT, page_sz - offset, pData + offset, (unsigned char*)buffer + offset);
694705
if(rc != SQLITE_OK) sqlcipher_codec_ctx_set_error(ctx, rc);
695706
return buffer; /* return persistent buffer data, pData remains intact */

‎src/crypto.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ int sqlcipher_codec_ctx_set_kdf_iter(codec_ctx *, int);
233233
int sqlcipher_codec_ctx_get_kdf_iter(codec_ctx *ctx);
234234

235235
int sqlcipher_codec_ctx_set_kdf_salt(codec_ctx *ctx, unsigned char *salt, int sz);
236-
void* sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx);
236+
int sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx, void **salt);
237237

238238
int sqlcipher_codec_ctx_set_fast_kdf_iter(codec_ctx *, int);
239239
int sqlcipher_codec_ctx_get_fast_kdf_iter(codec_ctx *);

‎src/crypto_impl.c‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,23 @@ void* sqlcipher_codec_ctx_get_data(codec_ctx *ctx) {
756756
return ctx->buffer;
757757
}
758758

759+
static int sqlcipher_codec_ctx_init_kdf_salt(codec_ctx *ctx) {
760+
sqlite3_file *fd = sqlite3PagerFile(ctx->pBt->pBt->pPager);
761+
762+
if(!ctx->need_kdf_salt) {
763+
return SQLITE_OK; /* don't reload salt when not needed */
764+
}
765+
766+
/* read salt from header, if present, otherwise generate a new random salt */
767+
CODEC_TRACE("sqlcipher_codec_ctx_init_kdf_salt: obtaining salt\n");
768+
if(fd == NULL || fd->pMethods == 0 || sqlite3OsRead(fd, ctx->kdf_salt, ctx->kdf_salt_sz, 0) != SQLITE_OK) {
769+
CODEC_TRACE("sqlcipher_codec_ctx_init_kdf_salt: unable to read salt from file header, generating random\n");
770+
if(ctx->provider->random(ctx->provider_ctx, ctx->kdf_salt, ctx->kdf_salt_sz) != SQLITE_OK) return SQLITE_ERROR;
771+
}
772+
ctx->need_kdf_salt = 0;
773+
return SQLITE_OK;
774+
}
775+
759776
int sqlcipher_codec_ctx_set_kdf_salt(codec_ctx *ctx, unsigned char *salt, int size) {
760777
if(size >= ctx->kdf_salt_sz) {
761778
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
765782
return SQLITE_ERROR;
766783
}
767784

768-
void* sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx) {
769-
return ctx->kdf_salt;
785+
int sqlcipher_codec_ctx_get_kdf_salt(codec_ctx *ctx, void** salt) {
786+
int rc = SQLITE_OK;
787+
if(ctx->need_kdf_salt) {
788+
rc = sqlcipher_codec_ctx_init_kdf_salt(ctx);
789+
}
790+
*salt = ctx->kdf_salt;
791+
return rc;
770792
}
771793

772794
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) {
10901112
ctx->hmac_kdf_salt, c_ctx->fast_kdf_iter, ctx->key_sz);
10911113

10921114

1093-
if(c_ctx->pass && c_ctx->pass_sz) { /* if pass is not null */
1115+
if(c_ctx->pass && c_ctx->pass_sz) { /* if key material is present on the context for derivation */
1116+
1117+
/* if necessary, initialize the salt from the header or random source */
10941118
if(ctx->need_kdf_salt) {
1095-
sqlite3_file *fd = sqlite3PagerFile(ctx->pBt->pBt->pPager);
1096-
/* read salt from header, if present, otherwise generate a new random salt */
1097-
CODEC_TRACE("sqlcipher_cipher_ctx_key_derive: obtaining salt\n");
1098-
if(fd == NULL || fd->pMethods == 0 || sqlite3OsRead(fd, ctx->kdf_salt, ctx->kdf_salt_sz, 0) != SQLITE_OK) {
1099-
CODEC_TRACE("sqlcipher_cipher_ctx_key_derive: unable to read salt from file header, generating random\n");
1100-
if(ctx->provider->random(ctx->provider_ctx, ctx->kdf_salt, ctx->kdf_salt_sz) != SQLITE_OK) return SQLITE_ERROR;
1101-
}
1102-
ctx->need_kdf_salt = 0;
1119+
if((rc = sqlcipher_codec_ctx_init_kdf_salt(ctx)) != SQLITE_OK) return rc;
11031120
}
1121+
11041122
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)) {
11051123
int n = c_ctx->pass_sz - 3; /* adjust for leading x' and tailing ' */
11061124
const unsigned char *z = c_ctx->pass + 2; /* adjust lead offset of x' */

‎test/sqlcipher-plaintext-header.test‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,9 @@ do_test test-plaintext-header-migrate-journal-wal-string-key-random-salt {
430430
db close
431431
file delete -force test.db
432432

433-
# when cipher_salt is the first statement the values are zeros
434-
# after the databse is first used and key derivation occurs it will change
433+
# when cipher_salt is the first statement a new salt should be generated
434+
# and it should match the salt after key derviation occurs. At no point
435+
# should the salt be zero
435436
do_test plaintext-header-size-salt-first-op {
436437
set rc {}
437438
sqlite_orig db test.db
@@ -447,10 +448,10 @@ do_test plaintext-header-size-salt-first-op {
447448
PRAGMA cipher_salt;
448449
}]
449450

450-
lappend rc $salt1
451451
lappend rc [string equal $salt1 "00000000000000000000000000000000"]
452452
lappend rc [string equal $salt2 "00000000000000000000000000000000"]
453-
} {00000000000000000000000000000000 1 0}
453+
lappend rc [string equal $salt1 $salt2]
454+
} {0 0 1}
454455
db close
455456
file delete -force test.db
456457

0 commit comments

Comments
 (0)