From 056089811ff0447bae9d983efdf1f52295a2db3c Mon Sep 17 00:00:00 2001 From: Pekka Riikonen Date: Thu, 5 Dec 2002 21:38:16 +0000 Subject: [PATCH] Added checks for encryption/decryption lengths that they must be multiple by the block size, added checks for decryption errors. --- CHANGES | 19 +++++++++++++++++++ TODO | 2 ++ lib/silccore/silcpacket.c | 17 ++++++++++++----- lib/silccrypt/silccipher.c | 12 ++++++++++++ lib/silccrypt/silcpkcs.c | 21 +++++++++++++-------- 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/CHANGES b/CHANGES index 65f6be31..6b660f0d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,22 @@ +Thu Dec 5 22:29:46 EET 2002 Pekka Riikonen + + * Fixed backup router bugs: When backup resumes router and + receives a CHANNEL_MESSAGE packet the backup must not act + as router since the packet header decryption would be + different. Also, when relaying packets to channel, do + not re-encrypt packets on backup that came from the primary + since the connection isn't really router-router connection. + Affected files silcd/server.c, silcd/packet_send.c. + + * Added checks in encryption/decryption that encryption/decryption + length sent as argument really is multiple by block size. Helps + catching really weird bugs like the above backup router bugs + when packets are being decrypted in wrong way. Affected files + lib/silccore/silcpacket.c, and lib/silccrypt/silccipher.c. + + * Fixed padding generation in private key file encryption. + Affected file lib/silccrypt/silcpkcs.c. + Thu Dec 5 16:35:23 EET 2002 Pekka Riikonen * Added ignore_message_signatures setting which can be used diff --git a/TODO b/TODO index 1d5f3a69..15318131 100644 --- a/TODO +++ b/TODO @@ -19,6 +19,8 @@ TODO for SILC Server 1.0 TODO/bugs In SILC Libraries =========================== + o Remove the old private key file format support in silcpkcs.c. + o Test cases for all cryptographic primitive in lib/silccrypt/ o Test cases for all payload encoding and decoding routins in lib/silccore/ diff --git a/lib/silccore/silcpacket.c b/lib/silccore/silcpacket.c index 0c3bcb91..bd5319b4 100644 --- a/lib/silccore/silcpacket.c +++ b/lib/silccore/silcpacket.c @@ -83,8 +83,8 @@ void silc_packet_encrypt(SilcCipher cipher, SilcHmac hmac, SilcUInt32 sequence, /* Encrypt the data area of the packet. */ if (cipher) { - SILC_LOG_DEBUG(("Encrypting packet, cipher %s, len %d", - silc_cipher_get_name(cipher), len)); + SILC_LOG_DEBUG(("Encrypting packet (%d), cipher %s, len %d", + sequence, silc_cipher_get_name(cipher), len)); silc_cipher_encrypt(cipher, buffer->data, buffer->data, len, NULL); } @@ -501,8 +501,11 @@ static int silc_packet_decrypt(SilcCipher cipher, SilcHmac hmac, if (cipher) { /* Decrypt rest of the packet */ SILC_LOG_DEBUG(("Decrypting the packet")); - silc_cipher_decrypt(cipher, buffer->data, buffer->data, buffer->len, - NULL); + if (!silc_cipher_decrypt(cipher, buffer->data, buffer->data, + buffer->len, NULL)) { + SILC_LOG_ERROR(("silc_cipher_decrypt failed")); + return -1; + } } return 0; @@ -527,7 +530,11 @@ static int silc_packet_decrypt(SilcCipher cipher, SilcHmac hmac, return -1; } silc_buffer_pull(buffer, block_len); - silc_cipher_decrypt(cipher, buffer->data, buffer->data, len, NULL); + if (!silc_cipher_decrypt(cipher, buffer->data, buffer->data, + len, NULL)) { + SILC_LOG_ERROR(("silc_cipher_decrypt failed")); + return -1; + } } return 1; diff --git a/lib/silccrypt/silccipher.c b/lib/silccrypt/silccipher.c index 72d984b3..db4a3329 100644 --- a/lib/silccrypt/silccipher.c +++ b/lib/silccrypt/silccipher.c @@ -315,6 +315,11 @@ bool silc_cipher_encrypt(SilcCipher cipher, const unsigned char *src, unsigned char *dst, SilcUInt32 len, unsigned char *iv) { +#ifdef SILC_DEBUG + assert((len & (cipher->cipher->block_len - 1)) == 0); +#endif + if (len & (cipher->cipher->block_len - 1)) + return FALSE; return cipher->cipher->encrypt(cipher->context, src, dst, len, iv ? iv : cipher->iv); } @@ -325,6 +330,13 @@ bool silc_cipher_decrypt(SilcCipher cipher, const unsigned char *src, unsigned char *dst, SilcUInt32 len, unsigned char *iv) { +#if 0 /* Remove to 0.9.11 */ +#ifdef SILC_DEBUG + assert((len & (cipher->cipher->block_len - 1)) == 0); +#endif + if (len & (cipher->cipher->block_len - 1)) + return FALSE; +#endif return cipher->cipher->decrypt(cipher->context, src, dst, len, iv ? iv : cipher->iv); } diff --git a/lib/silccrypt/silcpkcs.c b/lib/silccrypt/silcpkcs.c index 6100cfea..bb2e65c8 100644 --- a/lib/silccrypt/silcpkcs.c +++ b/lib/silccrypt/silcpkcs.c @@ -1190,7 +1190,7 @@ static bool silc_pkcs_save_private_key_internal(const char *filename, SilcHash sha1; SilcHmac sha1hmac; SilcBuffer buf, enc; - SilcUInt32 len, blocklen; + SilcUInt32 len, blocklen, padlen; unsigned char tmp[32], keymat[64]; int i; @@ -1241,8 +1241,8 @@ static bool silc_pkcs_save_private_key_internal(const char *filename, /* Allocate buffer for encryption */ len = silc_hmac_len(sha1hmac); - enc = silc_buffer_alloc_size(data_len + 4 + 4 + - (blocklen + (data_len % blocklen)) + len); + padlen = 16 + (16 - ((data_len + 4) % blocklen)); + enc = silc_buffer_alloc_size(4 + 4 + data_len + padlen + len); if (!enc) { silc_hmac_free(sha1hmac); silc_hash_free(sha1); @@ -1251,7 +1251,7 @@ static bool silc_pkcs_save_private_key_internal(const char *filename, } /* Generate padding */ - for (i = 0; i < blocklen + (data_len % blocklen); i++) + for (i = 0; i < padlen; i++) tmp[i] = silc_rng_global_get_byte_fast(); /* Put magic number */ @@ -1262,8 +1262,7 @@ static bool silc_pkcs_save_private_key_internal(const char *filename, silc_buffer_format(enc, SILC_STR_UI_INT(data_len), SILC_STR_UI_XNSTRING(data, data_len), - SILC_STR_UI_XNSTRING(tmp, blocklen + (data_len % - blocklen)), + SILC_STR_UI_XNSTRING(tmp, padlen), SILC_STR_END); /* Encrypt. */ @@ -1578,6 +1577,7 @@ bool silc_pkcs_load_private_key(const char *filename, /* Old support */ struct MD5Context md5; bool oldsupport = FALSE; + int oldlen = 0; #endif /* 1 */ SILC_LOG_DEBUG(("Loading private key `%s' with %s encoding", filename, @@ -1729,8 +1729,13 @@ bool silc_pkcs_load_private_key(const char *filename, data += 4; len -= 4; +#if 1 + /* Old support */ + oldlen = len - mac_len; +#endif + /* Decrypt the private key buffer */ - silc_cipher_decrypt(aes, data, data, len - mac_len, silc_cipher_get_iv(aes)); + silc_cipher_decrypt(aes, data, data, len - mac_len, NULL); SILC_GET32_MSB(i, data); if (i > len) { SILC_LOG_DEBUG(("Bad private key length in buffer!")); @@ -1762,7 +1767,7 @@ bool silc_pkcs_load_private_key(const char *filename, #if 1 /* Old support */ - if (oldsupport) + if (oldsupport || (oldlen & 15)) silc_pkcs_save_private_key((char *)filename, *private_key, passphrase, passphrase_len, encoding); #endif -- 2.24.0