Skip to content

Commit 8fc6448

Browse files
ariloudjc
authored andcommitted
Remove the hash-based filename requirements
When loading certificates from a directory, remove the requirement that the filenames have a hash pattern. Signed-off-by: Jon Doron <[email protected]>
1 parent 6f3124a commit 8fc6448

File tree

2 files changed

+33
-76
lines changed

2 files changed

+33
-76
lines changed

src/lib.rs

Lines changed: 5 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
2323

2424
use std::error::Error as StdError;
25-
use std::ffi::OsStr;
2625
use std::path::{Path, PathBuf};
2726
use std::{env, fmt, fs, io};
2827

@@ -51,7 +50,7 @@ use macos as platform;
5150
/// | Env. Var. | Description |
5251
/// |----------------|---------------------------------------------------------------------------------------|
5352
/// | SSL_CERT_FILE | File containing an arbitrary number of certificates in PEM format. |
54-
/// | SSL_CERT_DIR | Directory utilizing the hierarchy and naming convention used by OpenSSL's [c_rehash]. |
53+
/// | SSL_CERT_DIR | Colon separated list of directories containing certificate files. |
5554
///
5655
/// If **either** (or **both**) are set, certificates are only loaded from
5756
/// the locations specified via environment variables and not the platform-
@@ -228,10 +227,9 @@ impl CertPaths {
228227
/// not considered part of a certificate. Certificates which are not in the right
229228
/// format (PEM) or are otherwise corrupted may get ignored silently.
230229
///
231-
/// If `dir` is defined, a directory must exist at this path, and all
232-
/// hash files contained in it must be loaded successfully,
233-
/// subject to the rules outlined above for `file`. The directory is not
234-
/// scanned recursively and may be empty.
230+
/// If `dir` is defined, a directory must exist at this path, and all files
231+
/// contained in it must be loaded successfully, subject to the rules outlined above for `file`.
232+
/// The directory is not scanned recursively and may be empty.
235233
pub fn load_certs_from_paths(file: Option<&Path>, dir: Option<&Path>) -> CertificateResult {
236234
let dir = match dir {
237235
Some(d) => vec![d],
@@ -265,12 +263,6 @@ fn load_certs_from_paths_internal(
265263
}
266264

267265
/// Load certificate from certificate directory (what OpenSSL calls CAdir)
268-
///
269-
/// This directory can contain other files and directories. CAfile tends
270-
/// to be in here too. To avoid loading something twice or something that
271-
/// isn't a valid certificate, we limit ourselves to loading those files
272-
/// that have a hash-based file name matching the pattern used by OpenSSL.
273-
/// The hash is not verified, however.
274266
fn load_pem_certs_from_dir(dir: &Path, out: &mut CertificateResult) {
275267
let dir_reader = match fs::read_dir(dir) {
276268
Ok(reader) => reader,
@@ -290,12 +282,6 @@ fn load_pem_certs_from_dir(dir: &Path, out: &mut CertificateResult) {
290282
};
291283

292284
let path = entry.path();
293-
let file_name = path
294-
.file_name()
295-
// We are looping over directory entries. Directory entries
296-
// always have a name (except "." and ".." which the iterator
297-
// never yields).
298-
.expect("dir entry with no name");
299285

300286
// `openssl rehash` used to create this directory uses symlinks. So,
301287
// make sure we resolve them.
@@ -311,7 +297,7 @@ fn load_pem_certs_from_dir(dir: &Path, out: &mut CertificateResult) {
311297
}
312298
};
313299

314-
if metadata.is_file() && is_hash_file_name(file_name) {
300+
if metadata.is_file() {
315301
load_pem_certs(&path, out);
316302
}
317303
}
@@ -334,35 +320,6 @@ fn load_pem_certs(path: &Path, out: &mut CertificateResult) {
334320
}
335321
}
336322

337-
/// Check if this is a hash-based file name for a certificate
338-
///
339-
/// According to the [c_rehash man page][]:
340-
///
341-
/// > The links created are of the form HHHHHHHH.D, where each H is a hexadecimal
342-
/// > character and D is a single decimal digit.
343-
///
344-
/// `c_rehash` generates lower-case hex digits but this is not clearly documented.
345-
/// Because of this, and because it could lead to issues on case-insensitive file
346-
/// systems, upper-case hex digits are accepted too.
347-
///
348-
/// [c_rehash man page]: https://www.openssl.org/docs/manmaster/man1/c_rehash.html
349-
fn is_hash_file_name(file_name: &OsStr) -> bool {
350-
let file_name = match file_name.to_str() {
351-
Some(file_name) => file_name,
352-
None => return false, // non-UTF8 can't be hex digits
353-
};
354-
355-
if file_name.len() != 10 {
356-
return false;
357-
}
358-
let mut iter = file_name.chars();
359-
let iter = iter.by_ref();
360-
iter.take(8)
361-
.all(|c| c.is_ascii_hexdigit())
362-
&& iter.next() == Some('.')
363-
&& matches!(iter.next(), Some(c) if c.is_ascii_digit())
364-
}
365-
366323
#[derive(Debug)]
367324
pub struct Error {
368325
pub context: &'static str,
@@ -415,34 +372,6 @@ mod tests {
415372
#[cfg(unix)]
416373
use std::os::unix::fs::PermissionsExt;
417374

418-
#[test]
419-
fn valid_hash_file_name() {
420-
let valid_names = [
421-
"f3377b1b.0",
422-
"e73d606e.1",
423-
"01234567.2",
424-
"89abcdef.3",
425-
"ABCDEF00.9",
426-
];
427-
for name in valid_names {
428-
assert!(is_hash_file_name(OsStr::new(name)));
429-
}
430-
}
431-
432-
#[test]
433-
fn invalid_hash_file_name() {
434-
let valid_names = [
435-
"f3377b1b.a",
436-
"e73d606g.1",
437-
"0123457.2",
438-
"89abcdef0.3",
439-
"name.pem",
440-
];
441-
for name in valid_names {
442-
assert!(!is_hash_file_name(OsStr::new(name)));
443-
}
444-
}
445-
446375
#[test]
447376
fn deduplication() {
448377
let temp_dir = tempfile::TempDir::new().unwrap();

tests/smoketests.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,34 @@ fn ssl_cert_dir_multiple_paths_are_respected() {
230230
check_site("self-signed.badssl.com").unwrap();
231231
}
232232

233+
#[test]
234+
#[serial]
235+
#[ignore]
236+
fn ssl_cert_dir_non_hash_based_name() {
237+
unsafe {
238+
// SAFETY: safe because of #[serial]
239+
common::clear_env();
240+
}
241+
242+
// Create temporary directory
243+
let temp_dir = tempfile::TempDir::new().unwrap();
244+
245+
// Copy the certificate to the dir
246+
let original = Path::new("tests/badssl-com-chain.pem")
247+
.canonicalize()
248+
.unwrap();
249+
let cert = temp_dir.path().join("test.pem");
250+
std::fs::copy(original, cert).unwrap();
251+
252+
env::set_var(
253+
"SSL_CERT_DIR",
254+
// The CA cert, downloaded directly from the site itself:
255+
temp_dir.path(),
256+
);
257+
258+
check_site("self-signed.badssl.com").unwrap();
259+
}
260+
233261
#[test]
234262
#[serial]
235263
#[ignore]

0 commit comments

Comments
 (0)