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

[RSDK-9433] - Store TLS certs in DER format to save space #360

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gvaradarajan
Copy link
Member

No description provided.

@gvaradarajan gvaradarajan requested a review from a team as a code owner December 10, 2024 20:40
Copy link
Member

@mattjperez mattjperez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM it it's been verified on hardware.

Not sure if possible but would be cool if we can see how much space we've saved

Comment on lines +6 to +7
use rustls_pki_types::pem::PemObject;
use rustls_pki_types::{CertificateDer, PrivateKeyDer};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: join these

Comment on lines +448 to +449
let _ = self.srv_cert.replace(srv_cert);
let _ = self.srv_key.replace(srv_key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Comment on lines +457 to +458
&self.srv_cert.as_ref().unwrap(),
&self.srv_key.as_ref().unwrap(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ? instead of unwrap since the return signature is compatible

Comment on lines +31 to +32
let priv_keys = rustls::PrivateKey(self.srv_key.clone().unwrap());
let cert_chain = vec![rustls::Certificate(self.srv_cert.clone().unwrap())];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here with unwraps

fn try_from(resp: CertificateResponse) -> Result<Self, Self::Error> {
// we convert the certificate and private key from PEM to DER format to save space
let private_key_bytes = resp.tls_private_key.into_bytes();
let private_key = PrivateKeyDer::from_pem_slice(&private_key_bytes[0..])?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this range should just work with [..] right? also, maybe check if the range is needed or if as_slice is available.
L102 does the same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants