On Thursday 20 July 2006 18:58, Luca wrote: > Il Wed, Jul 19, 2006 at 10:13:00PM +0200, Rafael J. Wysocki ha scritto: > > On Wednesday 19 July 2006 21:38, Luca wrote: > > > 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. > > > > No, it won't. The format of the key file has changed entirely. The new > > code > > will not work with and old key file at all. > > Yeah, I didn't noticed that use_RSA was set only after the key had been > loaded succesfully. > > > > > + 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). > > > > Well, I've considered doing this too, but it seems to me the result won't be > > very much more readable. Maybe I'll try in the next version. > > With a couple of gotos it's possible to remove the two outer if blocks
Yup. Please have a look at the patch I'm going to post in a while. > (btw, rsa_data_set was leacked): Yes, it was intentional. If you free the key created out of a data set, you can't free the data set itself, apparently, or you get a double free (libgcrypt "feature", I would guess ;-) ). > --- a/suspend.c 2006-07-20 18:19:10.242010000 +0200 > +++ b/suspend.c 2006-07-20 18:25:15.656847000 +0200 > @@ -983,48 +983,47 @@ > ret = gcry_ac_key_init(&rsa_pub, rsa_hd, GCRY_AC_KEY_PUBLIC, > rsa_data_set); > > - 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); > + if (ret) > + goto Free_data; > + > + ret = gcry_ac_data_new(&key_set); > + if (ret) { > + gcry_ac_key_destroy(rsa_pub); > + goto Free_data; > + } > + > + 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'; > } > - gcry_ac_data_destroy(key_set); > } > - gcry_ac_key_destroy(rsa_pub); > - } else { > - gcry_ac_data_destroy(rsa_data_set); > + close(rnd_fd); > } > + gcry_ac_data_destroy(key_set); > + gcry_ac_key_destroy(rsa_pub); > > +Free_data: > + gcry_ac_data_destroy(rsa_data_set); > Free_rsa: > gcry_ac_close(rsa_hd); > Close: > > > > > > > > > + 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. > > > > RSA encryption doesn't work without this. Worse yet, if this is not done, > > libgcrypt just makes the whole program exit. :-( > > I meant: > > gcry_control(GCRYCTL_DISABLE_SECMEM, 0); > > Not really important though ;) Ah, I haven't found that. Anyway I'm inclined to let libgcrypt behave as it wants here. Greetings, Rafael ------------------------------------------------------------------------- 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