From 823a099e5ecfad4b9b3c710e27a69562c4312ee6 Mon Sep 17 00:00:00 2001 From: Stephen Lombardo Date: Tue, 27 Dec 2011 11:30:54 -0500 Subject: [PATCH] improvements to error handling --- src/crypto.c | 6 ++++- src/crypto_impl.c | 58 +++++++++++++++++++++++++++++++---------------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/crypto.c b/src/crypto.c index eddcd04..dfc3deb 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -148,7 +148,11 @@ void* sqlite3Codec(void *iCtx, void *data, Pgno pgno, int mode) { void *kdf_salt = sqlcipher_codec_ctx_get_kdf_salt(ctx); CODEC_TRACE(("sqlite3Codec: entered pgno=%d, mode=%d, page_sz=%d\n", pgno, mode, page_sz)); - sqlcipher_codec_key_derive(ctx); /* call to derive keys if not present yet */ + /* call to derive keys if not present yet */ + if((rc = sqlcipher_codec_key_derive(ctx)) != SQLITE_OK) { + sqlcipher_codec_ctx_set_error(ctx, rc); + return NULL; + } if(pgno == 1) offset = FILE_HEADER_SZ; /* adjust starting pointers in data page for header offset on first page*/ diff --git a/src/crypto_impl.c b/src/crypto_impl.c index 67d15fa..e6db124 100644 --- a/src/crypto_impl.c +++ b/src/crypto_impl.c @@ -189,17 +189,21 @@ int sqlcipher_cipher_ctx_set_pass(cipher_ctx *ctx, const void *zKey, int nKey) { int sqlcipher_codec_ctx_set_pass(codec_ctx *ctx, const void *zKey, int nKey, int for_ctx) { cipher_ctx *c_ctx = for_ctx ? ctx->write_ctx : ctx->read_ctx; + int rc; - sqlcipher_cipher_ctx_set_pass(c_ctx, zKey, nKey); + if((rc = sqlcipher_cipher_ctx_set_pass(c_ctx, zKey, nKey)) != SQLITE_OK) return rc; c_ctx->derive_key = 1; - /* FIXME: return value of copy */ - if(for_ctx == 2) sqlcipher_cipher_ctx_copy( for_ctx ? ctx->read_ctx : ctx->write_ctx, c_ctx); + 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_cipher(codec_ctx *ctx, const char *cipher_name, int for_ctx) { cipher_ctx *c_ctx = for_ctx ? ctx->write_ctx : ctx->read_ctx; + int rc; c_ctx->evp_cipher = (EVP_CIPHER *) EVP_get_cipherbyname(cipher_name); c_ctx->key_sz = EVP_CIPHER_key_length(c_ctx->evp_cipher); @@ -208,18 +212,24 @@ int sqlcipher_codec_ctx_set_cipher(codec_ctx *ctx, const char *cipher_name, int c_ctx->hmac_sz = EVP_MD_size(EVP_sha1()); c_ctx->derive_key = 1; - if(for_ctx == 2) sqlcipher_cipher_ctx_copy( for_ctx ? ctx->read_ctx : ctx->write_ctx, c_ctx); + 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_kdf_iter(codec_ctx *ctx, int kdf_iter, int for_ctx) { cipher_ctx *c_ctx = for_ctx ? ctx->write_ctx : ctx->read_ctx; + int rc; c_ctx->kdf_iter = kdf_iter; c_ctx->derive_key = 1; - if(for_ctx == 2) sqlcipher_cipher_ctx_copy( for_ctx ? ctx->read_ctx : ctx->write_ctx, c_ctx); + 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; } @@ -315,15 +325,15 @@ int sqlcipher_codec_ctx_init(codec_ctx **iCtx, Db *pDb, Pager *pPager, sqlite3_f if(sqlcipher_random(ctx->kdf_salt, FILE_HEADER_SZ) != 1) return SQLITE_ERROR; } - sqlcipher_codec_ctx_set_cipher(ctx, CIPHER, 0); - sqlcipher_codec_ctx_set_kdf_iter(ctx, PBKDF2_ITER, 0); - sqlcipher_codec_ctx_set_pass(ctx, zKey, nKey, 0); + 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_pass(ctx, zKey, nKey, 0)) != SQLITE_OK) return rc; /* Use HMAC signatures by default. Note that codec_set_use_hmac will implicity call codec_set_page_size to set the default */ if((rc = sqlcipher_codec_ctx_set_use_hmac(ctx, DEFAULT_USE_HMAC)) != SQLITE_OK) return rc; - sqlcipher_cipher_ctx_copy(ctx->write_ctx, ctx->read_ctx); + if((rc = sqlcipher_cipher_ctx_copy(ctx->write_ctx, ctx->read_ctx)) != SQLITE_OK) return rc; return SQLITE_OK; } @@ -345,16 +355,17 @@ void sqlcipher_codec_ctx_free(codec_ctx **iCtx) { int sqlcipher_page_hmac(cipher_ctx *ctx, Pgno pgno, unsigned char *in, int in_sz, unsigned char *out) { HMAC_CTX hctx; HMAC_CTX_init(&hctx); - HMAC_Init_ex(&hctx, ctx->hmac_key, ctx->key_sz, EVP_sha1(), NULL); + + if(!HMAC_Init_ex(&hctx, ctx->hmac_key, ctx->key_sz, EVP_sha1(), NULL)) return SQLITE_ERROR; /* include the encrypted page data, initialization vector, and page number in HMAC. This will prevent both tampering with the ciphertext, manipulation of the IV, or resequencing otherwise valid pages out of order in a database */ - HMAC_Update(&hctx, in, in_sz); - HMAC_Update(&hctx, (const unsigned char*) &pgno, sizeof(Pgno)); - HMAC_Final(&hctx, out, NULL); + if(!HMAC_Update(&hctx, in, in_sz)) return SQLITE_ERROR; + if(!HMAC_Update(&hctx, (const unsigned char*) &pgno, sizeof(Pgno))) return SQLITE_ERROR; + if(!HMAC_Final(&hctx, out, NULL)) return SQLITE_ERROR; HMAC_CTX_cleanup(&hctx); - return SQLITE_OK; /* FIXME: check for errors in HMAC routine to be safe */ + return SQLITE_OK; } /* @@ -369,7 +380,7 @@ int sqlcipher_page_cipher(codec_ctx *ctx, int for_ctx, Pgno pgno, int mode, int cipher_ctx *c_ctx = for_ctx ? ctx->write_ctx : ctx->read_ctx; EVP_CIPHER_CTX ectx; unsigned char *iv_in, *iv_out, *hmac_in, *hmac_out, *out_start; - int tmp_csz, csz, size; + int tmp_csz, csz, size, rc; /* calculate some required positions into various buffers */ size = page_sz - c_ctx->reserve_sz; /* adjust size to useable size and memset reserve at end of page */ @@ -399,7 +410,11 @@ int sqlcipher_page_cipher(codec_ctx *ctx, int for_ctx, Pgno pgno, int mode, int } if(c_ctx->use_hmac && (mode == CIPHER_DECRYPT)) { - sqlcipher_page_hmac(c_ctx, pgno, in, size + c_ctx->iv_sz, hmac_out); + if(sqlcipher_page_hmac(c_ctx, pgno, in, size + c_ctx->iv_sz, hmac_out) != SQLITE_OK) { + memset(out, 0, page_sz); + CODEC_TRACE(("codec_cipher: hmac operations failed for pgno=%d\n", pgno)); + return SQLITE_ERROR; + } CODEC_TRACE(("codec_cipher: comparing hmac on in=%d out=%d hmac_sz=%d\n", hmac_in, hmac_out, c_ctx->hmac_sz)); if(sqlcipher_memcmp(hmac_in, hmac_out, c_ctx->hmac_sz) != 0) { @@ -478,19 +493,22 @@ int sqlcipher_cipher_ctx_key_derive(codec_ctx *ctx, cipher_ctx *c_ctx) { } int sqlcipher_codec_key_derive(codec_ctx *ctx) { + int rc; + /* derive key on first use if necessary */ if(ctx->read_ctx->derive_key) { - sqlcipher_cipher_ctx_key_derive(ctx, ctx->read_ctx); + if(sqlcipher_cipher_ctx_key_derive(ctx, ctx->read_ctx) != SQLITE_OK) return SQLITE_ERROR; } if(ctx->write_ctx->derive_key) { if(sqlcipher_cipher_ctx_cmp(ctx->write_ctx, ctx->read_ctx) == 0) { - sqlcipher_cipher_ctx_copy(ctx->write_ctx, ctx->read_ctx); // the relevant parameters are the same, just copy read key + // the relevant parameters are the same, just copy read key + if(sqlcipher_cipher_ctx_copy(ctx->write_ctx, ctx->read_ctx) != SQLITE_OK) return SQLITE_ERROR; } else { - sqlcipher_cipher_ctx_key_derive(ctx, ctx->write_ctx); + if(sqlcipher_cipher_ctx_key_derive(ctx, ctx->write_ctx) != SQLITE_OK) return SQLITE_ERROR; } } - return SQLITE_OK; /* FIXME set proper return value */ + return SQLITE_OK; } int sqlcipher_codec_key_copy(codec_ctx *ctx, int source) {