swarm-controller: read the queue client secret from the store, drop the file
Some checks were skipped
public bin cache / build + push to preem:grid (push) Has been skipped
Some checks were skipped
public bin cache / build + push to preem:grid (push) Has been skipped
The controller's OIDC client secret (client `swarm-controller`, used for the queue connection, the auth-bridge bearer and the OTLP push) came from an operator-placed file, `deploy.swarm-controller.queue.clientSecretFile`, handed in by `LoadCredential=`. Now `swarm-secret-publish`, which already copies authelia's minted OIDC secrets into the store, also publishes this one, to `swarm/controller/swarm-controller/oidc/client`. That path sits under `controller/`, which no hive's policy reads. The controller reads it once at start with its existing store certificate and holds it in memory, as `swarm_queue_client::ClientSecret::Value`. If the store is down, it retries for about a minute and then fails the start, so `Restart=` tries again. Policy delta: the controller gets `read` on that leaf, and the publisher gets `create`/`update` on that leaf. Removed: the `queue.clientSecretFile` option (both spellings, now removed options with a message), its singleHostSwarm default, the credential and placeholder, and the path watcher plus its restart oneshot. A controller without a store identity is now an eval error, because it has no other way to get the secret.
This commit is contained in:
parent
97c4771514
commit
e94406cdb9
22 changed files with 595 additions and 247 deletions
|
|
@ -43,10 +43,13 @@ environment is a hard error, because the failure it would otherwise produce is
|
|||
the expensive kind — the process comes up "fine", never connects, and the data
|
||||
it was supposed to move silently stops.
|
||||
|
||||
The client secret is a **path, not a value**: putting it in the environment
|
||||
would publish it to anything that can read `/proc/<pid>/environ`. It is read
|
||||
per token request rather than cached, so a rotation the operator believes took
|
||||
effect actually did.
|
||||
The client secret is never in the environment, which would publish it to
|
||||
anything that can read `/proc/<pid>/environ`. `from_env` takes it as a
|
||||
**path** (`ClientSecret::File`), read per token request rather than cached, so
|
||||
a rotation the operator believes took effect actually did. A caller that
|
||||
fetched the secret from somewhere else builds the config itself with
|
||||
`ClientSecret::Value`, held in memory; `swarm-controller` does this with the
|
||||
secret it reads from the swarm's secret store.
|
||||
|
||||
## Token request shape: HTTP Basic, `audience`, `scope`
|
||||
|
||||
|
|
|
|||
|
|
@ -347,12 +347,9 @@ pub struct QueueConfig {
|
|||
pub token_endpoint: String,
|
||||
/// The controller's own `OAuth2` client id.
|
||||
pub client_id: String,
|
||||
/// File holding the client secret's PLAINTEXT.
|
||||
///
|
||||
/// A path and not a value: the secret is minted on the authelia host and
|
||||
/// read here, and putting it in the environment would publish it to
|
||||
/// anything that can read `/proc/<pid>/environ`.
|
||||
pub client_secret_file: PathBuf,
|
||||
/// Where the client secret's plaintext comes from. Never the environment,
|
||||
/// which would publish it to anything that can read `/proc/<pid>/environ`.
|
||||
pub client_secret: ClientSecret,
|
||||
/// Extra trust anchor for the token endpoint and the queue itself, when
|
||||
/// they are not signed by a publicly-trusted CA.
|
||||
///
|
||||
|
|
@ -368,6 +365,55 @@ pub struct QueueConfig {
|
|||
pub ca_file: Option<PathBuf>,
|
||||
}
|
||||
|
||||
/// Where a [`QueueConfig`]'s client secret comes from.
|
||||
#[derive(Clone)]
|
||||
pub enum ClientSecret {
|
||||
/// A file holding the plaintext, read at every token mint so a rotated
|
||||
/// file takes effect without a restart.
|
||||
File(PathBuf),
|
||||
/// The plaintext itself, held only in this process's memory.
|
||||
Value(String),
|
||||
}
|
||||
|
||||
/// Redacts [`ClientSecret::Value`]: a [`QueueConfig`] is `Debug`, and printing
|
||||
/// one must not put the secret in a log.
|
||||
impl std::fmt::Debug for ClientSecret {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
Self::File(path) => f.debug_tuple("File").field(path).finish(),
|
||||
Self::Value(_) => f.write_str("Value(<redacted>)"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ClientSecret {
|
||||
async fn read(&self) -> Result<String, Error> {
|
||||
match self {
|
||||
Self::File(path) => {
|
||||
tokio::fs::read_to_string(path)
|
||||
.await
|
||||
.map_err(|source| Error::ClientSecret {
|
||||
path: path.display().to_string(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
Self::Value(value) => Ok(value.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
fn read_blocking(&self) -> Result<String, Error> {
|
||||
match self {
|
||||
Self::File(path) => {
|
||||
std::fs::read_to_string(path).map_err(|source| Error::ClientSecret {
|
||||
path: path.display().to_string(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
Self::Value(value) => Ok(value.clone()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl QueueConfig {
|
||||
/// Read the config from `<prefix>_NATS_URL`, `<prefix>_OIDC_TOKEN_ENDPOINT`,
|
||||
/// `<prefix>_OIDC_CLIENT_ID` and `<prefix>_OIDC_CLIENT_SECRET_FILE`, or
|
||||
|
|
@ -404,7 +450,7 @@ impl QueueConfig {
|
|||
url,
|
||||
token_endpoint,
|
||||
client_id,
|
||||
client_secret_file: PathBuf::from(secret),
|
||||
client_secret: ClientSecret::File(PathBuf::from(secret)),
|
||||
ca_file,
|
||||
})),
|
||||
// A partially-set environment is a deployment bug, and the failure
|
||||
|
|
@ -433,12 +479,7 @@ async fn mint_token(
|
|||
) -> Result<CachedToken, Error> {
|
||||
// Read per call rather than caching: the file is small, and a cached
|
||||
// secret would survive a rotation that the operator believes took effect.
|
||||
let secret = tokio::fs::read_to_string(&cfg.client_secret_file)
|
||||
.await
|
||||
.map_err(|source| Error::ClientSecret {
|
||||
path: cfg.client_secret_file.display().to_string(),
|
||||
source,
|
||||
})?;
|
||||
let secret = cfg.client_secret.read().await?;
|
||||
|
||||
let response = token_request(http, cfg, secret.trim(), audience, scope)
|
||||
.send()
|
||||
|
|
@ -590,11 +631,7 @@ pub fn mint_token_for_blocking(
|
|||
// Read per call rather than caching — see `mint_token`'s identical
|
||||
// comment on the async path; the reasoning does not change with the
|
||||
// client type.
|
||||
let secret =
|
||||
std::fs::read_to_string(&cfg.client_secret_file).map_err(|source| Error::ClientSecret {
|
||||
path: cfg.client_secret_file.display().to_string(),
|
||||
source,
|
||||
})?;
|
||||
let secret = cfg.client_secret.read_blocking()?;
|
||||
|
||||
let mut form = vec![("grant_type", "client_credentials")];
|
||||
if let Some(audience) = audience {
|
||||
|
|
@ -869,12 +906,32 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_held_secret_is_read_back_as_itself() {
|
||||
let secret = ClientSecret::Value("s3cret".to_owned());
|
||||
assert_eq!(
|
||||
secret.read_blocking().expect("a value needs no I/O"),
|
||||
"s3cret"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_debug_print_of_a_config_does_not_carry_a_held_secret() {
|
||||
let cfg = QueueConfig {
|
||||
client_secret: ClientSecret::Value("s3cret".to_owned()),
|
||||
..token_cfg()
|
||||
};
|
||||
let printed = format!("{cfg:?}");
|
||||
assert!(!printed.contains("s3cret"), "{printed}");
|
||||
assert!(printed.contains("<redacted>"), "{printed}");
|
||||
}
|
||||
|
||||
fn token_cfg() -> QueueConfig {
|
||||
QueueConfig {
|
||||
url: "nats://127.0.0.1:4222".to_owned(),
|
||||
token_endpoint: "https://auth.example.com/api/oidc/token".to_owned(),
|
||||
client_id: "hive-alpha".to_owned(),
|
||||
client_secret_file: PathBuf::from("/nonexistent"),
|
||||
client_secret: ClientSecret::File(PathBuf::from("/nonexistent")),
|
||||
ca_file: None,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue