From 07fb1e7b435ce005fe629be07d05de912d61307f Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Thu, 18 Feb 2010 18:04:47 +0000 Subject: [PATCH] Fix cipher state chaining in OpenSSL back end Make cipher state chaining work in the OpenSSL back end for des, des3, and arcfour enc providers. Subtleties: * DES and DES3 have checks to avoid clobbering ivec with uninitialized data if there is no data to encrypt. * Arcfour saves the OpenSSL cipher context across calls. To protect against a caller improperly copying the state (which happens to work with other enc providers), a loopback pointer is used, as in GSSAPI. * EVP_EncryptFinal_ex is unnecessary with stream ciphers and would interfere with cipher state chaining if it did anything, so just remove it. ticket: 6665 target_version: 1.8 tags: pullup git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@23734 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/crypto/openssl/enc_provider/des.c | 22 +++-- src/lib/crypto/openssl/enc_provider/des3.c | 23 +++-- src/lib/crypto/openssl/enc_provider/rc4.c | 106 ++++++++++++--------- 3 files changed, 90 insertions(+), 61 deletions(-) diff --git a/src/lib/crypto/openssl/enc_provider/des.c b/src/lib/crypto/openssl/enc_provider/des.c index a1a524537..bf56cf9fe 100644 --- a/src/lib/crypto/openssl/enc_provider/des.c +++ b/src/lib/crypto/openssl/enc_provider/des.c @@ -60,8 +60,8 @@ #define DES_KEY_BYTES 7 static krb5_error_code -validate(krb5_key key, const krb5_data *ivec, - const krb5_crypto_iov *data, size_t num_data) +validate(krb5_key key, const krb5_data *ivec, const krb5_crypto_iov *data, + size_t num_data, krb5_boolean *empty) { size_t i, input_length; @@ -78,6 +78,7 @@ validate(krb5_key key, const krb5_data *ivec, if (ivec && (ivec->length != 8)) return(KRB5_BAD_MSIZE); + *empty = (input_length == 0); return 0; } @@ -89,13 +90,13 @@ k5_des_encrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, unsigned char iblock[MIT_DES_BLOCK_LENGTH], oblock[MIT_DES_BLOCK_LENGTH]; struct iov_block_state input_pos, output_pos; EVP_CIPHER_CTX ciph_ctx; + krb5_boolean empty; IOV_BLOCK_STATE_INIT(&input_pos); IOV_BLOCK_STATE_INIT(&output_pos); - - ret = validate(key, ivec, data, num_data); - if (ret) + ret = validate(key, ivec, data, num_data, &empty); + if (ret != 0 || empty) return ret; EVP_CIPHER_CTX_init(&ciph_ctx); @@ -122,6 +123,9 @@ k5_des_encrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, &output_pos); } + if (ivec != NULL) + memcpy(ivec->data, oblock, MIT_DES_BLOCK_LENGTH); + EVP_CIPHER_CTX_cleanup(&ciph_ctx); zap(iblock, sizeof(iblock)); @@ -140,12 +144,13 @@ k5_des_decrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, unsigned char iblock[MIT_DES_BLOCK_LENGTH], oblock[MIT_DES_BLOCK_LENGTH]; struct iov_block_state input_pos, output_pos; EVP_CIPHER_CTX ciph_ctx; + krb5_boolean empty; IOV_BLOCK_STATE_INIT(&input_pos); IOV_BLOCK_STATE_INIT(&output_pos); - ret = validate(key, ivec, data, num_data); - if (ret) + ret = validate(key, ivec, data, num_data, &empty); + if (ret != 0 || empty) return ret; EVP_CIPHER_CTX_init(&ciph_ctx); @@ -172,6 +177,9 @@ k5_des_decrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, MIT_DES_BLOCK_LENGTH, &output_pos); } + if (ivec != NULL) + memcpy(ivec->data, iblock, MIT_DES_BLOCK_LENGTH); + EVP_CIPHER_CTX_cleanup(&ciph_ctx); zap(iblock, sizeof(iblock)); diff --git a/src/lib/crypto/openssl/enc_provider/des3.c b/src/lib/crypto/openssl/enc_provider/des3.c index 3f0d2139a..b62d6e4ee 100644 --- a/src/lib/crypto/openssl/enc_provider/des3.c +++ b/src/lib/crypto/openssl/enc_provider/des3.c @@ -59,8 +59,8 @@ #define DES_BLOCK_SIZE 8 static krb5_error_code -validate(krb5_key key, const krb5_data *ivec, - const krb5_crypto_iov *data, size_t num_data) +validate(krb5_key key, const krb5_data *ivec, const krb5_crypto_iov *data, + size_t num_data, krb5_boolean *empty) { size_t i, input_length; @@ -77,6 +77,7 @@ validate(krb5_key key, const krb5_data *ivec, if (ivec && (ivec->length != 8)) return(KRB5_BAD_MSIZE); + *empty = (input_length == 0); return 0; } @@ -88,9 +89,10 @@ k5_des3_encrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, unsigned char iblock[MIT_DES_BLOCK_LENGTH], oblock[MIT_DES_BLOCK_LENGTH]; struct iov_block_state input_pos, output_pos; EVP_CIPHER_CTX ciph_ctx; + krb5_boolean empty; - ret = validate(key, ivec, data, num_data); - if (ret) + ret = validate(key, ivec, data, num_data, &empty); + if (ret != 0 || empty) return ret; IOV_BLOCK_STATE_INIT(&input_pos); @@ -121,8 +123,8 @@ k5_des3_encrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, oblock, MIT_DES_BLOCK_LENGTH, &output_pos); } - /*if (ivec != NULL && ivec->data) - memcpy(ivec->data, oblock, MIT_DES_BLOCK_LENGTH); */ + if (ivec != NULL) + memcpy(ivec->data, oblock, MIT_DES_BLOCK_LENGTH); EVP_CIPHER_CTX_cleanup(&ciph_ctx); @@ -142,9 +144,10 @@ k5_des3_decrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, unsigned char iblock[MIT_DES_BLOCK_LENGTH], oblock[MIT_DES_BLOCK_LENGTH]; struct iov_block_state input_pos, output_pos; EVP_CIPHER_CTX ciph_ctx; + krb5_boolean empty; - ret = validate(key, ivec, data, num_data); - if (ret) + ret = validate(key, ivec, data, num_data, &empty); + if (ret != 0 || empty) return ret; IOV_BLOCK_STATE_INIT(&input_pos); @@ -175,8 +178,8 @@ k5_des3_decrypt(krb5_key key, const krb5_data *ivec, krb5_crypto_iov *data, &output_pos); } - /*if (ivec != NULL && ivec->data) - memcpy(ivec->data, oblock, MIT_DES_BLOCK_LENGTH); */ + if (ivec != NULL) + memcpy(ivec->data, iblock, MIT_DES_BLOCK_LENGTH); EVP_CIPHER_CTX_cleanup(&ciph_ctx); diff --git a/src/lib/crypto/openssl/enc_provider/rc4.c b/src/lib/crypto/openssl/enc_provider/rc4.c index 7ce9436bc..a41b2d652 100644 --- a/src/lib/crypto/openssl/enc_provider/rc4.c +++ b/src/lib/crypto/openssl/enc_provider/rc4.c @@ -40,19 +40,17 @@ #include #include -typedef struct -{ - EVP_CIPHER_CTX evp_ctx; - unsigned int x; - unsigned int y; - unsigned char state[256]; - -} ArcfourContext; - -typedef struct { - int initialized; - ArcfourContext ctx; -} ArcFourCipherState; +/* + * The loopback field is NULL if ctx is uninitialized (no encrypt or decrypt + * operation has taken place), or a pointer to the structure address if ctx is + * initialized. If the application copies the state (not a valid operation, + * but one which happens to works with some other enc providers), we can detect + * it via the loopback field and return a sane error code. + */ +struct arcfour_state { + struct arcfour_state *loopback; + EVP_CIPHER_CTX ctx; +}; #define RC4_KEY_SIZE 16 #define RC4_BLOCK_SIZE 1 @@ -76,59 +74,79 @@ k5_arcfour_docrypt(krb5_key key,const krb5_data *state, krb5_crypto_iov *data, size_t num_data) { size_t i; - int ret = 0, tmp_len = 0; - unsigned char *tmp_buf = NULL; + int ret = 1, tmp_len = 0; krb5_crypto_iov *iov = NULL; - EVP_CIPHER_CTX ciph_ctx; - - - EVP_CIPHER_CTX_init(&ciph_ctx); - - ret = EVP_EncryptInit_ex(&ciph_ctx, EVP_rc4(), NULL, key->keyblock.contents, NULL); - if (!ret){ - EVP_CIPHER_CTX_cleanup(&ciph_ctx); - return KRB5_CRYPTO_INTERNAL; + EVP_CIPHER_CTX ciph_ctx, *ctx; + struct arcfour_state *arcstate; + krb5_boolean do_init = TRUE; + + arcstate = (state != NULL) ? (struct arcfour_state *) state->data : NULL; + if (arcstate != NULL) { + ctx = &arcstate->ctx; + if (arcstate->loopback == arcstate) + do_init = FALSE; + else if (arcstate->loopback != NULL) + return KRB5_CRYPTO_INTERNAL; + } else { + ctx = &ciph_ctx; + } + if (do_init) { + EVP_CIPHER_CTX_init(ctx); + ret = EVP_EncryptInit_ex(ctx, EVP_rc4(), NULL, key->keyblock.contents, + NULL); + if (!ret) + return KRB5_CRYPTO_INTERNAL; } for (i = 0; i < num_data; i++) { iov = &data[i]; - if (iov->data.length <= 0) break; - tmp_len = iov->data.length; - if (ENCRYPT_IOV(iov)) { - tmp_buf=(unsigned char *)iov->data.data; - ret = EVP_EncryptUpdate(&ciph_ctx, - tmp_buf, &tmp_len, - (unsigned char *)iov->data.data, iov->data.length); - if (!ret) break; - iov->data.length = tmp_len; + ret = EVP_EncryptUpdate(ctx, + (unsigned char *) iov->data.data, &tmp_len, + (unsigned char *) iov->data.data, + iov->data.length); + if (!ret) + break; } } - if(ret) - ret = EVP_EncryptFinal_ex(&ciph_ctx, (unsigned char *)tmp_buf, &tmp_len); - EVP_CIPHER_CTX_cleanup(&ciph_ctx); + if (arcstate) /* Context is saved; mark as initialized. */ + arcstate->loopback = arcstate; + else /* Context is not saved; clean it up now. */ + EVP_CIPHER_CTX_cleanup(ctx); - if (ret != 1) + if (!ret) return KRB5_CRYPTO_INTERNAL; - iov->data.length += tmp_len; - return 0; } static krb5_error_code k5_arcfour_free_state ( krb5_data *state) { - return 0; /* not implemented */ + struct arcfour_state *arcstate = (struct arcfour_state *) state->data; + + /* Clean up the OpenSSL context if it was initialized. */ + if (arcstate && arcstate->loopback == arcstate) + EVP_CIPHER_CTX_cleanup(&arcstate->ctx); + free(arcstate); + return 0; } static krb5_error_code k5_arcfour_init_state (const krb5_keyblock *key, krb5_keyusage keyusage, krb5_data *new_state) { - return 0; /* not implemented */ - + struct arcfour_state *arcstate; + + /* Create a state structure with an uninitialized context. */ + arcstate = calloc(1, sizeof(*arcstate)); + if (arcstate == NULL) + return ENOMEM; + arcstate->loopback = NULL; + new_state->data = (char *) arcstate; + new_state->length = sizeof(*arcstate); + return 0; } /* Since the arcfour cipher is identical going forwards and backwards, @@ -147,6 +165,6 @@ const struct krb5_enc_provider krb5int_enc_arcfour = { k5_arcfour_docrypt, NULL, krb5int_arcfour_make_key, - k5_arcfour_init_state, /*xxx not implemented */ - k5_arcfour_free_state /*xxx not implemented */ + k5_arcfour_init_state, + k5_arcfour_free_state }; -- 2.26.2