Skip to content

Commit

Permalink
Update chrono, remove time to fix audit failures (#2285)
Browse files Browse the repository at this point in the history
Closes #1366 and incidentally closes #1266.

Requires using a not-yet-merged modification to the latest version of `appinsights-rs` to remove the `time` feature from the `chrono` dependency (dmolokanov/appinsights-rs#280). 

There are changes to use the new (tokio-based) version of `appinsights-rs`, e.g., made `set_appinsights_clients` async to ensure it is always called from an async context, since the constructor for appinsights now invokes `Tokio::spawn`.
  • Loading branch information
MichaelMatschiner authored Sep 7, 2022
1 parent 8adc6b0 commit 2cad805
Show file tree
Hide file tree
Showing 17 changed files with 204 additions and 112 deletions.
35 changes: 14 additions & 21 deletions src/agent/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/agent/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,8 @@ members = [

[profile.release]
lto = "thin"

[patch.crates-io]
# TODO: remove once PR is merged upstream https://github.com/dmolokanov/appinsights-rs/pull/280
# this rev is on the branch 'audit-fixes'
appinsights = { git = "https://github.com/Porges/appinsights-rs", rev="8264e0236c7a2729c4af4a2ba0017ca0270d7d0e" }
4 changes: 3 additions & 1 deletion src/agent/coverage/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ winapi = "0.3"

[target.'cfg(target_os = "linux")'.dependencies]
pete = "0.9"
procfs = "0.12"
# For procfs, opt out of the `chrono` freature; it pulls in an old version
# of `time`. We do not use the methods that the `chrono` feature enables.
procfs = { version = "0.12", default-features = false, features=["flate2"] }

[dev-dependencies]
env_logger = "0.9"
Expand Down
17 changes: 10 additions & 7 deletions src/agent/onefuzz-agent/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,17 @@ fn run(opt: RunOpt) -> Result<()> {
}

// We can't send telemetry if this fails.
let config = load_config(opt);
let rt = tokio::runtime::Runtime::new()?;
let config = rt.block_on(load_config(opt));

// We can't send telemetry, because we couldn't get a telemetry key from the config.
// Instead, log to an assumed-redirected stdout for the sake of debugging.
if let Err(err) = &config {
error!("error loading supervisor agent config: {:?}", err);
}

let config = config?;

let rt = tokio::runtime::Runtime::new()?;
let result = rt.block_on(run_agent(config));

if let Err(err) = &result {
Expand All @@ -191,20 +193,20 @@ fn run(opt: RunOpt) -> Result<()> {
}
}

telemetry::try_flush_and_close();
rt.block_on(telemetry::try_flush_and_close());

result
}

fn load_config(opt: RunOpt) -> Result<StaticConfig> {
async fn load_config(opt: RunOpt) -> Result<StaticConfig> {
info!("loading supervisor agent config");

let config = match &opt.config_path {
Some(config_path) => StaticConfig::from_file(config_path)?,
None => StaticConfig::from_env()?,
};

init_telemetry(&config);
init_telemetry(&config).await;

Ok(config)
}
Expand Down Expand Up @@ -320,9 +322,10 @@ async fn run_agent(config: StaticConfig) -> Result<()> {
Ok(())
}

fn init_telemetry(config: &StaticConfig) {
async fn init_telemetry(config: &StaticConfig) {
telemetry::set_appinsights_clients(
config.instance_telemetry_key.clone(),
config.microsoft_telemetry_key.clone(),
);
)
.await;
}
2 changes: 1 addition & 1 deletion src/agent/onefuzz-task/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ tokio-stream = "0.1"
tui = { version = "0.18", default-features = false, features = ['crossterm'] }
url = { version = "2.2", features = ["serde"] }
uuid = { version = "0.8", features = ["serde", "v4"] }
chrono = "0.4"
chrono = { version = "0.4", default-features = false, features = ["clock", "std"] }

azure_core = { version = "0.4", default-features = false, features = ["enable_reqwest_rustls"] }
azure_storage = { version = "0.5", default-features = false, features = ["enable_reqwest_rustls"] }
Expand Down
9 changes: 5 additions & 4 deletions src/agent/onefuzz-task/src/managed/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub async fn run(args: &clap::ArgMatches<'_>) -> Result<()> {
let setup_dir = value_t!(args, "setup_dir", PathBuf)?;
let config = Config::from_file(config_path, setup_dir)?;

init_telemetry(config.common());
init_telemetry(config.common()).await;

let min_available_memory_bytes = 1_000_000 * config.common().min_available_memory_mb;

Expand Down Expand Up @@ -54,7 +54,7 @@ pub async fn run(args: &clap::ArgMatches<'_>) -> Result<()> {
error!("error running task: {:?}", err);
}

onefuzz_telemetry::try_flush_and_close();
onefuzz_telemetry::try_flush_and_close().await;

// wait for the task logger to finish
if let Some(task_logger) = task_logger {
Expand Down Expand Up @@ -111,11 +111,12 @@ struct OutOfMemory {
min_bytes: u64,
}

fn init_telemetry(config: &CommonConfig) {
async fn init_telemetry(config: &CommonConfig) {
onefuzz_telemetry::set_appinsights_clients(
config.instance_telemetry_key.clone(),
config.microsoft_telemetry_key.clone(),
);
)
.await;
}

pub fn args(name: &str) -> App<'static, 'static> {
Expand Down
8 changes: 2 additions & 6 deletions src/agent/onefuzz-telemetry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,13 @@ z3 = ["z3-sys"]
intel_instructions = ["iced-x86"]

[dependencies]
# appinsights-rs haas included optional support for rustls since 2020-10, but
# not the feature has not been released yet. This is the pinned to the most
# recent git hash as of 2021-06-30. Once released, this should be reverted to
# use released versions
anyhow = "1.0"
appinsights = { git = "https://github.com/dmolokanov/appinsights-rs", rev = "0af6ec83bad1c050160f5258ab08e9834596ce20", features=["rustls"], default-features = false }
appinsights = { version = "0.2.2", default-features = false, features = ["rustls"] }
log = "0.4"
uuid = { version = "0.8", features = ["serde", "v4"] }
serde = { version = "1.0", features = ["derive"] }
z3-sys = { version = "0.6", optional = true}
iced-x86 = { version = "1.17", optional = true}
tokio = { version = "1.16", features = ["full"] }
lazy_static = "1.4"
chrono = "0.4"
chrono = { version = "0.4", default-features = false, features = ["clock", "std"] }
12 changes: 7 additions & 5 deletions src/agent/onefuzz-telemetry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl fmt::Display for InstanceTelemetryKey {
}
}

pub type TelemetryClient = appinsights::TelemetryClient<appinsights::InMemoryChannel>;
pub type TelemetryClient = appinsights::TelemetryClient;
pub enum ClientType {
Instance,
Microsoft,
Expand Down Expand Up @@ -450,7 +450,10 @@ mod global {
}

const REDACTED: &str = "Redacted";
pub fn set_appinsights_clients(
// This function doesn't do anything async, but TelemetryClient::new must be invoked
// upon a Tokio runtime task, since it calls Tokio::spawn. The easiest way to ensure this
// statically is to make this function async.
pub async fn set_appinsights_clients(
instance_key: Option<InstanceTelemetryKey>,
microsoft_key: Option<MicrosoftTelemetryKey>,
) {
Expand Down Expand Up @@ -481,12 +484,11 @@ pub fn set_appinsights_clients(
/// Meant for a final attempt at flushing pending items before an abnormal exit.
/// After calling this function, any existing telemetry client will be dropped,
/// and subsequent telemetry submission will be a silent no-op.
pub fn try_flush_and_close() {
pub async fn try_flush_and_close() {
let clients = global::take_clients();

for client in clients {
client.flush_channel();
client.close_channel();
client.close_channel().await;
}

// dropping the broadcast sender to make sure all pending events are sent
Expand Down
9 changes: 1 addition & 8 deletions src/ci/agent.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,9 @@ if [ X${CARGO_INCREMENTAL} == X ]; then
fi

cargo fmt -- --check
# RUSTSEC-2020-0016: a dependency `net2` (pulled in from tokio) is deprecated
# RUSTSEC-2020-0036: a dependency `failure` (pulled from proc-maps) is deprecated
# RUSTSEC-2019-0036: a dependency `failure` (pulled from proc-maps) has type confusion vulnerability
# RUSTSEC-2021-0065: a dependency `anymap` is no longer maintained
# RUSTSEC-2020-0077: `memmap` dependency unmaintained, via `symbolic` (see: `getsentry/symbolic#304`)
# RUSTSEC-2020-0159: potential segfault in `time`, not yet patched (#1366)
# RUSTSEC-2020-0071: potential segfault in `chrono`, not yet patched (#1366)
# RUSTSEC-2022-0048: xml-rs is unmaintained
# RUSTSEC-2021-0139: ansi_term is unmaintained
cargo audit --deny warnings --deny unmaintained --deny unsound --deny yanked --ignore RUSTSEC-2020-0016 --ignore RUSTSEC-2020-0036 --ignore RUSTSEC-2019-0036 --ignore RUSTSEC-2021-0065 --ignore RUSTSEC-2020-0159 --ignore RUSTSEC-2020-0071 --ignore RUSTSEC-2020-0077 --ignore RUSTSEC-2022-0048 --ignore RUSTSEC-2021-0139
cargo audit --deny warnings --deny unmaintained --deny unsound --deny yanked --ignore RUSTSEC-2022-0048 --ignore RUSTSEC-2021-0139
cargo-license -j > data/licenses.json
cargo build --release --locked
cargo clippy --release --locked --all-targets -- -D warnings
Expand Down
6 changes: 1 addition & 5 deletions src/ci/proxy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,9 @@ mkdir -p artifacts/proxy
cd src/proxy-manager
cargo fmt -- --check
cargo clippy --release --all-targets -- -D warnings
# RUSTSEC-2020-0016: a dependency `net2` (pulled in from `tokio`) is deprecated
# RUSTSEC-2021-0065: a dependency `anymap` is no longer supported
# RUSTSEC-2020-0159: potential segfault in `time`, not yet patched (#1366)
# RUSTSEC-2020-0071: potential segfault in `chrono`, not yet patched (#1366)
# RUSTSEC-2022-0048: xml-rs is unmaintained
# RUSTSEC-2021-0139: ansi_term is unmaintained
cargo audit --deny warnings --deny unmaintained --deny unsound --deny yanked --ignore RUSTSEC-2020-0016 --ignore RUSTSEC-2021-0065 --ignore RUSTSEC-2020-0159 --ignore RUSTSEC-2020-0071 --ignore RUSTSEC-2022-0048 --ignore RUSTSEC-2021-0139
cargo audit --deny warnings --deny unmaintained --deny unsound --deny yanked --ignore RUSTSEC-2022-0048 --ignore RUSTSEC-2021-0139
cargo-license -j > data/licenses.json
cargo build --release --locked
# export RUST_LOG=trace
Expand Down
4 changes: 2 additions & 2 deletions src/integration-tests/integration-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ def check_logs_for_errors(self) -> None:
# about errors
if (
entry.get("severityLevel") == 2
and entry.get("sdkVersion") == "rust:0.1.5"
and "rust" in entry.get("sdkVersion")
):
continue

Expand All @@ -897,7 +897,7 @@ def check_logs_for_errors(self) -> None:
if (
"storage queue pop failed" in message
or "storage queue delete failed" in message
) and entry.get("sdkVersion") == "rust:0.1.5":
) and ("rust" in entry.get("sdkVersion")):
continue

if message is None:
Expand Down
Loading

0 comments on commit 2cad805

Please sign in to comment.