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/block proposal authorization password for v2/block_proposal endpoint #4471

Merged
merged 2 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions stacks-signer/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ pub struct GenerateFilesArgs {
/// The number of milliseconds to wait when polling for events from the stacker-db instance.
#[arg(long)]
pub timeout: Option<u64>,
#[arg(long)]
/// The authorization password to use to connect to the validate block proposal node endpoint
pub password: String,
}

#[derive(Clone, Debug)]
Expand Down
15 changes: 13 additions & 2 deletions stacks-signer/src/client/stacks_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use blockstack_lib::net::api::postblock_proposal::NakamotoBlockProposal;
use blockstack_lib::util_lib::boot::{boot_code_addr, boot_code_id};
use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier};
use clarity::vm::{ClarityName, ContractName, Value as ClarityValue};
use reqwest::header::AUTHORIZATION;
use serde_json::json;
use slog::slog_debug;
use stacks_common::codec::StacksMessageCodec;
Expand Down Expand Up @@ -63,6 +64,8 @@ pub struct StacksClient {
mainnet: bool,
/// The Client used to make HTTP connects
stacks_node_client: reqwest::blocking::Client,
/// the auth password for the stacks node
auth_password: String,
}

impl From<&GlobalConfig> for StacksClient {
Expand All @@ -75,13 +78,19 @@ impl From<&GlobalConfig> for StacksClient {
chain_id: config.network.to_chain_id(),
stacks_node_client: reqwest::blocking::Client::new(),
mainnet: config.network.is_mainnet(),
auth_password: config.auth_password.clone(),
}
}
}

impl StacksClient {
/// Create a new signer StacksClient with the provided private key, stacks node host endpoint, and version
pub fn new(stacks_private_key: StacksPrivateKey, node_host: SocketAddr, mainnet: bool) -> Self {
/// Create a new signer StacksClient with the provided private key, stacks node host endpoint, version, and auth password
pub fn new(
stacks_private_key: StacksPrivateKey,
node_host: SocketAddr,
auth_password: String,
mainnet: bool,
) -> Self {
let pubkey = StacksPublicKey::from_private(&stacks_private_key);
let tx_version = if mainnet {
TransactionVersion::Mainnet
Expand All @@ -102,6 +111,7 @@ impl StacksClient {
chain_id,
stacks_node_client: reqwest::blocking::Client::new(),
mainnet,
auth_password,
}
}

Expand Down Expand Up @@ -214,6 +224,7 @@ impl StacksClient {
self.stacks_node_client
.post(self.block_proposal_path())
.header("Content-Type", "application/json")
.header(AUTHORIZATION, self.auth_password.clone())
.json(&block_proposal)
.send()
.map_err(backoff::Error::transient)
Expand Down
7 changes: 7 additions & 0 deletions stacks-signer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ pub struct GlobalConfig {
pub sign_timeout: Option<Duration>,
/// the STX tx fee to use in uSTX
pub tx_fee_ustx: u64,
/// the authorization password for the block proposal endpoint
pub auth_password: String,
}

/// Internal struct for loading up the config file
Expand Down Expand Up @@ -222,6 +224,8 @@ struct RawConfigFile {
pub sign_timeout_ms: Option<u64>,
/// the STX tx fee to use in uSTX
pub tx_fee_ustx: Option<u64>,
/// The authorization password for the block proposal endpoint
pub auth_password: String,
}

impl RawConfigFile {
Expand Down Expand Up @@ -320,6 +324,7 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
nonce_timeout,
sign_timeout,
tx_fee_ustx: raw_data.tx_fee_ustx.unwrap_or(TX_FEE_USTX),
auth_password: raw_data.auth_password,
})
}
}
Expand Down Expand Up @@ -350,6 +355,7 @@ pub fn build_signer_config_tomls(
node_host: &str,
timeout: Option<Duration>,
network: &Network,
password: &str,
) -> Vec<String> {
let mut signer_config_tomls = vec![];

Expand All @@ -364,6 +370,7 @@ stacks_private_key = "{stacks_private_key}"
node_host = "{node_host}"
endpoint = "{endpoint}"
network = "{network}"
auth_password = "{password}"
"#
);

Expand Down
1 change: 1 addition & 0 deletions stacks-signer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ fn handle_generate_files(args: GenerateFilesArgs) {
&args.host.to_string(),
args.timeout.map(Duration::from_millis),
&args.network,
&args.password,
);
debug!("Built {:?} signer config tomls.", signer_config_tomls.len());
for (i, file_contents) in signer_config_tomls.iter().enumerate() {
Expand Down
1 change: 1 addition & 0 deletions stacks-signer/src/tests/conf/signer-0.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ stacks_private_key = "6a1fc1a3183018c6d79a4e11e154d2bdad2d89ac8bc1b0a021de8b4d28
node_host = "127.0.0.1:20443"
endpoint = "localhost:30000"
network = "testnet"
auth_password = "12345"
1 change: 1 addition & 0 deletions stacks-signer/src/tests/conf/signer-1.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ stacks_private_key = "126e916e77359ccf521e168feea1fcb9626c59dc375cae00c746430338
node_host = "127.0.0.1:20444"
endpoint = "localhost:30001"
network = "testnet"
auth_password = "12345"
1 change: 1 addition & 0 deletions stacks-signer/src/tests/conf/signer-4.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ stacks_private_key = "e427196ae29197b1db6d5495ff26bf0675f48a4f07b200c0814b95734e
node_host = "127.0.0.1:20443"
endpoint = "localhost:30004"
network = "testnet"
auth_password = "12345"
4 changes: 3 additions & 1 deletion stackslib/src/net/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ impl StacksHttp {
liststackerdbreplicas::RPCListStackerDBReplicasRequestHandler::new(),
);
self.register_rpc_endpoint(postblock::RPCPostBlockRequestHandler::new());
self.register_rpc_endpoint(postblock_proposal::RPCBlockProposalRequestHandler::new());
self.register_rpc_endpoint(postblock_proposal::RPCBlockProposalRequestHandler::new(
self.block_proposal_token.clone(),
));
self.register_rpc_endpoint(postfeerate::RPCPostFeeRateRequestHandler::new());
self.register_rpc_endpoint(postmempoolquery::RPCMempoolQueryRequestHandler::new());
self.register_rpc_endpoint(postmicroblock::RPCPostMicroblockRequestHandler::new());
Expand Down
26 changes: 14 additions & 12 deletions stackslib/src/net/api/postblock_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,15 @@ impl NakamotoBlockProposal {
#[derive(Clone, Default)]
pub struct RPCBlockProposalRequestHandler {
pub block_proposal: Option<NakamotoBlockProposal>,
pub auth: Option<String>,
}

impl RPCBlockProposalRequestHandler {
pub fn new() -> Self {
Self::default()
pub fn new(auth: Option<String>) -> Self {
Self {
block_proposal: None,
auth,
}
}

/// Decode a JSON-encoded block proposal
Expand Down Expand Up @@ -375,24 +379,22 @@ impl HttpRequest for RPCBlockProposalRequestHandler {
query: Option<&str>,
body: &[u8],
) -> Result<HttpRequestContents, Error> {
// Only accept requests from localhost
let is_loopback = match preamble.host {
// Should never be DNS
PeerHost::DNS(..) => false,
PeerHost::IP(addr, ..) => addr.is_loopback(),
// If no authorization is set, then the block proposal endpoint is not enabled
let Some(password) = &self.auth else {
return Err(Error::Http(400, "Bad Request.".into()));
};

if !is_loopback {
return Err(Error::Http(403, "Forbidden".into()));
let Some(auth_header) = preamble.headers.get("authorization") else {
return Err(Error::Http(401, "Unauthorized".into()));
};
if auth_header != password {
return Err(Error::Http(401, "Unauthorized".into()));
}

if preamble.get_content_length() == 0 {
return Err(Error::DecodeError(
"Invalid Http request: expected non-zero-length body for block proposal endpoint"
.to_string(),
));
}

if preamble.get_content_length() > MAX_PAYLOAD_LEN {
return Err(Error::DecodeError(
"Invalid Http request: BlockProposal body is too big".to_string(),
Expand Down
3 changes: 3 additions & 0 deletions stackslib/src/net/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,8 @@ pub struct ConnectionOptions {
/// the reward cycle in which Nakamoto activates, and thus needs to run both the epoch
/// 2.x and Nakamoto state machines.
pub force_nakamoto_epoch_transition: bool,
/// The authorization token to enable the block proposal RPC endpoint
pub block_proposal_token: Option<String>,
}

impl std::default::Default for ConnectionOptions {
Expand Down Expand Up @@ -508,6 +510,7 @@ impl std::default::Default for ConnectionOptions {
disable_stackerdb_get_chunks: false,
force_disconnect_interval: None,
force_nakamoto_epoch_transition: false,
block_proposal_token: None,
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions stackslib/src/net/httpcore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,8 @@ pub struct StacksHttp {
pub maximum_call_argument_size: u32,
/// Maximum execution budget of a read-only call
pub read_only_call_limit: ExecutionCost,
/// The authorization token to enable the block proposal RPC endpoint
pub block_proposal_token: Option<String>,
}

impl StacksHttp {
Expand All @@ -886,6 +888,7 @@ impl StacksHttp {
request_handlers: vec![],
maximum_call_argument_size: conn_opts.maximum_call_argument_size,
read_only_call_limit: conn_opts.read_only_call_limit.clone(),
block_proposal_token: conn_opts.block_proposal_token.clone(),
};
http.register_rpc_methods();
http
Expand Down
21 changes: 21 additions & 0 deletions testnet/stacks-node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,25 @@ mod tests {
"ST2TFVBMRPS5SSNP98DQKQ5JNB2B6NZM91C4K3P7B"
);
}

#[test]
fn should_load_block_proposal_token() {
let config = Config::from_config_file(
ConfigFile::from_str(
r#"
[connection_options]
block_proposal_token = "password"
"#,
)
.unwrap(),
)
.expect("Expected to be able to parse block proposal token from file");

assert_eq!(
config.connection_options.block_proposal_token,
Some("password".to_string())
);
}
}

impl ConfigFile {
Expand Down Expand Up @@ -2106,6 +2125,7 @@ pub struct ConnectionOptionsFile {
pub force_disconnect_interval: Option<u64>,
pub antientropy_public: Option<bool>,
pub private_neighbors: Option<bool>,
pub block_proposal_token: Option<String>,
}

impl ConnectionOptionsFile {
Expand Down Expand Up @@ -2229,6 +2249,7 @@ impl ConnectionOptionsFile {
max_sockets: self.max_sockets.unwrap_or(800) as usize,
antientropy_public: self.antientropy_public.unwrap_or(true),
private_neighbors: self.private_neighbors.unwrap_or(true),
block_proposal_token: self.block_proposal_token,
..ConnectionOptions::default()
})
}
Expand Down
47 changes: 36 additions & 11 deletions testnet/stacks-node/src/tests/nakamoto_integrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use std::{env, thread};
use clarity::vm::ast::ASTRules;
use clarity::vm::costs::ExecutionCost;
use clarity::vm::types::PrincipalData;
use http_types::headers::AUTHORIZATION;
use lazy_static::lazy_static;
use libsigner::{SignerSession, StackerDBSession};
use stacks::burnchains::MagicBytes;
Expand Down Expand Up @@ -1402,6 +1403,8 @@ fn block_proposal_api_endpoint() {
}

let (mut conf, _miner_account) = naka_neon_integration_conf(None);
let password = "12345".to_string();
conf.connection_options.block_proposal_token = Some(password.clone());
let account_keys = add_initial_balances(&mut conf, 10, 1_000_000);
let stacker_sk = setup_stacker(&mut conf);
let sender_signer_sk = Secp256k1PrivateKey::new();
Expand Down Expand Up @@ -1593,6 +1596,7 @@ fn block_proposal_api_endpoint() {

const HTTP_ACCEPTED: u16 = 202;
const HTTP_TOO_MANY: u16 = 429;
const HTTP_NOT_AUTHORIZED: u16 = 401;
let test_cases = [
(
"Valid Nakamoto block proposal",
Expand Down Expand Up @@ -1631,6 +1635,12 @@ fn block_proposal_api_endpoint() {
HTTP_ACCEPTED,
Some(Err(ValidateRejectCode::ChainstateError)),
),
(
"Not authorized",
sign(proposal.clone()),
HTTP_NOT_AUTHORIZED,
None,
),
];

// Build HTTP client
Expand All @@ -1647,12 +1657,18 @@ fn block_proposal_api_endpoint() {
test_cases.iter().enumerate()
{
// Send POST request
let mut response = client
let request_builder = client
.post(&path)
.header("Content-Type", "application/json")
.json(block_proposal)
.send()
.expect("Failed to POST");
.json(block_proposal);
let mut response = if expected_http_code == &HTTP_NOT_AUTHORIZED {
request_builder.send().expect("Failed to POST")
} else {
request_builder
.header(AUTHORIZATION.to_string(), password.to_string())
.send()
.expect("Failed to POST")
};
let start_time = Instant::now();
while ix != 1 && response.status().as_u16() == HTTP_TOO_MANY {
if start_time.elapsed() > Duration::from_secs(30) {
Expand All @@ -1661,20 +1677,29 @@ fn block_proposal_api_endpoint() {
}
info!("Waiting for prior request to finish processing, and then resubmitting");
thread::sleep(Duration::from_secs(5));
response = client
let request_builder = client
.post(&path)
.header("Content-Type", "application/json")
.json(block_proposal)
.send()
.expect("Failed to POST");
.json(block_proposal);
response = if expected_http_code == &HTTP_NOT_AUTHORIZED {
request_builder.send().expect("Failed to POST")
} else {
request_builder
.header(AUTHORIZATION.to_string(), password.to_string())
.send()
.expect("Failed to POST")
};
}

let response_code = response.status().as_u16();
let response_json = response.json::<serde_json::Value>();

let response_json = if expected_http_code != &HTTP_NOT_AUTHORIZED {
response.json::<serde_json::Value>().unwrap().to_string()
} else {
"No json response".to_string()
};
info!(
"Block proposal submitted and checked for HTTP response";
"response_json" => %response_json.unwrap(),
"response_json" => response_json,
"request_json" => serde_json::to_string(block_proposal).unwrap(),
"response_code" => response_code,
"test_description" => test_description,
Expand Down
12 changes: 11 additions & 1 deletion testnet/stacks-node/src/tests/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,18 @@ impl SignerTest {

let (mut naka_conf, _miner_account) = naka_neon_integration_conf(None);
naka_conf.miner.self_signing_key = None;
// So the combination is... one, two, three, four, five? That's the stupidest combination I've ever heard in my life!
// That's the kind of thing an idiot would have on his luggage!
let password = "12345";
naka_conf.connection_options.block_proposal_token = Some(password.to_string());

// Setup the signer and coordinator configurations
let signer_configs = build_signer_config_tomls(
&signer_stacks_private_keys,
&naka_conf.node.rpc_bind,
Some(Duration::from_millis(128)), // Timeout defaults to 5 seconds. Let's override it to 128 milliseconds.
&Network::Testnet,
password,
);

let mut running_signers = Vec::new();
Expand Down Expand Up @@ -726,7 +731,12 @@ impl SignerTest {
)
.unwrap();

let invalid_stacks_client = StacksClient::new(StacksPrivateKey::new(), host, false);
let invalid_stacks_client = StacksClient::new(
StacksPrivateKey::new(),
host,
"12345".to_string(), // That's amazing. I've got the same combination on my luggage!
false,
);
let invalid_signer_tx = invalid_stacks_client
.build_vote_for_aggregate_public_key(0, round, point, reward_cycle, None, 0)
.expect("FATAL: failed to build vote for aggregate public key");
Expand Down