proposal: crypto/rc4: remove assembly implementations #25417
Labels
FrozenDueToAge
Proposal
Proposal-Accepted
Proposal-Crypto
Proposal related to crypto packages or other security issues
Milestone
Comments
mundaym
added
the
Proposal-Crypto
|
Change https://golang.org/cl/130397 mentions this issue: |
|
RC4 is still used in many applications (for instance BitTorrent stream obfuscation). I don't think its performance should be allowed to slip just because it's not considered secure by modern standards. |
|
@anacrolix, it's a trade-off and we chose code size & maintainability & principles (we should fix the compiler instead) over the concerns of zero vocal users at the time. Even if you'd spoken up earlier we still probably would've made the same choice. |
|
@anacrolix, also, if you look at the CL, it might actually be faster on your CPU now with the pure Go implementation. Or it might be slower. In any case, it's a compiler bug now. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
@bradfitz investigated doing this in CL 102255. He found that on some CPUs the assembly was considerably faster and so abandoned the change.
At this point we are very unlikely to accept new RC4 assembly (e.g. CL 101916). This begs the question of whether we should have any assembly in this package at all. For reference, BoringSSL has removed all assembly for RC4: https://boringssl-review.googlesource.com/c/boringssl/+/10720. Removing all the assembly from crypto/rc4 would simplify the package significantly.
Is the performance of RC4 still important enough to warrant having assembly in this package?
/cc @FiloSottile @agl
The text was updated successfully, but these errors were encountered: