-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-15934] Add agent-forwarding detection and git signing detection parsers #12371
base: main
Are you sure you want to change the base?
Changes from 7 commits
53f2b6d
761c28c
4236ae9
088a72b
29ccb47
684862d
a530f91
d4796d1
f5472df
4c78e3b
fac5e38
e456a91
6413645
7956ff5
9053721
18e50de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
#[derive(Debug)] | ||
pub(crate) struct SshSigRequest { | ||
pub namespace: String, | ||
} | ||
|
||
#[derive(Debug)] | ||
pub(crate) struct SignRequest {} | ||
|
||
#[derive(Debug)] | ||
pub(crate) enum SshAgentSignRequest { | ||
SshSigRequest(SshSigRequest), | ||
SignRequest(SignRequest), | ||
} | ||
|
||
pub(crate) fn parse_request(data: &[u8]) -> Result<SshAgentSignRequest, anyhow::Error> { | ||
let magic_header = "SSHSIG"; | ||
let mut data_iter = data.to_vec().into_iter(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is getting a clippy lint for me, I think you can change it to That said, using iterators like this is kind of unergonomic, for this type of byte handling I usually recommend the pub(crate) fn parse_request(data: &[u8]) -> Result<SshAgentSignRequest, anyhow::Error> {
let mut data = Bytes::copy_from_slice(data);
let magic_header = "SSHSIG";
// This splits the six first characters into `header` and `data` advances it's internal cursor by 6
let header = data.split_to(magic_header.len());
// sshsig; based on https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.sshsig
if header == magic_header.as_bytes() {
// This automatically advances the internal cursor by 4
let _version = data.get_u32();
// read until null byte
let namespace = data
.into_iter()
.take_while(|&x| x != 0)
.collect::<Vec<u8>>(); PD: Another option is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, this is exactly what I was looking for! |
||
let header = data_iter | ||
.by_ref() | ||
.take(magic_header.len()) | ||
.collect::<Vec<u8>>(); | ||
|
||
// sshsig; based on https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.sshsig | ||
if header == magic_header.as_bytes() { | ||
let version = data_iter.by_ref().take(4).collect::<Vec<u8>>(); | ||
let _version = u32::from_be_bytes( | ||
version | ||
.try_into() | ||
.map_err(|_| anyhow::anyhow!("Invalid version"))?, | ||
); | ||
|
||
// read until null byte | ||
let namespace = data_iter | ||
.by_ref() | ||
.take_while(|&x| x != 0) | ||
.collect::<Vec<u8>>(); | ||
let namespace = | ||
String::from_utf8(namespace).map_err(|_| anyhow::anyhow!("Invalid namespace"))?; | ||
|
||
Ok(SshAgentSignRequest::SshSigRequest(SshSigRequest { | ||
namespace, | ||
})) | ||
} else { | ||
// regular sign request | ||
Ok(SshAgentSignRequest::SignRequest(SignRequest {})) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny non-blocking nit, instead of returning an
is_git
boolean, would it make more sense to return the namespace directly and let the TypeScript UI choose which message to show then?Would mean less changes if we ever need to add more messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, passing this through now.