Niels, Thank you so much for taking the time to look at my code. I really do appreciate it.
I have made the modifications you suggested on AS_DCP_AES.cpp file, and pushed to github. For KM_prng.cpp , are you suggesting that I use 128 bit AES instead of 256 bit ? I think the original code used 256 bit. Still working on the mpz changes that you suggested. Thanks again, Aaron On Mon, May 30, 2016 at 9:57 AM, Niels Möller <[email protected]> wrote: > Aaron Boxer <[email protected]> writes: > > > Here is a link to my changes, if you have time to take a look. > > > > > https://github.com/GrokImageCompression/asdcplib/commit/39cbb6188bce7112d826e6045cb739bfd1c8653b > > I've had a quick look. I'm assuming you intend to delete the commented > out parts, so I'm not saying anything more about them? > > src/AS_DCP_AES.cpp: > > * If you use only 128-bit aes, then it's better to use the aes128_ctx > and related functions. aes_ctx should be used only if for some reason > you really need an interface with variable key size at runtime. > > * The m_KeyBuf member variable seems useless. > > * You may want to use nettle's HMAC implementation, instead of > implementing it yourself. > > src/KM_prng.cpp: > > + sha1_update(&SHA, sizeof(m_Context), (byte_t*)&m_Context ); > > This won't work, m_Context is an aes_ctx, and you'll get different > results on big-endian and little-endian. Worse, it has space for more > subkeys than are used for aes128, so you will hash uninitialized data. > You probably want to hash the original, unexpanded, 128-bit key? > > + sha1_digest(&SHA,SHA1_DIGEST_SIZE, sha_buf); > > I tend to use the style > > sha1_digest (sizeof(sha_buf), sha_buf); > > + aes_set_encrypt_key(&m_Context, RNG_KEY_SIZE_BITS, sha_buf); > > Size argument to aes_set_encrypt_key is in octets, not bits. (And if you > use aes128_set_encrypt_key instead, as I suggest, there's no size > argument). > > + aes_encrypt(&m_Context,AES128_KEY_SIZE, buf + gen_count, > m_ctr_buf); > > AES128_KEYSIZE here looks a bit strange. sizeof(m_ctr_buf)? (Even if the > value, 16, is intentionally the same). > > Delete the code computing c_2powb, and use mpz_fdiv_r_2exp instead of > mpz_mod. > > You don't need bn_tmp, the mpz interface allows in-place operations. > > Dob't access _mp_size field, if you really want the (platform dependent) > limb size, use the mpz_size function. But generally, mpz_sizeinbase is > more useful. > > I don't think your use of mpz_export is correct in the case that the > base-256 representation has leading zeros. I think you need to handle > the zero-padding yourself (or maybe the initial memset is enough? But > for big-endian, you still have to know the number of leading zeros). > > Regards, > /Niels > > -- > Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. > Internet email is subject to wholesale government surveillance. > _______________________________________________ nettle-bugs mailing list [email protected] http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
