Open tisonkun opened 9 months ago
I believe it is already clear which parts of the crate are covered by which license as each source file has a license header at the top. The code in question comes from BoringSSL and they didn't do the relicensing. Unfortunately they (BoringSSL) put some important bits in OpenSSL-license-licensed header files. Our goal is to replace all the OpenSSL-license-licensed code as we move completely away from C to Rust. Once that is done there won't be any issue regarding this anymore.
See also #902 which tracks making the licensing use SPDX.
@briansmith Thanks for your reply. I have one more question:
it is already clear which parts of the crate are covered by which license
For a takeaway, are those files OpenSSL-license-licensed included in any downstream software that use ring? Or it's optional if certain feature unused.
I did a check and it seems the fileset is:
You can check if there is other missing, and perhaps we can divide and conquer them :D
crypto/constant_time_test.c
This is test code that shouldn't be linked into the ring library. Fixing this is tracked in https://github.com/briansmith/ring/issues/1705. I would appreciate a PR.
crypto/cpu_intel.c
I have sent some patches upstream to BoringSSL which, when they are all merged, will allow us to more easily replace cpu_intel.c with Rust code.
crypto/fipsmodule/bn/internal.h crypto/fipsmodule/bn/montgomery.c
include/ring-core/aes.h
We need to vendor the rust-crypto aes/soft code at https://github.com/RustCrypto/block-ciphers/tree/master/aes/src/soft into src/third_party/rust_crypto/aes/soft
, replacing aes_nohw.c. Then we can remove aes_nohw.c and aes.h.
include/ring-core/mem.h
fe_isnonzero
in curve25519.c to fe_isnonzero_vartime
. Update the caller. Change fe_isnonzero_vartime to do a variable-time comparison instead of CRYPTO_memcmp. Stop including mem.h and remove mem.h.include/ring-core/base.h include/ring-core/type_check.h
crypto/mem.c
value_barrier_w
in Rust or assembly. Then we can rewrite crypto/mem.c in Rust, using the value barrier.crypto/internal.h
include/ring-core/arm_arch.h
We probably need to work with BoringSSL upstream to relicense this since it is used by the assembly code.
include/ring-core/aes.h We need to vendor the rust-crypto aes/soft code at https://github.com/RustCrypto/block-ciphers/tree/master/aes/src/soft into src/third_party/rust_crypto/aes/soft, replacing aes_nohw.c. Then we can remove aes_nohw.c and aes.h.
This is now tracked as #1886 with more details.
Thanks a lot for the updates @briansmith!
For crypto/mem.c
, I found src/constant_time.rs
uses CRYPTO_memcmp
also:
/// Returns `Ok(())` if `a == b` and `Err(error::Unspecified)` otherwise.
/// The comparison of `a` and `b` is done in constant time with respect to the
/// contents of each, but NOT in constant time with respect to the lengths of
/// `a` and `b`.
pub fn verify_slices_are_equal(a: &[u8], b: &[u8]) -> Result<(), error::Unspecified> {
if a.len() != b.len() {
return Err(error::Unspecified);
}
let result = unsafe { CRYPTO_memcmp(a.as_ptr(), b.as_ptr(), a.len()) };
match result {
0 => Ok(()),
_ => Err(error::Unspecified),
}
}
prefixed_extern! {
fn CRYPTO_memcmp(a: *const u8, b: *const u8, len: c::size_t) -> c::int;
}
Maybe we compare a[i] != b[i]
iteratively?
Can we replace it with variable-time comparison? And it seems a common use case to do CRYPTO_memcmp
, I wonder if we can "reimplement it" in a different way like:
int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len) {
const unsigned *a = in_a;
const unsigned *b = in_b;
for (unsigned i = 0; i < len; i++) {
if (a[i] ^ b[i]) return 1;
}
return 0;
}
or just use memcmp
.
PR #2070 and PR #1899, and @tisonkun's PR #1893, amongst other small changes, are making progress on this.
The refactoring to remove crypto/cpu_intel.c
is well underway.
Currently, the LICENSE file is large and complicated, while can be vaguely regarded as a combination of MIT, ISC, and OpenSSL’s licenses.
Although the latest OpenSSL license is Apache License 2.0, in this repo, we use a few of sentences refer to the BSD-style licenses with advertising clauses:
https://github.com/briansmith/ring/blob/6c29bf61cd0e9d478deeec11e922e8513d5d0f3c/LICENSE#L66-L83
https://github.com/briansmith/ring/blob/6c29bf61cd0e9d478deeec11e922e8513d5d0f3c/LICENSE#L137-L145
It causes concerns in downstream for using this software in a mindset like so-called "permissive OSS license" or "weak copyleft license": https://lists.apache.org/thread/ptwdv18z4wd9r11nmdwj7wgzwvm3b8l2
@briansmith do you have more background of the license content, or how generally a downstream user use it?
The background of this questions is from https://www.apache.org/legal/resolved.html#category-x that "BSD-4-Clause/BSD-4-Clause (University of California-Specific)" can introduce burden for users to convey this software - they're, be required, to include extra acknowledgement for certern actions. And while if we can either: