fix(errors)!: forbid interpolated details in wire-facing internal errors
This commit is contained in:
@@ -23,6 +23,7 @@ use arbiter_proto::{
|
||||
use chrono::DateTime;
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[non_exhaustive]
|
||||
pub enum AuthError {
|
||||
#[error("Server sent invalid auth challenge")]
|
||||
InvalidChallenge,
|
||||
|
||||
@@ -17,6 +17,7 @@ use tokio_stream::wrappers::ReceiverStream;
|
||||
use tonic::transport::ClientTlsConfig;
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[non_exhaustive]
|
||||
pub enum ArbiterClientError {
|
||||
#[error("Authentication error")]
|
||||
Authentication(#[from] AuthError),
|
||||
|
||||
@@ -4,6 +4,7 @@ use arbiter_proto::home_path;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[non_exhaustive]
|
||||
pub enum StorageError {
|
||||
#[error("Invalid signing key length in storage: expected {expected} bytes, got {actual} bytes")]
|
||||
InvalidKeyLength { expected: usize, actual: usize },
|
||||
|
||||
@@ -11,6 +11,7 @@ pub fn next_request_id() -> i32 {
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[non_exhaustive]
|
||||
pub enum ClientSignError {
|
||||
#[error("Transport channel closed")]
|
||||
ChannelClosed,
|
||||
|
||||
@@ -30,6 +30,7 @@ impl Display for ArbiterUrl {
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error, miette::Diagnostic)]
|
||||
#[non_exhaustive]
|
||||
pub enum Error {
|
||||
#[error("Invalid URL scheme, expected '{ARBITER_URL_SCHEME}://'")]
|
||||
#[diagnostic(
|
||||
|
||||
@@ -13,6 +13,7 @@ use diesel_async::{AsyncConnection, RunQueryDsl};
|
||||
use hmac::Hmac;
|
||||
use kameo::{actor::ActorRef, error::SendError};
|
||||
use sha2::{Digest as _, Sha256};
|
||||
use tracing::error;
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum Error {
|
||||
@@ -121,7 +122,10 @@ pub async fn sign_entity<E: Integrable>(
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
SendError::HandlerError(inner) => Error::Vault(inner),
|
||||
_ => Error::VaultSend,
|
||||
other => {
|
||||
error!(?other, "Vault unreachable while signing integrity envelope");
|
||||
Error::VaultSend
|
||||
}
|
||||
})?;
|
||||
|
||||
insert_into(integrity_envelope::table)
|
||||
@@ -195,12 +199,18 @@ pub async fn verify_entity<E: Integrable>(
|
||||
Err(SendError::HandlerError(
|
||||
vault::Error::Sealed | vault::Error::KeyVersionMismatch { .. },
|
||||
)) => Ok(AttestationStatus::Unavailable),
|
||||
Err(_) => Err(Error::VaultSend),
|
||||
Err(other) => {
|
||||
error!(?other, "Vault unreachable while verifying integrity envelope");
|
||||
Err(Error::VaultSend)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn is_signing_available(vault: &ActorRef<Vault>) -> Result<bool, Error> {
|
||||
let state = vault.ask(GetState).await.map_err(|_| Error::VaultSend)?;
|
||||
let state = vault.ask(GetState).await.map_err(|err| {
|
||||
error!(?err, "Vault unreachable while querying signing availability");
|
||||
Error::VaultSend
|
||||
})?;
|
||||
Ok(matches!(state, vault::VaultState::Unsealed))
|
||||
}
|
||||
|
||||
|
||||
@@ -26,21 +26,22 @@ pub enum Error {
|
||||
UnregisteredPublicKey,
|
||||
InvalidChallengeSolution,
|
||||
InvalidBootstrapToken,
|
||||
Internal { details: String },
|
||||
/// Reaches the operator verbatim via `Status::internal`, so the payload is
|
||||
/// `&'static str`: the type makes it impossible to interpolate an inner
|
||||
/// error. Log the cause, send the constant.
|
||||
Internal { details: &'static str },
|
||||
Transport,
|
||||
}
|
||||
|
||||
impl Error {
|
||||
fn internal(details: impl Into<String>) -> Self {
|
||||
Self::Internal {
|
||||
details: details.into(),
|
||||
}
|
||||
const fn internal(details: &'static str) -> Self {
|
||||
Self::Internal { details }
|
||||
}
|
||||
}
|
||||
|
||||
impl From<diesel::result::Error> for Error {
|
||||
fn from(e: diesel::result::Error) -> Self {
|
||||
error!(?e, "Database error");
|
||||
error!(error = %crate::utils::error_chain(&e), "Database error");
|
||||
Self::internal("Database error")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,13 +31,11 @@ pub enum Error {
|
||||
#[error("State transition failed")]
|
||||
State,
|
||||
|
||||
/// Reaches the operator verbatim via `Status::internal`, so the payload is
|
||||
/// `&'static str`: the type makes it impossible to interpolate an inner
|
||||
/// error. Log the cause, send the constant.
|
||||
#[error("Internal error: {0}")]
|
||||
Internal(String),
|
||||
}
|
||||
impl Error {
|
||||
fn internal(message: impl Into<String>) -> Self {
|
||||
Self::Internal(message.into())
|
||||
}
|
||||
Internal(&'static str),
|
||||
}
|
||||
|
||||
pub struct HandshakeResponse {
|
||||
@@ -179,7 +177,7 @@ impl VaultGate {
|
||||
}
|
||||
Err(err) => {
|
||||
error!(?err, "Failed to send unseal request to vault");
|
||||
Err(Error::internal("Vault actor error"))
|
||||
Err(Error::Internal("Vault actor error"))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -221,7 +219,7 @@ impl VaultGate {
|
||||
}
|
||||
Err(err) => {
|
||||
error!(?err, "Failed to send bootstrap request to vault");
|
||||
Err(Error::internal("Vault error"))
|
||||
Err(Error::Internal("Vault error"))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -233,7 +231,10 @@ impl VaultGate {
|
||||
.vault
|
||||
.ask(GetState {})
|
||||
.await
|
||||
.map_err(|_| Error::internal("failed to query vault"))?;
|
||||
.map_err(|err| {
|
||||
error!(?err, "Failed to query vault state");
|
||||
Error::Internal("failed to query vault")
|
||||
})?;
|
||||
|
||||
Ok(answer)
|
||||
}
|
||||
@@ -252,7 +253,10 @@ impl Message<events::Bootstrapped> for VaultGate {
|
||||
.db
|
||||
.get()
|
||||
.await
|
||||
.map_err(|_| Error::internal("DB unavailable"))?;
|
||||
.map_err(|err| {
|
||||
error!(error = %crate::utils::error_chain(&err), "DB unavailable on bootstrap");
|
||||
Error::Internal("DB unavailable")
|
||||
})?;
|
||||
integrity::sign_entity(
|
||||
&mut conn,
|
||||
&self.actors.vault,
|
||||
@@ -260,9 +264,12 @@ impl Message<events::Bootstrapped> for VaultGate {
|
||||
self.auth_creds.id,
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
error!(?e, "Failed to sign integrity envelope on bootstrap");
|
||||
Error::internal("Integrity sign failed")
|
||||
.map_err(|err| {
|
||||
error!(
|
||||
error = %crate::utils::error_chain(&err),
|
||||
"Failed to sign integrity envelope on bootstrap"
|
||||
);
|
||||
Error::Internal("Integrity sign failed")
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -14,3 +14,47 @@ impl<F: FnOnce()> Drop for DeferClosure<F> {
|
||||
pub fn defer<F: FnOnce()>(f: F) -> impl Drop + Sized {
|
||||
DeferClosure { f: Some(f) }
|
||||
}
|
||||
|
||||
/// Renders an error together with its full `source` chain as `outer: inner: root`.
|
||||
///
|
||||
/// Error variants in this crate deliberately keep `Display` terse so that no
|
||||
/// internal detail can leak across the gRPC boundary. That same terseness would
|
||||
/// hide the cause in the logs, so use this for `tracing` fields, never in a
|
||||
/// wire payload.
|
||||
pub fn error_chain(err: &dyn core::error::Error) -> String {
|
||||
let mut out = err.to_string();
|
||||
let mut current = err.source();
|
||||
while let Some(source) = current {
|
||||
out.push_str(": ");
|
||||
out.push_str(&source.to_string());
|
||||
current = source.source();
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::error_chain;
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[error("root")]
|
||||
struct Root;
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[error("middle")]
|
||||
struct Middle(#[source] Root);
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[error("outer")]
|
||||
struct Outer(#[source] Middle);
|
||||
|
||||
#[test]
|
||||
fn walks_the_whole_source_chain() {
|
||||
assert_eq!(error_chain(&Root), "root", "a leaf error renders alone");
|
||||
assert_eq!(
|
||||
error_chain(&Outer(Middle(Root))),
|
||||
"outer: middle: root",
|
||||
"every source link must appear, in order"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user