Skip to content

Commit

Permalink
signer: Report policy errors back to the node
Browse files Browse the repository at this point in the history
When the signer rejects a request we had no indication as to what went
wrong until now. With this change we (a) send back the error as if it
were a normal response and then (b) print the error in the logs,
allowing us to collate the operations causing the rejection with the
rejection itself.
  • Loading branch information
cdecker committed Jul 22, 2024
1 parent fab2993 commit 26775e9
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 1 deletion.
6 changes: 6 additions & 0 deletions libs/gl-client/.resources/proto/glclient/greenlight.proto
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ message HsmResponse {
// A list of updated key-value-version tuples that is to be
// merged into the state tracked by the plugin.
repeated SignerStateEntry signer_state = 5;

// If the signer reported an error, and did therefore not include
// `raw`, this is the stringified error, so we can print it in the
// logs. This should help us collate policy errors with the changes
// proposed by CLN
string error = 6;
}

message HsmRequest {
Expand Down
12 changes: 12 additions & 0 deletions libs/gl-client/src/signer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ impl Signer {
return Ok(());
}
};
let request_id = req.request_id;
let hex_req = hex::encode(&req.raw);
let signer_state = req.signer_state.clone();
trace!("Received request {}", hex_req);
Expand All @@ -410,6 +411,16 @@ impl Signer {
.map_err(|e| Error::NodeDisconnect(e))?;
}
Err(e) => {
let response = HsmResponse {
raw: vec![],
request_id,
error: format!("{:?}", e),
signer_state: vec![],
};
client
.respond_hsm_request(response)
.await
.map_err(|e| Error::NodeDisconnect(e))?;
warn!(
"Ignoring error {} for request {} with state {:?}",
e, hex_req, signer_state,
Expand Down Expand Up @@ -561,6 +572,7 @@ impl Signer {
raw: response.0.as_vec(),
request_id: req.request_id,
signer_state,
error: "".to_owned(),
})
}

Expand Down
5 changes: 4 additions & 1 deletion libs/gl-plugin/src/hsm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,27 +63,30 @@ impl Hsm for StagingHsmServer {
request_id: req.request_id,
raw: response,
signer_state: Vec::new(),
error: "".into(),
}));
} else if req.get_type() == 11 {
debug!("Returning stashed init msg: {:?}", self.node_info.initmsg);
return Ok(Response::new(HsmResponse {
request_id: req.request_id,
raw: self.node_info.initmsg.clone(),
signer_state: Vec::new(), // the signerproxy doesn't care about state
error: "".into(),
}));
} else if req.get_type() == 33 {
debug!("Returning stashed dev-memleak response");
return Ok(Response::new(HsmResponse {
request_id: req.request_id,
raw: vec![0, 133, 0],
signer_state: Vec::new(), // the signerproxy doesn't care about state
error: "".into(),
}));
}

let mut chan = match self.stage.send(req).await {
Err(e) => {
return Err(Status::unknown(format!(
"Error while queing request from node: {:?}",
"Error while queuing request from node: {:?}",
e
)))
}
Expand Down
7 changes: 7 additions & 0 deletions libs/gl-plugin/src/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,13 @@ impl Node for PluginNodeServer {
request: Request<pb::HsmResponse>,
) -> Result<Response<pb::Empty>, Status> {
let req = request.into_inner();

if req.error != "" {
log::error!("Signer reports an error: {}", req.error);
log::warn!("The above error was returned instead of a response.");
return Ok(Response::new(pb::Empty::default()));
}

// Create a state from the key-value-version tuples. Need to
// convert here, since `pb` is duplicated in the two different
// crates.
Expand Down
2 changes: 2 additions & 0 deletions libs/gl-plugin/src/stager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ mod test {
request_id: r.request.request_id,
raw: vec![],
signer_state: vec![],
error: "".into(),
})
.await
{
Expand All @@ -194,6 +195,7 @@ mod test {
request_id: r.request.request_id,
raw: vec![],
signer_state: vec![],
error: "".into(),
})
.await
{
Expand Down
6 changes: 6 additions & 0 deletions libs/proto/glclient/greenlight.proto
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ message HsmResponse {
// A list of updated key-value-version tuples that is to be
// merged into the state tracked by the plugin.
repeated SignerStateEntry signer_state = 5;

// If the signer reported an error, and did therefore not include
// `raw`, this is the stringified error, so we can print it in the
// logs. This should help us collate policy errors with the changes
// proposed by CLN
string error = 6;
}

message HsmRequest {
Expand Down

0 comments on commit 26775e9

Please sign in to comment.