Skip to content

Conversation

YuJuncen
Copy link
Contributor

@YuJuncen YuJuncen commented Nov 9, 2022

Signed-off-by: Yu Juncen [email protected]

Motivation

This PR tries to fix #1143. Which caused tonic reports error when using EC keys as client keys.

Solution

The solution has been described in the issue. Copy & paste it here:

By reading the code, I have noticed that we are going to extract key via applying pkcs8_private_keys and rsa_private_keys. It seems rustls-pemfile supports SEC1 EC keys however doesn't provide something like ec_private_keys (!).
Perhaps we can use read_once direcly, so we only need to iterate the key file once and can extract the EC keys:

fn load_rustls_private_key(
    mut cursor: std::io::Cursor<&[u8]>,
) -> Result<PrivateKey, crate::Error> {
    while let Ok(Some(item)) = rustls_pemfile::read_one(&mut cursor) {
        match item {
            rustls_pemfile::Item::RSAKey(key)
            | rustls_pemfile::Item::PKCS8Key(key)
            | rustls_pemfile::Item::ECKey(key) => return Ok(PrivateKey(key)),
            _ => continue,
        }
    }

    // Otherwise we have a Private Key parsing problem
    Err(Box::new(TlsError::PrivateKeyParseError))
}

@YuJuncen YuJuncen changed the title fix: added support to EC keys fix: added support for EC keys Nov 9, 2022
use crate::transport::Identity;

fn load_rustls_private_key(
pub(crate) fn load_rustls_private_key(
Copy link
Member

@LucioFranco LucioFranco Nov 9, 2022

Choose a reason for hiding this comment

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

Why pub(crate), I don't see this used outside of the crate or am I going crazy?

Copy link
Contributor Author

@YuJuncen YuJuncen Nov 10, 2022

Choose a reason for hiding this comment

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

I want to use it in the tests module, if I remove this rustc would blame me load_rustls_private_key is a private function... Are there some better solutions?

Well, I guess pub(super) is what we need?

@LucioFranco LucioFranco changed the title fix: added support for EC keys feat(transport): added support for EC keys Feb 13, 2023
@LucioFranco LucioFranco added this pull request to the merge queue Feb 13, 2023
Merged via the queue into hyperium:master with commit 17d6a4b Feb 13, 2023
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.

bug?: cannot extract client key from pem file if it encoded by SEC1 EC

2 participants