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
(btw, rsa_data_set was leacked):
--- 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 ;)
> > Will test it ASAP and report back ;)
>
> Thanks!
Both encryption modes are working as expected.
Luca
--
Home: http://kronoz.cjb.net
Se alla sera, dopo una strepitosa vittoria, guardandoti allo
specchio dovessi notare un secondo paio di palle, che il tuo
cuore non si riempia d'orgoglio, perche` vuol dire che ti
stanno inculando -- Saggio Cinese
-------------------------------------------------------------------------
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
