Jonas/fix bogo shim (#59)

* Add shim-id to bogo-shim. * Do not ignore -ipv6 argument. Silence failed tests. * remove evercypt ci job --------- Co-authored-by: Franziskus Kiefer <[email protected]>
cryspen · Nov 7, 2023 · 23b5c68 · 23b5c68
1 parent 151f427
commit 23b5c68
Show file tree

Hide file tree

Showing 11 changed files with 144 additions and 73 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -26,10 +26,6 @@ jobs:
           toolchain: ${{ matrix.rust }}
           override: true
 
-      - name: Setup HACL toolchain
-        if: ${{ matrix.os == 'ubuntu-latest' }}
-        run: sudo apt-get install ninja-build
-
       - name: Checkout code
         uses: actions/checkout@v3
 
@@ -64,13 +60,6 @@ jobs:
         if: matrix.os != 'windows-latest'
         run: BORINGSSL_ROOT=./boringssl ./bogo_shim/run.sh
 
-      - name: Test code /w hacl
-        if: ${{ matrix.os == 'ubuntu-latest' }}
-        uses: actions-rs/cargo@v1
-        with:
-          command: test
-          args: --all --all-targets --no-default-features --features evercrypt
-
   audit:
     needs: test
     runs-on: ubuntu-latest

diff --git a/bogo_shim/assets/config.json b/bogo_shim/assets/config.json
@@ -216,6 +216,82 @@
     "RSAKeyUsage-Client-WantSignature-GotEncipherment-AlwaysEnforcedTLS13": "",
     "RSAKeyUsage-Client-WantSignature-GotEncipherment-Enforced-TLS13": "",
 
+    "### NEWLY ADDED AND UNREVIEWED 2023/11/06": "###",
+    "ServerHelloBogusCipher-TLS13": "",
+    "BadECDSA-1-1-TLS13": "",
+    "BadECDSA-1-2-TLS13": "",
+    "BadECDSA-1-3-TLS13": "",
+    "BadECDSA-1-4-TLS13": "",
+    "BadECDSA-2-1-TLS13": "",
+    "BadECDSA-2-2-TLS13": "",
+    "BadECDSA-2-3-TLS13": "",
+    "BadECDSA-2-4-TLS13": "",
+    "BadECDSA-3-2-TLS13": "",
+    "BadECDSA-3-1-TLS13": "",
+    "BadECDSA-3-3-TLS13": "",
+    "BadECDSA-3-4-TLS13": "",
+    "BadECDSA-4-1-TLS13": "",
+    "BadECDSA-4-2-TLS13": "",
+    "BadECDSA-4-3-TLS13": "",
+    "BadECDSA-4-4-TLS13": "",
+    "ALPS-OmitClientApplicationSettings-New-TLS-TLS13": "",
+    "ALPS-UnsupportedProtocol-Server-New-TLS-TLS13": "",
+    "ExtraClientEncryptedExtension-New-TLS-TLS13": "",
+    "ALPS-OmitClientEncryptedExtensions-New-TLS-TLS13": "",
+    "ALPS-UnsupportedProtocol-Server-Old-TLS-TLS13": "",
+    "ALPS-OmitClientApplicationSettings-Old-TLS-TLS13": "",
+    "ExtraClientEncryptedExtension-Old-TLS-TLS13": "",
+    "ALPS-OmitClientEncryptedExtensions-Old-TLS-TLS13": "",
+    "UnexpectedClientEncryptedExtensions-New-TLS-TLS13": "",
+    "UnexpectedClientEncryptedExtensions-Old-TLS-TLS13": "",
+    "Server-Sign-RSA_PKCS1_SHA1-TLS13": "",
+    "Server-Sign-RSA_PKCS1_SHA384-TLS13": "",
+    "Server-Sign-RSA_PKCS1_SHA256-TLS13": "",
+    "Client-Sign-RSA_PKCS1_SHA256-TLS13": "",
+    "Client-Sign-RSA_PKCS1_SHA384-TLS13": "",
+    "Client-Sign-RSA_PKCS1_SHA1-TLS13": "",
+    "Client-Sign-RSA_PKCS1_SHA512-TLS13": "",
+    "Server-Sign-RSA_PKCS1_SHA512-TLS13": "",
+    "Client-Sign-ECDSA_P256_SHA256-TLS13": "",
+    "Client-Sign-ECDSA_SHA1-TLS13": "",
+    "Server-Sign-ECDSA_P256_SHA256-TLS13": "",
+    "Server-Sign-ECDSA_P224_SHA256-TLS13": "",
+    "Server-Sign-ECDSA_SHA1-TLS13": "",
+    "Client-Sign-ECDSA_P224_SHA256-TLS13": "",
+    "Client-Sign-ECDSA_P384_SHA384-TLS13": "",
+    "Server-Sign-ECDSA_P384_SHA384-TLS13": "",
+    "Client-Sign-ECDSA_P521_SHA512-TLS13": "",
+    "Client-Sign-RSA_PSS_SHA256-TLS13": "",
+    "Server-Sign-ECDSA_P521_SHA512-TLS13": "",
+    "Client-Sign-RSA_PSS_SHA512-TLS13": "",
+    "Client-Sign-RSA_PSS_SHA384-TLS13": "",
+    "Server-Sign-RSA_PSS_SHA256-TLS13": "",
+    "Server-Sign-RSA_PSS_SHA384-TLS13": "",
+    "Server-Sign-RSA_PSS_SHA512-TLS13": "",
+    "Client-Sign-Ed25519-TLS13": "",
+    "Server-Sign-Ed25519-TLS13": "",
+    "KyberNotEnabledByDefaultForAServer": "",
+    "KyberNotEnabledByDefaultInClients": "",
+    "RSAKeyUsage-Client-WantSignature-GotEncipherment-AlwaysEnforced-TLS13": "",
+    "Compliance-wpa-202304-TLS-Server-ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": "",
+    "Compliance-wpa-202304-TLS-Client-ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": "",
+    "Compliance-wpa-202304-TLS-Server-ECDHE_RSA_WITH_AES_256_GCM_SHA384": "",
+    "Compliance-wpa-202304-TLS-Client-ECDHE_RSA_WITH_AES_256_GCM_SHA384": "",
+    "Compliance-wpa-202304-TLS-Server-AES_256_GCM_SHA384": "",
+    "Compliance-wpa-202304-TLS-Client-AES_256_GCM_SHA384": "",
+    "Compliance-wpa-202304-TLS-Client-P-384": "",
+    "Compliance-wpa-202304-TLS-Server-P-384": "",
+    "Compliance-wpa-202304-TLS-Server-RSA_PKCS1_SHA512": "",
+    "Compliance-wpa-202304-TLS-Client-RSA_PKCS1_SHA384": "",
+    "Compliance-wpa-202304-TLS-Server-RSA_PKCS1_SHA384": "",
+    "Compliance-wpa-202304-TLS-Client-RSA_PKCS1_SHA512": "",
+    "Compliance-wpa-202304-TLS-Client-ECDSA_P384_SHA384": "",
+    "Compliance-wpa-202304-TLS-Server-ECDSA_P384_SHA384": "",
+    "Compliance-wpa-202304-TLS-Client-RSA_PSS_SHA384": "",
+    "Compliance-wpa-202304-TLS-Server-RSA_PSS_SHA384": "",
+    "Compliance-wpa-202304-TLS-Client-RSA_PSS_SHA512": "",
+    "Compliance-wpa-202304-TLS-Server-RSA_PSS_SHA512": "",
+
     "### BERTIE AS SERVER": "###",
     "FallbackSCSV": "TLS 1.1",
     "BadFinished-Server": "TLS 1.2",

diff --git a/bogo_shim/src/main.rs b/bogo_shim/src/main.rs
@@ -7,7 +7,7 @@
 //     https://github.com/rustls/rustls/blob/main/rustls/examples/internal/bogo_shim.rs
 //
 
-use std::{env, net::TcpStream, process};
+use std::{env, io::Write, net::TcpStream, process};
 
 use simple_https_client::tls13client;
 use simple_https_server::{tls13server, AppError};
@@ -18,6 +18,8 @@ static BOGO_NACK: i32 = 89;
 #[derive(Debug, Default)]
 struct Options {
     port: u16,
+    shim_id: u16,
+    ipv6: bool,
     role: Role,
     key_file: String,
     cert_file: String,
@@ -26,18 +28,13 @@ struct Options {
     expect_extended_master_secret: bool,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Default)]
 enum Role {
+    #[default]
     Client,
     Server,
 }
 
-impl Default for Role {
-    fn default() -> Self {
-        Role::Client
-    }
-}
-
 /// When a BoGo test contains one of these parameters, it will be skipped.
 /// BoGo will be notified about the skip via the return code BOGO_NACK.
 /// (See https://github.com/google/boringssl/blob/master/ssl/test/PORTING.md#unimplemented-features.)
@@ -153,6 +150,12 @@ fn main() {
             "-port" => {
                 options.port = args.remove(0).parse::<u16>().unwrap();
             }
+            "-shim-id" => {
+                options.shim_id = args.remove(0).parse::<u16>().unwrap();
+            }
+            "-ipv6" => {
+                options.ipv6 = true;
+            }
             "-fallback-scsv" => {
                 options.expect_fallback_scsv = true;
                 skip_currently(&arg);
@@ -187,23 +190,21 @@ fn main() {
 
     println!("{:#?}", options);
 
+    let addrs = [
+        std::net::SocketAddr::from((std::net::Ipv6Addr::LOCALHOST, options.port)),
+        std::net::SocketAddr::from((std::net::Ipv4Addr::LOCALHOST, options.port)),
+    ];
+    let mut stream = TcpStream::connect(&addrs[..]).expect("Can't connect to BoGo.");
+
+    stream
+        .write_all(&(options.shim_id as u64).to_le_bytes())
+        .unwrap();
+
     match options.role {
         Role::Client => {
-            let addrs = [
-                std::net::SocketAddr::from((std::net::Ipv6Addr::LOCALHOST, options.port)),
-                std::net::SocketAddr::from((std::net::Ipv4Addr::LOCALHOST, options.port)),
-            ];
-            let stream = TcpStream::connect(&addrs[..]).expect("Can't connect to BoGo.");
-
             let _ = tls13client(&options.hostname, stream, None, "hello");
         }
         Role::Server => {
-            let addrs = [
-                std::net::SocketAddr::from((std::net::Ipv6Addr::LOCALHOST, options.port)),
-                std::net::SocketAddr::from((std::net::Ipv4Addr::LOCALHOST, options.port)),
-            ];
-            let stream = TcpStream::connect(&addrs[..]).expect("Can't connect to BoGo.");
-
             if let Err(e) = tls13server(stream, &options.hostname) {
                 match e {
                     AppError::TLS(137) => eprintln!("Wrong TLS protocol version {:?}", e),

diff --git a/simple_https_client/src/lib.rs b/simple_https_client/src/lib.rs
@@ -205,7 +205,7 @@ where
 
     let mut cf_rec = None;
     let mut cstate = cstate;
-    while cf_rec == None {
+    while cf_rec.is_none() {
         let rec = stream.read_record()?;
 
         let (new_cf_rec, new_cstate) = match client_read_handshake(&rec, cstate) {
@@ -246,7 +246,7 @@ where
 
     let mut ad = None;
     let mut cstate = cstate;
-    while ad == None {
+    while ad.is_none() {
         let rec = stream.read_record()?;
 
         let (new_ad, new_cstate) = client_read(&rec, cstate)?;
@@ -275,7 +275,7 @@ where
 {
     let mut ad = None;
     let mut cstate = cstate;
-    while ad == None {
+    while ad.is_none() {
         let rec = stream.read_record()?;
 
         let (new_ad, new_cstate) = client_read(&rec, cstate)?;

diff --git a/simple_https_client/src/tls13client.rs b/simple_https_client/src/tls13client.rs
@@ -10,6 +10,7 @@ use tracing::{error, trace};
 ///
 /// The client connects to host:port via TCP, executes a TLS 1.3 handshake,
 /// sends an encrypted HTTP GET, and prints the servers HTTP response.
+#[allow(clippy::never_loop)]
 fn main() -> anyhow::Result<()> {
     // Setup tracing.
     tracing_subscriber::fmt::init();
@@ -57,7 +58,7 @@ fn main() -> anyhow::Result<()> {
     }
     if response_prefix.is_empty() {
         error!("Unable to connect with the configured ciphersuites.");
-        return Err(AppError::TLS(UNSUPPORTED_ALGORITHM.into()).into());
+        return Err(AppError::TLS(UNSUPPORTED_ALGORITHM).into());
     }
 
     println!("[!] Received HTTP response (prefix):");

diff --git a/src/server.rs b/src/server.rs
@@ -14,7 +14,7 @@ pub fn lookup_db(
     tkt: &Option<Bytes>,
 ) -> Result<(Bytes, SignatureKey, Option<PSK>), TLSError> {
     let ServerDB(server_name, cert, sk, psk_opt) = db;
-    if eq(&sni, &Bytes::new()) || eq(&sni, &server_name) {
+    if eq(sni, &Bytes::new()) || eq(sni, server_name) {
         match (crate::psk_mode(&algs), tkt, psk_opt) {
             (true, Some(ctkt), Some((stkt, psk))) => {
                 check_eq(ctkt, stkt)?;

diff --git a/src/tls13cert.rs b/src/tls13cert.rs
@@ -240,15 +240,13 @@ fn read_spki(cert: &Bytes, mut offset: usize) -> SpkiResult {
             SignatureScheme::EcdsaSecp256r1Sha256,
             CertificateKey(offset, bit_string_len - 1),
         ))
+    } else if rsa_pk_oid {
+        SpkiResult::Ok((
+            SignatureScheme::RsaPssRsaSha256,
+            CertificateKey(offset, bit_string_len - 1),
+        ))
     } else {
-        if rsa_pk_oid {
-            SpkiResult::Ok((
-                SignatureScheme::RsaPssRsaSha256,
-                CertificateKey(offset, bit_string_len - 1),
-            ))
-        } else {
-            asn1err(ASN1_INVALID_CERTIFICATE)
-        }
+        asn1err(ASN1_INVALID_CERTIFICATE)
     }
 }
 

diff --git a/src/tls13crypto.rs b/src/tls13crypto.rs
@@ -104,8 +104,8 @@ pub fn to_libcrux_hkdf_alg(alg: &HashAlgorithm) -> Result<hkdf::Algorithm, TLSEr
 pub fn hkdf_extract(alg: &HashAlgorithm, salt: &Bytes, ikm: &Bytes) -> Result<Bytes, TLSError> {
     Ok(hkdf::extract(
         to_libcrux_hkdf_alg(alg)?,
-        &salt.declassify(),
-        &ikm.declassify(),
+        salt.declassify(),
+        ikm.declassify(),
     )
     .into())
 }
@@ -118,8 +118,8 @@ pub fn hkdf_expand(
 ) -> Result<Bytes, TLSError> {
     match hkdf::expand(
         to_libcrux_hkdf_alg(alg)?,
-        &prk.declassify(),
-        &info.declassify(),
+        prk.declassify(),
+        info.declassify(),
         len,
     ) {
         Ok(x) => Ok(x.into()),

diff --git a/src/tls13formats.rs b/src/tls13formats.rs
@@ -1,6 +1,5 @@
-/// A module that for the formatting code needed by TLS 1.3
-/// Import hacspec and all needed definitions.
-#[allow(clippy::manual_range_contains)]
+//! A module that for the formatting code needed by TLS 1.3
+#![allow(clippy::manual_range_contains)]
 use crate::*;
 
 /// Well Known Constants
@@ -134,21 +133,19 @@ pub fn check_psk_key_exchange_modes(_algs: &Algorithms, ch: &Bytes) -> Result<()
 }
 
 pub fn key_shares(algs: &Algorithms, gx: &KemPk) -> Result<Bytes, TLSError> {
-    let ks = supported_group(algs)?.concat(&lbytes2(&gx)?);
+    let ks = supported_group(algs)?.concat(&lbytes2(gx)?);
     Ok(bytes2(0, 0x33).concat(&lbytes2(&lbytes2(&ks)?)?))
 }
 
 pub fn find_key_share(g: &Bytes, ch: &Bytes) -> Result<Bytes, TLSError> {
     if ch.len() < 4 {
         tlserr(parse_failed())
+    } else if eq(g, &ch.slice_range(0..2)) {
+        let len = check_lbytes2(&ch.slice_range(2..ch.len()))?;
+        Ok(ch.slice_range(4..4 + len))
     } else {
-        if eq(g, &ch.slice_range(0..2)) {
-            let len = check_lbytes2(&ch.slice_range(2..ch.len()))?;
-            Ok(ch.slice_range(4..4 + len))
-        } else {
-            let len = check_lbytes2(&ch.slice_range(2..ch.len()))?;
-            find_key_share(g, &ch.slice_range(4 + len..ch.len()))
-        }
+        let len = check_lbytes2(&ch.slice_range(2..ch.len()))?;
+        find_key_share(g, &ch.slice_range(4 + len..ch.len()))
     }
 }
 
@@ -652,7 +649,7 @@ pub fn client_hello(
 
     let ch = handshake_message(
         HandshakeType::ClientHello,
-        &ver.concat(&cr)
+        &ver.concat(cr)
             .concat(&sid)
             .concat(&cip)
             .concat(&comp)
@@ -687,6 +684,7 @@ fn invalid_compression_list() -> Result<(), TLSError> {
     Result::<(), TLSError>::Err(INVALID_COMPRESSION_LIST)
 }
 
+#[allow(clippy::type_complexity)]
 pub fn parse_client_hello(
     algs: &Algorithms,
     ch: &HandshakeData,
@@ -766,7 +764,7 @@ pub fn server_hello(
     }
     let sh = handshake_message(
         HandshakeType::ServerHello,
-        &ver.concat(&sr)
+        &ver.concat(sr)
             .concat(&sid)
             .concat(&cip)
             .concat(&comp)

diff --git a/src/tls13handshake.rs b/src/tls13handshake.rs
@@ -24,7 +24,7 @@ pub fn hkdf_expand_label(
         let info = lenb
             .concat(&lbytes1(&tls13_label)?)
             .concat(&lbytes1(context)?);
-        hkdf_expand(ha, k, &info, len as usize)
+        hkdf_expand(ha, k, &info, len)
     }
 }
 
@@ -560,6 +560,7 @@ fn put_client_finished(
 // server_init -> (decrypt_zerortt)* | (encrypt_handshake | decrypt_handshake)* ->
 // server_finish -> (encrypt_data | decrypt_data)*
 
+#[allow(clippy::type_complexity)]
 pub fn server_init(
     algs: Algorithms,
     ch: &HandshakeData,