From 2d5246e7fa26b82eeff8787ac281b97ed27b13f6 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Fri, 15 Jun 2012 08:56:57 +0900 Subject: [PATCH] protection improvements (2): Use ECB for DEK encryption, use IV, etc. --- ChangeLog | 18 ++++ NEWS | 21 +++-- polarssl-0.14.0/include/polarssl/aes.h | 2 + polarssl-0.14.0/library/aes.c | 2 + src/gnuk.h | 21 +++-- src/openpgp-do.c | 117 +++++++++++++++---------- src/random.c | 8 +- 7 files changed, 122 insertions(+), 67 deletions(-) diff --git a/ChangeLog b/ChangeLog index d7c39a2..6fe635d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2012-06-15 Niibe Yutaka + More protection improvements. + * src/random.c (RANDOM_BYTES_LENGTH): It's 32 now (was: 16). + * src/gnuk.h (struct key_data_internal): Remove check, random, + magic. Add checksum. + (struct prvkey_data): Remove crm_encrypted. Add iv and + checksum_encrypted. + * src/openpgp-do.c (encrypt, decrypt): Add IV argument. + (encrypt_dek, decrypt_dek): New. It's in ECB mode. + (compute_key_data_checksum): New. + (gpg_do_load_prvkey): Handle initial vector and checksum. + Use decrypt_dek to decrypt DEK. Clear DEK after use. + (calc_check32):Remove. + (gpg_do_write_prvkey): Use encrypt_dek to encrypt DEK. + (gpg_do_chks_prvkey): Likewise. + + * polarssl-0.14.0/include/polarssl/aes.h (aes_crypt_cbc) + * polarssl-0.14.0/library/aes.c (aes_crypt_cbc): ifdef-out. + * src/configure (--enable-pinpad): Deprecate DND. 2012-06-14 Niibe Yutaka diff --git a/NEWS b/NEWS index 6b6a4e8..21b3f0b 100644 --- a/NEWS +++ b/NEWS @@ -6,16 +6,23 @@ Gnuk NEWS - User visible changes ** Key generation feature added Finally, key generation is supported. Note that it is very slow. It -will takes a few minutes to generate two keys. +will take a few minutes (or more) to generate two or three keys, when +you are unlucky. + +** DnD pinentry support is deprecated +Once, DnD pinentry was considered a great feature, but it found that +it is difficult to remember moves of folders. ** KDF (Key Derivation Function) is now SHA-256 -Data encryption key for private keys are computed by KDF (Key -Derivation Function, sometimes also is refered as string to key -function, S2K). It was SHA1 before, but it is replaced by SHA-256. +Keystring is now computed by SHA-256 (it was SHA1 before). -** Protection improvement (even when data is disclosed) -Even if PW1 and Reset-code is same, content of encripted DEK is -different now. +** Protection improvements (even when internal data is disclosed) +Three improvements. (1) Even if PW1 and Reset-code is same, content +of encripted DEK is different now. (2) DEK is now encrypted and +decrypted by keystring in ECB mode (it was just a kind of xor by +single block CFB mode). (3) Key data plus checksum are encrypted in +CFB mode with initial vector (it will be able to switch OCB mode +easily). * Major changes in Gnuk 0.19 diff --git a/polarssl-0.14.0/include/polarssl/aes.h b/polarssl-0.14.0/include/polarssl/aes.h index 5576680..04d47ab 100644 --- a/polarssl-0.14.0/include/polarssl/aes.h +++ b/polarssl-0.14.0/include/polarssl/aes.h @@ -83,6 +83,7 @@ int aes_crypt_ecb( aes_context *ctx, const unsigned char input[16], unsigned char output[16] ); +#if 0 /** * \brief AES-CBC buffer encryption/decryption * Length should be a multiple of the block @@ -103,6 +104,7 @@ int aes_crypt_cbc( aes_context *ctx, unsigned char iv[16], const unsigned char *input, unsigned char *output ); +#endif /** * \brief AES-CFB128 buffer encryption/decryption. diff --git a/polarssl-0.14.0/library/aes.c b/polarssl-0.14.0/library/aes.c index ea4904c..58c1fb1 100644 --- a/polarssl-0.14.0/library/aes.c +++ b/polarssl-0.14.0/library/aes.c @@ -753,6 +753,7 @@ int aes_crypt_ecb( aes_context *ctx, return( 0 ); } +#if 0 /* * AES-CBC buffer encryption/decryption */ @@ -816,6 +817,7 @@ int aes_crypt_cbc( aes_context *ctx, return( 0 ); } +#endif /* * AES-CFB128 buffer encryption/decryption diff --git a/src/gnuk.h b/src/gnuk.h index c0fed78..76559ca 100644 --- a/src/gnuk.h +++ b/src/gnuk.h @@ -156,9 +156,9 @@ extern int flash_write_binary (uint8_t file_id, const uint8_t *data, uint16_t le extern uint8_t ch_certificate_start; extern uint8_t random_bits_start; -#define KEY_MAGIC_LEN 8 #define KEY_CONTENT_LEN 256 /* p and q */ -#define GNUK_MAGIC "Gnuk KEY" +#define INITIAL_VECTOR_SIZE 16 +#define DATA_ENCRYPTION_KEY_SIZE 16 /* encrypted data content */ struct key_data { @@ -167,22 +167,21 @@ struct key_data { struct key_data_internal { uint8_t data[KEY_CONTENT_LEN]; /* p and q */ - uint32_t check; - uint32_t random; - char magic[KEY_MAGIC_LEN]; + uint8_t checksum[DATA_ENCRYPTION_KEY_SIZE]; }; -#define ADDITIONAL_DATA_SIZE 16 -#define DATA_ENCRYPTION_KEY_SIZE 16 struct prvkey_data { const uint8_t *key_addr; /* - * CRM: [C]heck, [R]andom, and [M]agic in struct key_data_internal - * + * IV: Initial Vector */ - uint8_t crm_encrypted[ADDITIONAL_DATA_SIZE]; + uint8_t iv[INITIAL_VECTOR_SIZE]; /* - * DEK: Data Encryption Key + * Checksum + */ + uint8_t checksum_encrypted[DATA_ENCRYPTION_KEY_SIZE]; + /* + * DEK (Data Encryption Key) encrypted */ uint8_t dek_encrypted_1[DATA_ENCRYPTION_KEY_SIZE]; /* For user */ uint8_t dek_encrypted_2[DATA_ENCRYPTION_KEY_SIZE]; /* For resetcode */ diff --git a/src/openpgp-do.c b/src/openpgp-do.c index 07c564a..300b50f 100644 --- a/src/openpgp-do.c +++ b/src/openpgp-do.c @@ -572,40 +572,58 @@ proc_resetting_code (const uint8_t *data, int len) } static void -encrypt (const uint8_t *key_str, uint8_t *data, int len) +encrypt (const uint8_t *key, const uint8_t *iv, uint8_t *data, int len) { aes_context aes; - uint8_t iv[16]; + uint8_t iv0[INITIAL_VECTOR_SIZE]; int iv_offset; DEBUG_INFO ("ENC\r\n"); DEBUG_BINARY (data, len); - aes_setkey_enc (&aes, key_str, 128); - memset (iv, 0, 16); + aes_setkey_enc (&aes, key, 128); + memcpy (iv0, iv, INITIAL_VECTOR_SIZE); iv_offset = 0; - aes_crypt_cfb128 (&aes, AES_ENCRYPT, len, &iv_offset, iv, data, data); + aes_crypt_cfb128 (&aes, AES_ENCRYPT, len, &iv_offset, iv0, data, data); } /* Signing, Decryption, and Authentication */ struct key_data kd[3]; static void -decrypt (const uint8_t *key_str, uint8_t *data, int len) +decrypt (const uint8_t *key, const uint8_t *iv, uint8_t *data, int len) { aes_context aes; - uint8_t iv[16]; + uint8_t iv0[INITIAL_VECTOR_SIZE]; int iv_offset; - aes_setkey_enc (&aes, key_str, 128); - memset (iv, 0, 16); + aes_setkey_enc (&aes, key, 128); /* This is setkey_enc, because of CFB. */ + memcpy (iv0, iv, INITIAL_VECTOR_SIZE); iv_offset = 0; - aes_crypt_cfb128 (&aes, AES_DECRYPT, len, &iv_offset, iv, data, data); + aes_crypt_cfb128 (&aes, AES_DECRYPT, len, &iv_offset, iv0, data, data); DEBUG_INFO ("DEC\r\n"); DEBUG_BINARY (data, len); } +static void +encrypt_dek (const uint8_t *key_string, uint8_t *dek) +{ + aes_context aes; + + aes_setkey_enc (&aes, key_string, 128); + aes_crypt_ecb (&aes, AES_ENCRYPT, dek, dek); +} + +static void +decrypt_dek (const uint8_t *key_string, uint8_t *dek) +{ + aes_context aes; + + aes_setkey_dec (&aes, key_string, 128); + aes_crypt_ecb (&aes, AES_DECRYPT, dek, dek); +} + static uint8_t get_do_ptr_nr_for_kk (enum kind_of_key kk) { @@ -627,6 +645,25 @@ gpg_do_clear_prvkey (enum kind_of_key kk) memset ((void *)&kd[kk], 0, sizeof (struct key_data)); } + +static int +compute_key_data_checksum (struct key_data_internal *kdi, int check_or_calc) +{ + unsigned int i; + uint32_t d[4] = { 0, 0, 0, 0 }; + + for (i = 0; i < KEY_CONTENT_LEN / sizeof (uint32_t); i++) + d[i&3] ^= *(uint32_t *)(&kdi->data[i*4]); + + if (check_or_calc == 0) /* store */ + { + memcpy (kdi->checksum, d, DATA_ENCRYPTION_KEY_SIZE); + return 0; + } + else /* check */ + return memcmp (kdi->checksum, d, DATA_ENCRYPTION_KEY_SIZE) == 0; +} + /* * Return 1 on success, * 0 if none, @@ -637,8 +674,9 @@ gpg_do_load_prvkey (enum kind_of_key kk, int who, const uint8_t *keystring) { uint8_t nr = get_do_ptr_nr_for_kk (kk); const uint8_t *do_data = do_ptr[nr - NR_DO__FIRST__]; - uint8_t *key_addr; + const uint8_t *key_addr; uint8_t dek[DATA_ENCRYPTION_KEY_SIZE]; + const uint8_t *iv; struct key_data_internal kdi; DEBUG_INFO ("Loading private key: "); @@ -647,38 +685,27 @@ gpg_do_load_prvkey (enum kind_of_key kk, int who, const uint8_t *keystring) if (do_data == NULL) return 0; - key_addr = *(uint8_t **)&(do_data)[1]; + key_addr = *(const uint8_t **)&(do_data)[1]; /* Possible unaligned access */ memcpy (kdi.data, key_addr, KEY_CONTENT_LEN); - memcpy (((uint8_t *)&kdi.check), do_data+5, ADDITIONAL_DATA_SIZE); + iv = do_data+5; + memcpy (kdi.checksum, iv + INITIAL_VECTOR_SIZE, DATA_ENCRYPTION_KEY_SIZE); - memcpy (dek, do_data+5+16*who, DATA_ENCRYPTION_KEY_SIZE); - decrypt (keystring, dek, DATA_ENCRYPTION_KEY_SIZE); + memcpy (dek, do_data+5+16*(who+1), DATA_ENCRYPTION_KEY_SIZE); + decrypt_dek (keystring, dek); - decrypt (dek, (uint8_t *)&kdi, sizeof (struct key_data_internal)); - if (memcmp (kdi.magic, GNUK_MAGIC, KEY_MAGIC_LEN) != 0) + decrypt (dek, iv, (uint8_t *)&kdi, sizeof (struct key_data_internal)); + memset (dek, 0, DATA_ENCRYPTION_KEY_SIZE); + if (!compute_key_data_checksum (&kdi, 1)) { DEBUG_INFO ("gpg_do_load_prvkey failed.\r\n"); return -1; } - /* more sanity check??? */ memcpy (kd[kk].data, kdi.data, KEY_CONTENT_LEN); DEBUG_BINARY (&kd[kk], KEY_CONTENT_LEN); return 1; } -static uint32_t -calc_check32 (const uint8_t *p, int len) -{ - uint32_t check = 0; - uint32_t *data = (uint32_t *)p; - int i; - - for (i = 0; i < len/4; i++) - check += data[i]; - - return check; -} static int8_t num_prv_keys; @@ -691,7 +718,7 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len, int r; struct prvkey_data *pd; uint8_t *key_addr; - const uint8_t *dek; + const uint8_t *dek, *iv; const uint8_t *do_data = do_ptr[nr - NR_DO__FIRST__]; const uint8_t *ks_pw1; const uint8_t *ks_rc; @@ -738,18 +765,17 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len, DEBUG_WORD ((uint32_t)key_addr); memcpy (kdi.data, key_data, KEY_CONTENT_LEN); - kdi.check = calc_check32 (key_data, KEY_CONTENT_LEN); - kdi.random = get_salt (); - memcpy (kdi.magic, GNUK_MAGIC, KEY_MAGIC_LEN); + compute_key_data_checksum (&kdi, 0); - dek = random_bytes_get (); /* 16-byte random bytes */ + dek = random_bytes_get (); /* 32-byte random bytes */ + iv = dek + DATA_ENCRYPTION_KEY_SIZE; memcpy (pd->dek_encrypted_1, dek, DATA_ENCRYPTION_KEY_SIZE); memcpy (pd->dek_encrypted_2, dek, DATA_ENCRYPTION_KEY_SIZE); memcpy (pd->dek_encrypted_3, dek, DATA_ENCRYPTION_KEY_SIZE); ks_pw1 = gpg_do_read_simple (NR_DO_KEYSTRING_PW1); ks_rc = gpg_do_read_simple (NR_DO_KEYSTRING_RC); - encrypt (dek, (uint8_t *)&kdi, sizeof (struct key_data_internal)); + encrypt (dek, iv, (uint8_t *)&kdi, sizeof (struct key_data_internal)); r = flash_key_write (key_addr, kdi.data, modulus); if (modulus_allocated_here) @@ -763,7 +789,8 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len, } pd->key_addr = key_addr; - memcpy (pd->crm_encrypted, (uint8_t *)&kdi.check, ADDITIONAL_DATA_SIZE); + memcpy (pd->iv, iv, INITIAL_VECTOR_SIZE); + memcpy (pd->checksum_encrypted, kdi.checksum, DATA_ENCRYPTION_KEY_SIZE); if (kk == GPG_KEY_FOR_SIGNING) { @@ -774,7 +801,7 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len, ac_reset_other (); if (ks_pw1) - encrypt (ks_pw1+1, pd->dek_encrypted_1, DATA_ENCRYPTION_KEY_SIZE); + encrypt_dek (ks_pw1+1, pd->dek_encrypted_1); else { uint8_t ks123_pw1[KEYSTRING_SIZE_PW1]; @@ -782,16 +809,16 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len, ks123_pw1[0] = strlen (OPENPGP_CARD_INITIAL_PW1); sha256 ((uint8_t *)OPENPGP_CARD_INITIAL_PW1, strlen (OPENPGP_CARD_INITIAL_PW1), ks123_pw1+1); - encrypt (ks123_pw1+1, pd->dek_encrypted_1, DATA_ENCRYPTION_KEY_SIZE); + encrypt_dek (ks123_pw1+1, pd->dek_encrypted_1); } if (ks_rc) - encrypt (ks_rc+1, pd->dek_encrypted_2, DATA_ENCRYPTION_KEY_SIZE); + encrypt_dek (ks_rc+1, pd->dek_encrypted_2); else memset (pd->dek_encrypted_2, 0, DATA_ENCRYPTION_KEY_SIZE); if (keystring_admin) - encrypt (keystring_admin, pd->dek_encrypted_3, DATA_ENCRYPTION_KEY_SIZE); + encrypt_dek (keystring_admin, pd->dek_encrypted_3); else memset (pd->dek_encrypted_3, 0, DATA_ENCRYPTION_KEY_SIZE); @@ -842,11 +869,11 @@ gpg_do_chks_prvkey (enum kind_of_key kk, return -1; memcpy (pd, &(do_data)[1], sizeof (struct prvkey_data)); - dek_p = ((uint8_t *)pd) + 4 + ADDITIONAL_DATA_SIZE - + DATA_ENCRYPTION_KEY_SIZE * (who_old - 1); + dek_p = ((uint8_t *)pd) + 4 + INITIAL_VECTOR_SIZE + + DATA_ENCRYPTION_KEY_SIZE * who_old; memcpy (dek, dek_p, DATA_ENCRYPTION_KEY_SIZE); - decrypt (old_ks, dek, DATA_ENCRYPTION_KEY_SIZE); - encrypt (new_ks, dek, DATA_ENCRYPTION_KEY_SIZE); + decrypt_dek (old_ks, dek); + encrypt_dek (new_ks, dek); dek_p += DATA_ENCRYPTION_KEY_SIZE * (who_new - who_old); memcpy (dek_p, dek, DATA_ENCRYPTION_KEY_SIZE); diff --git a/src/random.c b/src/random.c index a2d5a79..200d471 100644 --- a/src/random.c +++ b/src/random.c @@ -1,7 +1,7 @@ /* * random.c -- get random bytes * - * Copyright (C) 2010, 2011 Free Software Initiative of Japan + * Copyright (C) 2010, 2011, 2012 Free Software Initiative of Japan * Author: NIIBE Yutaka * * This file is a part of Gnuk, a GnuPG USB Token implementation. @@ -26,7 +26,7 @@ #include "gnuk.h" #include "neug.h" -#define RANDOM_BYTES_LENGTH 16 +#define RANDOM_BYTES_LENGTH 32 static uint32_t random_word[RANDOM_BYTES_LENGTH/sizeof (uint32_t)]; void @@ -43,7 +43,7 @@ random_init (void) } /* - * Return pointer to random 16-byte + * Return pointer to random 32-byte */ const uint8_t * random_bytes_get (void) @@ -53,7 +53,7 @@ random_bytes_get (void) } /* - * Free pointer to random 16-byte + * Free pointer to random 32-byte */ void random_bytes_free (const uint8_t *p)