-
Notifications
You must be signed in to change notification settings - Fork 733
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve safety patterns in ring::endian
#1662
Comments
IIUC, the ideal situation would be to provide the ability to depend on zerocopy in such a way that it, in turn, has no dependencies? Right now, the
Would it help if we published a release specific to ring (e.g.,
Just to make sure I understand correctly: The endianness utilities are used to store secret data (keys, plaintexts, etc), and so any operation on them (e.g. performing a byteorder swap) could in principle be performed in a data-dependent way and thus introduce a timing side-channel? |
google/zerocopy#450 tracks changes to zerocopy to support this. MSRV was bumped to 1.61.0 in ring in part to make it possible to use zerocopy. |
OK, after looking at everything (I think), I think the long-term solution is to "just" eliminate
etc., etc. |
Note to help explain why And, perhaps I was just trying to be too clever, such as with |
I did a sketch of this tonight and it's 159 insertions(+), 176 deletions(-) including license boilerplate so I think it is very practical to eliminate almost all of |
Oh awesome! |
So, what did we learn from this?:
I think there are still probably some good uses of zerocopy elsewhere in ring, especially if/where we actually need zero-copy type conversions or where we can make better use of the zerocopy traits to model safety. In the case of |
Yeah that sounds like a really good summary. |
Well, |
Recently @joshlf commented, roughly, that it would be easier to audit the code in the internal
ring::endian
module if we made use of thezerocopy
crate and/or thebyteorder
crate. During the discussion several issues came up including:zerocopy
has aderive
feature that uses proc macros to derive implementations of the traits that help makeunsafe
code type-safe, but that requires (a) using proc macros, and (b) adding syn, quote, and proc-macro2 as dependencies. So far we have gone out of our way to avoid using proc macros to avoid this.ring uses
ring::endian
directly in the implementation of cryptography primitives. So, as far as the FIPS project is concerned, whatever dependencies we use, if any, would be affected by our FIPS policy. For example, we might not be able to upgrade these dependencies as freely as we would like. ring would likely feel the need to lock itself to specific versions of these dependencies and/or fork/vendor them. Until we are further along in the FIPS project it is hard to say what the effect on dependencies would be.Because ring uses
ring::endian
directly in the implementation of cryptographic primitives, any dependencies used in its implementation need to have a plan for preventing compiler-introduced side channels. Currently we all kind of just hope the compiler does the right thing. But in the long term we need to find a solution that goes beyond hope, and any dependencies need to commit to that solution.ring::endian
has a potentially huge impact on performance, so changes toring::endian
also need to be benchmarked.The text was updated successfully, but these errors were encountered: