(But note that SHA_CBLOCK is less than "the whole pad", which is 128B! I don't know whether that too is an error.
On Mon, Jan 30, 2012 at 3:10 PM, Eddie Kohler <[email protected]> wrote: > Hi Markku, > > One of Click's great benefits is that its modularity supports code > from different authors... unfortunately that means it's not all > equally well tested. :( > > It looks to me like this code was created by taking code from openssl > and then modifying it by "inlining" some calls out to other functions. > For example the OpenSSL version is > > for (i=0; i<HMAC_MAX_MD_CBLOCK; i++) > pad[i]=0x5c^ctx->key[i]; > if (!EVP_DigestInit_ex(&ctx->o_ctx,md, impl)) > goto err; > if (!EVP_DigestUpdate(&ctx->o_ctx,pad,EVP_MD_block_size(md))) > goto err; > > and the Click > > for (i=0; i<HMAC_MAX_MD_CBLOCK; i++) > pad[i]=0x5c^ctx->key[i]; > SHA1_init(&ctx->o_ctx); > SHA1_update(&ctx->o_ctx,pad,SHA_BLOCK); > > And as you guessed, EVP_MD_block_size(md) for the EVP_sha1() structure > is SHA_CBLOCK, not SHA_BLOCK. > > Would love it if you created a pull request on github with this fix. > > It would be great to have some ipsec tests, or to go back & redo the > sfotware engineering here -- it's often a mistake to copy *and modify* > code to this degree. But at least the pull request is in the right > direction. > Eddie > > > On Mon, Jan 30, 2012 at 2:48 AM, Markku Savela <[email protected]> wrote: >> Even if based on Openssl, it appears to be some copy/paste error >> then. In sha1_impl >> >> #define SHA_CBLOCK 64 >> #define SHA_LBLOCK 16 >> #define SHA_BLOCK 16 >> >> The code I proposed to be fixed is feeding the HMAC pads into hash >> algorithm. Obviously, the whole pad (SHA_CBLOCK == 64) should be >> fed in, instead of only the first 16 bytes (SHA_BLOCK). >> >> >> On 01/27/2012 05:32 PM, Dimitris Syrivelis wrote: >>> Hi Markku, >>> >>> The code fragment you are referring to, is copied as is from Eric Young's >>> Openssl library Implementation. In comments there is a notice about using >>> this >>> library and i confirm that this code fragment is from there. The last time i >>> checked, IPsec flow was working on click ver 1.8. You have to set up a >>> configuration that uses SA tables as depicted in the example >>> simple_ipsec.click >>> and documentation. >>> >>> Dimitris >>> >>>> Hi, >>>> >>>> Has anyone actually worked on those elements? I just tried the HMAC, >>>> and couldn't get it to match my other implementation. On quick browse, >>>> it looks like it's using wrong constant in few places (SHA_BLOCK where >>>> SHA_CBLOCK should be?). Haven't really tried this yet -- will return >>>> to issue next week... >>>> >>>> >>>> >>>> @@ -97,12 +97,12 @@ void HMAC_Init_ex(HMAC_CTX *ctx, const void *key, >>>> int len) >>>> for (i=0; i<HMAC_MAX_MD_CBLOCK; i++) >>>> pad[i]=0x36^ctx->key[i]; >>>> SHA1_init(&ctx->i_ctx); >>>> - SHA1_update(&ctx->i_ctx,pad,SHA_BLOCK); >>>> + SHA1_update(&ctx->i_ctx,pad,SHA_CBLOCK); >>>> >>>> for (i=0; i<HMAC_MAX_MD_CBLOCK; i++) >>>> pad[i]=0x5c^ctx->key[i]; >>>> SHA1_init(&ctx->o_ctx); >>>> - SHA1_update(&ctx->o_ctx,pad,SHA_BLOCK); >>>> + SHA1_update(&ctx->o_ctx,pad,SHA_CBLOCK); >>>> } >>>> memcpy((void *)&ctx->md_ctx,(void*)&ctx->i_ctx,sizeof(SHA1_ctx)); >>>> } >>>> >>>> _______________________________________________ >>>> click mailing list >>>> [email protected] >>>> https://amsterdam.lcs.mit.edu/mailman/listinfo/click >>>> >>> >>> >> >> _______________________________________________ >> click mailing list >> [email protected] >> https://amsterdam.lcs.mit.edu/mailman/listinfo/click _______________________________________________ click mailing list [email protected] https://amsterdam.lcs.mit.edu/mailman/listinfo/click
