Hi Steve,
thanks for the review, but I will work out another patch.
This one has something that doesn´t feel right in my head.
Fabio
On 3/14/2012 3:44 AM, Steven Dake wrote:
> Reviewed-by: Steven Dake <sdake@xxxxxxxxxx>
>
> On 03/13/2012 10:25 AM, Fabio M. Di Nitto wrote:
>> From: "Fabio M. Di Nitto" <fdinitto@xxxxxxxxxx>
>>
>> keep totem.secauth config key for compatibility
>>
>> if the key is NOT set, crypto will default to aes256/sha1
>> if the key is set to "off", crypto is disabled.
>> this reflects pretty much old behavior
>>
>> keywords totem.crypto_cipher and totem.crypto_hash can
>> override secauth individually.
>>
>> it is not yet possible to enable encryption without hashing
>> (totemcrypt will fail) but it is planned in the next patchset.
>>
>> this one will restore basic operations for developers.
>>
>> Signed-off-by: Fabio M. Di Nitto <fdinitto@xxxxxxxxxx>
>> ---
>> exec/totemconfig.c | 9 +---
>> exec/totemcrypto.c | 106 +++++++++++++++++++++++++---------------
>> exec/totemudp.c | 90 +++++++++++++++-------------------
>> exec/totemudpu.c | 90 +++++++++++++++-------------------
>> include/corosync/totem/totem.h | 2 -
>> 5 files changed, 148 insertions(+), 149 deletions(-)
>>
>> diff --git a/exec/totemconfig.c b/exec/totemconfig.c
>> index 931bd7a..1138963 100644
>> --- a/exec/totemconfig.c
>> +++ b/exec/totemconfig.c
>> @@ -129,11 +129,9 @@ static void totem_get_crypto(struct totem_config *totem_config)
>>
>> tmp_hash = "sha1";
>> tmp_cipher = "aes256";
>> - totem_config->secauth = 1;
>>
>> if (icmap_get_string("totem.secauth", &str) == CS_OK) {
>> if (strcmp (str, "off") == 0) {
>> - totem_config->secauth = 0;
>> tmp_hash = "none";
>> tmp_cipher = "none";
>> }
>> @@ -160,10 +158,6 @@ static void totem_get_crypto(struct totem_config *totem_config)
>> free(str);
>> }
>>
>> - if (strcmp(tmp_hash, "none") == 0 && strcmp(tmp_cipher, "none") == 0) {
>> - totem_config->secauth = 0;
>> - }
>> -
>> free(totem_config->crypto_cipher_type);
>> free(totem_config->crypto_hash_type);
>>
>> @@ -1019,7 +1013,8 @@ int totem_config_keyread (
>> memset (totem_config->private_key, 0, 128);
>> totem_config->private_key_len = 128;
>>
>> - if (totem_config->secauth == 0) {
>> + if (strcmp(totem_config->crypto_cipher_type, "none") == 0 &&
>> + strcmp(totem_config->crypto_hash_type, "none") == 0) {
>> return (0);
>> }
>>
>> diff --git a/exec/totemcrypto.c b/exec/totemcrypto.c
>> index 8c0ec91..b2141a8 100644
>> --- a/exec/totemcrypto.c
>> +++ b/exec/totemcrypto.c
>> @@ -145,39 +145,27 @@ do { \
>>
>> static void init_nss_crypto(struct crypto_instance *instance)
>> {
>> - PK11SlotInfo* aes_slot = NULL;
>> - PK11SlotInfo* sha1_slot = NULL;
>> + PK11SlotInfo* crypt_slot = NULL;
>> + PK11SlotInfo* hash_slot = NULL;
>> SECItem key_item;
>> - SECStatus rv;
>> +
>> + if ((!cipher_to_nss[instance->crypto_cipher_type]) &&
>> + (!hash_to_nss[instance->crypto_hash_type])) {
>> + log_printf(instance->log_level_notice,
>> + "Initializing transmit/receive security: NONE");
>> + return;
>> + }
>>
>> log_printf(instance->log_level_notice,
>> "Initializing transmit/receive security: NSS AES256CBC/SHA1HMAC (mode %u).", 0);
>> - rv = NSS_NoDB_Init(".");
>> - if (rv != SECSuccess)
>> - {
>> - log_printf(instance->log_level_security, "NSS initialization failed (err %d)",
>> - PR_GetError());
>> - goto out;
>> - }
>>
>> - /*
>> - * TODO: use instance info!
>> - */
>> - aes_slot = PK11_GetBestSlot(cipher_to_nss[instance->crypto_cipher_type], NULL);
>> - if (aes_slot == NULL)
>> + if (NSS_NoDB_Init(".") != SECSuccess)
>> {
>> - log_printf(instance->log_level_security, "Unable to find security slot (err %d)",
>> + log_printf(instance->log_level_security, "NSS initialization failed (err %d)",
>> PR_GetError());
>> goto out;
>> }
>>
>> - sha1_slot = PK11_GetBestSlot(hash_to_nss[instance->crypto_hash_type], NULL);
>> - if (sha1_slot == NULL)
>> - {
>> - log_printf(instance->log_level_security, "Unable to find security slot (err %d)",
>> - PR_GetError());
>> - goto out;
>> - }
>> /*
>> * Make the private key into a SymKey that we can use
>> */
>> @@ -185,26 +173,46 @@ static void init_nss_crypto(struct crypto_instance *instance)
>> key_item.data = instance->private_key;
>> key_item.len = cipher_key_len[instance->crypto_cipher_type];
>>
>> - instance->nss_sym_key = PK11_ImportSymKey(aes_slot,
>> - cipher_to_nss[instance->crypto_cipher_type],
>> - PK11_OriginUnwrap, CKA_ENCRYPT|CKA_DECRYPT,
>> - &key_item, NULL);
>> - if (instance->nss_sym_key == NULL)
>> - {
>> - log_printf(instance->log_level_security, "Failure to import key into NSS (err %d)",
>> - PR_GetError());
>> - goto out;
>> + if (cipher_to_nss[instance->crypto_cipher_type]) {
>> + crypt_slot = PK11_GetBestSlot(cipher_to_nss[instance->crypto_cipher_type], NULL);
>> + if (crypt_slot == NULL)
>> + {
>> + log_printf(instance->log_level_security, "Unable to find security slot (err %d)",
>> + PR_GetError());
>> + goto out;
>> + }
>> + instance->nss_sym_key = PK11_ImportSymKey(crypt_slot,
>> + cipher_to_nss[instance->crypto_cipher_type],
>> + PK11_OriginUnwrap, CKA_ENCRYPT|CKA_DECRYPT,
>> + &key_item, NULL);
>> + if (instance->nss_sym_key == NULL)
>> + {
>> + log_printf(instance->log_level_security, "Failure to import key into NSS (err %d)",
>> + PR_GetError());
>> + goto out;
>> + }
>> }
>>
>> - instance->nss_sym_key_sign = PK11_ImportSymKey(sha1_slot,
>> - hash_to_nss[instance->crypto_hash_type],
>> - PK11_OriginUnwrap, CKA_SIGN,
>> - &key_item, NULL);
>> - if (instance->nss_sym_key_sign == NULL) {
>> - log_printf(instance->log_level_security, "Failure to import key into NSS (err %d)",
>> - PR_GetError());
>> - goto out;
>> + if (hash_to_nss[instance->crypto_hash_type]) {
>> + hash_slot = PK11_GetBestSlot(hash_to_nss[instance->crypto_hash_type], NULL);
>> + if (hash_slot == NULL)
>> + {
>> + log_printf(instance->log_level_security, "Unable to find security slot (err %d)",
>> + PR_GetError());
>> + goto out;
>> + }
>> +
>> + instance->nss_sym_key_sign = PK11_ImportSymKey(hash_slot,
>> + hash_to_nss[instance->crypto_hash_type],
>> + PK11_OriginUnwrap, CKA_SIGN,
>> + &key_item, NULL);
>> + if (instance->nss_sym_key_sign == NULL) {
>> + log_printf(instance->log_level_security, "Failure to import key into NSS (err %d)",
>> + PR_GetError());
>> + goto out;
>> + }
>> }
>> +
>> out:
>> return;
>> }
>> @@ -447,6 +455,16 @@ int crypto_encrypt_and_sign (
>> unsigned char *buf_out,
>> size_t *buf_out_len)
>> {
>> + /*
>> + * if crypto is totally disabled, let's skip complex parsing
>> + */
>> + if ((!cipher_to_nss[instance->crypto_cipher_type]) &&
>> + (!hash_to_nss[instance->crypto_hash_type])) {
>> + memcpy(buf_out, buf_in, buf_in_len);
>> + *buf_out_len = buf_in_len;
>> + return 0;
>> + }
>> +
>> return (encrypt_and_sign_nss(instance, buf_in, buf_in_len, buf_out, buf_out_len));
>> }
>>
>> @@ -454,6 +472,14 @@ int crypto_authenticate_and_decrypt (struct crypto_instance *instance,
>> unsigned char *buf,
>> int *buf_len)
>> {
>> + /*
>> + * if crypto is totally disabled, there is no work for us
>> + */
>> + if ((!cipher_to_nss[instance->crypto_cipher_type]) &&
>> + (!hash_to_nss[instance->crypto_hash_type])) {
>> + return 0;
>> + }
>> +
>> return (authenticate_and_decrypt_nss(instance, buf, buf_len));
>> }
>>
>> diff --git a/exec/totemudp.c b/exec/totemudp.c
>> index 21a6122..0ccc55b 100644
>> --- a/exec/totemudp.c
>> +++ b/exec/totemudp.c
>> @@ -259,26 +259,24 @@ static inline void ucast_sendmsg (
>> struct iovec iovec;
>> int addrlen;
>>
>> - if (instance->totem_config->secauth == 1) {
>> + /*
>> + * Encrypt and digest the message
>> + */
>> + if (crypto_encrypt_and_sign (
>> + instance->crypto_inst,
>> + (const unsigned char *)msg,
>> + msg_len,
>> + buf_out,
>> + &buf_out_len) != 0) {
>> /*
>> - * Encrypt and digest the message
>> + * TODO: how to handle error here
>> */
>> - if (crypto_encrypt_and_sign (
>> - instance->crypto_inst,
>> - (const unsigned char *)msg,
>> - msg_len,
>> - buf_out,
>> - &buf_out_len) != 0) {
>> - log_printf(LOGSYS_LEVEL_CRIT, "Unable to crypt? now what?");
>> - }
>> -
>> - iovec.iov_base = (void *)buf_out;
>> - iovec.iov_len = buf_out_len;
>> - } else {
>> - iovec.iov_base = (void *)msg;
>> - iovec.iov_len = msg_len;
>> + log_printf(LOGSYS_LEVEL_CRIT, "Unable to crypt? now what?");
>> }
>>
>> + iovec.iov_base = (void *)buf_out;
>> + iovec.iov_len = buf_out_len;
>> +
>> /*
>> * Build unicast message
>> */
>> @@ -323,26 +321,24 @@ static inline void mcast_sendmsg (
>> struct sockaddr_storage sockaddr;
>> int addrlen;
>>
>> - if (instance->totem_config->secauth == 1) {
>> + /*
>> + * Encrypt and digest the message
>> + */
>> + if (crypto_encrypt_and_sign (
>> + instance->crypto_inst,
>> + (const unsigned char *)msg,
>> + msg_len,
>> + buf_out,
>> + &buf_out_len) != 0) {
>> /*
>> - * Encrypt and digest the message
>> + * TODO: how to handle error here
>> */
>> - if (crypto_encrypt_and_sign (
>> - instance->crypto_inst,
>> - (const unsigned char *)msg,
>> - msg_len,
>> - buf_out,
>> - &buf_out_len) != 0) {
>> - log_printf(LOGSYS_LEVEL_CRIT, "unable to crypt? now what?");
>> - }
>> -
>> - iovec.iov_base = (void *)&buf_out;
>> - iovec.iov_len = buf_out_len;
>> - } else {
>> - iovec.iov_base = (void *)msg;
>> - iovec.iov_len = msg_len;
>> + log_printf(LOGSYS_LEVEL_CRIT, "unable to crypt? now what?");
>> }
>>
>> + iovec.iov_base = (void *)&buf_out;
>> + iovec.iov_len = buf_out_len;
>> +
>> /*
>> * Build multicast message
>> */
>> @@ -443,18 +439,16 @@ static int net_deliver_fn (
>> instance->stats_recv += bytes_received;
>> }
>>
>> - if (instance->totem_config->secauth == 1) {
>> - /*
>> - * Authenticate and if authenticated, decrypt datagram
>> - */
>> - res = crypto_authenticate_and_decrypt (instance->crypto_inst, iovec->iov_base, &bytes_received);
>> - if (res == -1) {
>> - log_printf (instance->totemudp_log_level_security, "Received message has invalid digest... ignoring.");
>> - log_printf (instance->totemudp_log_level_security,
>> - "Invalid packet data");
>> - iovec->iov_len = FRAME_SIZE_MAX;
>> - return 0;
>> - }
>> + /*
>> + * Authenticate and if authenticated, decrypt datagram
>> + */
>> + res = crypto_authenticate_and_decrypt (instance->crypto_inst, iovec->iov_base, &bytes_received);
>> + if (res == -1) {
>> + log_printf (instance->totemudp_log_level_security, "Received message has invalid digest... ignoring.");
>> + log_printf (instance->totemudp_log_level_security,
>> + "Invalid packet data");
>> + iovec->iov_len = FRAME_SIZE_MAX;
>> + return 0;
>> }
>> iovec->iov_len = bytes_received;
>>
>> @@ -1179,12 +1173,8 @@ extern int totemudp_iface_check (void *udp_context)
>> extern void totemudp_net_mtu_adjust (void *udp_context, struct totem_config *totem_config)
>> {
>> #define UDPIP_HEADER_SIZE (20 + 8) /* 20 bytes for ip 8 bytes for udp */
>> - if (totem_config->secauth == 1) {
>> - totem_config->net_mtu -= crypto_sec_header_size(totem_config->crypto_hash_type) +
>> - UDPIP_HEADER_SIZE;
>> - } else {
>> - totem_config->net_mtu -= UDPIP_HEADER_SIZE;
>> - }
>> + totem_config->net_mtu -= crypto_sec_header_size(totem_config->crypto_hash_type) +
>> + UDPIP_HEADER_SIZE;
>> }
>>
>> const char *totemudp_iface_print (void *udp_context) {
>> diff --git a/exec/totemudpu.c b/exec/totemudpu.c
>> index be4ca50..7d1d31c 100644
>> --- a/exec/totemudpu.c
>> +++ b/exec/totemudpu.c
>> @@ -247,26 +247,24 @@ static inline void ucast_sendmsg (
>> struct iovec iovec;
>> int addrlen;
>>
>> - if (instance->totem_config->secauth == 1) {
>> + /*
>> + * Encrypt and digest the message
>> + */
>> + if (crypto_encrypt_and_sign (
>> + instance->crypto_inst,
>> + (const unsigned char *)msg,
>> + msg_len,
>> + buf_out,
>> + &buf_out_len) != 0) {
>> /*
>> - * Encrypt and digest the message
>> + * TODO: how to handle error here
>> */
>> - if (crypto_encrypt_and_sign (
>> - instance->crypto_inst,
>> - (const unsigned char *)msg,
>> - msg_len,
>> - buf_out,
>> - &buf_out_len) != 0) {
>> - log_printf(LOGSYS_LEVEL_CRIT, "unable to crypt? now what?");
>> - }
>> -
>> - iovec.iov_base = (void *)buf_out;
>> - iovec.iov_len = buf_out_len;
>> - } else {
>> - iovec.iov_base = (void *)msg;
>> - iovec.iov_len = msg_len;
>> + log_printf(LOGSYS_LEVEL_CRIT, "unable to crypt? now what?");
>> }
>>
>> + iovec.iov_base = (void *)buf_out;
>> + iovec.iov_len = buf_out_len;
>> +
>> /*
>> * Build unicast message
>> */
>> @@ -312,26 +310,24 @@ static inline void mcast_sendmsg (
>> struct list_head *list;
>> struct totemudpu_member *member;
>>
>> - if (instance->totem_config->secauth == 1) {
>> + /*
>> + * Encrypt and digest the message
>> + */
>> + if (crypto_encrypt_and_sign (
>> + instance->crypto_inst,
>> + (const unsigned char *)msg,
>> + msg_len,
>> + buf_out,
>> + &buf_out_len) != 0) {
>> /*
>> - * Encrypt and digest the message
>> + * TODO: how to handle error here
>> */
>> - if(crypto_encrypt_and_sign (
>> - instance->crypto_inst,
>> - (const unsigned char *)msg,
>> - msg_len,
>> - buf_out,
>> - &buf_out_len) != 0) {
>> - log_printf(LOGSYS_LEVEL_CRIT, "Unable to crypt? now what?");
>> - }
>> -
>> - iovec.iov_base = (void *)buf_out;
>> - iovec.iov_len = buf_out_len;
>> - } else {
>> - iovec.iov_base = (void *)msg;
>> - iovec.iov_len = msg_len;
>> + log_printf(LOGSYS_LEVEL_CRIT, "Unable to crypt? now what?");
>> }
>>
>> + iovec.iov_base = (void *)buf_out;
>> + iovec.iov_len = buf_out_len;
>> +
>> /*
>> * Build multicast message
>> */
>> @@ -422,19 +418,17 @@ static int net_deliver_fn (
>> instance->stats_recv += bytes_received;
>> }
>>
>> - if (instance->totem_config->secauth == 1) {
>> - /*
>> - * Authenticate and if authenticated, decrypt datagram
>> - */
>> + /*
>> + * Authenticate and if authenticated, decrypt datagram
>> + */
>>
>> - res = crypto_authenticate_and_decrypt (instance->crypto_inst, iovec->iov_base, &bytes_received);
>> - if (res == -1) {
>> - log_printf (instance->totemudpu_log_level_security, "Received message has invalid digest... ignoring.");
>> - log_printf (instance->totemudpu_log_level_security,
>> - "Invalid packet data");
>> - iovec->iov_len = FRAME_SIZE_MAX;
>> - return 0;
>> - }
>> + res = crypto_authenticate_and_decrypt (instance->crypto_inst, iovec->iov_base, &bytes_received);
>> + if (res == -1) {
>> + log_printf (instance->totemudpu_log_level_security, "Received message has invalid digest... ignoring.");
>> + log_printf (instance->totemudpu_log_level_security,
>> + "Invalid packet data");
>> + iovec->iov_len = FRAME_SIZE_MAX;
>> + return 0;
>> }
>> iovec->iov_len = bytes_received;
>>
>> @@ -884,12 +878,8 @@ extern int totemudpu_iface_check (void *udpu_context)
>> extern void totemudpu_net_mtu_adjust (void *udpu_context, struct totem_config *totem_config)
>> {
>> #define UDPIP_HEADER_SIZE (20 + 8) /* 20 bytes for ip 8 bytes for udp */
>> - if (totem_config->secauth == 1) {
>> - totem_config->net_mtu -= crypto_sec_header_size(totem_config->crypto_hash_type) +
>> - UDPIP_HEADER_SIZE;
>> - } else {
>> - totem_config->net_mtu -= UDPIP_HEADER_SIZE;
>> - }
>> + totem_config->net_mtu -= crypto_sec_header_size(totem_config->crypto_hash_type) +
>> + UDPIP_HEADER_SIZE;
>> }
>>
>> const char *totemudpu_iface_print (void *udpu_context) {
>> diff --git a/include/corosync/totem/totem.h b/include/corosync/totem/totem.h
>> index eba3850..08a459e 100644
>> --- a/include/corosync/totem/totem.h
>> +++ b/include/corosync/totem/totem.h
>> @@ -151,8 +151,6 @@ struct totem_config {
>>
>> struct totem_logging_configuration totem_logging_configuration;
>>
>> - unsigned int secauth;
>> -
>> unsigned int net_mtu;
>>
>> unsigned int threads;
>
> _______________________________________________
> discuss mailing list
> discuss@xxxxxxxxxxxx
> http://lists.corosync.org/mailman/listinfo/discuss
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
