Skip to content

Commit

Permalink
Clear sensitive data after request handling to prevent leaks.
Browse files Browse the repository at this point in the history
  • Loading branch information
wa5i committed Feb 27, 2024
1 parent ca86f4d commit 39bf749
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 15 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ delay_timer = "0.11"
as-any = "0.3.1"
pem = "3.0"
chrono = "0.4"
zeroize = { version = "1.4.0", features= ["derive"] }
zeroize = { version = "1.7.0", features= ["zeroize_derive"] }
bcrypt = "0.15"

[target.'cfg(unix)'.dependencies]
Expand Down
3 changes: 2 additions & 1 deletion src/http/logical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Default for LogicalResponse {

async fn logical_request_handler(
req: HttpRequest,
body: web::Bytes,
mut body: web::Bytes,
method: Method,
path: web::Path<String>,
core: web::Data<Arc<RwLock<Core>>>,
Expand All @@ -67,6 +67,7 @@ async fn logical_request_handler(
if body.len() > 0 {
let payload = serde_json::from_slice(&body)?;
r.body = Some(payload);
body.clear();
}
}
Method::DELETE => {
Expand Down
15 changes: 10 additions & 5 deletions src/http/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,11 @@ async fn sys_init_get_request_handler(

async fn sys_init_put_request_handler(
_req: HttpRequest,
body: web::Bytes,
mut body: web::Bytes,
core: web::Data<Arc<RwLock<Core>>>,
) -> Result<HttpResponse, RvError> {
let payload = serde_json::from_slice::<InitRequest>(&body)?;
body.clear();
let seal_config = SealConfig { secret_shares: payload.secret_shares, secret_threshold: payload.secret_threshold };

let mut core = core.write()?;
Expand Down Expand Up @@ -125,11 +126,12 @@ async fn sys_seal_request_handler(

async fn sys_unseal_request_handler(
_req: HttpRequest,
body: web::Bytes,
mut body: web::Bytes,
core: web::Data<Arc<RwLock<Core>>>,
) -> Result<HttpResponse, RvError> {
// TODO
let payload = serde_json::from_slice::<UnsealRequest>(&body)?;
body.clear();
let key = hex::decode(payload.key)?;

{
Expand All @@ -154,11 +156,12 @@ async fn sys_list_mounts_request_handler(
async fn sys_mount_request_handler(
req: HttpRequest,
path: web::Path<String>,
body: web::Bytes,
mut body: web::Bytes,
core: web::Data<Arc<RwLock<Core>>>,
) -> Result<HttpResponse, RvError> {
let _test = serde_json::from_slice::<MountRequest>(&body)?;
let payload = serde_json::from_slice(&body)?;
body.clear();
let mount_path = path.into_inner();
if mount_path.len() == 0 {
return Ok(response_error(StatusCode::NOT_FOUND, ""));
Expand Down Expand Up @@ -191,11 +194,12 @@ async fn sys_unmount_request_handler(

async fn sys_remount_request_handler(
req: HttpRequest,
body: web::Bytes,
mut body: web::Bytes,
core: web::Data<Arc<RwLock<Core>>>,
) -> Result<HttpResponse, RvError> {
let _test = serde_json::from_slice::<RemountRequest>(&body)?;
let payload = serde_json::from_slice(&body)?;
body.clear();

let mut r = request_auth(&req);
r.operation = Operation::Write;
Expand All @@ -218,11 +222,12 @@ async fn sys_list_auth_mounts_request_handler(
async fn sys_auth_enable_request_handler(
req: HttpRequest,
path: web::Path<String>,
body: web::Bytes,
mut body: web::Bytes,
core: web::Data<Arc<RwLock<Core>>>,
) -> Result<HttpResponse, RvError> {
let _test = serde_json::from_slice::<MountRequest>(&body)?;
let payload = serde_json::from_slice(&body)?;
body.clear();
let mount_path = path.into_inner();
if mount_path.len() == 0 {
return Ok(response_error(StatusCode::NOT_FOUND, ""));
Expand Down
30 changes: 28 additions & 2 deletions src/logical/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{collections::HashMap, sync::Arc};
use regex::Regex;
use serde_json::{Map, Value};

use super::{path::Path, request::Request, response::Response, secret::Secret, Backend, Operation};
use super::{path::Path, request::Request, response::Response, secret::Secret, FieldType, Backend, Operation};
use crate::errors::RvError;

type BackendOperationHandler = dyn Fn(&dyn Backend, &mut Request) -> Result<Option<Response>, RvError> + Send + Sync;
Expand Down Expand Up @@ -86,7 +86,9 @@ impl Backend for LogicalBackend {
req.match_path = Some(path.clone());
for operation in &path.operations {
if operation.op == req.operation {
return operation.handle_request(self, req);
let ret = operation.handle_request(self, req);
self.clear_secret_field(req);
return ret;
}
}

Expand Down Expand Up @@ -177,6 +179,16 @@ impl LogicalBackend {

None
}

fn clear_secret_field(&self, req: &mut Request) {
for path in &self.paths {
for (key, field) in &path.fields {
if field.field_type == FieldType::SecretStr {
req.clear_data(key);
}
}
}
}
}

#[macro_export]
Expand Down Expand Up @@ -235,6 +247,7 @@ mod test {
use std::{collections::HashMap, env, fs, sync::Arc, time::Duration};

use go_defer::defer;
use serde_json::json;

use super::*;
use crate::{
Expand Down Expand Up @@ -310,6 +323,10 @@ mod test {
"mypath": {
field_type: FieldType::Str,
description: "hehe"
},
"mypassword": {
field_type: FieldType::SecretStr,
description: "password"
}
},
operations: [
Expand Down Expand Up @@ -408,8 +425,17 @@ mod test {

assert!(logical_backend.handle_request(&mut req).is_err());

let body: Map<String, Value> = json!({
"mytype": 1,
"mypath": "/pp",
"mypassword": "123qwe",
}).as_object().unwrap().clone();
req.body = Some(body);
req.storage = Some(Arc::new(barrier));
assert!(logical_backend.handle_request(&mut req).is_ok());
let mypassword = req.body.as_ref().unwrap().get("mypassword");
assert!(mypassword.is_some());
assert_eq!(mypassword.unwrap(), "");

let unauth_paths = logical_backend.get_unauth_paths();
assert!(unauth_paths.is_some());
Expand Down
3 changes: 3 additions & 0 deletions src/logical/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use crate::errors::RvError;
pub enum FieldType {
#[strum(to_string = "string")]
Str,
#[strum(to_string = "secret_string")]
SecretStr,
#[strum(to_string = "int")]
Int,
#[strum(to_string = "bool")]
Expand Down Expand Up @@ -40,6 +42,7 @@ impl Field {
pub fn get_default(&self) -> Result<Value, RvError> {
match &self.field_type {
FieldType::Str => self.cast_value::<String>(),
FieldType::SecretStr => self.cast_value::<String>(),
FieldType::Int => self.cast_value::<i32>(),
FieldType::Bool => self.cast_value::<bool>(),
FieldType::Map => self.cast_value::<Value>(),
Expand Down
18 changes: 18 additions & 0 deletions src/logical/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,24 @@ impl Request {
return field.get_default();
}

pub fn clear_data(&mut self, key: &str) {
if self.data.is_some() {
if let Some(secret_str) = self.data.as_mut().unwrap().get_mut(key) {
if let Value::String(ref mut s) = *secret_str {
*s = "".to_owned();
}
}
}

if self.body.is_some() {
if let Some(secret_str) = self.body.as_mut().unwrap().get_mut(key) {
if let Value::String(ref mut s) = *secret_str {
*s = "".to_owned();
}
}
}
}

pub fn storage_list(&self, prefix: &str) -> Result<Vec<String>, RvError> {
if self.storage.is_none() {
return Err(RvError::ErrRequestNotReady);
Expand Down
10 changes: 5 additions & 5 deletions src/modules/credential/userpass/path_login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl UserPassBackend {
description: "Username of the user."
},
"password": {
field_type: FieldType::Str,
field_type: FieldType::SecretStr,
required: true,
description: "Password for this user."
}
Expand All @@ -38,10 +38,10 @@ impl UserPassBackend {
impl UserPassBackendInner {
pub fn login(&self, _backend: &dyn Backend, req: &mut Request) -> Result<Option<Response>, RvError> {
let err_info = "invalid username or password";
let username_vale = req.get_data("username")?;
let username = username_vale.as_str().unwrap().to_lowercase();
let password_vale = req.get_data("password")?;
let password = password_vale.as_str().unwrap();
let username_value = req.get_data("username")?;
let username = username_value.as_str().unwrap().to_lowercase();
let password_value = req.get_data("password")?;
let password = password_value.as_str().unwrap();

let user = self.get_user(req, &username)?;
if user.is_none() {
Expand Down
2 changes: 1 addition & 1 deletion src/modules/credential/userpass/path_users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl UserPassBackend {
description: "Username of the user."
},
"password": {
field_type: FieldType::Str,
field_type: FieldType::SecretStr,
required: false,
description: "Password for this user."
},
Expand Down

0 comments on commit 39bf749

Please sign in to comment.