Skip to content
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

Replace dangerous cargo features with rustc flags #263

Merged
merged 3 commits into from Dec 22, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 0 additions & 7 deletions Cargo.toml
Expand Up @@ -26,13 +26,6 @@ endomorphism = ["secp256k1-sys/endomorphism"]
lowmemory = ["secp256k1-sys/lowmemory"]
global-context = ["std", "rand-std"]

# Use this feature to not compile the bundled libsecp256k1 C symbols,
# but use external ones. Use this only if you know what you are doing!
external-symbols = ["secp256k1-sys/external-symbols"]

# Do not use this feature! HAZMAT. (meant for Fuzzing only. this is *BROKEN CRYPTOGRAPHY*)
fuzztarget = ["secp256k1-sys/fuzztarget"]

[dependencies]
secp256k1-sys = { version = "0.3.1", default-features = false, path = "./secp256k1-sys" }
bitcoin_hashes = { version = "0.9", optional = true }
Expand Down
9 changes: 9 additions & 0 deletions README.md
Expand Up @@ -35,3 +35,12 @@ before_script:
cargo generate-lockfile --verbose && cargo update -p cc --precise "1.0.41" --verbose;
fi
```

## Fuzzing

If you want to fuzz this library, or any library which depends on it, you will
probably want to disable the actual cryptography, since fuzzers are unable to
forge signatures and therefore won't test many interesting codepaths. To instead
use a trivially-broken but fuzzer-accessible signature scheme, compile with
`--cfg=rust_secp_fuzz` in your `RUSTFLAGS` variable.

4 changes: 2 additions & 2 deletions contrib/test.sh
Expand Up @@ -31,8 +31,8 @@ if [ "$DO_FEATURE_MATRIX" = true ]; then
done

# Other combos
cargo test --no-run --verbose --features="fuzztarget"
cargo test --no-run --verbose --features="fuzztarget recovery"
RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --no-run --verbose
RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --no-run --verbose --features="recovery"
cargo test --verbose --features="rand rand-std"
cargo test --verbose --features="rand serde"

Expand Down
6 changes: 0 additions & 6 deletions secp256k1-sys/Cargo.toml
Expand Up @@ -31,9 +31,3 @@ endomorphism = []
lowmemory = []
std = []

# Use this feature to not compile the bundled libsecp256k1 C symbols,
# but use external ones. Use this only if you know what you are doing!
external-symbols = []

# Do not use this feature! HAZMAT. (meant for Fuzzing only. this is *BROKEN CRYPTOGRAPHY*)
fuzztarget = []
7 changes: 4 additions & 3 deletions secp256k1-sys/README.md
Expand Up @@ -29,6 +29,7 @@ $ ./vendor-libsecp.sh depend <version-code> <rev>

## Linking to external symbols

For the more exotic use cases, this crate can be used with existing libsecp256k1
symbols by using the `external-symbols` feature. How to setup rustc to link
against those existing symbols is left as an exercise to the reader.
If you want to compile this library without using the bundled symbols (which may
be required for integration into other build systems), you can do so by adding
`--cfg=rust_secp_no_symbol_renaming'` to your `RUSTFLAGS` variable.

5 changes: 0 additions & 5 deletions secp256k1-sys/build.rs
Expand Up @@ -26,11 +26,6 @@ extern crate cc;
use std::env;

fn main() {
if cfg!(feature = "external-symbols") {
println!("cargo:rustc-link-lib=static=secp256k1");
return;
}

Comment on lines -29 to -33
Copy link
Member

@elichai elichai Dec 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to leave if cfg!("rust_secp_no_symbol_renaming") here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, all this does is set a flag that Cargo understands ... but if you are using Cargo then you shouldn't be using this flag, since you'll get symbol collisions and other messy results.

// Actual build
let mut base_config = cc::Build::new();
base_config.include("depend/secp256k1/")
Expand Down
119 changes: 60 additions & 59 deletions secp256k1-sys/src/lib.rs

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions secp256k1-sys/src/recovery.rs
Expand Up @@ -16,7 +16,7 @@
//! # FFI of the recovery module

use ::types::*;
#[cfg(not(feature = "fuzztarget"))]
#[cfg(not(rust_secp_fuzz))]
use ::{Context, Signature, NonceFn, PublicKey};

/// Library-internal representation of a Secp256k1 signature + recovery ID
Expand All @@ -36,23 +36,23 @@ impl Default for RecoverableSignature {
}
}

#[cfg(not(feature = "fuzztarget"))]
#[cfg(not(rust_secp_fuzz))]
extern "C" {
#[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_3_1_ecdsa_recoverable_signature_parse_compact")]
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_3_1_ecdsa_recoverable_signature_parse_compact")]
pub fn secp256k1_ecdsa_recoverable_signature_parse_compact(cx: *const Context, sig: *mut RecoverableSignature,
input64: *const c_uchar, recid: c_int)
-> c_int;

#[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_3_1_ecdsa_recoverable_signature_serialize_compact")]
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_3_1_ecdsa_recoverable_signature_serialize_compact")]
pub fn secp256k1_ecdsa_recoverable_signature_serialize_compact(cx: *const Context, output64: *mut c_uchar,
recid: *mut c_int, sig: *const RecoverableSignature)
-> c_int;

#[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_3_1_ecdsa_recoverable_signature_convert")]
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_3_1_ecdsa_recoverable_signature_convert")]
pub fn secp256k1_ecdsa_recoverable_signature_convert(cx: *const Context, sig: *mut Signature,
input: *const RecoverableSignature)
-> c_int;
#[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_3_1_ecdsa_sign_recoverable")]
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_3_1_ecdsa_sign_recoverable")]
pub fn secp256k1_ecdsa_sign_recoverable(cx: *const Context,
sig: *mut RecoverableSignature,
msg32: *const c_uchar,
Expand All @@ -61,7 +61,7 @@ extern "C" {
noncedata: *const c_void)
-> c_int;

#[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_3_1_ecdsa_recover")]
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_3_1_ecdsa_recover")]
pub fn secp256k1_ecdsa_recover(cx: *const Context,
pk: *mut PublicKey,
sig: *const RecoverableSignature,
Expand All @@ -70,7 +70,7 @@ extern "C" {
}


#[cfg(feature = "fuzztarget")]
#[cfg(rust_secp_fuzz)]
mod fuzz_dummy {
extern crate std;
use self::std::ptr;
Expand Down Expand Up @@ -126,6 +126,6 @@ mod fuzz_dummy {
unimplemented!();
}
}
#[cfg(feature = "fuzztarget")]
#[cfg(rust_secp_fuzz)]
pub use self::fuzz_dummy::*;

2 changes: 1 addition & 1 deletion secp256k1-sys/src/types.rs
Expand Up @@ -40,7 +40,7 @@ impl AlignedType {
}
}

#[cfg(all(feature = "std", not(feature = "external-symbols")))]
#[cfg(all(feature = "std", not(rust_secp_no_symbol_renaming)))]
pub(crate) const ALIGN_TO: usize = mem::align_of::<AlignedType>();


Expand Down