Skip to content

Commit

Permalink
chore: split jni and ffi out as separate features (cloudflare#288)
Browse files Browse the repository at this point in the history
We should not be unconditionally building FFI. We also should not be just building JNI on a per-target basis. Both of these should be behind feature flags.

This PR also makes the integration tests ignored, so that they get run in a separate pass and thus don't get invoked repeatedly by `cargo hack`.
  • Loading branch information
Noah-Kennedy authored Jul 11, 2022
1 parent 04eb355 commit 0085c5d
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 506 deletions.
15 changes: 13 additions & 2 deletions .github/workflows/on-push.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ jobs:
toolchain: stable
- uses: taiki-e/install-action@cargo-hack
- run: cargo hack check --each-feature
- run: RUSTFLAGS="--cfg loom" cargo hack check --each-feature
clippy:
strategy:
matrix:
Expand All @@ -30,7 +29,8 @@ jobs:
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
- run: cargo clippy
- uses: taiki-e/install-action@cargo-hack
- run: cargo hack clippy --each-feature
rustfmt:
strategy:
matrix:
Expand All @@ -54,3 +54,14 @@ jobs:
toolchain: stable
- uses: taiki-e/install-action@cargo-hack
- run: cargo hack test --each-feature
integration-tests:
strategy:
matrix:
os: [ ubuntu-latest, macos-latest ]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
- run: cargo test -- --ignored
16 changes: 9 additions & 7 deletions boringtun/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ repository = "https://github.com/cloudflare/boringtun"
documentation = "https://docs.rs/boringtun/0.4.0/boringtun/"
edition = "2018"

[features]
jni-bindings = ["ffi-bindings", "jni"]
ffi-bindings = []

[dependencies]
base64 = "0.13"
hex = "0.4"
Expand All @@ -19,22 +23,20 @@ ip_network = "0.4.1"
ip_network_table = "0.2.0"
ring = "0.16"
x25519-dalek = { version = "2.0.0-pre.1", features = ["reusable_secrets"] }
rand_core = { version = "0.6.3", features = ["getrandom"]}
rand_core = { version = "0.6.3", features = ["getrandom"] }
chacha20poly1305 = "0.10.0-pre.1"
aead = "0.5.0-pre.2"
blake2 = "0.10"
hmac = "0.12"
jni = { version = "0.19.0", optional = true }

[target.'cfg(target_os="macos")'.dependencies]
nix = "0.24.1"

[dev-dependencies]
tracing-subscriber = "0.3"
criterion = { version = "0.3.5", features = ["html_reports"] }

[target.'cfg(target_os="android")'.dependencies]
jni = "0.19.0"

[target.'cfg(target_os="macos")'.dependencies]
nix = "0.24.1"

[[bench]]
name = "crypto_benches"
harness = false
9 changes: 9 additions & 0 deletions boringtun/src/device/integration_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ mod tests {
}

#[test]
#[ignore]
/// Test if wireguard starts and creates a unix socket that we can read from
fn test_wireguard_get() {
let wg = WGHandle::init("192.0.2.0".parse().unwrap(), "::2".parse().unwrap());
Expand All @@ -475,6 +476,7 @@ mod tests {
}

#[test]
#[ignore]
/// Test if wireguard starts and creates a unix socket that we can use to set settings
fn test_wireguard_set() {
let port = next_port();
Expand Down Expand Up @@ -542,6 +544,7 @@ mod tests {

/// Test if wireguard can handle simple ipv4 connections, don't use a connected socket
#[test]
#[ignore]
fn test_wg_start_ipv4_non_connected() {
let port = next_port();
let private_key = StaticSecret::new(OsRng);
Expand Down Expand Up @@ -588,6 +591,7 @@ mod tests {

/// Test if wireguard can handle simple ipv4 connections
#[test]
#[ignore]
fn test_wg_start_ipv4() {
let port = next_port();
let private_key = StaticSecret::new(OsRng);
Expand Down Expand Up @@ -622,6 +626,7 @@ mod tests {
}

#[test]
#[ignore]
/// Test if wireguard can handle simple ipv6 connections
fn test_wg_start_ipv6() {
let port = next_port();
Expand Down Expand Up @@ -657,6 +662,7 @@ mod tests {

/// Test if wireguard can handle connection with an ipv6 endpoint
#[test]
#[ignore]
#[cfg(target_os = "linux")] // Can't make docker work with ipv6 on macOS ATM
fn test_wg_start_ipv6_endpoint() {
let port = next_port();
Expand Down Expand Up @@ -695,6 +701,7 @@ mod tests {

/// Test if wireguard can handle connection with an ipv6 endpoint
#[test]
#[ignore]
#[cfg(target_os = "linux")] // Can't make docker work with ipv6 on macOS ATM
fn test_wg_start_ipv6_endpoint_not_connected() {
let port = next_port();
Expand Down Expand Up @@ -744,6 +751,7 @@ mod tests {

/// Test many concurrent connections
#[test]
#[ignore]
fn test_wg_concurrent() {
let port = next_port();
let private_key = StaticSecret::new(OsRng);
Expand Down Expand Up @@ -794,6 +802,7 @@ mod tests {

/// Test many concurrent connections
#[test]
#[ignore]
fn test_wg_concurrent_v6() {
let port = next_port();
let private_key = StaticSecret::new(OsRng);
Expand Down
9 changes: 5 additions & 4 deletions boringtun/src/jni.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Copyright (c) 2019 Cloudflare, Inc. All rights reserved.
// SPDX-License-Identifier: BSD-3-Clause

// temporary, we need to do some verification around these bindings later
#![allow(clippy::missing_safety_doc)]

/// JNI bindings for BoringTun library
use std::os::raw::c_char;
use std::ptr;
Expand All @@ -10,8 +13,6 @@ use jni::strings::JNIStr;
use jni::sys::{jbyteArray, jint, jlong, jshort, jstring};
use jni::JNIEnv;

use std::str::FromStr;

use crate::ffi::new_tunnel;
use crate::ffi::wireguard_read;
use crate::ffi::wireguard_result;
Expand All @@ -36,7 +37,7 @@ pub extern "C" fn log_print(_log_string: *const c_char) {
#[no_mangle]
#[export_name = "Java_com_cloudflare_app_boringtun_BoringTunJNI_x25519_1secret_1key"]
pub extern "C" fn generate_secret_key(env: JNIEnv, _class: JClass) -> jbyteArray {
match env.byte_array_from_slice(&x25519_secret_key().as_bytes()) {
match env.byte_array_from_slice(&x25519_secret_key().key) {
Ok(v) => v,
Err(_) => ptr::null_mut(),
}
Expand All @@ -63,7 +64,7 @@ pub unsafe extern "C" fn generate_public_key1(
key: std::mem::transmute::<[i8; 32], [u8; 32]>(key_inner),
};

match env.byte_array_from_slice(&x25519_public_key(secret_key).as_bytes()) {
match env.byte_array_from_slice(&x25519_public_key(secret_key).key) {
Ok(v) => v,
Err(_) => ptr::null_mut(),
}
Expand Down
3 changes: 2 additions & 1 deletion boringtun/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
#[cfg(not(any(target_os = "windows", target_os = "android", target_os = "ios")))]
pub mod device;

#[cfg(feature = "ffi-bindings")]
pub mod ffi;
pub mod noise;

#[cfg(target_os = "android")]
#[cfg(feature = "jni-bindings")]
pub mod jni;

pub(crate) mod serialization;
2 changes: 0 additions & 2 deletions boringtun/src/noise/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ pub mod handshake;
pub mod rate_limiter;

mod session;
#[cfg(test)]
mod tests;
mod timers;

use crate::noise::errors::WireGuardError;
Expand Down
Loading

0 comments on commit 0085c5d

Please sign in to comment.