Skip to content

Commit

Permalink
Fix handling of Monero daemon connections when using an authenticated…
Browse files Browse the repository at this point in the history
… RPC

The lack of locking the connection when making an authenticated request, which
is actually two sequential requests, risked another caller making a request in
between, invalidating the state.

Now, only unauthenticated connections share a connection object.
  • Loading branch information
kayabaNerve committed Oct 26, 2023
1 parent f22aedc commit 3069138
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 14 deletions.
40 changes: 30 additions & 10 deletions coins/monero/src/rpc/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,21 @@ use reqwest::Client;

use crate::rpc::{RpcError, RpcConnection, Rpc};

#[derive(Clone, Debug)]
enum Authentication {
// If unauthenticated, reuse a single client
Unauthenticated(Client),
// If authenticated, don't reuse clients so that each connection makes its own connection
// This ensures that if a nonce is requested, another caller doesn't make a request invalidating
// it
// We could acquire a mutex over the client, yet creating a new client is preferred for the
// possibility of parallelism
Authenticated(String, String),
}

#[derive(Clone, Debug)]
pub struct HttpRpc {
client: Client,
userpass: Option<(String, String)>,
authentication: Authentication,
url: String,
}

Expand All @@ -18,8 +29,8 @@ impl HttpRpc {
/// A daemon requiring authentication can be used via including the username and password in the
/// URL.
pub fn new(mut url: String) -> Result<Rpc<HttpRpc>, RpcError> {
// Parse out the username and password
let userpass = if url.contains('@') {
let authentication = if url.contains('@') {
// Parse out the username and password
let url_clone = url;
let split_url = url_clone.split('@').collect::<Vec<_>>();
if split_url.len() != 2 {
Expand All @@ -42,22 +53,31 @@ impl HttpRpc {
if split_userpass.len() != 2 {
Err(RpcError::InvalidNode)?;
}
Some((split_userpass[0].to_string(), split_userpass[1].to_string()))
Authentication::Authenticated(split_userpass[0].to_string(), split_userpass[1].to_string())
} else {
None
Authentication::Unauthenticated(Client::new())
};

Ok(Rpc(HttpRpc { client: Client::new(), userpass, url }))
Ok(Rpc(HttpRpc { authentication, url }))
}
}

#[async_trait]
impl RpcConnection for HttpRpc {
async fn post(&self, route: &str, body: Vec<u8>) -> Result<Vec<u8>, RpcError> {
let mut builder = self.client.post(self.url.clone() + "/" + route).body(body);
#[allow(unused_assignments)] // False positive
let mut client_storage = None;
let client = match &self.authentication {
Authentication::Unauthenticated(client) => client,
Authentication::Authenticated(_, _) => {
client_storage = Some(Client::new());
client_storage.as_ref().unwrap()
}
};

if let Some((user, pass)) = &self.userpass {
let req = self.client.post(&self.url).send().await.map_err(|_| RpcError::InvalidNode)?;
let mut builder = client.post(self.url.clone() + "/" + route).body(body);
if let Authentication::Authenticated(user, pass) = &self.authentication {
let req = client.post(&self.url).send().await.map_err(|_| RpcError::InvalidNode)?;
// Only provide authentication if this daemon actually expects it
if let Some(header) = req.headers().get("www-authenticate") {
builder = builder.header(
Expand Down
6 changes: 3 additions & 3 deletions orchestration/coins/monero/scripts/entry-dev.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ RPC_PASS="${RPC_PASS:=seraidex}"

# Run Monero
# TODO: Restore Auth
monerod --regtest --offline --fixed-difficulty=1 \
--rpc-bind-ip=0.0.0.0 --rpc-access-control-origins * --confirm-external-bind \
--non-interactive
monerod --non-interactive --regtest --offline --fixed-difficulty=1 \
--no-zmq --rpc-bind-ip=0.0.0.0 --confirm-external-bind \
--rpc-access-control-origins * --disable-rpc-ban
3 changes: 2 additions & 1 deletion processor/src/tests/literal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ mod monero {
"--no-zmq".to_string(),
"--disable-rpc-ban".to_string(),
"--rpc-bind-ip=0.0.0.0".to_string(),
"--rpc-login=serai:seraidex".to_string(),
"--rpc-access-control-origins=*".to_string(),
"--confirm-external-bind".to_string(),
"--non-interactive".to_string(),
Expand All @@ -110,7 +111,7 @@ mod monero {
let handle = ops.handle("serai-dev-monero").host_port(18081).unwrap();
// TODO: Replace with a check if the node has booted
tokio::time::sleep(core::time::Duration::from_secs(20)).await;
let monero = Monero::new(format!("http://{}:{}", handle.0, handle.1));
let monero = Monero::new(format!("http://serai:seraidex@{}:{}", handle.0, handle.1));
while monero.get_latest_block_number().await.unwrap() < 150 {
monero.mine_block().await;
}
Expand Down

0 comments on commit 3069138

Please sign in to comment.