On 02/06/2013 10:39 PM, Andrew Beekhof wrote: > I admit to having missed this at the time, but doesn't this warrant a > little more by way of a commit message? > What was the rationale and where was the big warning that "Corosync > version X is no longer compatible with versions less than Y" in the > documentation? CVE-2013-0250 and it was also added to the 2.3.0 release notes by Honza. We didn't have the CVE at the time of commit/release, so we couldn't include it directly. Fabio > > On Mon, Jan 14, 2013 at 9:46 PM, Fabio M. Di Nitto <fdinitto@xxxxxxxxxx> wrote: >> From: "Fabio M. Di Nitto" <fdinitto@xxxxxxxxxx> >> >> Signed-off-by: Fabio M. Di Nitto <fdinitto@xxxxxxxxxx> >> --- >> conf/corosync.conf.example | 7 - >> conf/lenses/corosync.aug | 1 - >> conf/lenses/tests/test_corosync.aug | 2 - >> exec/coroparse.c | 8 - >> exec/main.c | 1 - >> exec/totemconfig.c | 14 -- >> exec/totemcrypto.c | 273 +++++++++-------------------------- >> exec/totemcrypto.h | 1 - >> exec/totemudp.c | 1 - >> exec/totemudpu.c | 1 - >> include/corosync/totem/totem.h | 2 - >> man/corosync.conf.5 | 13 -- >> 12 files changed, 65 insertions(+), 259 deletions(-) >> >> diff --git a/conf/corosync.conf.example b/conf/corosync.conf.example >> index 6ffb4cf..7121548 100644 >> --- a/conf/corosync.conf.example >> +++ b/conf/corosync.conf.example >> @@ -8,13 +8,6 @@ totem { >> crypto_cipher: none >> crypto_hash: none >> >> - # crypto_compat: 2.0|2.2 (default higher) can be used to change >> - # on-wire crypto packet format. Unless performing some special >> - # rolling upgrades from corosync < 2.2 to 2.2, to keep the cluster >> - # running, do not touch this option. This option cannot be changed >> - # at runtime. >> - #crypto_compat: 2.2 >> - >> # interface: define at least one interface to communicate >> # over. If you define more than one interface stanza, you must >> # also set rrp_mode. >> diff --git a/conf/lenses/corosync.aug b/conf/lenses/corosync.aug >> index cc2311c..1418c30 100644 >> --- a/conf/lenses/corosync.aug >> +++ b/conf/lenses/corosync.aug >> @@ -53,7 +53,6 @@ let totem = >> |kv "crypto_type" /nss|aes256|aes192|aes128|3des/ >> |kv "crypto_cipher" /none|nss|aes256|aes192|aes128|3des/ >> |kv "crypto_hash" /none|md5|sha1|sha256|sha384|sha512/ >> - |kv "crypto_compat" /2.0|2.2/ >> |kv "transport" /udp|iba/ >> |kv "version" Rx.integer >> |kv "nodeid" Rx.integer >> diff --git a/conf/lenses/tests/test_corosync.aug b/conf/lenses/tests/test_corosync.aug >> index 71e41bf..486b543 100644 >> --- a/conf/lenses/tests/test_corosync.aug >> +++ b/conf/lenses/tests/test_corosync.aug >> @@ -7,7 +7,6 @@ totem { >> secauth: off >> crypto_cipher: none >> crypto_hash: none >> - crypto_compat: 2.2 >> threads: 0 >> clear_node_high_bit: no >> rrp_mode: none >> @@ -97,7 +96,6 @@ test Corosync.lns get conf = >> { "secauth" = "off" } >> { "crypto_cipher" = "none" } >> { "crypto_hash" = "none" } >> - { "crypto_compat" = "2.2" } >> { "threads" = "0" } >> { "clear_node_high_bit" = "no" } >> { "rrp_mode" = "none" } >> diff --git a/exec/coroparse.c b/exec/coroparse.c >> index d5a50bb..da25225 100644 >> --- a/exec/coroparse.c >> +++ b/exec/coroparse.c >> @@ -543,14 +543,6 @@ static int main_config_parser_cb(const char *path, >> return (0); >> } >> } >> - if (strcmp(path, "totem.crypto_compat") == 0) { >> - if ((strcmp(value, "2.0") != 0) && >> - (strcmp(value, "2.2") != 0)) { >> - *error_string = "Invalid crypto compat type"; >> - >> - return (0); >> - } >> - } >> break; >> >> case MAIN_CP_CB_DATA_STATE_QB: >> diff --git a/exec/main.c b/exec/main.c >> index 6a5dc45..7707a9d 100644 >> --- a/exec/main.c >> +++ b/exec/main.c >> @@ -913,7 +913,6 @@ static void set_icmap_ro_keys_flag (void) >> */ >> icmap_set_ro_access("totem.crypto_cipher", CS_FALSE, CS_TRUE); >> icmap_set_ro_access("totem.crypto_hash", CS_FALSE, CS_TRUE); >> - icmap_set_ro_access("totem.crypto_compat", CS_FALSE, CS_TRUE); >> icmap_set_ro_access("totem.secauth", CS_FALSE, CS_TRUE); >> icmap_set_ro_access("totem.ip_version", CS_FALSE, CS_TRUE); >> icmap_set_ro_access("totem.rrp_mode", CS_FALSE, CS_TRUE); >> diff --git a/exec/totemconfig.c b/exec/totemconfig.c >> index fc833f5..3257dd4 100644 >> --- a/exec/totemconfig.c >> +++ b/exec/totemconfig.c >> @@ -119,11 +119,9 @@ static void totem_get_crypto(struct totem_config *totem_config) >> char *str; >> const char *tmp_cipher; >> const char *tmp_hash; >> - const char *tmp_compat; >> >> tmp_hash = "sha1"; >> tmp_cipher = "aes256"; >> - tmp_compat = "2.2"; >> >> if (icmap_get_string("totem.secauth", &str) == CS_OK) { >> if (strcmp (str, "off") == 0) { >> @@ -174,23 +172,11 @@ static void totem_get_crypto(struct totem_config *totem_config) >> free(str); >> } >> >> - if (icmap_get_string("totem.crypto_compat", &str) == CS_OK) { >> - if (strcmp(str, "2.0") == 0) { >> - tmp_compat = "2.0"; >> - } >> - if (strcmp(str, "2.2") == 0) { >> - tmp_compat = "2.2"; >> - } >> - free(str); >> - } >> - >> free(totem_config->crypto_cipher_type); >> free(totem_config->crypto_hash_type); >> - free(totem_config->crypto_compat_type); >> >> totem_config->crypto_cipher_type = strdup(tmp_cipher); >> totem_config->crypto_hash_type = strdup(tmp_hash); >> - totem_config->crypto_compat_type = strdup(tmp_compat); >> } >> >> static uint16_t generate_cluster_id (const char *cluster_name) >> diff --git a/exec/totemcrypto.c b/exec/totemcrypto.c >> index 90cbb24..69818b8 100644 >> --- a/exec/totemcrypto.c >> +++ b/exec/totemcrypto.c >> @@ -82,8 +82,8 @@ struct crypto_config_header { >> #endif >> >> /* >> - * while CRYPTO_CIPHER_TYPE_2_2 is not a real cipher at all, >> - * we still allocate a value for it because we use crypto_crypt_t >> + * while CRYPTO_CIPHER_TYPE_2_X are not a real cipher at all, >> + * we still allocate a value for them because we use crypto_crypt_t >> * internally and we don't want overlaps >> */ >> >> @@ -93,6 +93,7 @@ enum crypto_crypt_t { >> CRYPTO_CIPHER_TYPE_AES192 = 2, >> CRYPTO_CIPHER_TYPE_AES128 = 3, >> CRYPTO_CIPHER_TYPE_3DES = 4, >> + CRYPTO_CIPHER_TYPE_2_3 = UINT8_MAX - 1, >> CRYPTO_CIPHER_TYPE_2_2 = UINT8_MAX >> }; >> >> @@ -125,8 +126,8 @@ size_t cypher_block_len[] = { >> */ >> >> /* >> - * while CRYPTO_HASH_TYPE_2_2 is not a real hash mechanism at all, >> - * we still allocate a value for it because we use crypto_hash_t >> + * while CRYPTO_HASH_TYPE_2_X are not a real hash mechanism at all, >> + * we still allocate a value for them because we use crypto_hash_t >> * internally and we don't want overlaps >> */ >> >> @@ -137,6 +138,7 @@ enum crypto_hash_t { >> CRYPTO_HASH_TYPE_SHA256 = 3, >> CRYPTO_HASH_TYPE_SHA384 = 4, >> CRYPTO_HASH_TYPE_SHA512 = 5, >> + CRYPTO_HASH_TYPE_2_3 = UINT8_MAX - 1, >> CRYPTO_HASH_TYPE_2_2 = UINT8_MAX >> }; >> >> @@ -167,15 +169,6 @@ size_t hash_block_len[] = { >> SHA512_BLOCK_LENGTH /* CRYPTO_HASH_TYPE_SHA512 */ >> }; >> >> -/* >> - * crypto on-wire compat >> - */ >> - >> -enum crypto_compat_t { >> - CRYPTO_COMPAT_2_0 = 0, >> - CRYPTO_COMPAT_2_2 = 1 >> -}; >> - >> struct crypto_instance { >> PK11SymKey *nss_sym_key; >> PK11SymKey *nss_sym_key_sign; >> @@ -188,8 +181,6 @@ struct crypto_instance { >> >> enum crypto_hash_t crypto_hash_type; >> >> - enum crypto_compat_t crypto_compat_type; >> - >> unsigned int crypto_header_size; >> >> void (*log_printf_func) ( >> @@ -216,20 +207,6 @@ do { \ >> } while (0); >> >> /* >> - * compat functions >> - */ >> - >> -static int string_to_crypto_compat_type(const char* crypto_compat_type) >> -{ >> - if (strcmp(crypto_compat_type, "2.0") == 0) { >> - return CRYPTO_COMPAT_2_0; >> - } else if (strcmp(crypto_compat_type, "2.1") == 0) { >> - return CRYPTO_COMPAT_2_2; >> - } >> - return CRYPTO_COMPAT_2_2; >> -} >> - >> -/* >> * crypt/decrypt functions >> */ >> >> @@ -595,12 +572,11 @@ static int init_nss_db(struct crypto_instance *instance) >> >> static int init_nss(struct crypto_instance *instance, >> const char *crypto_cipher_type, >> - const char *crypto_hash_type, >> - const char *crypto_compat_type) >> + const char *crypto_hash_type) >> { >> log_printf(instance->log_level_notice, >> - "Initializing transmit/receive security (NSS) crypto: %s hash: %s compat: %s", >> - crypto_cipher_type, crypto_hash_type, crypto_compat_type); >> + "Initializing transmit/receive security (NSS) crypto: %s hash: %s", >> + crypto_cipher_type, crypto_hash_type); >> >> if (init_nss_db(instance) < 0) { >> return -1; >> @@ -617,31 +593,7 @@ static int init_nss(struct crypto_instance *instance, >> return 0; >> } >> >> -static int encrypt_and_sign_nss_2_0 ( >> - struct crypto_instance *instance, >> - const unsigned char *buf_in, >> - const size_t buf_in_len, >> - unsigned char *buf_out, >> - size_t *buf_out_len) >> -{ >> - unsigned char *hash = buf_out; >> - unsigned char *data = hash + hash_len[instance->crypto_hash_type]; >> - >> - if (encrypt_nss(instance, buf_in, buf_in_len, data, buf_out_len) < 0) { >> - return -1; >> - } >> - >> - if (hash_to_nss[instance->crypto_hash_type]) { >> - if (calculate_nss_hash(instance, data, *buf_out_len, hash) < 0) { >> - return -1; >> - } >> - *buf_out_len = *buf_out_len + hash_len[instance->crypto_hash_type]; >> - } >> - >> - return 0; >> -} >> - >> -static int encrypt_and_sign_nss_2_2 ( >> +static int encrypt_and_sign_nss_2_3 ( >> struct crypto_instance *instance, >> const unsigned char *buf_in, >> const size_t buf_in_len, >> @@ -666,38 +618,7 @@ static int encrypt_and_sign_nss_2_2 ( >> return 0; >> } >> >> -static int authenticate_and_decrypt_nss_2_0 ( >> - struct crypto_instance *instance, >> - unsigned char *buf, >> - int *buf_len) >> -{ >> - if (hash_to_nss[instance->crypto_hash_type]) { >> - unsigned char tmp_hash[hash_len[instance->crypto_hash_type]]; >> - unsigned char *hash = buf; >> - unsigned char *data = hash + hash_len[instance->crypto_hash_type]; >> - int datalen = *buf_len - hash_len[instance->crypto_hash_type]; >> - >> - if (calculate_nss_hash(instance, data, datalen, tmp_hash) < 0) { >> - return -1; >> - } >> - >> - if (memcmp(tmp_hash, hash, hash_len[instance->crypto_hash_type]) != 0) { >> - log_printf(instance->log_level_error, "Digest does not match"); >> - return -1; >> - } >> - >> - memmove(buf, data, datalen); >> - *buf_len = datalen; >> - } >> - >> - if (decrypt_nss(instance, buf, buf_len) < 0) { >> - return -1; >> - } >> - >> - return 0; >> -} >> - >> -static int authenticate_nss_2_2 ( >> +static int authenticate_nss_2_3 ( >> struct crypto_instance *instance, >> unsigned char *buf, >> int *buf_len) >> @@ -720,7 +641,7 @@ static int authenticate_nss_2_2 ( >> return 0; >> } >> >> -static int decrypt_nss_2_2 ( >> +static int decrypt_nss_2_3 ( >> struct crypto_instance *instance, >> unsigned char *buf, >> int *buf_len) >> @@ -765,7 +686,7 @@ size_t crypto_sec_header_size( >> * crypto_cipher_type | crypto_hash_type | __pad0 | __pad1 | hash | salt | data >> * only data is encrypted, hash only covers salt + data >> * >> - * 2.2 packet format >> + * 2.2/2.3 packet format >> * fake_crypto_cipher_type | fake_crypto_hash_type | __pad0 | __pad1 | salt | data | hash >> * only data is encrypted, hash covers the whole packet >> * >> @@ -784,35 +705,14 @@ int crypto_encrypt_and_sign ( >> struct crypto_config_header *cch = (struct crypto_config_header *)buf_out; >> int err; >> >> - switch (instance->crypto_compat_type) { >> - case CRYPTO_COMPAT_2_0: >> - cch->crypto_cipher_type = instance->crypto_cipher_type; >> - cch->crypto_hash_type = instance->crypto_hash_type; >> - cch->__pad0 = 0; >> - cch->__pad1 = 0; >> - >> - buf_out += sizeof(struct crypto_config_header); >> - >> - err = encrypt_and_sign_nss_2_0(instance, >> - buf_in, buf_in_len, >> - buf_out, buf_out_len); >> + cch->crypto_cipher_type = CRYPTO_CIPHER_TYPE_2_3; >> + cch->crypto_hash_type = CRYPTO_HASH_TYPE_2_3; >> + cch->__pad0 = 0; >> + cch->__pad1 = 0; >> >> - *buf_out_len = *buf_out_len + sizeof(struct crypto_config_header); >> - break; >> - case CRYPTO_COMPAT_2_2: >> - cch->crypto_cipher_type = CRYPTO_CIPHER_TYPE_2_2; >> - cch->crypto_hash_type = CRYPTO_HASH_TYPE_2_2; >> - cch->__pad0 = 0; >> - cch->__pad1 = 0; >> - >> - err = encrypt_and_sign_nss_2_2(instance, >> - buf_in, buf_in_len, >> - buf_out, buf_out_len); >> - break; >> - default: >> - err = -1; >> - break; >> - } >> + err = encrypt_and_sign_nss_2_3(instance, >> + buf_in, buf_in_len, >> + buf_out, buf_out_len); >> >> return err; >> } >> @@ -823,92 +723,51 @@ int crypto_authenticate_and_decrypt (struct crypto_instance *instance, >> { >> struct crypto_config_header *cch = (struct crypto_config_header *)buf; >> >> - switch (instance->crypto_compat_type) { >> - case CRYPTO_COMPAT_2_0: >> - >> - /* >> - * decode crypto config of incoming packets >> - */ >> - >> - if (cch->crypto_cipher_type != instance->crypto_cipher_type) { >> - log_printf(instance->log_level_security, >> - "Incoming packet has different crypto type. Rejecting"); >> - return -1; >> - } >> - >> - if (cch->crypto_hash_type != instance->crypto_hash_type) { >> - log_printf(instance->log_level_security, >> - "Incoming packet has different hash type. Rejecting"); >> - return -1; >> - } >> - >> - if ((cch->__pad0 != 0) || (cch->__pad1 != 0)) { >> - log_printf(instance->log_level_security, >> - "Incoming packet appears to have features not supported by this version of corosync. Rejecting"); >> - return -1; >> - } >> - >> - /* >> - * invalidate config header and kill it >> - */ >> - >> - cch = NULL; >> - *buf_len -= sizeof(struct crypto_config_header); >> - memmove(buf, buf + sizeof(struct crypto_config_header), *buf_len); >> - >> - return authenticate_and_decrypt_nss_2_0(instance, buf, buf_len); >> - break; >> - case CRYPTO_COMPAT_2_2: >> - if (cch->crypto_cipher_type != CRYPTO_CIPHER_TYPE_2_2) { >> - log_printf(instance->log_level_security, >> - "Incoming packet has different crypto type. Rejecting"); >> - return -1; >> - } >> - >> - if (cch->crypto_hash_type != CRYPTO_HASH_TYPE_2_2) { >> - log_printf(instance->log_level_security, >> - "Incoming packet has different hash type. Rejecting"); >> - return -1; >> - } >> - >> - /* >> - * authenticate packet first >> - */ >> - >> - if (authenticate_nss_2_2(instance, buf, buf_len) != 0) { >> - return -1; >> - } >> - >> - /* >> - * now we can "trust" the padding bytes/future features >> - */ >> - >> - if ((cch->__pad0 != 0) || (cch->__pad1 != 0)) { >> - log_printf(instance->log_level_security, >> - "Incoming packet appears to have features not supported by this version of corosync. Rejecting"); >> - return -1; >> - } >> - >> - /* >> - * decrypt >> - */ >> - >> - if (decrypt_nss_2_2(instance, buf, buf_len) != 0) { >> - return -1; >> - } >> - >> - /* >> - * invalidate config header and kill it >> - */ >> - cch = NULL; >> - memmove(buf, buf + sizeof(struct crypto_config_header), *buf_len); >> - >> - return 0; >> - break; >> - default: >> - return -1; >> - break; >> + if (cch->crypto_cipher_type != CRYPTO_CIPHER_TYPE_2_3) { >> + log_printf(instance->log_level_security, >> + "Incoming packet has different crypto type. Rejecting"); >> + return -1; >> + } >> + >> + if (cch->crypto_hash_type != CRYPTO_HASH_TYPE_2_3) { >> + log_printf(instance->log_level_security, >> + "Incoming packet has different hash type. Rejecting"); >> + return -1; >> + } >> + >> + /* >> + * authenticate packet first >> + */ >> + >> + if (authenticate_nss_2_3(instance, buf, buf_len) != 0) { >> + return -1; >> + } >> + >> + /* >> + * now we can "trust" the padding bytes/future features >> + */ >> + >> + if ((cch->__pad0 != 0) || (cch->__pad1 != 0)) { >> + log_printf(instance->log_level_security, >> + "Incoming packet appears to have features not supported by this version of corosync. Rejecting"); >> + return -1; >> } >> + >> + /* >> + * decrypt >> + */ >> + >> + if (decrypt_nss_2_3(instance, buf, buf_len) != 0) { >> + return -1; >> + } >> + >> + /* >> + * invalidate config header and kill it >> + */ >> + cch = NULL; >> + memmove(buf, buf + sizeof(struct crypto_config_header), *buf_len); >> + >> + return 0; >> } >> >> struct crypto_instance *crypto_init( >> @@ -916,7 +775,6 @@ struct crypto_instance *crypto_init( >> unsigned int private_key_len, >> const char *crypto_cipher_type, >> const char *crypto_hash_type, >> - const char *crypto_compat_type, >> void (*log_printf_func) ( >> int level, >> int subsys, >> @@ -942,7 +800,6 @@ struct crypto_instance *crypto_init( >> >> instance->crypto_cipher_type = string_to_crypto_cipher_type(crypto_cipher_type); >> instance->crypto_hash_type = string_to_crypto_hash_type(crypto_hash_type); >> - instance->crypto_compat_type = string_to_crypto_compat_type(crypto_compat_type); >> >> instance->crypto_header_size = crypto_sec_header_size(crypto_cipher_type, crypto_hash_type); >> >> @@ -952,7 +809,7 @@ struct crypto_instance *crypto_init( >> instance->log_level_error = log_level_error; >> instance->log_subsys_id = log_subsys_id; >> >> - if (init_nss(instance, crypto_cipher_type, crypto_hash_type, crypto_compat_type) < 0) { >> + if (init_nss(instance, crypto_cipher_type, crypto_hash_type) < 0) { >> free(instance); >> return(NULL); >> } >> diff --git a/exec/totemcrypto.h b/exec/totemcrypto.h >> index 4577050..7c06c39 100644 >> --- a/exec/totemcrypto.h >> +++ b/exec/totemcrypto.h >> @@ -61,7 +61,6 @@ extern struct crypto_instance *crypto_init( >> unsigned int private_key_len, >> const char *crypto_cipher_type, >> const char *crypto_hash_type, >> - const char *crypto_compat_type, >> void (*log_printf_func) ( >> int level, >> int subsys, >> diff --git a/exec/totemudp.c b/exec/totemudp.c >> index 5208961..a5169c2 100644 >> --- a/exec/totemudp.c >> +++ b/exec/totemudp.c >> @@ -1148,7 +1148,6 @@ int totemudp_initialize ( >> totem_config->private_key_len, >> totem_config->crypto_cipher_type, >> totem_config->crypto_hash_type, >> - totem_config->crypto_compat_type, >> instance->totemudp_log_printf, >> instance->totemudp_log_level_security, >> instance->totemudp_log_level_notice, >> diff --git a/exec/totemudpu.c b/exec/totemudpu.c >> index 14163c5..12ec63c 100644 >> --- a/exec/totemudpu.c >> +++ b/exec/totemudpu.c >> @@ -779,7 +779,6 @@ int totemudpu_initialize ( >> totem_config->private_key_len, >> totem_config->crypto_cipher_type, >> totem_config->crypto_hash_type, >> - totem_config->crypto_compat_type, >> instance->totemudpu_log_printf, >> instance->totemudpu_log_level_security, >> instance->totemudpu_log_level_notice, >> diff --git a/include/corosync/totem/totem.h b/include/corosync/totem/totem.h >> index 7ef8652..72a3e1a 100644 >> --- a/include/corosync/totem/totem.h >> +++ b/include/corosync/totem/totem.h >> @@ -176,8 +176,6 @@ struct totem_config { >> >> char *crypto_hash_type; >> >> - char *crypto_compat_type; >> - >> totem_transport_t transport_number; >> >> unsigned int miss_count_const; >> diff --git a/man/corosync.conf.5 b/man/corosync.conf.5 >> index 43a339b..becbf86 100644 >> --- a/man/corosync.conf.5 >> +++ b/man/corosync.conf.5 >> @@ -181,19 +181,6 @@ Valid values are none (no encryption), aes256, aes192, aes128 and 3des. >> The default is aes256. >> >> .TP >> -crypto_compat >> -This specifies which crypto protocol version should be used to encrypt all messages. >> -Valid values are 2.0 and 2.2. >> - >> -The default is always is the higher supported version. >> - >> -This value should only be used when performing rolling upgrades from older >> -versions of corosync to newer ones. It cannot be changed at runtime. >> - >> -Once the upgrade is completed, the cluster should be temporary halted to >> -switch to the latest version of the protocol. >> - >> -.TP >> secauth >> This specifies that HMAC/SHA1 authentication should be used to authenticate >> all messages. It further specifies that all data should be encrypted with the >> -- >> 1.7.7.6 >> >> _______________________________________________ >> discuss mailing list >> discuss@xxxxxxxxxxxx >> http://lists.corosync.org/mailman/listinfo/discuss _______________________________________________ discuss mailing list discuss@xxxxxxxxxxxx http://lists.corosync.org/mailman/listinfo/discuss