mono / boringssl

Custom version of Boring SSL used by Mono
Other
6 stars 21 forks source link

Unbreak secp256r1 on big endian by disabling the optimized path #9

Closed NattyNarwhal closed 6 years ago

NattyNarwhal commented 6 years ago

Neale seems to have tried to fix this for s390x, but it's still not working there nor on ppc64. We can just fall back to a less optimized implementation however, which does work fine.

Upstream OpenSSL, LibreSSL, and BoringSSL have diverged heavily from this implementation anyways; so we'd need to rework support anyways.

Fixes #8.

nealef commented 6 years ago

I had implemented this algorithm based on the Putty code and found that it was “simply” a matter of reversing the order of the bignumbers. The Open Mainframe Project has an internship program over the summer so I have nominated the completion of big-endian enablement of BTLS as one of the projects. I am not sure if we have filled the position though.

Neale seems to have tried to fix this for s390x, but it's still not working there nor on ppc64. We can just fall back to a less optimized implementation however, which does work fine.

Upstream OpenSSL, LibreSSL, and BoringSSL have diverged heavily from this implementation anyways; so we'd need to rework support anyways.

Fixes #8https://github.com/mono/boringssl/issues/8.

akoeplinger commented 6 years ago

So should we take this now?

NattyNarwhal commented 6 years ago

Well, it's all blocking on mono/mono#8679 for it to be useful anyways.

Right now, this patch would as work fine a workaround; I ship this patch for AIX/i builds. (as secp256r1 is a cipher used by things like say, api.nuget.org) Efforts to fix the thing properly may be wasted though, as aren't you guys going to try to upgrade this BTLS version anyways? (I think Google rewrote it a year ago upstream anyways, and IIRC OpenSSL wrote theirs after Google had forked initially.)

akoeplinger commented 6 years ago

Efforts to fix the thing properly may be wasted though, as aren't you guys going to try to upgrade this BTLS version anyways?

I'm not in the loop on when that will happen so I guess it's fine to get this workaround in for now :)