Skip to content

certificate: accept pem format - #829

Merged
djc merged 2 commits into
quinn-rs:masterfrom
SSebo:master
Jul 18, 2020
Merged

certificate: accept pem format#829
djc merged 2 commits into
quinn-rs:masterfrom
SSebo:master

Conversation

@SSebo

@SSebo SSebo commented Jul 16, 2020

Copy link
Copy Markdown
Contributor

I found quinn::Certificate missing a from_pem function when I use pem format certificates, just add it.

@djc

djc commented Jul 16, 2020

Copy link
Copy Markdown
Member

Seems okay to me (modulo the formatting nit from CI -- ignore the clippy warning). @Ralith, any thoughts?

@Ralith

Ralith commented Jul 16, 2020

Copy link
Copy Markdown
Collaborator

I don't have a great grasp on the pem format itself; are there times when it'd be more appropriate to use this over CertificateChain::from_der?

@stammw

stammw commented Jul 17, 2020

Copy link
Copy Markdown
Contributor

Pem seems easier to handle from an ops point of view as most providers (i know) use this format. This PR simplify user's life by saving them the converting work IMHO.

@Ralith

Ralith commented Jul 17, 2020

Copy link
Copy Markdown
Collaborator

Er, sorry, that was a typo. I meant to ask whether there are situations where you can't use CertificateChain::from_pem. There is no CertificateChain::from_der.

@SSebo

SSebo commented Jul 17, 2020

Copy link
Copy Markdown
Contributor Author

I found der format in server.rs example is:

let key = quinn::PrivateKey::from_der(&key)?;
let cert = quinn::Certificate::from_der(&cert)?;
server_config.certificate(quinn::CertificateChain::from_certs(vec![cert]), key)?;

I followed the example, think maybe pem format should be this way:

let key = quinn::PrivateKey::from_pem(&key)?;
let cert = quinn::Certificate::from_pem(&cert)?;
server_config.certificate(quinn::CertificateChain::from_certs(vec![cert]), key)?;

after I read @Ralith 's comment, I found following way is working as well:

let key = quinn::PrivateKey::from_pem(&key)?;
server_config.certificate(quinn::CertificateChain::from_pem(&cert.to_vec())?, key)?;

In my case, client.rs need this Certificate::from_pem:

client_config.add_certificate_authority(quinn::Certificate::from_pem(&cert)?)?;

Ralith
Ralith previously approved these changes Jul 17, 2020

@Ralith Ralith left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems reasonable to me.

@djc

djc commented Jul 17, 2020

Copy link
Copy Markdown
Member

@SSebo please fix up the formatting, then we can merge it! Thanks!

Signed-off-by: ssebo <ssebo.zzz@gmail.com>
@djc
djc merged commit 2892490 into quinn-rs:master Jul 18, 2020
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.

4 participants