From d1cfa2abd6e9cc41fe6227e27abaa3b6ab564ae8 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 28 Sep 2020 14:59:57 -0700 Subject: [PATCH] Redact work set fields (#34) - Provide full set of `expose[..]` methods for `Secret` - Redact serialized work unit config in logging - Use `BlobContainerUrl` for work set setup URL --- src/agent/onefuzz-supervisor/src/agent/tests.rs | 9 +++++---- src/agent/onefuzz-supervisor/src/auth.rs | 16 +++++++++++++++- src/agent/onefuzz-supervisor/src/config.rs | 4 ++-- src/agent/onefuzz-supervisor/src/coordinator.rs | 8 ++++---- src/agent/onefuzz-supervisor/src/debug.rs | 5 +++-- src/agent/onefuzz-supervisor/src/setup.rs | 9 +++------ src/agent/onefuzz-supervisor/src/work.rs | 7 ++++--- src/agent/onefuzz-supervisor/src/worker.rs | 2 +- src/agent/onefuzz-supervisor/src/worker/tests.rs | 2 +- src/agent/onefuzz/src/blob/url.rs | 12 +++++++++++- 10 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/agent/onefuzz-supervisor/src/agent/tests.rs b/src/agent/onefuzz-supervisor/src/agent/tests.rs index 4c41df696..f9acde8fc 100644 --- a/src/agent/onefuzz-supervisor/src/agent/tests.rs +++ b/src/agent/onefuzz-supervisor/src/agent/tests.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use url::Url; +use onefuzz::blob::BlobContainerUrl; use uuid::Uuid; use crate::coordinator::double::*; @@ -68,12 +68,13 @@ impl Fixture { } } - pub fn setup_url(&self) -> Url { - "https://contoso.com/my-setup-container".parse().unwrap() + pub fn setup_url(&self) -> BlobContainerUrl { + let url = "https://contoso.com/my-setup-container"; + BlobContainerUrl::parse(&url).unwrap() } pub fn work_unit(&self) -> WorkUnit { - let config = r#"{ "hello": "world" }"#.into(); + let config = r#"{ "hello": "world" }"#.to_owned().into(); WorkUnit { job_id: self.job_id(), diff --git a/src/agent/onefuzz-supervisor/src/auth.rs b/src/agent/onefuzz-supervisor/src/auth.rs index 5b727c7a3..528ec1e93 100644 --- a/src/agent/onefuzz-supervisor/src/auth.rs +++ b/src/agent/onefuzz-supervisor/src/auth.rs @@ -11,9 +11,17 @@ use uuid::Uuid; pub struct Secret(T); impl Secret { - pub fn expose(&self) -> &T { + pub fn expose(self) -> T { + self.0 + } + + pub fn expose_ref(&self) -> &T { &self.0 } + + pub fn expose_mut(&mut self) -> &mut T { + &mut self.0 + } } impl From for Secret { @@ -28,6 +36,12 @@ impl fmt::Debug for Secret { } } +impl fmt::Display for Secret { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "") + } +} + #[derive(Clone, Deserialize, Eq, PartialEq)] pub struct AccessToken { secret: Secret, diff --git a/src/agent/onefuzz-supervisor/src/config.rs b/src/agent/onefuzz-supervisor/src/config.rs index 5faf53c45..dbba31c6d 100644 --- a/src/agent/onefuzz-supervisor/src/config.rs +++ b/src/agent/onefuzz-supervisor/src/config.rs @@ -126,7 +126,7 @@ impl Registration { let response = reqwest::Client::new() .post(url.clone()) .header("Content-Length", "0") - .bearer_auth(token.secret().expose()) + .bearer_auth(token.secret().expose_ref()) .body("") .send() .await? @@ -174,7 +174,7 @@ impl Registration { let response = reqwest::Client::new() .get(url) - .bearer_auth(token.secret().expose()) + .bearer_auth(token.secret().expose_ref()) .send() .await? .error_for_status()?; diff --git a/src/agent/onefuzz-supervisor/src/coordinator.rs b/src/agent/onefuzz-supervisor/src/coordinator.rs index f474d3f75..158686863 100644 --- a/src/agent/onefuzz-supervisor/src/coordinator.rs +++ b/src/agent/onefuzz-supervisor/src/coordinator.rs @@ -242,7 +242,7 @@ impl Coordinator { let request = self .client .get(url) - .bearer_auth(self.token.secret().expose()) + .bearer_auth(self.token.secret().expose_ref()) .json(&request) .build()?; @@ -259,7 +259,7 @@ impl Coordinator { let request = self .client .delete(url) - .bearer_auth(self.token.secret().expose()) + .bearer_auth(self.token.secret().expose_ref()) .json(&request) .build()?; @@ -271,7 +271,7 @@ impl Coordinator { let request = self .client .post(url) - .bearer_auth(self.token.secret().expose()) + .bearer_auth(self.token.secret().expose_ref()) .json(event) .build()?; @@ -294,7 +294,7 @@ impl Coordinator { let request = self .client .get(url) - .bearer_auth(self.token.secret().expose()) + .bearer_auth(self.token.secret().expose_ref()) .json(&task_search) .build()?; diff --git a/src/agent/onefuzz-supervisor/src/debug.rs b/src/agent/onefuzz-supervisor/src/debug.rs index fcc220862..7e2c45147 100644 --- a/src/agent/onefuzz-supervisor/src/debug.rs +++ b/src/agent/onefuzz-supervisor/src/debug.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use anyhow::Result; +use onefuzz::blob::BlobContainerUrl; use structopt::StructOpt; use url::Url; use uuid::Uuid; @@ -136,13 +137,13 @@ fn debug_run_worker(opt: RunWorkerOpt) -> Result<()> { }; let work_unit = WorkUnit { - config, + config: config.into(), job_id: Uuid::new_v4(), task_id, }; let work_set = WorkSet { reboot: false, - setup_url: opt.setup_url, + setup_url: BlobContainerUrl::new(opt.setup_url)?, script: opt.script, work_units: vec![work_unit], }; diff --git a/src/agent/onefuzz-supervisor/src/setup.rs b/src/agent/onefuzz-supervisor/src/setup.rs index a77df79cc..d99ef37f7 100644 --- a/src/agent/onefuzz-supervisor/src/setup.rs +++ b/src/agent/onefuzz-supervisor/src/setup.rs @@ -7,7 +7,6 @@ use std::process::Stdio; use anyhow::Result; use downcast_rs::Downcast; use onefuzz::az_copy; -use onefuzz::blob::BlobContainerUrl; use tokio::fs; use tokio::process::Command; @@ -40,17 +39,15 @@ impl SetupRunner { info!("running setup for work set"); // Download the setup container. - let setup_url = work_set.setup_url.clone(); - let setup_url = BlobContainerUrl::new(setup_url)?; - - let setup_dir = setup_url.container(); + let setup_url = work_set.setup_url.url(); + let setup_dir = work_set.setup_url.container(); let setup_dir = onefuzz::fs::onefuzz_root()? .join("blob-containers") .join(setup_dir); // `azcopy sync` requires the local dir to exist. fs::create_dir_all(&setup_dir).await?; - az_copy::sync(work_set.setup_url.to_string(), &setup_dir).await?; + az_copy::sync(setup_url.to_string(), &setup_dir).await?; verbose!( "synced setup container from {} to {}", diff --git a/src/agent/onefuzz-supervisor/src/work.rs b/src/agent/onefuzz-supervisor/src/work.rs index c4fd6cee3..91e2e0800 100644 --- a/src/agent/onefuzz-supervisor/src/work.rs +++ b/src/agent/onefuzz-supervisor/src/work.rs @@ -5,10 +5,11 @@ use std::path::PathBuf; use anyhow::Result; use downcast_rs::Downcast; +use onefuzz::blob::BlobContainerUrl; use storage_queue::QueueClient; -use url::Url; use uuid::Uuid; +use crate::auth::Secret; use crate::config::Registration; pub type JobId = Uuid; @@ -18,7 +19,7 @@ pub type TaskId = Uuid; #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct WorkSet { pub reboot: bool, - pub setup_url: Url, + pub setup_url: BlobContainerUrl, pub script: bool, pub work_units: Vec, } @@ -32,7 +33,7 @@ pub struct WorkUnit { pub task_id: TaskId, /// JSON-serialized task config. - pub config: String, + pub config: Secret, } impl WorkUnit { diff --git a/src/agent/onefuzz-supervisor/src/worker.rs b/src/agent/onefuzz-supervisor/src/worker.rs index 9918aefe7..166acadf2 100644 --- a/src/agent/onefuzz-supervisor/src/worker.rs +++ b/src/agent/onefuzz-supervisor/src/worker.rs @@ -199,7 +199,7 @@ impl IWorkerRunner for WorkerRunner { let config_path = work.config_path()?; - fs::write(&config_path, &work.config).await?; + fs::write(&config_path, work.config.expose_ref()).await?; verbose!( "wrote worker config to config_path = {}", diff --git a/src/agent/onefuzz-supervisor/src/worker/tests.rs b/src/agent/onefuzz-supervisor/src/worker/tests.rs index 330ab7669..e99f61380 100644 --- a/src/agent/onefuzz-supervisor/src/worker/tests.rs +++ b/src/agent/onefuzz-supervisor/src/worker/tests.rs @@ -11,7 +11,7 @@ impl Fixture { fn work(&self) -> WorkUnit { let job_id = "d4e6cb4a-917e-4826-8a44-7646938c80a8".parse().unwrap(); let task_id = "1cfcdfe6-df10-42a5-aab7-1a45db0d0e48".parse().unwrap(); - let config = r#"{ "some": "config" }"#.to_owned(); + let config = r#"{ "some": "config" }"#.to_owned().into(); WorkUnit { job_id, diff --git a/src/agent/onefuzz/src/blob/url.rs b/src/agent/onefuzz/src/blob/url.rs index 4eb8cbbfe..02c900cb6 100644 --- a/src/agent/onefuzz/src/blob/url.rs +++ b/src/agent/onefuzz/src/blob/url.rs @@ -5,7 +5,7 @@ use std::fmt; use anyhow::Result; use reqwest::Url; -use serde::de; +use serde::{de, Serialize, Serializer}; #[derive(Clone, Eq, PartialEq)] pub struct BlobUrl { @@ -219,6 +219,16 @@ fn possible_blob_container_url(url: &Url) -> bool { possible_blob_storage_url(url, true) } +impl Serialize for BlobContainerUrl { + fn serialize(&self, serializer: S) -> std::result::Result + where + S: Serializer, + { + let url = self.url.to_string(); + serializer.serialize_str(&url) + } +} + impl<'de> de::Deserialize<'de> for BlobContainerUrl { fn deserialize(de: D) -> std::result::Result where