From cf7e9f94512d003d4e76a81a2d05d554718113e2 Mon Sep 17 00:00:00 2001 From: Stephen Lombardo Date: Fri, 21 Sep 2018 16:01:17 -0400 Subject: [PATCH] improved error handling for providers --- src/crypto.h | 2 +- src/crypto_cc.c | 16 ++++----- src/crypto_impl.c | 42 ++++++++++++----------- src/crypto_libtomcrypt.c | 9 ----- src/crypto_openssl.c | 73 ++++++++++++++++++++++++++-------------- 5 files changed, 80 insertions(+), 62 deletions(-) diff --git a/src/crypto.h b/src/crypto.h index 11576c97..1a9f8a8a 100644 --- a/src/crypto.h +++ b/src/crypto.h @@ -262,7 +262,7 @@ void sqlcipher_codec_set_store_pass(codec_ctx *ctx, int value); int sqlcipher_codec_fips_status(codec_ctx *ctx); const char* sqlcipher_codec_get_provider_version(codec_ctx *ctx); -int sqlcipher_codec_hmac(const codec_ctx *ctx, const unsigned char *hmac_key, int key_sz, +int sqlcipher_codec_hmac_sha1(const codec_ctx *ctx, const unsigned char *hmac_key, int key_sz, unsigned char* in, int in_sz, unsigned char *in2, int in2_sz, unsigned char *out); diff --git a/src/crypto_cc.c b/src/crypto_cc.c index d7a05191..7327ad67 100644 --- a/src/crypto_cc.c +++ b/src/crypto_cc.c @@ -43,7 +43,7 @@ static int sqlcipher_cc_add_random(void *ctx, void *buffer, int length) { /* generate a defined number of random bytes */ static int sqlcipher_cc_random (void *ctx, void *buffer, int length) { - return (SecRandomCopyBytes(kSecRandomDefault, length, (uint8_t *)buffer) == 0) ? SQLITE_OK : SQLITE_ERROR; + return (SecRandomCopyBytes(kSecRandomDefault, length, (uint8_t *)buffer) == kCCSuccess) ? SQLITE_OK : SQLITE_ERROR; } static const char* sqlcipher_cc_get_provider_name(void *ctx) { @@ -89,13 +89,13 @@ static int sqlcipher_cc_hmac(void *ctx, int algorithm, unsigned char *hmac_key, static int sqlcipher_cc_kdf(void *ctx, int algorithm, const unsigned char *pass, int pass_sz, unsigned char* salt, int salt_sz, int workfactor, int key_sz, unsigned char *key) { switch(algorithm) { case SQLCIPHER_HMAC_SHA1: - CCKeyDerivationPBKDF(kCCPBKDF2, (const char *)pass, pass_sz, salt, salt_sz, kCCPRFHmacAlgSHA1, workfactor, key, key_sz); + if(CCKeyDerivationPBKDF(kCCPBKDF2, (const char *)pass, pass_sz, salt, salt_sz, kCCPRFHmacAlgSHA1, workfactor, key, key_sz) != kCCSuccess) return SQLITE_ERROR; break; case SQLCIPHER_HMAC_SHA256: - CCKeyDerivationPBKDF(kCCPBKDF2, (const char *)pass, pass_sz, salt, salt_sz, kCCPRFHmacAlgSHA256, workfactor, key, key_sz); + if(CCKeyDerivationPBKDF(kCCPBKDF2, (const char *)pass, pass_sz, salt, salt_sz, kCCPRFHmacAlgSHA256, workfactor, key, key_sz) != kCCSuccess) return SQLITE_ERROR; break; case SQLCIPHER_HMAC_SHA512: - CCKeyDerivationPBKDF(kCCPBKDF2, (const char *)pass, pass_sz, salt, salt_sz, kCCPRFHmacAlgSHA512, workfactor, key, key_sz); + if(CCKeyDerivationPBKDF(kCCPBKDF2, (const char *)pass, pass_sz, salt, salt_sz, kCCPRFHmacAlgSHA512, workfactor, key, key_sz) != kCCSuccess) return SQLITE_ERROR; break; default: return SQLITE_ERROR; @@ -108,13 +108,13 @@ static int sqlcipher_cc_cipher(void *ctx, int mode, unsigned char *key, int key_ size_t tmp_csz, csz; CCOperation op = mode == CIPHER_ENCRYPT ? kCCEncrypt : kCCDecrypt; - CCCryptorCreate(op, kCCAlgorithmAES128, 0, key, kCCKeySizeAES256, iv, &cryptor); - CCCryptorUpdate(cryptor, in, in_sz, out, in_sz, &tmp_csz); + if(CCCryptorCreate(op, kCCAlgorithmAES128, 0, key, kCCKeySizeAES256, iv, &cryptor) != kCCSuccess) return SQLITE_ERROR; + if(CCCryptorUpdate(cryptor, in, in_sz, out, in_sz, &tmp_csz) != kCCSuccess) return SQLITE_ERROR; csz = tmp_csz; out += tmp_csz; - CCCryptorFinal(cryptor, out, in_sz - csz, &tmp_csz); + if(CCCryptorFinal(cryptor, out, in_sz - csz, &tmp_csz) != kCCSuccess) return SQLITE_ERROR; csz += tmp_csz; - CCCryptorRelease(cryptor); + if(CCCryptorRelease(cryptor) != kCCSuccess) return SQLITE_ERROR; assert(in_sz == csz); return SQLITE_OK; diff --git a/src/crypto_impl.c b/src/crypto_impl.c index f73ee721..9166d69d 100644 --- a/src/crypto_impl.c +++ b/src/crypto_impl.c @@ -969,12 +969,11 @@ static int sqlcipher_page_hmac(codec_ctx *ctx, cipher_ctx *c_ctx, Pgno pgno, uns /* 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 */ - ctx->provider->hmac( + return ctx->provider->hmac( ctx->provider_ctx, ctx->hmac_algorithm, c_ctx->hmac_key, ctx->key_sz, in, in_sz, (unsigned char*) &pgno_raw, sizeof(pgno), out); - return SQLITE_OK; } /* @@ -1007,22 +1006,20 @@ int sqlcipher_page_cipher(codec_ctx *ctx, int for_ctx, Pgno pgno, int mode, int /* the key size should never be zero. If it is, error out. */ if(ctx->key_sz == 0) { CODEC_TRACE("codec_cipher: error possible context corruption, key_sz is zero for pgno=%d\n", pgno); - sqlcipher_memset(out, 0, page_sz); - return SQLITE_ERROR; + goto error; } if(mode == CIPHER_ENCRYPT) { /* start at front of the reserve block, write random data to the end */ - if(ctx->provider->random(ctx->provider_ctx, iv_out, ctx->reserve_sz) != SQLITE_OK) return SQLITE_ERROR; + if(ctx->provider->random(ctx->provider_ctx, iv_out, ctx->reserve_sz) != SQLITE_OK) goto error; } else { /* CIPHER_DECRYPT */ memcpy(iv_out, iv_in, ctx->iv_sz); /* copy the iv from the input to output buffer */ } if((ctx->flags & CIPHER_FLAG_HMAC) && (mode == CIPHER_DECRYPT) && !ctx->skip_read_hmac) { if(sqlcipher_page_hmac(ctx, c_ctx, pgno, in, size + ctx->iv_sz, hmac_out) != SQLITE_OK) { - sqlcipher_memset(out, 0, page_sz); - CODEC_TRACE("codec_cipher: hmac operations failed for pgno=%d\n", pgno); - return SQLITE_ERROR; + CODEC_TRACE("codec_cipher: hmac operation on decrypt failed for pgno=%d\n", pgno); + goto error; } CODEC_TRACE("codec_cipher: comparing hmac on in=%p out=%p hmac_sz=%d\n", hmac_in, hmac_out, ctx->hmac_sz); @@ -1040,21 +1037,29 @@ int sqlcipher_page_cipher(codec_ctx *ctx, int for_ctx, Pgno pgno, int mode, int since the check failed, the page was either tampered with or corrupted. wipe the output buffer, and return SQLITE_ERROR to the caller */ CODEC_TRACE("codec_cipher: hmac check failed for pgno=%d returning SQLITE_ERROR\n", pgno); - sqlcipher_memset(out, 0, page_sz); - return SQLITE_ERROR; + goto error; } } } - ctx->provider->cipher(ctx->provider_ctx, mode, c_ctx->key, ctx->key_sz, iv_out, in, size, out); + if(ctx->provider->cipher(ctx->provider_ctx, mode, c_ctx->key, ctx->key_sz, iv_out, in, size, out) != SQLITE_OK) { + CODEC_TRACE("codec_cipher: cipher operation mode=%d failed for pgno=%d returning SQLITE_ERROR\n", mode, pgno); + goto error; + }; if((ctx->flags & CIPHER_FLAG_HMAC) && (mode == CIPHER_ENCRYPT)) { - sqlcipher_page_hmac(ctx, c_ctx, pgno, out_start, size + ctx->iv_sz, hmac_out); + if(sqlcipher_page_hmac(ctx, c_ctx, pgno, out_start, size + ctx->iv_sz, hmac_out) != SQLITE_OK) { + CODEC_TRACE("codec_cipher: hmac operation on encrypt failed for pgno=%d\n", pgno); + goto error; + }; } CODEC_HEXDUMP("codec_cipher: output page data", out_start, page_sz); return SQLITE_OK; +error: + sqlcipher_memset(out, 0, page_sz); + return SQLITE_ERROR; } /** @@ -1099,9 +1104,9 @@ static int sqlcipher_cipher_ctx_key_derive(codec_ctx *ctx, cipher_ctx *c_ctx) { cipher_hex2bin(z + (ctx->key_sz * 2), (ctx->kdf_salt_sz * 2), ctx->kdf_salt); } else { CODEC_TRACE("cipher_ctx_key_derive: deriving key using full PBKDF2 with %d iterations\n", c_ctx->kdf_iter); - ctx->provider->kdf(ctx->provider_ctx, ctx->kdf_algorithm, c_ctx->pass, c_ctx->pass_sz, + if(ctx->provider->kdf(ctx->provider_ctx, ctx->kdf_algorithm, c_ctx->pass, c_ctx->pass_sz, ctx->kdf_salt, ctx->kdf_salt_sz, ctx->kdf_iter, - ctx->key_sz, c_ctx->key); + ctx->key_sz, c_ctx->key) != SQLITE_OK) return SQLITE_ERROR; } /* set the context "keyspec" containing the hex-formatted key and salt to be used when attaching databases */ @@ -1127,9 +1132,9 @@ static int sqlcipher_cipher_ctx_key_derive(codec_ctx *ctx, cipher_ctx *c_ctx) { c_ctx->fast_kdf_iter); - ctx->provider->kdf(ctx->provider_ctx, ctx->kdf_algorithm, c_ctx->key, ctx->key_sz, + if(ctx->provider->kdf(ctx->provider_ctx, ctx->kdf_algorithm, c_ctx->key, ctx->key_sz, ctx->hmac_kdf_salt, ctx->kdf_salt_sz, ctx->fast_kdf_iter, - ctx->key_sz, c_ctx->hmac_key); + ctx->key_sz, c_ctx->hmac_key) != SQLITE_OK) return SQLITE_ERROR; } c_ctx->derive_key = 0; @@ -1465,11 +1470,10 @@ const char* sqlcipher_codec_get_provider_version(codec_ctx *ctx) { return ctx->provider->get_provider_version(ctx->provider_ctx); } -int sqlcipher_codec_hmac(const codec_ctx *ctx, const unsigned char *hmac_key, int key_sz, +int sqlcipher_codec_hmac_sha1(const codec_ctx *ctx, const unsigned char *hmac_key, int key_sz, unsigned char* in, int in_sz, unsigned char *in2, int in2_sz, unsigned char *out) { - ctx->provider->hmac(ctx->provider_ctx, SQLCIPHER_HMAC_SHA1, (unsigned char *)hmac_key, key_sz, in, in_sz, in2, in2_sz, out); - return SQLITE_OK; + return ctx->provider->hmac(ctx->provider_ctx, SQLCIPHER_HMAC_SHA1, (unsigned char *)hmac_key, key_sz, in, in_sz, in2, in2_sz, out); } diff --git a/src/crypto_libtomcrypt.c b/src/crypto_libtomcrypt.c index 52bcd72b..efb24fdc 100644 --- a/src/crypto_libtomcrypt.c +++ b/src/crypto_libtomcrypt.c @@ -175,9 +175,6 @@ static int sqlcipher_ltc_hmac(void *ctx, int algorithm, unsigned char *hmac_key, static int sqlcipher_ltc_kdf(void *ctx, int algorithm, const unsigned char *pass, int pass_sz, unsigned char* salt, int salt_sz, int workfactor, int key_sz, unsigned char *key) { int rc, hash_idx; unsigned long outlen = key_sz; - unsigned long random_buffer_sz = sizeof(char) * 256; - unsigned char *random_buffer = sqlcipher_malloc(random_buffer_sz); - sqlcipher_memset(random_buffer, 0, random_buffer_sz); switch(algorithm) { case SQLCIPHER_HMAC_SHA1: @@ -198,12 +195,6 @@ static int sqlcipher_ltc_kdf(void *ctx, int algorithm, const unsigned char *pass workfactor, hash_idx, key, &outlen)) != CRYPT_OK) { return SQLITE_ERROR; } - if((rc = pkcs_5_alg2(key, key_sz, salt, salt_sz, - 1, hash_idx, random_buffer, &random_buffer_sz)) != CRYPT_OK) { - return SQLITE_ERROR; - } - sqlcipher_ltc_add_random(ctx, random_buffer, random_buffer_sz); - sqlcipher_free(random_buffer, random_buffer_sz); return SQLITE_OK; } diff --git a/src/crypto_openssl.c b/src/crypto_openssl.c index 4dbb4176..e0017e3a 100644 --- a/src/crypto_openssl.c +++ b/src/crypto_openssl.c @@ -209,63 +209,86 @@ static int sqlcipher_openssl_random (void *ctx, void *buffer, int length) { static int sqlcipher_openssl_hmac(void *ctx, int algorithm, unsigned char *hmac_key, int key_sz, unsigned char *in, int in_sz, unsigned char *in2, int in2_sz, unsigned char *out) { unsigned int outlen; - HMAC_CTX* hctx = HMAC_CTX_new(); - if(hctx == NULL || in == NULL) return SQLITE_ERROR; + int rc = SQLITE_OK; + HMAC_CTX* hctx = NULL; + + if(in == NULL) goto error; + + hctx = HMAC_CTX_new(); + if(hctx == NULL) goto error; switch(algorithm) { case SQLCIPHER_HMAC_SHA1: - HMAC_Init_ex(hctx, hmac_key, key_sz, EVP_sha1(), NULL); + if(!HMAC_Init_ex(hctx, hmac_key, key_sz, EVP_sha1(), NULL)) goto error; break; case SQLCIPHER_HMAC_SHA256: - HMAC_Init_ex(hctx, hmac_key, key_sz, EVP_sha256(), NULL); - return EVP_MD_size(EVP_sha256());; + if(!HMAC_Init_ex(hctx, hmac_key, key_sz, EVP_sha256(), NULL)) goto error; break; case SQLCIPHER_HMAC_SHA512: - HMAC_Init_ex(hctx, hmac_key, key_sz, EVP_sha512(), NULL); + if(!HMAC_Init_ex(hctx, hmac_key, key_sz, EVP_sha512(), NULL)) goto error; break; default: - return SQLITE_ERROR; + goto error; } - HMAC_Update(hctx, in, in_sz); - if(in2 != NULL) HMAC_Update(hctx, in2, in2_sz); - HMAC_Final(hctx, out, &outlen); - HMAC_CTX_free(hctx); - return SQLITE_OK; + if(!HMAC_Update(hctx, in, in_sz)) goto error; + if(in2 != NULL) { + if(!HMAC_Update(hctx, in2, in2_sz)) goto error; + } + if(!HMAC_Final(hctx, out, &outlen)) goto error; + + goto cleanup; +error: + rc = SQLITE_ERROR; +cleanup: + if(hctx) HMAC_CTX_free(hctx); + return rc; } static int sqlcipher_openssl_kdf(void *ctx, int algorithm, const unsigned char *pass, int pass_sz, unsigned char* salt, int salt_sz, int workfactor, int key_sz, unsigned char *key) { + int rc = SQLITE_OK; + switch(algorithm) { case SQLCIPHER_HMAC_SHA1: - PKCS5_PBKDF2_HMAC((const char *)pass, pass_sz, salt, salt_sz, workfactor, EVP_sha1(), key_sz, key); + if(!PKCS5_PBKDF2_HMAC((const char *)pass, pass_sz, salt, salt_sz, workfactor, EVP_sha1(), key_sz, key)) goto error; break; case SQLCIPHER_HMAC_SHA256: - PKCS5_PBKDF2_HMAC((const char *)pass, pass_sz, salt, salt_sz, workfactor, EVP_sha256(), key_sz, key); + if(!PKCS5_PBKDF2_HMAC((const char *)pass, pass_sz, salt, salt_sz, workfactor, EVP_sha256(), key_sz, key)) goto error; break; case SQLCIPHER_HMAC_SHA512: - PKCS5_PBKDF2_HMAC((const char *)pass, pass_sz, salt, salt_sz, workfactor, EVP_sha512(), key_sz, key); + if(!PKCS5_PBKDF2_HMAC((const char *)pass, pass_sz, salt, salt_sz, workfactor, EVP_sha512(), key_sz, key)) goto error; break; default: return SQLITE_ERROR; } - return SQLITE_OK; + + goto cleanup; +error: + rc = SQLITE_ERROR; +cleanup: + return rc; } static int sqlcipher_openssl_cipher(void *ctx, int mode, unsigned char *key, int key_sz, unsigned char *iv, unsigned char *in, int in_sz, unsigned char *out) { - int tmp_csz, csz; + int tmp_csz, csz, rc = SQLITE_OK; EVP_CIPHER_CTX* ectx = EVP_CIPHER_CTX_new(); - if(ectx == NULL) return SQLITE_ERROR; - EVP_CipherInit_ex(ectx, ((openssl_ctx *)ctx)->evp_cipher, NULL, NULL, NULL, mode); - EVP_CIPHER_CTX_set_padding(ectx, 0); /* no padding */ - EVP_CipherInit_ex(ectx, NULL, NULL, key, iv, mode); - EVP_CipherUpdate(ectx, out, &tmp_csz, in, in_sz); + if(ectx == NULL) goto error; + if(!EVP_CipherInit_ex(ectx, ((openssl_ctx *)ctx)->evp_cipher, NULL, NULL, NULL, mode)) goto error; + if(!EVP_CIPHER_CTX_set_padding(ectx, 0)) goto error; /* no padding */ + if(!EVP_CipherInit_ex(ectx, NULL, NULL, key, iv, mode)) goto error; + if(!EVP_CipherUpdate(ectx, out, &tmp_csz, in, in_sz)) goto error; csz = tmp_csz; out += tmp_csz; - EVP_CipherFinal_ex(ectx, out, &tmp_csz); + if(!EVP_CipherFinal_ex(ectx, out, &tmp_csz)) goto error; csz += tmp_csz; - EVP_CIPHER_CTX_free(ectx); assert(in_sz == csz); - return SQLITE_OK; + + goto cleanup; +error: + rc = SQLITE_ERROR; +cleanup: + if(ectx) EVP_CIPHER_CTX_free(ectx); + return rc; } static const char* sqlcipher_openssl_get_cipher(void *ctx) {