>> You make it sound like the fact that c_d is a stack array is an excuse >> for doing wrong:-) I mean it's clearly a compiler bug and reasoning as >> above does not make it understandable/justifiable/excusable. Formally >> it's not OpenSSL bug and shouldn't be reported as one... > > > I thought I'd made it quite clear that it was a compiler bug, but since a > single line of code can "fix" it in my case, I was hoping it wouldn't be > too much trouble... > > I don't think Microsoft is going to be very responsive to changing some old > DDK compiler, after all... and I'm stuck with it :( The only alternative > would be to go back to the 0.9.x series of OpenSSL where the file in > question is different, or maintain my own patch set (I do like your > alternate fix).
Question was if you can *confirm* if it does the trick. > If it's not to be fixed because it's actually a compiler bug then please > simply close the bug report. Working around compiler bugs is a gray zone. It's not exactly appropriate to do it, but it was done at several occasions. But there is one thing that is not gray zone, formulations like "I *assume* the same applies to other keys" are not. In other words if something will be done, no room for such assumptions will be left. I.e. it won't be about your particular usage case, but whole bn_nist.c. So that if you want to see it happen, you ought to confirm that the workaround applies to all subroutines. Best would be if you can confirm that all relevant EC tests fail and then after patch succeed.
Index: crypto/bn/bn_nist.c =================================================================== RCS file: /e/openssl/cvs/openssl/crypto/bn/bn_nist.c,v retrieving revision 1.36 diff -u -w -r1.36 bn_nist.c --- crypto/bn/bn_nist.c 2 Feb 2012 07:46:05 -0000 1.36 +++ crypto/bn/bn_nist.c 26 Jun 2012 12:33:08 -0000 @@ -474,8 +474,9 @@ */ mask = 0-(PTR_SIZE_INT)bn_sub_words(c_d,r_d,_nist_p_192[0],BN_NIST_192_TOP); mask &= 0-(PTR_SIZE_INT)carry; + res = c_d; res = (BN_ULONG *) - (((PTR_SIZE_INT)c_d&~mask) | ((PTR_SIZE_INT)r_d&mask)); + (((PTR_SIZE_INT)res&~mask) | ((PTR_SIZE_INT)r_d&mask)); nist_cp_bn(r_d, res, BN_NIST_192_TOP); r->top = BN_NIST_192_TOP; bn_correct_top(r); @@ -632,7 +633,8 @@ /* otherwise it's effectively same as in BN_nist_mod_192... */ mask = 0-(PTR_SIZE_INT)(*u.f)(c_d,r_d,_nist_p_224[0],BN_NIST_224_TOP); mask &= 0-(PTR_SIZE_INT)carry; - res = (BN_ULONG *)(((PTR_SIZE_INT)c_d&~mask) | + res = c_d; + res = (BN_ULONG *)(((PTR_SIZE_INT)res&~mask) | ((PTR_SIZE_INT)r_d&mask)); nist_cp_bn(r_d, res, BN_NIST_224_TOP); r->top = BN_NIST_224_TOP; @@ -831,7 +833,8 @@ mask = 0-(PTR_SIZE_INT)(*u.f)(c_d,r_d,_nist_p_256[0],BN_NIST_256_TOP); mask &= 0-(PTR_SIZE_INT)carry; - res = (BN_ULONG *)(((PTR_SIZE_INT)c_d&~mask) | + res = c_d; + res = (BN_ULONG *)(((PTR_SIZE_INT)res&~mask) | ((PTR_SIZE_INT)r_d&mask)); nist_cp_bn(r_d, res, BN_NIST_256_TOP); r->top = BN_NIST_256_TOP; @@ -1052,7 +1055,8 @@ mask = 0-(PTR_SIZE_INT)(*u.f)(c_d,r_d,_nist_p_384[0],BN_NIST_384_TOP); mask &= 0-(PTR_SIZE_INT)carry; - res = (BN_ULONG *)(((PTR_SIZE_INT)c_d&~mask) | + res = c_d; + res = (BN_ULONG *)(((PTR_SIZE_INT)res&~mask) | ((PTR_SIZE_INT)r_d&mask)); nist_cp_bn(r_d, res, BN_NIST_384_TOP); r->top = BN_NIST_384_TOP; @@ -1118,7 +1122,8 @@ bn_add_words(r_d,r_d,t_d,BN_NIST_521_TOP); mask = 0-(PTR_SIZE_INT)bn_sub_words(t_d,r_d,_nist_p_521,BN_NIST_521_TOP); - res = (BN_ULONG *)(((PTR_SIZE_INT)t_d&~mask) | + res = t_d; + res = (BN_ULONG *)(((PTR_SIZE_INT)res&~mask) | ((PTR_SIZE_INT)r_d&mask)); nist_cp_bn(r_d,res,BN_NIST_521_TOP); r->top = BN_NIST_521_TOP;
