Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,14 @@ tooling; otherwise follow the example's README.
describe what well-named code already says.
- **Backwards compatibility:** don't add compatibility shims for behavior
the task didn't ask for. See `AGENTS.md` §3, rule 6.
- **Secrets in server memory:** every long-lived secret-bearing field in
`crates/server` must use a wrapper from `crates/server/src/secret/`
(`FixedKey<N>`, `SecretBytes`, `SecretString`, `CredentialUrl`). Read a
secret-bearing env var and wrap it in a single expression — never bind
the raw `String` to a config field or local. Reviewers must confirm this
for any new secret-shaped field; the compile-time assertions in
`crates/server/src/secret/tests.rs` enforce that wrappers can never be
logged (`Display`), serialized (`Serialize`), or placed in a response DTO.

## Docs

Expand Down
22 changes: 18 additions & 4 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/miden-keystore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ miden-protocol = { workspace = true }
thiserror = { workspace = true }
hex = { workspace = true }
rand = "0.9.2"
zeroize = "1.7"

[dev-dependencies]
rand_chacha = { workspace = true }
Expand Down
18 changes: 8 additions & 10 deletions crates/miden-keystore/src/ecdsa_keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::hash::{DefaultHasher, Hash, Hasher};
use std::io::{BufRead, BufReader, BufWriter, Write};
use std::path::{Path, PathBuf};
use std::sync::Mutex;
use zeroize::Zeroizing;

type Result<T> = std::result::Result<T, KeyStoreError>;

Expand Down Expand Up @@ -62,13 +63,10 @@ impl FilesystemEcdsaKeyStore {
})?;

let mut writer = BufWriter::new(file);
writer
.write_all(hex::encode(key_bytes).as_bytes())
.map_err(|error| {
KeyStoreError::StorageError(format!(
"Failed to write key to file {filename}: {error}"
))
})?;
let encoded = Zeroizing::new(hex::encode(key_bytes));
writer.write_all(encoded.as_bytes()).map_err(|error| {
KeyStoreError::StorageError(format!("Failed to write key to file {filename}: {error}"))
})?;

writer.flush().map_err(|error| {
KeyStoreError::StorageError(format!("Failed to flush key file {filename}: {error}"))
Expand All @@ -89,16 +87,16 @@ impl FilesystemEcdsaKeyStore {
})?;

let mut reader = BufReader::new(file);
let mut hex_encoded = String::new();
let mut hex_encoded = Zeroizing::new(String::new());
reader.read_line(&mut hex_encoded).map_err(|error| {
KeyStoreError::StorageError(format!("Failed to read key from file {filename}: {error}"))
})?;

let key_bytes = hex::decode(hex_encoded.trim()).map_err(|error| {
let key_bytes = Zeroizing::new(hex::decode(hex_encoded.trim()).map_err(|error| {
KeyStoreError::DecodingError(format!(
"Failed to decode hex key from file {filename}: {error}"
))
})?;
})?);

SecretKey::read_from_bytes(&key_bytes).map_err(|error| {
KeyStoreError::DecodingError(format!(
Expand Down
18 changes: 8 additions & 10 deletions crates/miden-keystore/src/keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::io::{BufRead, BufReader, BufWriter, Write};
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};
use thiserror::Error;
use zeroize::Zeroizing;

#[derive(Debug, Error)]
pub enum KeyStoreError {
Expand Down Expand Up @@ -66,13 +67,10 @@ impl<R: RngCore + Send + Sync> FilesystemKeyStore<R> {
})?;

let mut writer = BufWriter::new(file);
writer
.write_all(hex::encode(key_bytes).as_bytes())
.map_err(|error| {
KeyStoreError::StorageError(format!(
"Failed to write key to file {filename}: {error}"
))
})?;
let encoded = Zeroizing::new(hex::encode(key_bytes));
writer.write_all(encoded.as_bytes()).map_err(|error| {
KeyStoreError::StorageError(format!("Failed to write key to file {filename}: {error}"))
})?;

writer.flush().map_err(|error| {
KeyStoreError::StorageError(format!("Failed to flush key file {filename}: {error}"))
Expand All @@ -93,16 +91,16 @@ impl<R: RngCore + Send + Sync> FilesystemKeyStore<R> {
})?;

let mut reader = BufReader::new(file);
let mut hex_encoded = String::new();
let mut hex_encoded = Zeroizing::new(String::new());
reader.read_line(&mut hex_encoded).map_err(|error| {
KeyStoreError::StorageError(format!("Failed to read key from file {filename}: {error}"))
})?;

let key_bytes = hex::decode(hex_encoded.trim()).map_err(|error| {
let key_bytes = Zeroizing::new(hex::decode(hex_encoded.trim()).map_err(|error| {
KeyStoreError::DecodingError(format!(
"Failed to decode hex key from file {filename}: {error}"
))
})?;
})?);

SecretKey::read_from_bytes(&key_bytes).map_err(|error| {
KeyStoreError::DecodingError(format!(
Expand Down
4 changes: 4 additions & 0 deletions crates/server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ hmac = "0.12"
sha2 = "0.10"
rand = "0.9"
rand_chacha = { workspace = true }
secrecy = { version = "0.10", default-features = false }
serde = { workspace = true }
subtle = "2.5"
zeroize = { version = "1.7", features = ["derive"] }
serde_json = { workspace = true }
tokio = { workspace = true, features = ["full"] }
tonic = { workspace = true }
Expand Down Expand Up @@ -63,6 +66,7 @@ tonic-prost-build = { workspace = true }

[dev-dependencies]
futures = "0.3"
static_assertions = "1.1"
tempfile = { workspace = true }
uuid = { version = "1.0", features = ["v4"] }
miden-standards = { workspace = true }
Expand Down
7 changes: 7 additions & 0 deletions crates/server/src/ack/miden_ecdsa/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ impl MidenEcdsaSigner {

impl MidenEcdsaSigner {
pub(crate) fn sign_with_server_key(&self, message: Word) -> crate::ack::Result<Signature> {
// Verified: miden-crypto 0.23.0 (pulled in by miden-protocol 0.14.5)
// implements `impl ZeroizeOnDrop for SecretKey {}` in
// src/dsa/ecdsa_k256_keccak/mod.rs (delegating to k256's
// ZeroizeOnDrop), so the per-call loaded key material is zeroed
// when the signing frame returns. The local file-read buffer inside
// miden-keystore is wrapped in zeroize::Zeroizing for the same
// reason.
Ok(self.keystore.ecdsa_sign(self.server_pubkey_word, message)?)
}

Expand Down
7 changes: 7 additions & 0 deletions crates/server/src/ack/miden_falcon_rpo/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ impl MidenFalconRpoSigner {

impl MidenFalconRpoSigner {
pub(crate) fn sign_with_server_key(&self, message: Word) -> crate::ack::Result<Signature> {
// Verified: miden-crypto 0.23.0 (pulled in by miden-protocol 0.14.5)
// implements `impl ZeroizeOnDrop for SecretKey {}` in
// src/dsa/falcon512_poseidon2/keys/secret_key.rs (the SecretKey type
// here), so the per-call loaded key material is zeroed when the
// signing frame returns. The local file-read buffer inside
// miden-keystore is wrapped in zeroize::Zeroizing for the same
// reason.
Ok(self.keystore.sign(self.server_pubkey_word, message)?)
}

Expand Down
30 changes: 18 additions & 12 deletions crates/server/src/ack/secrets_manager.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::error::{GuardianError, Result};
use crate::secret::{SecretBytes, SecretString};
use async_trait::async_trait;
use aws_config::BehaviorVersion;
use aws_sdk_secretsmanager::Client;
Expand Down Expand Up @@ -39,7 +40,7 @@ impl AwsSecretsManagerProvider {
})
}

async fn secret_string(&self, secret_id: &str) -> Result<String> {
async fn secret_string(&self, secret_id: &str) -> Result<SecretString> {
let response = self
.client
.get_secret_value()
Expand All @@ -52,25 +53,30 @@ impl AwsSecretsManagerProvider {
))
})?;

response.secret_string().map(str::to_owned).ok_or_else(|| {
GuardianError::ConfigurationError(format!(
"Secret {secret_id} does not contain a secret string value"
))
})
response
.secret_string()
.map(|s| SecretString::new(s.to_owned()))
.ok_or_else(|| {
GuardianError::ConfigurationError(format!(
"Secret {secret_id} does not contain a secret string value"
))
})
}

async fn parsed_secret_key<T, F>(&self, secret_id: &str, parser: F) -> Result<T>
where
F: FnOnce(&[u8]) -> std::result::Result<T, String>,
{
let secret_hex = self.secret_string(secret_id).await?;
let secret_bytes = hex::decode(secret_hex.trim()).map_err(|error| {
GuardianError::ConfigurationError(format!(
"Secret {secret_id} must contain valid hex-encoded key bytes: {error}"
))
})?;
let secret_bytes = SecretBytes::new(
hex::decode(secret_hex.expose_secret().trim()).map_err(|error| {
GuardianError::ConfigurationError(format!(
"Secret {secret_id} must contain valid hex-encoded key bytes: {error}"
))
})?,
);

parser(&secret_bytes).map_err(|error| {
parser(secret_bytes.expose_secret()).map_err(|error| {
GuardianError::ConfigurationError(format!(
"Secret {secret_id} does not contain a valid key: {error}"
))
Expand Down
11 changes: 7 additions & 4 deletions crates/server/src/audit/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,15 @@ mod tests {
use diesel::QueryDsl;
use diesel_async::RunQueryDsl;

let database_url = std::env::var("DATABASE_URL")
.expect("DATABASE_URL must be set; this test is marked #[ignore] by default");
crate::storage::postgres::run_migrations(&database_url)
let database_url = crate::secret::CredentialUrl::new(
std::env::var("DATABASE_URL")
.expect("DATABASE_URL must be set; this test is marked #[ignore] by default"),
);
let raw_url = database_url.expose_secret();
crate::storage::postgres::run_migrations(raw_url)
.await
.expect("migrations run");
let pool = build_postgres_pool_lazy(&database_url, 1).expect("pool builds");
let pool = build_postgres_pool_lazy(raw_url, 1).expect("pool builds");
// Sanity: prove the pool is reachable; if not, abort early
// with a clear error rather than a confusing trigger failure.
let _ = pool
Expand Down
Loading
Loading