-
Notifications
You must be signed in to change notification settings - Fork 14
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
Enclave signing #199
base: main
Are you sure you want to change the base?
Enclave signing #199
Conversation
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.
Thank you for your contribution! It's something that has been on our mind so great to see it make its way in.
A concern with signing long lived artifacts is that it pushes one to use long lived certs. Unfortunately there's no revocation mechanism so one should be careful.
Just a few changes and I think it'll be good to merge.
cmd.push("/var/run/key"); | ||
|
||
let certificate_path = canonicalize(PathBuf::from(manifest_path).parent().unwrap().join(&signature.certificate)).await?; | ||
let key_path = canonicalize(PathBuf::from(manifest_path).parent().unwrap().join(&signature.key)).await?; |
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.
I would move these two lines to the caller and just path the paths for the cert and key into the function.
cmd.push("--private-key"); | ||
cmd.push("/var/run/key"); | ||
|
||
let certificate_path = canonicalize(PathBuf::from(manifest_path).parent().unwrap().join(&signature.certificate)).await?; |
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.
Use .parent()?
instead to avoid panicking if someone passes in a bad path.
#[serde(deny_unknown_fields)] | ||
pub struct Signature { | ||
pub certificate: String, | ||
pub key: String, |
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.
Make these PathBuf
to be clear that they're paths and also avoid conversions later.
Warning!!! I am less than a n00b in rust, this was done to scratch my own itch and it worked for me.