Il Wed, Jul 19, 2006 at 05:55:17PM +0200, Rafael J. Wysocki ha scritto:
> The libgcrypt's AES seems to be significantly slower than the openssl's
> Blowfish, but well. Also IMHO libgcrypt is less convenient than openssl
> and the documentation sucks.
Agree...
Btw, it should be made very clear that the user has to regenerate his
RSA key after the update. I think that if the old key is left in place
the image will be encrypted with a random key, since generate_key()
will leave key_data->key uninitialized. See below.
Also I find the code hard to read especially due to the "allocate one
big buffer and use it multiple times for different stuff"... I know that
mlockall() may cause future allocations to fail but we are talking about
a couple of pages at most.
> Index: suspend/keygen.c
> ===================================================================
> --- suspend.orig/keygen.c 2006-07-19 17:23:54.000000000 +0200
> +++ suspend/keygen.c 2006-07-19 17:24:17.000000000 +0200
> @@ -55,18 +66,51 @@ int main(int argc, char *argv[])
> } while (len < MIN_KEY_BITS || len > MAX_KEY_BITS);
>
> Retry:
> - printf("Generating %d-bit RSA keys.\n", len);
> - rsa = RSA_generate_key(len, RSA_F4, NULL, NULL);
> - if (!rsa) {
> - fprintf(stderr, "Key generation failed.\n");
> + printf("Generating %d-bit RSA keys. Please wait.\n", len);
> + ret = gcry_ac_key_pair_generate(rsa_hd, len, NULL, &rsa_key_pair, NULL);
> + if (ret) {
> + fprintf(stderr, "Key generation failed: %s\n",
> gcry_strerror(ret));
> ret = EXIT_FAILURE;
> - goto Free_RSA;
> + goto Close_RSA;
> }
> - if (RSA_check_key(rsa) <= 0) {
> + printf("Testing the private key. Please wait.\n");
> + rsa_priv = gcry_ac_key_pair_extract(rsa_key_pair, GCRY_AC_KEY_SECRET);
> + ret = gcry_ac_key_test(rsa_hd, rsa_priv);
> + if (ret) {
> printf("RSA key test failed. Retrying.\n");
> goto Retry;
> }
>
> + rsa_data_set = gcry_ac_key_data_get(rsa_priv);
> + if (gcry_ac_data_length(rsa_data_set) != RSA_FIELDS) {
> + fprintf(stderr, "Wrong number of key fields\n");
> + ret = -EINVAL;
> + goto Free_RSA;
> + }
> +
> + /* Convert the key length into bytes */
> + size = (len + 7) >> 3;
> + /* Copy the public key components to struct RSA_data */
> + offset = 0;
> + for (j = 0; j < RSA_FIELDS_PUB; j++) {
> + char *str;
> + size_t s;
Is the order of exponents and primes guaranteed to be always the same?
Maybe it's better to get them using their names.
> +
> + if (offset + size >= RSA_DATA_SIZE)
> + goto Free_RSA;
> +
> + gcry_ac_data_get_index(rsa_data_set, GCRY_AC_FLAG_COPY, j,
> + (const char **)&str, &mpi);
> + gcry_mpi_print(GCRYMPI_FMT_USG, rsa.data + offset,
> + size, &s, mpi);
> + rsa.field[j][0] = str[0];
> + rsa.field[j][1] = '\0';
> + rsa.size[j] = s;
> + offset += s;
> + gcry_mpi_release(mpi);
> + gcry_free(str);
> + }
> +
> tcgetattr(0, &termios);
> termios.c_lflag &= ~ECHO;
> termios.c_lflag |= ICANON | ECHONL;
> --- suspend.orig/suspend.c 2006-07-19 17:23:54.000000000 +0200
> +++ suspend/suspend.c 2006-07-19 17:24:17.000000000 +0200
> @@ -521,31 +521,51 @@ int write_image(int snapshot_fd, int res
> #ifdef CONFIG_ENCRYPT
> if (encrypt) {
> if (use_RSA) {
> - BF_set_key(&handle.key, KEY_SIZE,
> key_data->key);
> - memcpy(handle.ivec, key_data->ivec, IVEC_SIZE);
> - handle.num = 0;
> - header->image_flags |= IMAGE_ENCRYPTED |
> IMAGE_USE_RSA;
> - memcpy(&header->rsa_data, &key_data->rsa_data,
> - sizeof(struct RSA_data));
> - memcpy(&header->key_data, key_data + 1,
> - sizeof(struct encrypted_key));
> + error = gcry_cipher_setkey(cipher_handle,
> + key_data->key, KEY_SIZE);
key_data->key is initialized in generate_key() which may fail silently.
> + if (!error)
> + error = gcry_cipher_setiv(cipher_handle,
> + key_data->ivec, CIPHER_BLOCK);
> +
> + if (!error) {
> + struct encrypted_key *key;
> +
> + header->image_flags |= IMAGE_ENCRYPTED |
> + IMAGE_USE_RSA;
> + memcpy(&header->rsa, &key_data->rsa,
> + sizeof(struct RSA_data));
> + key = (struct encrypted_key *)(key_data
> + 1);
> + memcpy(&header->key, key,
> + sizeof(struct encrypted_key));
> + }
> @@ -918,10 +938,14 @@ static inline void close_swappiness(void
> #ifdef CONFIG_ENCRYPT
> static void generate_key(void)
> {
> - RSA *rsa;
> - int fd, rnd_fd;
> - struct RSA_data *rsa_data;
> + gcry_ac_handle_t rsa_hd;
> + gcry_ac_data_t rsa_data_set, key_set;
> + gcry_ac_key_t rsa_pub;
> + gcry_mpi_t mpi;
> + int ret, fd, rnd_fd;
> + struct RSA_data *rsa;
> unsigned char *buf;
> + int j;
>
> if (!key_data)
> return;
> @@ -930,41 +954,79 @@ static void generate_key(void)
> if (fd < 0)
> return;
>
> - rsa = RSA_new();
> - if (!rsa)
> + rsa = &key_data->rsa;
> + if (read(fd, rsa, sizeof(struct RSA_data)) <= 0)
> goto Close;
>
> - rsa_data = &key_data->rsa_data;
> - if (read(fd, rsa_data, sizeof(struct RSA_data)) <= 0)
> - goto Free_rsa;
> + ret = gcry_ac_open(&rsa_hd, GCRY_AC_RSA, 0);
> + if (ret)
> + goto Close;
>
> - buf = rsa_data->data;
> - rsa->n = BN_mpi2bn(buf, rsa_data->n_size, NULL);
> - buf += rsa_data->n_size;
> - rsa->e = BN_mpi2bn(buf, rsa_data->e_size, NULL);
> - if (!rsa->n || !rsa->e)
> + buf = rsa->data;
> + ret = gcry_ac_data_new(&rsa_data_set);
> + if (ret)
> goto Free_rsa;
>
> - rnd_fd = open("/dev/urandom", O_RDONLY);
> - if (rnd_fd > 0) {
> - unsigned short size = KEY_SIZE + IVEC_SIZE;
> -
> - if (read(rnd_fd, key_data->key, size) == size) {
> - struct encrypted_key *enc_key;
> - int ret;
> -
> - enc_key = (struct encrypted_key *)(key_data + 1);
> - ret = RSA_public_encrypt(size, key_data->key,
> - enc_key->data, rsa, RSA_PKCS1_PADDING);
> - if (ret > 0) {
> - enc_key->size = ret;
> - use_RSA = 'y';
> + for (j = 0; j < RSA_FIELDS_PUB; j++) {
> + size_t s = rsa->size[j];
> +
> + gcry_mpi_scan(&mpi, GCRYMPI_FMT_USG, buf, s, NULL);
> + ret = gcry_ac_data_set(rsa_data_set, GCRY_AC_FLAG_COPY,
> + rsa->field[j], mpi);
> + gcry_mpi_release(mpi);
> + if (ret)
> + break;
> +
> + buf += s;
> + }
> + if (!ret)
> + ret = gcry_ac_key_init(&rsa_pub, rsa_hd, GCRY_AC_KEY_PUBLIC,
> + rsa_data_set);
Suppose old key file was left in place. gcry_ac_key_init will probably
fail, since rsa_data_set is filled with "garbage". key_data->key is left
uninitlialed and will be used for encrypting the image.
Easy fix is to propagated the error, if generate_key() fails suspend
aborts.
> +
> + if (!ret) {
> + ret = gcry_ac_data_new(&key_set);
> + if (!ret) {
> + rnd_fd = open("/dev/urandom", O_RDONLY);
> + if (rnd_fd > 0) {
> + unsigned short size = KEY_SIZE + CIPHER_BLOCK;
> +
> + if (read(rnd_fd, key_data->key, size) == size) {
> + gcry_mpi_scan(&mpi, GCRYMPI_FMT_USG,
> + key_data->key, size, NULL);
> + ret = gcry_ac_data_encrypt(rsa_hd, 0,
> + rsa_pub, mpi, &key_set);
> + gcry_mpi_release(mpi);
> + if (!ret) {
> + struct encrypted_key *key;
> + char *str;
> + size_t s;
> +
> + key = (struct encrypted_key *)
> + (key_data + 1);
> + gcry_ac_data_get_index(key_set,
> + GCRY_AC_FLAG_COPY, 0,
> + (const char **)&str,
> &mpi);
> +
> + gcry_free(str);
> + gcry_mpi_print(GCRYMPI_FMT_USG,
> + key->data,
> KEY_DATA_SIZE,
> + &s, mpi);
> + gcry_mpi_release(mpi);
> + key->size = s;
> + use_RSA = 'y';
> + }
> + }
> + close(rnd_fd);
> }
> + gcry_ac_data_destroy(key_set);
> }
Hum, I'd reorganize the code to reduce the indendation (and the
wrapping).
> - close(rnd_fd);
> + gcry_ac_key_destroy(rsa_pub);
> + } else {
> + gcry_ac_data_destroy(rsa_data_set);
> }
> +
> Free_rsa:
> - RSA_free(rsa);
> + gcry_ac_close(rsa_hd);
> Close:
> close(fd);
> }
> @@ -1026,6 +1088,13 @@ int main(int argc, char *argv[])
> }
> #ifdef CONFIG_ENCRYPT
> if (encrypt) {
> + printf("suspend: libgcrypt version: %s\n",
> + gcry_check_version(NULL));
Btw, gcry_check_version should be used to ensure that the expected
version of the library is in place. Bebian for example ships at least 3
different versions (I don't know if/how the API differs).
> + gcry_control(GCRYCTL_INIT_SECMEM, page_size, 0);
You can also disable secure memory, it's nothing more than a mlock()'ed
area and we call mlockall() anyway.
> + encrypt = !gcry_cipher_open(&cipher_handle, GCRY_CIPHER_AES,
> + GCRY_CIPHER_MODE_CFB, GCRY_CIPHER_SECURE);
> + }
> + if (encrypt) {
> mem_size -= buffer_size;
> key_data = (struct key_data *)((char *)mem_pool + mem_size);
> generate_key();
> @@ -1138,6 +1207,10 @@ Close_snapshot_fd:
> Close_resume_fd:
> close(resume_fd);
>
> +#ifdef CONFIG_ENCRYPT
> + if (encrypt)
> + gcry_cipher_close(cipher_handle);
> +#endif
> free(mem_pool);
>
> return ret;
Will test it ASAP and report back ;)
Luca
--
Home: http://kronoz.cjb.net
Al termine di un pranzo di nozze mi hanno dato un
amaro alle erbe cosi' schifoso che perfino sull'etichetta
c'era un frate che vomitava.
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Suspend-devel mailing list
Suspend-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/suspend-devel
