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

Expose CertPaths directly #147

Open
0rphon opened this issue Oct 30, 2024 · 3 comments
Open

Expose CertPaths directly #147

0rphon opened this issue Oct 30, 2024 · 3 comments

Comments

@0rphon
Copy link

0rphon commented Oct 30, 2024

This crate allows environment variables to set a custom ssl cert file/dir. It would be great if you also included a way for the caller to set these values directly. By either making CertPaths and CertPaths::load public, or by exposing a function similar to the following:

/// The same as [`load_native_certs()`], but allows specifying the `SSL_CERT_FILE` and `SSL_CERT_DIR` directly
pub fn load_certs_from_path(file: Option<PathBuf>, dir: Option<PathBuf>) -> CertificateResult {
    CertPaths { file, dir }.load()
}

My specific use case that requires this is because we use cert paths specified from a configuration file. We have our own code in place to do this, but if you exposed functionality like the above then it would allow us to fully switch over to your crate.

@0rphon
Copy link
Author

0rphon commented Oct 30, 2024

i actually tweaked my own fork to remove the path allocations with the following

/// Load certificates from the paths.
///
/// If both are `None`, return `Ok(None)`.
///
/// If `file` is `Some`, it is always used, so it must be a path to an existing,
/// accessible file from which certificates can be loaded successfully. While parsing,
/// the rustls-pki-types PEM parser will ignore parts of the file which are
/// not considered part of a certificate. Certificates which are not in the right
/// format (PEM) or are otherwise corrupted may get ignored silently.
///
/// If `dir` is defined, a directory must exist at this path, and all
/// hash files contained in it must be loaded successfully,
/// subject to the rules outlined above for `file`. The directory is not
/// scanned recursively and may be empty.
pub fn load_certs_from_path<T: AsRef<Path>>(file: Option<T>, dir: Option<T>) -> CertificateResult {
    let mut out = CertificateResult::default();
    if file.is_none() && dir.is_none() {
        return out;
    }

    if let Some(cert_file) = file {
        load_pem_certs(cert_file.as_ref(), &mut out);
    }

    if let Some(cert_dir) = dir {
        load_pem_certs_from_dir(cert_dir.as_ref(), &mut out);
    }

    out.certs
        .sort_unstable_by(|a, b| a.cmp(b));
    out.certs.dedup();
    out
}

/// Certificate paths from `SSL_CERT_FILE` and/or `SSL_CERT_DIR`.
struct CertPaths {
    file: Option<PathBuf>,
    dir: Option<PathBuf>,
}

impl CertPaths {
    fn from_env() -> Self {
        Self {
            file: env::var_os(ENV_CERT_FILE).map(PathBuf::from),
            dir: env::var_os(ENV_CERT_DIR).map(PathBuf::from),
        }
    }

    /// Calls [`load_certs_from_path`] with the given cert paths.
    fn load(&self) -> CertificateResult {
        load_certs_from_path(self.file.as_ref(), self.dir.as_ref())
    }
}

If this is a change you guys are happy with, id be fine with PRing it

@djc
Copy link
Member

djc commented Oct 30, 2024

This will be easier to judge if you just submit a PR. I'd certainly be willing to consider it.

@0rphon
Copy link
Author

0rphon commented Oct 30, 2024

Can do!

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

No branches or pull requests

2 participants