Skip to content

Commit

Permalink
QoL changes for better CI integration (tlspuffin#304)
Browse files Browse the repository at this point in the history 
* feat(openssl): implement non-clear mode for OpenSSL PUT

During PUT OpenSSL initialization a confusing message about the
non-availability of clear mode is logged, but only when clear mode is NOT
selected. In fact, the OpenSSL PUT only implements clear mode rather than a
full reset of the agents.

This is confusing on two levels:
- the message hints that a full reset of OpenSSL agents is always performed but
  they are, on the contrary, always soft reset.
- the process continues with the soft reset without making it clear to the user
  that the option they have selected won't be applied.

We implement the non-clear mode in the OpenSSL PUT to get rid of the message.

Fixes: tlspuffin#154

* fix(cli): don't use tlspuffin exit code to convey semantically meaningful data

The `tlspuffin execute` command exits with 0 or 1 to signal if there are
potentially more inputs that were not executed. This has been used for batch
executions during coverage analysis.

Numerous tools, including the running shell, associate a single meaning to
non-zero exit code: the command failed to execute in the way intended by the
caller. Depending on how they are configured, these tools might behave
differently when they detect a non-zero return code (e.g. bash "set -e").

To avoid any confusion, we instead log the total number of inputs, so that
calling scripts can still retrieve it without relying on the exit code.

* fix(cli): display missing trace file when the `execute` command fails

* docs(cli): fix seeds directory name in CLI help and documentation

* fix(tlspuffin): don't generate incompatible seeds when there is no built-in PUT

When no PUT is selected at build-time, the `tlspuffin seed` command still
generates seeds. These seeds are incompatible with the default TCP PUT and
`tlspuffin execute` fails on this initial corpus.

We fix the `tlspuffin seed` command to make sure that it only generates seeds
that are compatible with the build-time configuration of tlspuffin.

* fix(cli): remove duplicated log message when generating seeds

* fix(cli): correct default `-n <num>` value when running `execute` on directories

When the `execute` subcommand is run, it executes by default all input traces
provided on the command line. This is done by defaulting the `-n <num>` CLI
argument to the number of provided arguments.

When a directory is part of the command line inputs, all input traces in that
directory will be executed. Since the number of inputs doesn't match the number
of arguments on the command line anymore, `execute` will wrongly ignore part of
the inputs and only run on a subset of the provided inputs.

We fix this inconsistency by using the total number of traces found as default
value for `-n <num>`.

* refactor(openssl): fix clippy errors in openssl-src

* revert(cli): revert changes to `execute` command, introduce `execute-traces` instead

We introduce a new command to avoid breaking coverage workflows. In the future,
we want to merge these commands but this will require having stable coverage
scripts first.
  • Loading branch information
@michaelmera
michaelmera authored Apr 2, 2024
1 parent 310bb2f commit 86bd731
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 75 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ Now we will go over the sub-commands execute, plot, experiment, and seed.
* **experiment**
> This sub-command initiates an experiment. Experiments are stored in a directory named experiments/ in the current working directory. An experiment consists of a directory which contains . The title and description of the experiment can be specified with --title ⟨t⟩ and --description ⟨d⟩ respectively. Both strings are persisted in the metadata of the experiment, together with the current commit hash of , the version and the current date and time.
* **seed**
> This sub-command serializes the default seed corpus in a directory named corpus/ in the current working directory. The default corpus is defined in the source code of using the trace dsl.
> This sub-command serializes the default seed corpus in a directory named seeds/ in the current working directory. The default corpus is defined in the source code using the trace dsl.

## Rust Setup
Expand Down
72 changes: 34 additions & 38 deletions crates/openssl-src-111/src/openssl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ extern crate cc;

use std::io::ErrorKind;
use std::{
env, fs,
fs::{canonicalize, File},
env,
fs::{self, canonicalize, File},
io,
io::Write,
path::{Path, PathBuf},
Expand Down Expand Up @@ -48,21 +48,6 @@ pub fn version() -> &'static str {
env!("CARGO_PKG_VERSION")
}

fn patch_openssl<P: AsRef<Path>>(out_dir: P, patch: &str) -> std::io::Result<()> {
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
let status = Command::new("git")
.current_dir(out_dir)
.arg("am")
.arg(root.join("patches").join(patch).to_str().unwrap())
.status()?;

if !status.success() {
return Err(io::Error::from(ErrorKind::Other));
}

Ok(())
}

pub struct Build {
out_dir: Option<PathBuf>,
target: Option<String>,
Expand Down Expand Up @@ -105,10 +90,10 @@ impl Build {
Command::new("make")
}

pub fn build_deterministic_rand(install_dir: &PathBuf) {
pub fn build_deterministic_rand(install_dir: &Path) {
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
let file = root.join("src").join("deterministic_rand.c");
let buf = canonicalize(&file).unwrap();
let buf = canonicalize(file).unwrap();
let deterministic_rand = buf.to_str().unwrap();

println!("cargo:rerun-if-changed={}", deterministic_rand);
Expand All @@ -119,7 +104,7 @@ impl Build {
.compile("deterministic_rand");
}

pub fn insert_claim_interface(additional_headers: &PathBuf) -> std::io::Result<()> {
pub fn insert_claim_interface(additional_headers: &Path) -> std::io::Result<()> {
let interface = security_claims::CLAIM_INTERFACE_H;

let path = additional_headers.join("claim-interface.h");
Expand Down Expand Up @@ -149,7 +134,7 @@ impl Build {
let inner_dir = build_dir.join("src");
fs::create_dir_all(&inner_dir).unwrap();

clone_repo(&inner_dir.to_str().unwrap()).unwrap();
clone_repo(inner_dir.to_str().unwrap()).unwrap();

let perl_program =
env::var("OPENSSL_SRC_PERL").unwrap_or(env::var("PERL").unwrap_or("perl".to_string()));
Expand All @@ -164,6 +149,7 @@ impl Build {
// No shared objects, we just want static libraries
.arg("no-dso")
.arg("no-shared");

// No need to build tests, we won't run them anyway
// TODO .arg("no-unit-test")
// Nothing related to zlib please
Expand All @@ -172,17 +158,23 @@ impl Build {
// TODO .arg("no-zlib-dynamic")

// TODO: does only work when combinded with rand.patch?
//configure.arg("--with-rand-seed=none");

if cfg!(feature = "weak-crypto") {
// TODO configure.arg("enable-md2").arg("enable-rc5").arg("enable-weak-ssl-ciphers");
} else {
// TODO configure.arg("no-md2").arg("no-rc5").arg("no-weak-ssl-ciphers");
}

if cfg!(not(feature = "seed")) {
// TODO configure.arg("no-seed");
}
// configure.arg("--with-rand-seed=none");

// if cfg!(feature = "weak-crypto") {
// configure
// .arg("enable-md2")
// .arg("enable-rc5")
// .arg("enable-weak-ssl-ciphers");
// } else {
// configure
// .arg("no-md2")
// .arg("no-rc5")
// .arg("no-weak-ssl-ciphers");
// }

// if cfg!(not(feature = "seed")) {
// configure.arg("no-seed");
// }

let os = match target {
"aarch64-apple-darwin" => "darwin64-arm64-cc",
Expand All @@ -192,9 +184,9 @@ impl Build {
};
configure.arg(os);

if cfg!(feature = "no-rand") {
// TODO configure.arg("-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION");
}
// if cfg!(feature = "no-rand") {
// configure.arg("-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION");
// }

let cc = "clang".to_owned();
let mut cflags = "".to_owned();
Expand Down Expand Up @@ -258,8 +250,6 @@ impl Build {

self.run_command(install, "installing OpenSSL");

let libs = vec!["ssl".to_string(), "crypto".to_string()];

if cfg!(feature = "no-rand") {
Self::build_deterministic_rand(&install_dir);
}
Expand All @@ -268,7 +258,7 @@ impl Build {
lib_dir: install_dir.join("lib"),
bin_dir: install_dir.join("bin"),
include_dir: install_dir.join("include"),
libs: libs,
libs: vec!["ssl".to_string(), "crypto".to_string()],
target: target.to_string(),
}
}
Expand All @@ -293,6 +283,12 @@ Error {}:
}
}

impl Default for Build {
fn default() -> Self {
Self::new()
}
}

impl Artifacts {
pub fn include_dir(&self) -> &Path {
&self.include_dir
Expand Down
5 changes: 1 addition & 4 deletions evaluation-paper-2023/coverage.sh
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#!/bin/bash


#!/usr/bin/env bash

source ~/venv/bin/activate
source "$HOME/.cargo/env"
Expand Down Expand Up @@ -47,7 +45,6 @@ generate_coverage () {

cd "$build_dir"

#cargo clean
find -name "*.gcda" -delete
cargo build -p tlspuffin --target x86_64-unknown-linux-gnu --features "$features,gcov_analysis" --no-default-features

Expand Down
69 changes: 61 additions & 8 deletions puffin/src/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn create_app() -> Command {
.arg(arg!(-t --title <t> "Title of the experiment"))
.arg(arg!(-d --description <d> "Descritpion of the experiment"))
,
Command::new("seed").about("Generates seeds to ./corpus"),
Command::new("seed").about("Generates seeds to ./seeds"),
Command::new("plot")
.about("Plots a trace stored in a file")
.arg(arg!(<input> "The file which stores a trace"))
Expand All @@ -66,6 +66,9 @@ fn create_app() -> Command {
.arg(arg!(-n --number <n> "Amount of files to execute starting at index.").value_parser(value_parser!(usize)))
.arg(arg!(-i --index <i> "Index of file to execute.").value_parser(value_parser!(usize)))
.arg(arg!(-s --sort "Sort files in ascending order by the creation date before executing")),
Command::new("execute-traces")
.about("Executes traces stored in files.")
.arg(arg!(<inputs> "The file which stores a trace").num_args(1..)),
Command::new("binary-attack")
.about("Serializes a trace as much as possible and output its")
.arg(arg!(<input> "The file which stores a trace"))
Expand Down Expand Up @@ -148,7 +151,6 @@ pub fn main<PB: ProtocolBehavior + Clone + 'static>(
} else if let Some(matches) = matches.subcommand_matches("execute") {
let inputs: ValuesRef<String> = matches.get_many("inputs").unwrap();
let index: usize = *matches.get_one("index").unwrap_or(&0);
let n: usize = *matches.get_one("number").unwrap_or(&inputs.len());

let mut paths = inputs
.flat_map(|input| {
Expand All @@ -159,7 +161,7 @@ pub fn main<PB: ProtocolBehavior + Clone + 'static>(
.expect("failed to read directory")
.map(|entry| entry.expect("failed to read path in directory").path())
.filter(|path| {
!path.file_name().unwrap().to_str().unwrap().starts_with(".")
!path.file_name().unwrap().to_str().unwrap().starts_with('.')
})
.collect()
} else {
Expand All @@ -168,7 +170,14 @@ pub fn main<PB: ProtocolBehavior + Clone + 'static>(
})
.collect::<Vec<_>>();

paths.sort_by_key(|path| fs::metadata(path).unwrap().modified().unwrap());
paths.sort_by_key(|path| {
fs::metadata(path)
.unwrap_or_else(|_| panic!("missing trace file {}", path.display()))
.modified()
.unwrap()
});

let n: usize = *matches.get_one("number").unwrap_or(&paths.len());

let mut end_reached = false;

Expand All @@ -185,16 +194,24 @@ pub fn main<PB: ProtocolBehavior + Clone + 'static>(
&paths[0..0]
};

info!("execute: found {} inputs", paths.len());
info!(
"execute: running on subset [{}..{}] ({} inputs)",
index,
index + n,
lookup_paths.len()
);

for path in lookup_paths {
info!("Executing: {}", path.display());
execute(&path, put_registry);
execute(path, put_registry);
}

if !lookup_paths.is_empty() {
println!(
"{}",
fs::metadata(&lookup_paths[0])
.unwrap()
.unwrap_or_else(|_| panic!("missing trace file {}", lookup_paths[0].display()))
.modified()
.unwrap()
.duration_since(std::time::UNIX_EPOCH)
Expand All @@ -208,6 +225,42 @@ pub fn main<PB: ProtocolBehavior + Clone + 'static>(
} else {
return ExitCode::SUCCESS;
}
} else if let Some(matches) = matches.subcommand_matches("execute-traces") {
let inputs: ValuesRef<String> = matches.get_many("inputs").unwrap();

let mut paths = inputs
.flat_map(|input| {
let input = PathBuf::from(input);

if input.is_dir() {
fs::read_dir(input)
.expect("failed to read directory")
.map(|entry| entry.expect("failed to read path in directory").path())
.filter(|path| {
!path.file_name().unwrap().to_str().unwrap().starts_with('.')
})
.collect()
} else {
vec![input]
}
})
.collect::<Vec<_>>();

paths.sort_by_key(|path| {
fs::metadata(path)
.unwrap_or_else(|_| panic!("missing trace file {}", path.display()))
.modified()
.unwrap()
});

info!("execute: found {} inputs", paths.len());

for path in paths {
info!("Executing: {}", path.display());
execute(path, put_registry);
}

return ExitCode::SUCCESS;
} else if let Some(matches) = matches.subcommand_matches("binary-attack") {
let input: &String = matches.get_one("input").unwrap();
let output: &String = matches.get_one("output").unwrap();
Expand Down Expand Up @@ -384,9 +437,9 @@ fn seed<PB: ProtocolBehavior>(
fs::create_dir_all("./seeds")?;
for (trace, name) in PB::create_corpus() {
trace.to_file(format!("./seeds/{}.trace", name))?;

info!("Generated seed traces into the directory ./corpus")
}

info!("Generated seed traces into the directory ./seeds");
Ok(())
}

Expand Down
Loading

0 comments on commit 86bd731

Please sign in to comment.