Fix fips-compat build

[kornelski]

Because fips feature implies fips-compat, the negation needs to be not(fips-compat). Except for the ABI changes, which change only with the fips feature.

Just to be safe, I've used .try_into().unwrap() in FFI instead of assert + casts. try_into() will be optimized out when it really doesn't matter.

[ghedo]

I'm not sure I follow, but AFAICT this change kind of defeats the purpose of fips-compat in the first place... what's actually broken?

The point of fips-compat is to have the same API as fips (i.e. BufLen, ProtosLen, ...) but without actually enabling FIPS-mode during build (which allows us to also keep the support for PQ curves, i.e. SSL_CURVE_X25519_KYBER768_DRAFT00), but this change seems to reverse that.

[kornelski]

what's actually broken?

1. Rust API that uses incorrect negation for not-FIPS, which works only with fips, but breaks fips-compat Rust API.
2. FFI that uses fips-compat flag to change the C types for BoringSSL, which do not change with just fips-compat enabled, because the whole point about fips-compat is that it uses the regular non-FIPS BoringSSL, which has its regular non-FIPS C types.

cargo test --all -F fips-compat
   Compiling boring v4.12.0 (boring/boring)
error[E0432]: unresolved import `super::CompliancePolicy`
   --> boring/src/ssl/test/mod.rs:25:5
    |
25  | use super::CompliancePolicy;
    |     ^^^^^^^^^^^^^^^^^^^^^^^ no `CompliancePolicy` in `ssl`
    |
note: found an item that was configured out
   --> boring/src/ssl/mod.rs:778:12
    |
778 | pub struct CompliancePolicy(ffi::ssl_compliance_policy_t);
    |            ^^^^^^^^^^^^^^^^
note: the item is gated here
   --> boring/src/ssl/mod.rs:777:1
    |
777 | #[cfg(not(feature = "fips-compat"))]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
    --> boring/src/bio.rs:33:17
     |
31   |             cvt_p(BIO_new_mem_buf(
     |                   --------------- arguments to this function are incorrect
32   |                 buf.as_ptr() as *const _,
33   |                 buf.len() as BufLen,
     |                 ^^^^^^^^^^^^^^^^^^^ expected `isize`, found `i32`
     |
note: function defined here
    --> boring/target/debug/build/boring-sys-72c63acc268224f2/out/bindings.rs:8555:12
     |
8555 |     pub fn BIO_new_mem_buf(buf: *const ::std::os::raw::c_void, len: ossl_ssize_t) -> *mut BIO;
     |            ^^^^^^^^^^^^^^^
help: you can convert an `i32` to an `isize` and panic if the converted value doesn't fit
     |
33   |                 (buf.len() as BufLen).try_into().unwrap(),
     |                 +                   +++++++++++++++++++++

error[E0308]: mismatched types
     --> boring/src/ssl/mod.rs:1528:17
      |
1525  |             let r = ffi::SSL_CTX_set_alpn_protos(
      |                     ---------------------------- arguments to this function are incorrect
...
1528  |                 protocols.len() as ProtosLen,
      |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `usize`, found `u32`
      |
note: function defined here
     --> boring/target/debug/build/boring-sys-72c63acc268224f2/out/bindings.rs:28525:12
      |
28525 |     pub fn SSL_CTX_set_alpn_protos(
      |            ^^^^^^^^^^^^^^^^^^^^^^^
help: you can convert a `u32` to a `usize` and panic if the converted value doesn't fit
      |
1528  |                 (protocols.len() as ProtosLen).try_into().unwrap(),
      |                 +                            +++++++++++++++++++++

error[E0599]: no method named `set_compliance_policy` found for struct `SslContextBuilder` in the current scope
   --> boring/src/ssl/test/mod.rs:993:9
    |
993 |     ctx.set_compliance_policy(CompliancePolicy::FIPS_202205)
    |         ^^^^^^^^^^^^^^^^^^^^^ method not found in `SslContextBuilder`
    |
   ::: boring/src/ssl/mod.rs:910:1
    |
910 | pub struct SslContextBuilder {
    | ---------------------------- method `set_compliance_policy` not found for this struct

error[E0308]: mismatched types
     --> boring/src/ssl/mod.rs:2953:17
      |
2950  |             let r = ffi::SSL_set_alpn_protos(
      |                     ------------------------ arguments to this function are incorrect
...
2953  |                 protocols.len() as ProtosLen,
      |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `usize`, found `u32`
      |
note: function defined here
     --> boring/target/debug/build/boring-sys-72c63acc268224f2/out/bindings.rs:28532:12
      |
28532 |     pub fn SSL_set_alpn_protos(
      |            ^^^^^^^^^^^^^^^^^^^
help: you can convert a `u32` to a `usize` and panic if the converted value doesn't fit
      |
2953  |                 (protocols.len() as ProtosLen).try_into().unwrap(),
      |                 +                            +++++++++++++++++++++

error[E0599]: no method named `set_compliance_policy` found for struct `SslContextBuilder` in the current scope
    --> boring/src/ssl/test/mod.rs:1014:9
     |
1014 |     ctx.set_compliance_policy(CompliancePolicy::WPA3_192_202304)
     |         ^^^^^^^^^^^^^^^^^^^^^ method not found in `SslContextBuilder`
     |
    ::: boring/src/ssl/mod.rs:910:1
     |
910  | pub struct SslContextBuilder {
     | ---------------------------- method `set_compliance_policy` not found for this struct

error[E0599]: no method named `set_compliance_policy` found for struct `SslContextBuilder` in the current scope
    --> boring/src/ssl/test/mod.rs:1032:9
     |
1032 |     ctx.set_compliance_policy(CompliancePolicy::NONE)
     |         ^^^^^^^^^^^^^^^^^^^^^ method not found in `SslContextBuilder`
     |
    ::: boring/src/ssl/mod.rs:910:1
     |
910  | pub struct SslContextBuilder {
     | ---------------------------- method `set_compliance_policy` not found for this struct

error[E0308]: mismatched types
     --> boring/src/x509/mod.rs:893:17
      |
888   |             cvt(ffi::X509_NAME_add_entry_by_txt(
      |                 ------------------------------- arguments to this function are incorrect
...
893   |                 value.len() as ValueLen,
      |                 ^^^^^^^^^^^^^^^^^^^^^^^ expected `isize`, found `i32`
      |
note: function defined here
     --> boring/target/debug/build/boring-sys-72c63acc268224f2/out/bindings.rs:23119:12
      |
23119 |     pub fn X509_NAME_add_entry_by_txt(
      |            ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: you can convert an `i32` to an `isize` and panic if the converted value doesn't fit
      |
893   |                 (value.len() as ValueLen).try_into().unwrap(),
      |                 +                       +++++++++++++++++++++

error[E0308]: mismatched types
     --> boring/src/x509/mod.rs:920:17
      |
915   |             cvt(ffi::X509_NAME_add_entry_by_txt(
      |                 ------------------------------- arguments to this function are incorrect
...
920   |                 value.len() as ValueLen,
      |                 ^^^^^^^^^^^^^^^^^^^^^^^ expected `isize`, found `i32`
      |
note: function defined here
     --> boring/target/debug/build/boring-sys-72c63acc268224f2/out/bindings.rs:23119:12
      |
23119 |     pub fn X509_NAME_add_entry_by_txt(
      |            ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: you can convert an `i32` to an `isize` and panic if the converted value doesn't fit
      |
920   |                 (value.len() as ValueLen).try_into().unwrap(),
      |                 +                       +++++++++++++++++++++

error[E0308]: mismatched types
     --> boring/src/x509/mod.rs:941:17
      |
936   |             cvt(ffi::X509_NAME_add_entry_by_NID(
      |                 ------------------------------- arguments to this function are incorrect
...
941   |                 value.len() as ValueLen,
      |                 ^^^^^^^^^^^^^^^^^^^^^^^ expected `isize`, found `i32`
      |
note: function defined here
     --> boring/target/debug/build/boring-sys-72c63acc268224f2/out/bindings.rs:23108:12
      |
23108 |     pub fn X509_NAME_add_entry_by_NID(
      |            ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: you can convert an `i32` to an `isize` and panic if the converted value doesn't fit
      |
941   |                 (value.len() as ValueLen).try_into().unwrap(),
      |                 +                       +++++++++++++++++++++

error[E0308]: mismatched types
     --> boring/src/x509/mod.rs:967:17
      |
962   |             cvt(ffi::X509_NAME_add_entry_by_NID(
      |                 ------------------------------- arguments to this function are incorrect
...
967   |                 value.len() as ValueLen,
      |                 ^^^^^^^^^^^^^^^^^^^^^^^ expected `isize`, found `i32`
      |
note: function defined here
     --> boring/target/debug/build/boring-sys-72c63acc268224f2/out/bindings.rs:23108:12
      |
23108 |     pub fn X509_NAME_add_entry_by_NID(
      |            ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: you can convert an `i32` to an `isize` and panic if the converted value doesn't fit
      |
967   |                 (value.len() as ValueLen).try_into().unwrap(),
      |                 +                       +++++++++++++++++++++

[ghedo]

Oh, I see what you mean. I think the missing context here is that fips-compat was intended to be used with a custom BoringSSL version and fips-link-precompiled (otherwise there's not much point in enabling the feature at all) https://github.com/cloudflare/boring/blob/master/boring/Cargo.toml#L24-L27

Though this is clearly not documented properly.

An alternative fix would be to switch the BoringSSL source to deps/boringssl-fips when fips-compat is enabled, just to maybe avoid the confusion?

[kornelski]

Oh, I see. I misunderstood this as testing the FIPS API without the FIPS build (because e.g. it's not possible to build FIPS on macOS any more — the newest FIPS-certified clang is older than the oldest Apple-supported clang).