Dave,

I had a suggestion for AES_unwrap() function. As of now, if IV doesn't
match it return 0. It would be good to change this to some other error
value which can be eye catchy. Normally the fist thing which comes to mind
when we see return 0 is things are fine... my 2cents....


On Fri, Apr 6, 2012 at 1:41 PM, Prashanth kumar N <
prashanth.kuma...@gmail.com> wrote:

> Thanks Dave for your great support... you rock...  after changing KEYBITS,
> it worked... my ignorance that i mistook it for Key and set it to 512...
> Please find my response below...
>
> Firstly Jeff,
>
> 256 is valid KEK and max one. Key can be of 'n' blocks each block being 64
> bits in size and 'n' should ne min of 2 blocks
>
>
>
> On Fri, Apr 6, 2012 at 5:16 AM, Dave Thompson <dthomp...@prinpay.com>wrote:
>
>> > From: owner-openssl-us...@openssl.org On Behalf Of pkumarn
>> > Sent: Wednesday, 04 April, 2012 05:41
>>
>> > I need to wrap 512bit key with 256 bit KEK key. When i do
>> > this, i am hitting
>> > seg fault in AES_wrap_key(). When i do gdb, it points to
>> > memcpy(). <snip>
>>
>> > #define KEY512  0
>> >
>> > #if KEY512
>> >     #define KEYLEN      64
>> >     #define KEYBITS     512
>> > #else
>> >     #define KEYLEN  32
>> >     #define KEYBITS 256
>> > #endif
>>
>> >     #if (!KEY512)
>> >     static const unsigned char kek[] = {
>> >       0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
>> >       0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
>> >       0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
>> >       0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f
>> >       };
>> >     #else
>> >     static const unsigned char kek[] = {
>> >     0xbc, 0x54, 0xd8, 0xa0, 0x6e, 0xab, 0x3b,
>> >     0x4c, 0x06, 0xf5, 0xbe, 0x01, 0xc5, 0x77,
>> >     0x28, 0x3d, 0x92, 0xda, 0xfb, 0xe8, 0x3f,
>> >     0xe0, 0x59, 0x57, 0xff, 0xbe, 0xfa, 0x5b,
>> >     0xe0, 0xd4, 0xfb, 0xb7
>> >     };
>> >     #endif
>>
>> >     #if (!KEY512)
>> >     static const unsigned char key[] = <snip: 32 bytes>
>> >     #else
>> >     static const unsigned char key[] = <snip: 64 bytes>
>> >     #endif
>> >
>> Suggestion: for hardcoded data like this which is supposed
>> to be an exact size, it's usually best to verify it is
>> the correct size before using it, because it's easy for
>> humans to mis-count and/or mistakenly change it.
>> In real use of course your key data should not be hardcoded
>> (because then it provides no actual security benefit) so
>> this issue doesn't arise; instead you should allocate
>> the correct size (by declaration or malloc/etc) and get
>> correct size data by some other means (e.g. RAND_bytes).
>> I will add this checks in my code...
>>
>> >
>> >     int ret, i;
>> >     unsigned char *otmp, *dtmp;
>> >
>> >     AES_KEY actx, dctx;
>> >    printf("\n keylen = %d; kebits= %d", KEYLEN, KEYBITS);
>> >
>> Get out of the habit of outputting 'partial' lines (not
>> terminated by \n) in C. Sometimes it works and sometimes
>> it doesn't. It appears in this case on your system it didn't.
>> The standard requires complete lines to work (up to possibly
>> a reasonable documented length limit) and if they don't (and
>> you didn't screw up something else) you can complain to your
>> implementor; incomplete lines are formally undefined behavior
>> which means the implementation can do anything it likes and
>> needn't even document it, although in practice implementors
>> try to do something reasonably sane if possible.
>>
> Above printf() is just for my reference ( i knew it was wrong). I had
> added them as a checkpoint.
>
>>
>> >     if (AES_set_encrypt_key(kek, KEYBITS, &actx))
>> >         printf("\n Error seeting AES key ");
>> >
>> This is the actual error. The KEK is an AES key and can't
>> ever be 512 bits. Your declarations above actually define
>> kek as 32 bytes = 256 bits for either setting of KEY512,
>> which is valid, so use 256 as kek length. Alternatively
>> choose a KEK which is another valid AES size and use that size.
>> Got that. i added another macro and it worked...
>>
>
>
>> >     otmp = (unsigned char *) malloc(sizeof(char) * (KEYLEN+8));
>> >     dtmp = (unsigned char *) malloc(sizeof(char) * KEYLEN);
>> >
>> Don't cast malloc in C, and in real code check for failure.
>> Or for a small known size like this don't malloc at all.
>> Yes i need to add checks.  Many of them advice me to cast malloc. What
>> would go wrong if i followed the above approach?
>>
>
>
>>  >   ret = AES_wrap_key(&actx, default_iv, otmp, key, KEYLEN);
>> >
>> Because AES_set_encrypt_key failed (but you ignored the failure)
>> this screws up; it does so differently on different systems,
>> and the only system I have where it segfaults (Windows) I can't
>> currently debug for 1.0.0. In any case it doesn't work as desired.
>> In my earlier code, ret was 72 which was right as i was wrapping 64 bytes
>> + 8 bytes of IV. Is my understanding right?
>>
>
>
>>  >     printf("\n AES wrap ; ret =  %d", ret);
>> >
>> >     if (ret < 0)
>> >         printf("\n AES wrap key failed");
>> >
>> >     printf("\n Wrapped key : ");
>> >
>> >     for (i = 0; i< (KEYLEN + 8); i++)
>> >         printf(" %02x", otmp[i]);
>> >
>> >
>> >     if (AES_set_decrypt_key(kek, KEYBITS, &dctx))
>> >         printf("\n Error setting decrypt key ");
>> >
>> Same here.
>>
>> >     ret = AES_unwrap_key(&dctx, default_iv, dtmp, otmp, ret);
>> >
>> >     printf("\n AES unwrap ; ret = %d", ret);
>> >
>> >     if (ret == 0)
>> >         printf("\n AES unwrapping failed ");
>> >
>> >     printf("\n Original key : ");
>> >     for (i = 0; i < KEYLEN ; i++)
>> >         printf(" %02x", dtmp[i]);
>> >
>> >     printf("\n");
>> >    free(otmp);
>> >    free(dtmp);
>> >
>> > }
>> >
>> With set_{en,de}crypt_key fixed it works for me.
>>
>>
>> ______________________________________________________________________
>> OpenSSL Project                                 http://www.openssl.org
>> User Support Mailing List                    openssl-users@openssl.org
>> Automated List Manager                           majord...@openssl.org
>>
>
>

Reply via email to