Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(composer)! avoid holding private key in env var #1074

Merged
merged 6 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
15 changes: 2 additions & 13 deletions Cargo.lock

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

4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ bytes = "1"
celestia-tendermint = "0.32.1"
celestia-types = "0.1.1"
clap = "4.5.4"
ed25519-consensus = "2.1.0"
ed25519-consensus = { version = "2.1.0", default-features = false, features = [
"std",
] }
ethers = "2.0.11"
futures = "0.3"
hex = "0.4"
Expand Down
2 changes: 1 addition & 1 deletion charts/evm-rollup/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.16.1
version: 0.16.2

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
17 changes: 15 additions & 2 deletions charts/evm-rollup/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ data:
OTEL_EXPORTER_OTLP_HEADERS: "{{ .Values.config.rollup.otel.otlpHeaders }}"
OTEL_EXPORTER_OTLP_TRACE_HEADERS: "{{ .Values.config.rollup.otel.traceHeaders }}"
OTEL_SERVICE_NAME: "{{ tpl .Values.config.rollup.otel.serviceNamePrefix . }}-composer"
{{- if not .Values.global.dev }}
{{- if not .Values.secretProvider.enabled }}
ASTRIA_COMPOSER_PRIVATE_KEY: "{{ .Values.config.sequencer.privateKey }}"
ASTRIA_COMPOSER_PRIVATE_KEY: "{{ .Values.config.sequencer.privateKey.devContent }}"
{{- end }}
{{- if not .Values.global.dev }}
{{- else }}
ASTRIA_COMPOSER_PRIVATE_KEY_FILE: "/var/secrets/{{ .Values.config.sequencer.privateKey.secret.filename }}"
{{- end }}
---
apiVersion: v1
Expand Down Expand Up @@ -259,3 +260,15 @@ metadata:
data:
VISUALIZER__SERVER__GRPC__ENABLED: "false"
VISUALIZER__SERVER__HTTP__ADDR: "0.0.0.0:8151"
---
{{- if not .Values.secretProvider.enabled }}
apiVersion: v1
kind: ConfigMap
metadata:
namespace: {{ include "rollup.namespace" . }}
name: sequencer-private-key
data:
{{ .Values.config.sequencer.privateKey.secret.filename }}: |
{{ .Values.config.sequencer.privateKey.devContent }}
---
{{- end }}
29 changes: 11 additions & 18 deletions charts/evm-rollup/templates/statefulsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ spec:
{{- else }}
- --state.scheme=path
{{- end }}
{{ if .Values.config.rollup.metrics.enabled }}
- --metrics
{{ if .Values.config.rollup.metrics.enabled }}
- --metrics
- --metrics.addr=0.0.0.0
- --metrics.port={{ .Values.ports.metrics }}
{{- end }}
Expand All @@ -79,12 +79,12 @@ spec:
name: ws-rpc
- containerPort: {{ .Values.ports.executionGRPC }}
name: execution-grpc
{{- if .Values.config.rollup.metrics.enabled }}
{{- if .Values.config.rollup.metrics.enabled }}
- containerPort: {{ .Values.ports.metrics }}
name: geth-metr
{{- end }}
resources:
{{- toYaml .Values.resources.geth | trim | nindent 12 }}
{{- toYaml .Values.resources.geth | trim | nindent 12 }}
- name: composer
image: {{ include "composer.image" . }}
command: [ "/usr/local/bin/astria-composer" ]
Expand All @@ -93,20 +93,10 @@ spec:
envFrom:
- configMapRef:
name: {{ .Values.config.rollup.name }}-composer-env
{{- if .Values.secretProvider.enabled }}
env:
- name: ASTRIA_COMPOSER_PRIVATE_KEY
valueFrom:
secretKeyRef:
name: sequencer-private-key
key: {{ .Values.secretProvider.secrets.sequencerPrivateKey.key }}
{{- end }}
volumeMounts:
{{- if .Values.secretProvider.enabled }}
- mountPath: "/var/secrets"
name: sequencer-private-key
{{- end }}
{{- if .Values.config.rollup.metrics.enabled }}
{{- if .Values.config.rollup.metrics.enabled }}
ports:
- containerPort: {{ .Values.ports.composerMetrics }}
name: composer-metr
Expand All @@ -123,7 +113,7 @@ spec:
name: {{ .Values.config.rollup.name }}-conductor-env
resources:
{{- toYaml .Values.resources.conductor | trim | nindent 12 }}
{{- if .Values.config.rollup.metrics.enabled }}
{{- if .Values.config.rollup.metrics.enabled }}
ports:
- containerPort: {{ .Values.ports.conductorMetrics }}
name: conductor-metr
Expand All @@ -140,14 +130,17 @@ spec:
{{- else }}
emptyDir: {}
{{- end }}
{{- if .Values.secretProvider.enabled }}
- name: sequencer-private-key
{{- if .Values.secretProvider.enabled }}
csi:
driver: secrets-store.csi.k8s.io
readOnly: true
volumeAttributes:
secretProviderClass: sequencer-private-key
{{- end }}
{{- else }}
configMap:
name: sequencer-private-key
{{- end }}
---
{{- if .Values.config.blockscout.enabled }}
apiVersion: apps/v1
Expand Down
6 changes: 5 additions & 1 deletion charts/evm-rollup/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,11 @@ config:
# Private key which is used for wrapping txs for sequencer submission
# Note: When secretProvider.enabled is true the secret provided by
# `sequencerPrivateKey` is used instead of this value.
privateKey: "2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90"
privateKey:
devContent: "2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90"
secret:
filename: "key.hex"
resourceName: "projects/$PROJECT_ID/secrets/sequencerPrivateKey/versions/latest"

celestia:
# if config.rollup.executionLevel is NOT 'SoftOnly' AND celestia-node is not enabled
Expand Down
6 changes: 2 additions & 4 deletions crates/astria-cli/src/commands/sequencer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use astria_core::{
crypto::SigningKey,
primitive::v1::asset,
protocol::transaction::v1alpha1::{
action::{
Expand Down Expand Up @@ -31,10 +32,7 @@ use color_eyre::{
Context,
},
};
use ed25519_consensus::{
SigningKey,
VerificationKeyBytes,
};
use ed25519_consensus::VerificationKeyBytes;
use rand::rngs::OsRng;

use crate::cli::sequencer::{
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ telemetry = { package = "astria-telemetry", path = "../astria-telemetry", featur
] }

pin-project-lite = "0.2.13"
secrecy = { version = "0.8", features = ["serde"] }
tonic-health = "0.10.2"

async-trait = { workspace = true }
Expand Down Expand Up @@ -70,6 +69,7 @@ test_utils = { package = "astria-test-utils", path = "../astria-test-utils", fea
"geth",
] }
insta = { workspace = true, features = ["json"] }
tempfile = { workspace = true }
tokio-test = { workspace = true }
astria-core = { path = "../astria-core", features = ["client"] }
tendermint-rpc = { workspace = true }
Expand Down
6 changes: 3 additions & 3 deletions crates/astria-composer/local.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ ASTRIA_COMPOSER_SEQUENCER_CHAIN_ID="astria-dev-1"
# names are sha256 hashed and used as the `rollup_id` in `SequenceAction`s
ASTRIA_COMPOSER_ROLLUPS="astriachain::ws://127.0.0.1:8545"

# Private key for the sequencer account used for signing transactions
# Must be a hex-encoded 32-byte array (64-character hex string)
ASTRIA_COMPOSER_PRIVATE_KEY="2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90"
# The path to the file storing the private key for the sequencer account used for signing
# transactions. The file should contain a hex-encoded Ed25519 secret key.
ASTRIA_COMPOSER_PRIVATE_KEY_FILE=/path/to/priv_sequencer_key.json

# Block time in milliseconds, used to force submitting of finished bundles.
# Should match the sequencer node configuration for 'timeout_commit', as
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/src/composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl Composer {
let (executor, executor_handle) = executor::Builder {
sequencer_url: cfg.sequencer_url.clone(),
sequencer_chain_id: cfg.sequencer_chain_id.clone(),
private_key: cfg.private_key.clone(),
private_key_file: cfg.private_key_file.clone(),
block_time_ms: cfg.block_time_ms,
max_bytes_per_bundle: cfg.max_bytes_per_bundle,
bundle_queue_capacity: cfg.bundle_queue_capacity,
Expand Down
27 changes: 2 additions & 25 deletions crates/astria-composer/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
use std::net::SocketAddr;

use secrecy::{
zeroize::ZeroizeOnDrop,
ExposeSecret as _,
SecretString,
};
use serde::{
Deserialize,
Serialize,
Serializer,
};

// this is a config, may have many boolean values
Expand All @@ -31,9 +25,8 @@ pub struct Config {
/// A list of <rollup_name>::<url> pairs
pub rollups: String,

/// Private key for the sequencer account used for signing transactions
#[serde(serialize_with = "serialize_private_key")]
pub private_key: SecretString,
/// Path to private key for the sequencer account used for signing transactions
pub private_key_file: String,

/// Sequencer block time in milliseconds
#[serde(alias = "max_submit_interval_ms")]
Expand Down Expand Up @@ -69,22 +62,6 @@ impl config::Config for Config {
const PREFIX: &'static str = "ASTRIA_COMPOSER_";
}

impl ZeroizeOnDrop for Config {}

fn serialize_private_key<S>(key: &SecretString, s: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
use serde::ser::Error as _;
let mut raw_key = key.expose_secret().clone().into_bytes();
if let Some(sub_slice) = raw_key.get_mut(4..) {
sub_slice.fill(b'#');
}
let sanitized_key = std::str::from_utf8(&raw_key)
.map_err(|_| S::Error::custom("private key hex contained non-ascii characters"))?;
s.serialize_str(sanitized_key)
}

#[cfg(test)]
mod tests {
use super::Config;
Expand Down
43 changes: 23 additions & 20 deletions crates/astria-composer/src/executor/builder.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
use std::time::Duration;
use std::{
fs,
path::Path,
time::Duration,
};

use astria_core::{
crypto::SigningKey,
primitive::v1::Address,
protocol::transaction::v1alpha1::action::SequenceAction,
};
use astria_eyre::{
use astria_eyre::eyre::{
self,
eyre,
eyre::{
eyre,
WrapErr as _,
},
};
use ed25519_consensus::SigningKey;
use secrecy::{
ExposeSecret,
SecretString,
Zeroize,
WrapErr as _,
};
use tokio::sync::watch;
use tokio_util::sync::CancellationToken;
Expand All @@ -28,7 +25,7 @@ use crate::{
pub(crate) struct Builder {
pub(crate) sequencer_url: String,
pub(crate) sequencer_chain_id: String,
pub(crate) private_key: SecretString,
pub(crate) private_key_file: String,
pub(crate) block_time_ms: u64,
pub(crate) max_bytes_per_bundle: usize,
pub(crate) bundle_queue_capacity: usize,
Expand All @@ -40,7 +37,7 @@ impl Builder {
let Self {
sequencer_url,
sequencer_chain_id,
private_key,
private_key_file,
block_time_ms,
max_bytes_per_bundle,
bundle_queue_capacity,
Expand All @@ -49,12 +46,10 @@ impl Builder {
let sequencer_client = sequencer_client::HttpClient::new(sequencer_url.as_str())
.wrap_err("failed constructing sequencer client")?;
let (status, _) = watch::channel(Status::new());
let mut private_key_bytes: [u8; 32] = hex::decode(private_key.expose_secret())
.wrap_err("failed to decode private key bytes from hex string")?
.try_into()
.map_err(|_| eyre!("invalid private key length; must be 32 bytes"))?;
let sequencer_key = SigningKey::from(private_key_bytes);
private_key_bytes.zeroize();

let sequencer_key = read_signing_key_from_file(&private_key_file).wrap_err_with(|| {
format!("failed reading signing key from file at path `{private_key_file}`")
})?;

let sequencer_address = Address::from_verification_key(sequencer_key.verification_key());

Expand All @@ -78,3 +73,11 @@ impl Builder {
))
}
}

fn read_signing_key_from_file<P: AsRef<Path>>(path: P) -> eyre::Result<SigningKey> {
let private_key_hex = fs::read_to_string(path)?;
let private_key_bytes: [u8; 32] = hex::decode(private_key_hex.trim())?
.try_into()
.map_err(|_| eyre!("invalid private key length; must be 32 bytes"))?;
Ok(SigningKey::from(private_key_bytes))
}
Loading
Loading