-
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
Added support for postgres certificate authentication for native tls #1166
Changes from all commits
2630b9c
019364d
dbce967
7e4cf5e
1463670
acdce64
adfe6d4
a10cb65
b92fee5
00e73be
691cbd6
673c9c2
13dc17a
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 |
---|---|---|
|
@@ -80,11 +80,15 @@ where | |
accept_invalid_certs: bool, | ||
accept_invalid_hostnames: bool, | ||
root_cert_path: Option<&CertificateInput>, | ||
client_cert_path: Option<&CertificateInput>, | ||
client_key_path: Option<&CertificateInput>, | ||
) -> Result<(), Error> { | ||
let connector = configure_tls_connector( | ||
accept_invalid_certs, | ||
accept_invalid_hostnames, | ||
root_cert_path, | ||
client_cert_path, | ||
client_key_path, | ||
) | ||
.await?; | ||
|
||
|
@@ -117,8 +121,16 @@ async fn configure_tls_connector( | |
accept_invalid_certs: bool, | ||
accept_invalid_hostnames: bool, | ||
root_cert_path: Option<&CertificateInput>, | ||
client_cert_path: Option<&CertificateInput>, | ||
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. Can we fix these warnings? If the |
||
client_key_path: Option<&CertificateInput>, | ||
) -> Result<sqlx_rt::TlsConnector, Error> { | ||
#[cfg(feature = "openssl-native")] | ||
use openssl::{pkcs12::Pkcs12, pkey::PKey, x509::X509}; | ||
#[cfg(feature = "openssl-native")] | ||
use sqlx_rt::native_tls::Identity; | ||
use sqlx_rt::native_tls::{Certificate, TlsConnector}; | ||
#[cfg(feature = "openssl-native")] | ||
const PASSWORD: &str = "temp-pkcs12"; | ||
|
||
let mut builder = TlsConnector::builder(); | ||
builder | ||
|
@@ -131,6 +143,26 @@ async fn configure_tls_connector( | |
let cert = Certificate::from_pem(&data)?; | ||
|
||
builder.add_root_certificate(cert); | ||
|
||
#[cfg(feature = "openssl-native")] | ||
if let (Some(cert), Some(key)) = (client_cert_path, client_key_path) { | ||
let cert_data = cert.data().await?; | ||
let key_data = key.data().await?; | ||
if let (Ok(pkey), Ok(cert)) = ( | ||
PKey::private_key_from_pem(&key_data), | ||
X509::from_pem(&cert_data), | ||
) { | ||
if let Ok(pkcs) = | ||
Pkcs12::builder().build(PASSWORD, PASSWORD, pkey.as_ref(), cert.as_ref()) | ||
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. I'm not a fan of this hack, though I do understand that the I would like to see this PR merged in 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. If nothing else I would like to see comments added to this code explaining exactly why it's doing this and what it needs to be better. I also don't really like how the errors are discarded here. 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. Honestly I would probably consider requiring the |
||
{ | ||
if let Ok(der) = pkcs.to_der() { | ||
let identity = Identity::from_pkcs12(&der, PASSWORD)?; | ||
|
||
builder.identity(identity); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ use rustls::{ | |
Certificate, ClientConfig, RootCertStore, ServerCertVerified, ServerCertVerifier, TLSError, | ||
WebPKIVerifier, | ||
}; | ||
use std::io::Cursor; | ||
use std::io::{BufReader, Cursor}; | ||
use std::sync::Arc; | ||
use webpki::DNSNameRef; | ||
|
||
|
@@ -13,6 +13,8 @@ pub async fn configure_tls_connector( | |
accept_invalid_certs: bool, | ||
accept_invalid_hostnames: bool, | ||
root_cert_path: Option<&CertificateInput>, | ||
client_cert_path: Option<&CertificateInput>, | ||
client_key_path: Option<&CertificateInput>, | ||
) -> Result<sqlx_rt::TlsConnector, Error> { | ||
let mut config = ClientConfig::new(); | ||
|
||
|
@@ -34,6 +36,21 @@ pub async fn configure_tls_connector( | |
.map_err(|_| Error::Tls(format!("Invalid certificate {}", ca).into()))?; | ||
} | ||
|
||
if let (Some(cert), Some(key)) = (client_cert_path, client_key_path) { | ||
let key_data = key.data().await?; | ||
let cert_data = cert.data().await?; | ||
let certs = to_certs(cert_data); | ||
let key = to_private_key(key_data)?; | ||
match config.set_single_client_cert(certs, key) { | ||
Ok(_) => (), | ||
Err(err) => { | ||
return Err(Error::Configuration( | ||
format!("no keys found in: {:?}", err).into(), | ||
)) | ||
} | ||
} | ||
} | ||
|
||
if accept_invalid_hostnames { | ||
config | ||
.dangerous() | ||
|
@@ -77,3 +94,29 @@ impl ServerCertVerifier for NoHostnameTlsVerifier { | |
} | ||
} | ||
} | ||
|
||
fn to_certs(pem: Vec<u8>) -> Vec<rustls::Certificate> { | ||
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 name isn't really idiomatic as it doesn't take I think |
||
let cur = Cursor::new(pem); | ||
let mut reader = BufReader::new(cur); | ||
rustls_pemfile::certs(&mut reader) | ||
.unwrap() | ||
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. The docs for |
||
.iter() | ||
.map(|v| rustls::Certificate(v.clone())) | ||
.collect() | ||
} | ||
|
||
fn to_private_key(pem: Vec<u8>) -> Result<rustls::PrivateKey, Error> { | ||
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. Same here, |
||
let cur = Cursor::new(pem); | ||
let mut reader = BufReader::new(cur); | ||
|
||
loop { | ||
match rustls_pemfile::read_one(&mut reader)? { | ||
Some(rustls_pemfile::Item::RSAKey(key)) => return Ok(rustls::PrivateKey(key)), | ||
Some(rustls_pemfile::Item::PKCS8Key(key)) => return Ok(rustls::PrivateKey(key)), | ||
None => break, | ||
_ => {} | ||
} | ||
} | ||
|
||
Err(Error::Configuration("no keys found pem file".into())) | ||
} |
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 don't really see the reasoning behind giving this a different feature name. Why not just
openssl
like in thesqlx
crate?If you tried to define an
openssl
feature but got an error, it's because you don't need to explicitly define features foroptional = true
dependencies.