Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@inikulin
inikulin committed Jul 10, 2023
1 parent d59d170 commit 0b54299
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 79 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ jobs:
shell: bash
- run: cargo test --features rpk
name: Run `rpk` tests
- run: cargo test --features post-quantum
name: Run `post-quantum` tests
- run: cargo test --features post-quantum,rpk
name: Run `post-quantum,rpk` tests
- run: cargo test --features pq-experimental
name: Run `pq-experimental` tests
- run: cargo test --features pq-experimental,rpk
name: Run `pq-experimental,rpk` tests
6 changes: 3 additions & 3 deletions boring-sys/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ include = [
]

[package.metadata.docs.rs]
features = ["rpk", "post-quantum"]
features = ["rpk", "pq-experimental"]
rustdoc-args = ["--cfg", "docsrs"]

[features]
Expand All @@ -39,8 +39,8 @@ fips = []
# Enables Raw public key API (https://datatracker.ietf.org/doc/html/rfc7250)
rpk = []

# Enables post-quantum crypto (https://blog.cloudflare.com/post-quantum-for-all/)
post-quantum = []
# Enables experimental post-quantum crypto (https://blog.cloudflare.com/post-quantum-for-all/)
pq-experimental = []

[build-dependencies]
bindgen = { workspace = true }
Expand Down
73 changes: 37 additions & 36 deletions boring-sys/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,14 +347,14 @@ fn ensure_patches_applied() -> io::Result<()> {

run_command(&mut cmd)?;

if cfg!(feature = "post-quantum") {
println!("cargo:warning=applying post quantum crypto patch to boringssl");
run_apply_patch_script("scripts/apply_pq_patch.sh", "")?;
if cfg!(feature = "pq-experimental") {
println!("cargo:warning=applying experimental post quantum crypto patch to boringssl");
run_apply_patch_script("scripts/apply_pq_patch.sh")?;
}

if cfg!(feature = "rpk") {
println!("cargo:warning=applying RPK patch to boringssl");
run_apply_patch_script("scripts/apply_rpk_patch.sh", "")?;
run_apply_patch_script("scripts/apply_rpk_patch.sh")?;
}

Ok(())
Expand All @@ -375,17 +375,9 @@ fn run_command(command: &mut Command) -> io::Result<()> {
Ok(())
}

fn run_apply_patch_script(
script_path: impl AsRef<Path>,
from_dir: impl AsRef<Path>,
) -> io::Result<()> {
fn run_apply_patch_script(script_path: impl AsRef<Path>) -> io::Result<()> {
let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));

let src_path = manifest_dir
.join(BORING_SSL_PATH)
.join(from_dir)
.canonicalize()?;

let src_path = manifest_dir.join(BORING_SSL_PATH).canonicalize()?;
let cmd_path = manifest_dir.join(script_path).canonicalize()?;

let mut cmd = Command::new(cmd_path);
Expand All @@ -395,12 +387,7 @@ fn run_apply_patch_script(
Ok(())
}

fn main() {
println!("cargo:rerun-if-env-changed=BORING_BSSL_PATH");

#[cfg(all(feature = "fips", feature = "rpk"))]
compile_error!("`fips` and `rpk` features are mutually exclusive");

fn build_boring_from_sources() -> String {
if !Path::new(BORING_SSL_PATH).join("CMakeLists.txt").exists() {
println!("cargo:warning=fetching boringssl git submodule");
// fetch the boringssl submodule
Expand All @@ -421,26 +408,40 @@ fn main() {

ensure_patches_applied().unwrap();

let bssl_dir = std::env::var("BORING_BSSL_PATH").unwrap_or_else(|_| {
let mut cfg = get_boringssl_cmake_config();
let mut cfg = get_boringssl_cmake_config();

if cfg!(feature = "fuzzing") {
cfg.cxxflag("-DBORINGSSL_UNSAFE_DETERMINISTIC_MODE")
.cxxflag("-DBORINGSSL_UNSAFE_FUZZER_MODE");
}
if cfg!(feature = "fips") {
let (clang, clangxx) = verify_fips_clang_version();
cfg.define("CMAKE_C_COMPILER", clang);
cfg.define("CMAKE_CXX_COMPILER", clangxx);
cfg.define("CMAKE_ASM_COMPILER", clang);
cfg.define("FIPS", "1");
}
if cfg!(feature = "fuzzing") {
cfg.cxxflag("-DBORINGSSL_UNSAFE_DETERMINISTIC_MODE")
.cxxflag("-DBORINGSSL_UNSAFE_FUZZER_MODE");
}
if cfg!(feature = "fips") {
let (clang, clangxx) = verify_fips_clang_version();
cfg.define("CMAKE_C_COMPILER", clang);
cfg.define("CMAKE_CXX_COMPILER", clangxx);
cfg.define("CMAKE_ASM_COMPILER", clang);
cfg.define("FIPS", "1");
}

cfg.build_target("ssl").build();
cfg.build_target("crypto").build().display().to_string()
});
cfg.build_target("ssl").build();
cfg.build_target("crypto").build().display().to_string()
}

fn main() {
println!("cargo:rerun-if-env-changed=BORING_BSSL_PATH");

#[cfg(all(feature = "fips", feature = "rpk"))]
compile_error!("`fips` and `rpk` features are mutually exclusive");

let bssl_dir = std::env::var("BORING_BSSL_PATH");

if bssl_dir.is_ok() && cfg!(any(feature = "rpk", feature = "pq-experimental")) {
panic!("precompiled BoringSSL was provided, optional patches can't be applied to it");
}

let bssl_dir = bssl_dir.unwrap_or_else(|_| build_boring_from_sources());

let build_path = get_boringssl_platform_output_path();

if cfg!(feature = "fips") {
println!(
"cargo:rustc-link-search=native={}/build/crypto/{}",
Expand Down
53 changes: 32 additions & 21 deletions boring-sys/patches/boring-pq.patch
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
From 968000d0f6c94f49fa9b1dc4d0acadca9fd78f58 Mon Sep 17 00:00:00 2001
From 4cba2164726c8d2647e38548a266a70c4942d567 Mon Sep 17 00:00:00 2001
From: Bas Westerbaan <[email protected]>
Date: Fri, 22 Jul 2022 16:43:48 +0200
Subject: [PATCH] Add temporary post-quantum key agreements
Expand Down Expand Up @@ -39,19 +39,19 @@ Cf RTG-2076 RTG-2051 RTG-2508 RTG-2707 RTG-2607
src/crypto/kyber/kyber768.c | 4 +
src/crypto/kyber/kyber_test.cc | 229 ---
src/crypto/kyber/kyber_tests.txt | 905 ---------
src/crypto/obj/obj_dat.h | 10 +-
src/crypto/obj/obj_mac.num | 2 +
src/crypto/obj/objects.txt | 4 +-
src/crypto/obj/obj_dat.h | 14 +-
src/crypto/obj/obj_mac.num | 3 +
src/crypto/obj/objects.txt | 5 +-
src/include/openssl/kyber.h | 199 +-
src/include/openssl/nid.h | 6 +
src/include/openssl/nid.h | 9 +
src/include/openssl/ssl.h | 3 +
src/sources.cmake | 2 -
src/ssl/extensions.cc | 3 +
src/ssl/ssl_key_share.cc | 412 +++-
src/ssl/ssl_lib.cc | 2 +-
src/ssl/ssl_test.cc | 25 +-
src/tool/speed.cc | 162 +-
26 files changed, 2788 insertions(+), 5447 deletions(-)
26 files changed, 2797 insertions(+), 5447 deletions(-)
delete mode 100644 src/crypto/kyber/internal.h
delete mode 100644 src/crypto/kyber/keccak.c
delete mode 100644 src/crypto/kyber/keccak_tests.txt
Expand Down Expand Up @@ -7743,76 +7743,83 @@ index 486b163ea..000000000
-ct = 7F60D2E6EE01AE6FBB198364141AF9A1AC4BA1B161CBDB16B86224FA77129864F00B71AA221DD1F300BA3EAFB2694610CA8F27C24CDFCD240961C27CF42D01561CCAF742379B19085C462270636676D6DCC5786E23D3881C83CD5FFD667C017FCA1F404A15A5BD368CD95A8FBDEBCCB9771B95958BDE3F65533190C0A758586094EBB86101582DC69FCCD7C2C1900D30648F360B4C805F143C8FE201D2DE242C66378439CBD304A30C67213F364F8FCBB202FFDCC290E85E8F0F718677FF8CB81D4AB3CF0DA5640C5F61B11A240099201D0BFAABECBBBB27A4623D77860BF52DC53A3AFB65ED2FCD4E9C9B1D1A996A643140CCE0D8B801728ED51BC3A47C9003375D8B19401E1C6841379A45BD2BE02A41222C4FE63E19C978D480E2D4E353AB957A1B81B74BBC7655A3D0261C8B2B06F6642BA4A5F8DD87B98B9AE355F57F8A5F978C6372BF594DF54DD8BF7650803EE13D9094B3EE86A1C3E2C21E4FD1F143D8A95CCE869DD365C29FA004070B0BDB899138FBF1D83EB9F76CBC112016C8CE1118FBDF76C3C907D1D3CAE2BA0BA5E419814DADEF0887CDC5817CA9C7B050574CF84009A6731136B39807687D3C3D6863C5D780BD6303F10A7D311D1C2A1C87039C17C6668501D572FC9D56B249ED0E95E17851AEBB45E288A32A1206C4FA475455309CD3D759CA19C47C0128A95DBD07E67937C0324B4B53BAB7402363FB32606387E2C419C0350943C8CB4760C2AA7D2FF9B1392E528B98493A61D2DC6D85CBF9E5759106D6BD1DAD276FB3AA45AAF75EB80BDB5D83AFFD7880371562100226B4E373B421B9EB3032938205012C6C083DD9E44BDFA842E280F03373F6E7E4DB5A50B4AA0B34789AF9AD051709B1E9ABDD71AD733DCC021DFA3A63609994F0FAFC5C1D88A39D46F81FA70E6164575321C2EBBB71F32893348AF887BAD2B1720CD5A86EA3774C1EBD53B15B9F01A4B5542A5F54053107E38ACE2594170B81DCD98D1CC47FEE808BE78DEBA05491F913ADE44B8914D65865EB0EB0F7E02DCBA1DE0D4B34E70485F83F96A5E41BBC064E3458473F43A70D51593F442EE2BD5B39E2C77C53BE83B9188101E3455C513BF4B8F744286BD529DA2804F804CE48E864A15F391A6793E4609279EAD144CDA084F3087678CA971EB22ED3B4B7FB740BE0407571F58DDE930D895140AD8EA096F06A3E255CD95AD48C4A46B99FF777B16452123A28D73B4A0CC0899C0CDAA1286FBE1C5FE95FA9418B5A473CC6ADBA14AFB06F080A544E49E148E492A8CE4BA9F4B5F781436F8FE30058A46F4D49A44E1FDE1B6F07E2A247CD3973AD76D3F473CAE88A2BAB3BEB78BD1875B240B23A901A9AD3F4421E5E355EAB0D3E1E197B246EB4F0861AEFB55BFD3BB54BF2CD6FFDCDFA0B78BAA674130CA2512B7069E7C33903A0A5EEEE7AF46FAFA52DD3F2E952CF909806B9CD2AA6826E823959A4BF03847BA50A4026FA0A72B78CBF6AA7FCD4908BA3BBCBDDE68708EC0AEC44B1E35C4E9E1EB7F01AFF0D9CE48F99BA01D78CEE
-ss = 4793F705AED572ACE61DB13BEDE3900F2538EADDB904988C1F015BAC605A1093
diff --git a/src/crypto/obj/obj_dat.h b/src/crypto/obj/obj_dat.h
index 654b3c08e..851e27ec3 100644
index 654b3c08e..06f80f971 100644
--- a/src/crypto/obj/obj_dat.h
+++ b/src/crypto/obj/obj_dat.h
@@ -57,7 +57,7 @@
/* This file is generated by crypto/obj/objects.go. */


-#define NUM_NID 965
+#define NUM_NID 967
+#define NUM_NID 968

static const uint8_t kObjectData[] = {
/* NID_rsadsi */
@@ -8784,6 +8784,10 @@ static const ASN1_OBJECT kObjects[NUM_NID] = {
@@ -8784,6 +8784,12 @@ static const ASN1_OBJECT kObjects[NUM_NID] = {
{"HKDF", "hkdf", NID_hkdf, 0, NULL, 0},
{"X25519Kyber768Draft00", "X25519Kyber768Draft00",
NID_X25519Kyber768Draft00, 0, NULL, 0},
+ {"X25519Kyber512Draft00", "X25519Kyber512Draft00",
+ NID_X25519Kyber512Draft00, 0, NULL, 0},
+ {"P256Kyber768Draft00", "P256Kyber768Draft00", NID_P256Kyber768Draft00, 0,
+ NULL, 0},
+ {"X25519Kyber768Draft00Old", "X25519Kyber768Draft00Old",
+ NID_X25519Kyber768Draft00Old, 0, NULL, 0},
};

static const uint16_t kNIDsInShortNameOrder[] = {
@@ -8916,6 +8920,7 @@ static const uint16_t kNIDsInShortNameOrder[] = {
@@ -8916,6 +8922,7 @@ static const uint16_t kNIDsInShortNameOrder[] = {
18 /* OU */,
749 /* Oakley-EC2N-3 */,
750 /* Oakley-EC2N-4 */,
+ 966 /* P256Kyber768Draft00 */,
9 /* PBE-MD2-DES */,
168 /* PBE-MD2-RC2-64 */,
10 /* PBE-MD5-DES */,
@@ -8982,6 +8987,7 @@ static const uint16_t kNIDsInShortNameOrder[] = {
@@ -8982,7 +8989,9 @@ static const uint16_t kNIDsInShortNameOrder[] = {
458 /* UID */,
0 /* UNDEF */,
948 /* X25519 */,
+ 965 /* X25519Kyber512Draft00 */,
964 /* X25519Kyber768Draft00 */,
+ 967 /* X25519Kyber768Draft00Old */,
961 /* X448 */,
11 /* X500 */,
@@ -9829,6 +9835,7 @@ static const uint16_t kNIDsInLongNameOrder[] = {
378 /* X500algorithms */,
@@ -9829,6 +9838,7 @@ static const uint16_t kNIDsInLongNameOrder[] = {
366 /* OCSP Nonce */,
371 /* OCSP Service Locator */,
180 /* OCSP Signing */,
+ 966 /* P256Kyber768Draft00 */,
161 /* PBES2 */,
69 /* PBKDF2 */,
162 /* PBMAC1 */,
@@ -9853,6 +9860,7 @@ static const uint16_t kNIDsInLongNameOrder[] = {
@@ -9853,7 +9863,9 @@ static const uint16_t kNIDsInLongNameOrder[] = {
133 /* Time Stamping */,
375 /* Trust Root */,
948 /* X25519 */,
+ 965 /* X25519Kyber512Draft00 */,
964 /* X25519Kyber768Draft00 */,
+ 967 /* X25519Kyber768Draft00Old */,
961 /* X448 */,
12 /* X509 */,
402 /* X509v3 AC Targeting */,
diff --git a/src/crypto/obj/obj_mac.num b/src/crypto/obj/obj_mac.num
index a0519acee..239780c9f 100644
index a0519acee..caeb5eaed 100644
--- a/src/crypto/obj/obj_mac.num
+++ b/src/crypto/obj/obj_mac.num
@@ -952,3 +952,5 @@ X448 961
@@ -952,3 +952,6 @@ X448 961
sha512_256 962
hkdf 963
X25519Kyber768Draft00 964
+X25519Kyber512Draft00 965
+P256Kyber768Draft00 966
+X25519Kyber768Draft00Old 967
diff --git a/src/crypto/obj/objects.txt b/src/crypto/obj/objects.txt
index 3ad32ea3d..a5d786a8e 100644
index 3ad32ea3d..aa1404d83 100644
--- a/src/crypto/obj/objects.txt
+++ b/src/crypto/obj/objects.txt
@@ -1332,8 +1332,10 @@ secg-scheme 14 3 : dhSinglePass-cofactorDH-sha512kdf-scheme
@@ -1332,8 +1332,11 @@ secg-scheme 14 3 : dhSinglePass-cofactorDH-sha512kdf-scheme
: dh-std-kdf
: dh-cofactor-kdf

Expand All @@ -7821,6 +7828,7 @@ index 3ad32ea3d..a5d786a8e 100644
+ : X25519Kyber512Draft00
: X25519Kyber768Draft00
+ : P256Kyber768Draft00
+ : X25519Kyber768Draft00Old

# See RFC 8410.
1 3 101 110 : X25519
Expand Down Expand Up @@ -8043,10 +8051,10 @@ index cafae9d17..074ac5906 100644
#if defined(__cplusplus)
} // extern C
diff --git a/src/include/openssl/nid.h b/src/include/openssl/nid.h
index 4dd8841b1..78747f437 100644
index 4dd8841b1..8237efb74 100644
--- a/src/include/openssl/nid.h
+++ b/src/include/openssl/nid.h
@@ -4255,6 +4255,12 @@ extern "C" {
@@ -4255,6 +4255,15 @@ extern "C" {
#define SN_X25519Kyber768Draft00 "X25519Kyber768Draft00"
#define NID_X25519Kyber768Draft00 964

Expand All @@ -8055,6 +8063,9 @@ index 4dd8841b1..78747f437 100644
+
+#define SN_P256Kyber768Draft00 "P256Kyber768Draft00"
+#define NID_P256Kyber768Draft00 966
+
+#define SN_X25519Kyber768Draft00Old "X25519Kyber768Draft00Old"
+#define NID_X25519Kyber768Draft00Old 967
+

#if defined(__cplusplus)
Expand Down Expand Up @@ -8101,7 +8112,7 @@ index 5ee280221..0a706c411 100644
default:
return false;
diff --git a/src/ssl/ssl_key_share.cc b/src/ssl/ssl_key_share.cc
index 09a9ad380..4e28112f4 100644
index 09a9ad380..f7d2226e3 100644
--- a/src/ssl/ssl_key_share.cc
+++ b/src/ssl/ssl_key_share.cc
@@ -26,6 +26,7 @@
Expand Down Expand Up @@ -8574,7 +8585,7 @@ index 09a9ad380..4e28112f4 100644
{NID_X25519Kyber768Draft00, SSL_CURVE_X25519_KYBER768_DRAFT00,
- "X25519Kyber768Draft00", ""},
+ "X25519Kyber768Draft00", "Xyber768D00"},
+ {NID_X25519Kyber768Draft00, SSL_CURVE_X25519_KYBER768_DRAFT00_OLD,
+ {NID_X25519Kyber768Draft00Old, SSL_CURVE_X25519_KYBER768_DRAFT00_OLD,
+ "X25519Kyber768Draft00Old", "Xyber768D00Old"},
+ {NID_P256Kyber768Draft00, SSL_CURVE_P256_KYBER768_DRAFT00,
+ "P256Kyber768Draft00", "P256Kyber768D00"}
Expand Down
6 changes: 3 additions & 3 deletions boring/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ categories = ["cryptography", "api-bindings"]
edition = { workspace = true }

[package.metadata.docs.rs]
features = ["rpk", "post-quantum"]
features = ["rpk", "pq-experimental"]
rustdoc-args = ["--cfg", "docsrs"]

[features]
Expand All @@ -22,8 +22,8 @@ fips = ["boring-sys/fips"]
# Enables Raw public key API (https://datatracker.ietf.org/doc/html/rfc7250)
rpk = ["boring-sys/rpk"]

# Enables post-quantum crypto (https://blog.cloudflare.com/post-quantum-for-all/)
post-quantum = ["boring-sys/post-quantum"]
# Enables experimental post-quantum crypto (https://blog.cloudflare.com/post-quantum-for-all/)
pq-experimental = ["boring-sys/pq-experimental"]

[dependencies]
bitflags = { workspace = true }
Expand Down
16 changes: 15 additions & 1 deletion boring/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,24 @@
//! The crate can be compiled with [RawPublicKey](https://datatracker.ietf.org/doc/html/rfc7250)
//! support by turning on `rpk` compilation feature.
//!
//! ## Post-quantum cryptography
//! ## Experimental post-quantum cryptography
//!
//! The crate can be compiled with [post-quantum cryptography](https://blog.cloudflare.com/post-quantum-for-all/)
//! support by turning on `post-quantum` compilation feature.
//!
//! Upstream BoringSSL support the post-quantum hybrid key agreement `X25519Kyber768Draft00`. Most
//! users should stick to that one. Enabling this feature, adds a few other post-quantum key
//! agreements:
//!
//! - `X25519Kyber768Draft00Old` is the same as `X25519Kyber768Draft00`, but under its old codepoint.
//! -`X25519Kyber512Draft00`. Similar to `X25519Kyber768Draft00`, but uses level 1 parameter set for
//! Kyber. Not recommended. It's useful to test whether the shorter ClientHello upsets fewer middle
//! boxes.
//! - `P256Kyber768Draft00`. Similar again to `X25519Kyber768Draft00`, but uses P256 as classical
//! part. It uses a non-standard codepoint. Not recommended.
//!
//! Presently all these key agreements are deployed by Cloudflare, but we do not guarantee continued
//! support for them.
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

Expand Down
13 changes: 8 additions & 5 deletions boring/src/ssl/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,14 +633,17 @@ impl SslCurve {

pub const X25519: SslCurve = SslCurve(ffi::NID_X25519);

#[cfg(feature = "post-quantum")]
#[cfg(not(feature = "fips"))]
pub const X25519_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::NID_X25519Kyber768Draft00);

#[cfg(feature = "pq-experimental")]
pub const X25519_KYBER768_DRAFT00_OLD: SslCurve = SslCurve(ffi::NID_X25519Kyber768Draft00Old);

#[cfg(feature = "pq-experimental")]
pub const X25519_KYBER512_DRAFT00: SslCurve = SslCurve(ffi::NID_X25519Kyber512Draft00);

#[cfg(feature = "post-quantum")]
#[cfg(feature = "pq-experimental")]
pub const P256_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::NID_P256Kyber768Draft00);

#[cfg(feature = "post-quantum")]
pub const X25519_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::NID_X25519Kyber768Draft00);
}

/// A standard implementation of protocol selection for Application Layer Protocol Negotiation
Expand Down
Loading

0 comments on commit 0b54299

Please sign in to comment.