security(bootstrap): use SafeCell for token storage instead of zeroize
This commit is contained in:
1
server/Cargo.lock
generated
1
server/Cargo.lock
generated
@@ -788,7 +788,6 @@ dependencies = [
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
"x25519-dalek 2.0.1",
|
||||
"zeroize",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -37,7 +37,6 @@ chrono.workspace = true
|
||||
kameo.workspace = true
|
||||
chacha20poly1305 = { version = "0.10.1", features = ["std"] }
|
||||
argon2 = { version = "0.5.3", features = ["zeroize"] }
|
||||
zeroize = "1.9"
|
||||
restructed = "0.2.2"
|
||||
strum = { version = "0.28.0", features = ["derive"] }
|
||||
pem = "3.0.6"
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::db::{self, DatabasePool, schema};
|
||||
use arbiter_crypto::safecell::{SafeCell, SafeCellHandle as _};
|
||||
use arbiter_proto::{BOOTSTRAP_PATH, home_path};
|
||||
|
||||
use diesel::QueryDsl;
|
||||
@@ -10,7 +11,6 @@ use std::path::{Path, PathBuf};
|
||||
use subtle::ConstantTimeEq as _;
|
||||
use thiserror::Error;
|
||||
use tracing::warn;
|
||||
use zeroize::Zeroizing;
|
||||
|
||||
const TOKEN_LENGTH: usize = 64;
|
||||
|
||||
@@ -26,18 +26,23 @@ async fn write_token_file(path: &Path, content: &str) -> Result<(), std::io::Err
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn generate_token(path: &Path) -> Result<String, std::io::Error> {
|
||||
let token = UnwrapErr(SysRng)
|
||||
.sample_iter(Alphanumeric)
|
||||
.take(TOKEN_LENGTH)
|
||||
.fold(String::default(), |mut accum, char| {
|
||||
accum += char.to_string().as_str();
|
||||
accum
|
||||
});
|
||||
async fn generate_token(path: &Path) -> Result<SafeCell<[u8; TOKEN_LENGTH]>, std::io::Error> {
|
||||
let mut cell = SafeCell::new([0u8; TOKEN_LENGTH]);
|
||||
{
|
||||
let mut buf = cell.write();
|
||||
for (slot, b) in buf
|
||||
.iter_mut()
|
||||
.zip(UnwrapErr(SysRng).sample_iter(Alphanumeric))
|
||||
{
|
||||
*slot = b;
|
||||
}
|
||||
}
|
||||
|
||||
write_token_file(path, &token).await?;
|
||||
let token_str = cell.read_inline(|buf| String::from_utf8_lossy(buf.as_ref()).into_owned());
|
||||
|
||||
Ok(token)
|
||||
write_token_file(path, &token_str).await?;
|
||||
|
||||
Ok(cell)
|
||||
}
|
||||
|
||||
#[derive(Error, Debug)]
|
||||
@@ -54,7 +59,7 @@ pub enum Error {
|
||||
|
||||
#[derive(Actor)]
|
||||
pub struct Bootstrapper {
|
||||
token: Option<Zeroizing<String>>,
|
||||
token: Option<SafeCell<[u8; TOKEN_LENGTH]>>,
|
||||
token_path: Option<PathBuf>,
|
||||
}
|
||||
|
||||
@@ -72,7 +77,7 @@ impl Bootstrapper {
|
||||
let (token, token_path) = if row_count == 0 {
|
||||
let path = home_path()?.join(BOOTSTRAP_PATH);
|
||||
let token = generate_token(&path).await?;
|
||||
(Some(Zeroizing::new(token)), Some(path))
|
||||
(Some(token), Some(path))
|
||||
} else {
|
||||
(None, None)
|
||||
};
|
||||
@@ -82,13 +87,9 @@ impl Bootstrapper {
|
||||
}
|
||||
|
||||
impl Bootstrapper {
|
||||
fn is_correct_token(&self, token: &str) -> bool {
|
||||
self.token.as_ref().is_some_and(|expected| {
|
||||
let expected_bytes = expected.as_bytes();
|
||||
let token_bytes = token.as_bytes();
|
||||
|
||||
let choice = expected_bytes.ct_eq(token_bytes);
|
||||
bool::from(choice)
|
||||
fn is_correct_token(&mut self, token: &[u8]) -> bool {
|
||||
self.token.as_mut().is_some_and(|expected| {
|
||||
expected.read_inline(|exp| bool::from(exp.as_ref().ct_eq(token)))
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -96,14 +97,14 @@ impl Bootstrapper {
|
||||
#[messages]
|
||||
impl Bootstrapper {
|
||||
#[message]
|
||||
pub async fn consume_token(&mut self, token: Zeroizing<String>) -> bool {
|
||||
pub async fn consume_token(&mut self, token: Vec<u8>) -> bool {
|
||||
if self.is_correct_token(&token) {
|
||||
self.token = None;
|
||||
if let Some(path) = self.token_path.take() {
|
||||
if let Err(e) = tokio::fs::remove_file(&path).await {
|
||||
if let Some(path) = self.token_path.take()
|
||||
&& let Err(e) = tokio::fs::remove_file(&path).await
|
||||
{
|
||||
warn!(error = ?e, path = ?path, "Failed to delete bootstrap token file after consumption");
|
||||
}
|
||||
}
|
||||
true
|
||||
} else {
|
||||
false
|
||||
@@ -114,7 +115,9 @@ impl Bootstrapper {
|
||||
#[messages]
|
||||
impl Bootstrapper {
|
||||
#[message]
|
||||
pub fn get_token(&self) -> Option<String> {
|
||||
self.token.as_ref().map(|token| token.to_string())
|
||||
pub fn get_token(&mut self) -> Option<String> {
|
||||
self.token
|
||||
.as_mut()
|
||||
.map(|cell| cell.read_inline(|buf| String::from_utf8_lossy(buf.as_ref()).into_owned()))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,6 @@ use arbiter_proto::{
|
||||
use async_trait::async_trait;
|
||||
use tonic::Status;
|
||||
use tracing::warn;
|
||||
use zeroize::Zeroizing;
|
||||
|
||||
pub(super) struct AuthTransportAdapter<'a> {
|
||||
pub(super) bi: &'a mut GrpcBi<OperatorRequest, OperatorResponse>,
|
||||
@@ -172,7 +171,7 @@ impl Receiver<auth::Inbound> for AuthTransportAdapter<'_> {
|
||||
|
||||
Some(auth::Inbound::AuthChallengeRequest {
|
||||
pubkey,
|
||||
bootstrap_token: bootstrap_token.map(Zeroizing::new),
|
||||
bootstrap_token: bootstrap_token.map(String::into_bytes),
|
||||
})
|
||||
}
|
||||
AuthRequestPayload::ChallengeSolution(ProtoAuthChallengeSolution { signature }) => {
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use super::{Credentials, OperatorConnection};
|
||||
use arbiter_crypto::authn::{self, AuthChallenge};
|
||||
use arbiter_proto::transport::Bi;
|
||||
use zeroize::Zeroizing;
|
||||
|
||||
use state::{
|
||||
AuthContext, AuthError, AuthEvents, AuthStateMachine, AuthStates, ChallengeRequest,
|
||||
@@ -15,7 +14,7 @@ mod state;
|
||||
pub enum Inbound {
|
||||
AuthChallengeRequest {
|
||||
pubkey: authn::PublicKey,
|
||||
bootstrap_token: Option<Zeroizing<String>>,
|
||||
bootstrap_token: Option<Vec<u8>>,
|
||||
},
|
||||
AuthChallengeSolution {
|
||||
signature: Vec<u8>,
|
||||
|
||||
@@ -9,7 +9,6 @@ use crate::{
|
||||
};
|
||||
use arbiter_crypto::authn::{self, AuthChallenge, OPERATOR_CONTEXT};
|
||||
use arbiter_proto::transport::Bi;
|
||||
use zeroize::Zeroizing;
|
||||
|
||||
use diesel::{ExpressionMethods as _, OptionalExtension as _, QueryDsl};
|
||||
use diesel_async::RunQueryDsl;
|
||||
@@ -17,13 +16,12 @@ use tracing::error;
|
||||
|
||||
pub(super) struct ChallengeRequest {
|
||||
pub(super) pubkey: authn::PublicKey,
|
||||
pub(super) bootstrap_token: Option<Zeroizing<String>>,
|
||||
pub(super) bootstrap_token: Option<Vec<u8>>,
|
||||
}
|
||||
|
||||
pub struct ChallengeContext {
|
||||
pub(super) challenge: AuthChallenge,
|
||||
pub(super) pubkey: authn::PublicKey,
|
||||
pub(super) bootstrap_token: Option<Zeroizing<String>>,
|
||||
}
|
||||
|
||||
pub(super) struct ChallengeSolution {
|
||||
@@ -80,11 +78,16 @@ async fn register_key(db: &DatabasePool, pubkey: &authn::PublicKey) -> Result<i3
|
||||
pub(super) struct AuthContext<'a, T: ?Sized> {
|
||||
pub(super) conn: &'a mut OperatorConnection,
|
||||
pub(super) transport: &'a mut T,
|
||||
bootstrap_token: Option<Vec<u8>>,
|
||||
}
|
||||
|
||||
impl<'a, T: ?Sized> AuthContext<'a, T> {
|
||||
pub(super) const fn new(conn: &'a mut OperatorConnection, transport: &'a mut T) -> Self {
|
||||
Self { conn, transport }
|
||||
Self {
|
||||
conn,
|
||||
transport,
|
||||
bootstrap_token: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,6 +112,8 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
self.bootstrap_token = bootstrap_token;
|
||||
|
||||
let challenge = AuthChallenge::generate(&mut rand::rng());
|
||||
|
||||
self.transport
|
||||
@@ -121,20 +126,12 @@ where
|
||||
Error::Transport
|
||||
})?;
|
||||
|
||||
Ok(ChallengeContext {
|
||||
challenge,
|
||||
pubkey,
|
||||
bootstrap_token,
|
||||
})
|
||||
Ok(ChallengeContext { challenge, pubkey })
|
||||
}
|
||||
|
||||
async fn verify_solution(
|
||||
&mut self,
|
||||
ChallengeContext {
|
||||
challenge,
|
||||
pubkey,
|
||||
bootstrap_token,
|
||||
}: &ChallengeContext,
|
||||
ChallengeContext { challenge, pubkey }: &ChallengeContext,
|
||||
ChallengeSolution { solution }: ChallengeSolution,
|
||||
) -> Result<Credentials, Self::Error> {
|
||||
let signature = authn::Signature::try_from(solution.as_slice()).map_err(|()| {
|
||||
@@ -153,7 +150,7 @@ where
|
||||
}
|
||||
|
||||
// Resolve client id: bootstrap (consume token + register) or lookup
|
||||
let id = match bootstrap_token.clone() {
|
||||
let id = match self.bootstrap_token.take() {
|
||||
Some(token) => {
|
||||
let token_ok: bool = self
|
||||
.conn
|
||||
|
||||
@@ -174,7 +174,7 @@ pub async fn bootstrap_token_auth() {
|
||||
test_transport
|
||||
.send(auth::Inbound::AuthChallengeRequest {
|
||||
pubkey: verifying_key(&new_key).into(),
|
||||
bootstrap_token: Some(token),
|
||||
bootstrap_token: Some(token.into_bytes()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -231,7 +231,7 @@ pub async fn bootstrap_invalid_token_auth() {
|
||||
test_transport
|
||||
.send(auth::Inbound::AuthChallengeRequest {
|
||||
pubkey: verifying_key(&new_key).into(),
|
||||
bootstrap_token: Some("invalid_token".to_owned()),
|
||||
bootstrap_token: Some(b"invalid_token".to_vec()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
Reference in New Issue
Block a user