config robustness + trivia batch: ALPN dedup, empty-domains validation, packaging excludes, https doc line (C-2, C-3, N-6, N-7)
- C-2: new_acme dedups the acme-tls/1 ALPN append (idempotent construction); ACME/non-ACME ALPN asymmetry documented on TlsServerConfig::new; pinned by new_acme_caller_supplied_acme_tls_alpn_is_not_duplicated - C-3: new_acme rejects an empty domains list with TlsError::AcmeConfig before spawning the order loop; contact stays unvalidated (RFC 8555 7.3 zero-contact accounts are legal); pinned by new_acme_empty_domains_returns_config_error (acme-gated) - N-6: Cargo.toml exclude gains tasks/ and docs/architecture/ - N-7: AcmeDirectory::Custom documents the https-only caller contract (no runtime validation, per finding) Verification: cargo test (default) and --all-features (77 lib tests, 2 new), clippy -D warnings, fmt --check, doc, package --list (no tasks/ or docs/architecture/), publish --dry-run — all green
This commit is contained in:
+1
-1
@@ -8,7 +8,7 @@ description = "Shared TLS setup types: server and client rustls configs, cert re
|
||||
repository = "https://git.alk.dev/alkdev/alktls"
|
||||
keywords = ["tls", "rustls", "acme", "noq", "network"]
|
||||
categories = ["network-programming", "cryptography", "asynchronous"]
|
||||
exclude = [".opencode/", "AGENTS.md", "docs/reviews/", "docs/research/", "docs/plans/", "docs/sdd_process.md"]
|
||||
exclude = [".opencode/", "AGENTS.md", "tasks/", "docs/reviews/", "docs/research/", "docs/plans/", "docs/architecture/", "docs/sdd_process.md"]
|
||||
|
||||
[lib]
|
||||
name = "alktls"
|
||||
|
||||
+5
-1
@@ -61,7 +61,11 @@ pub enum AcmeDirectory {
|
||||
/// The Let's Encrypt staging directory (rate limits are relaxed;
|
||||
/// certificates it issues are not trusted by browsers).
|
||||
Staging,
|
||||
/// A custom ACME directory URL.
|
||||
/// A custom ACME directory URL. The URL is passed to rustls-acme
|
||||
/// verbatim and **must be an `https://` ACME directory URL** — an
|
||||
/// `http://` URL would run ACME (token-bearing) over plaintext.
|
||||
/// No runtime validation is applied (a non-https test directory
|
||||
/// must stay usable); this is a caller contract.
|
||||
Custom(String),
|
||||
}
|
||||
|
||||
|
||||
+57
-1
@@ -30,6 +30,11 @@ pub struct TlsServerConfig {
|
||||
impl TlsServerConfig {
|
||||
/// Build a server config from a [`TlsIdentity`] and ALPN list.
|
||||
/// ACME identities spawn a background cert-renewal task.
|
||||
///
|
||||
/// ALPN asymmetry: ACME identities always serve `acme-tls/1`, so it
|
||||
/// is appended to the caller's list here (idempotently — a caller
|
||||
/// who already includes it gets a single entry); non-ACME identities
|
||||
/// use the ALPN list verbatim.
|
||||
pub async fn new(tls_identity: &TlsIdentity, alpns: &[Vec<u8>]) -> Result<Self, TlsError> {
|
||||
match tls_identity {
|
||||
TlsIdentity::Acme {
|
||||
@@ -72,6 +77,12 @@ impl TlsServerConfig {
|
||||
use rustls_acme::caches::DirCache;
|
||||
use rustls_acme::{AcmeConfig, EventError, EventOk};
|
||||
|
||||
if domains.is_empty() {
|
||||
return Err(TlsError::AcmeConfig(
|
||||
"TlsIdentity::Acme requires a non-empty domain list".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let acme_config = AcmeConfig::new(domains.to_vec())
|
||||
.cache(DirCache::new(cache_dir.to_path_buf()))
|
||||
.directory(directory.url())
|
||||
@@ -88,7 +99,9 @@ impl TlsServerConfig {
|
||||
config.max_early_data_size = u32::MAX;
|
||||
|
||||
let mut alpn = alpns.to_vec();
|
||||
alpn.push(b"acme-tls/1".to_vec());
|
||||
if !alpn.contains(&b"acme-tls/1".to_vec()) {
|
||||
alpn.push(b"acme-tls/1".to_vec());
|
||||
}
|
||||
config.alpn_protocols = alpn;
|
||||
|
||||
let domains_owned: Vec<String> = domains.to_vec();
|
||||
@@ -755,6 +768,49 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(feature = "acme")]
|
||||
#[tokio::test]
|
||||
async fn new_acme_caller_supplied_acme_tls_alpn_is_not_duplicated() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let identity = TlsIdentity::Acme {
|
||||
domains: vec!["localhost".to_string()],
|
||||
cache_dir: dir.path().join("cache"),
|
||||
directory: crate::identity::AcmeDirectory::Custom(
|
||||
"http://127.0.0.1:9/directory".to_string(),
|
||||
),
|
||||
contact: vec!["mailto:dev@example.com".to_string()],
|
||||
};
|
||||
let alpn = vec![b"alktls/test".to_vec(), b"acme-tls/1".to_vec()];
|
||||
let setup = TlsServerConfig::new(&identity, &alpn)
|
||||
.await
|
||||
.expect("ACME config builds without awaiting the order");
|
||||
assert_eq!(
|
||||
setup.rustls_config().alpn_protocols,
|
||||
vec![b"alktls/test".to_vec(), b"acme-tls/1".to_vec()],
|
||||
"a caller-supplied acme-tls/1 must yield exactly one entry"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(feature = "acme")]
|
||||
#[tokio::test]
|
||||
async fn new_acme_empty_domains_returns_config_error() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let identity = TlsIdentity::Acme {
|
||||
domains: vec![],
|
||||
cache_dir: dir.path().join("cache"),
|
||||
directory: crate::identity::AcmeDirectory::Staging,
|
||||
contact: vec!["mailto:dev@example.com".to_string()],
|
||||
};
|
||||
let err = match TlsServerConfig::new(&identity, &[b"alktls/test".to_vec()]).await {
|
||||
Ok(_) => panic!("empty domain list must not construct an ACME config"),
|
||||
Err(e) => e,
|
||||
};
|
||||
assert!(
|
||||
matches!(err, TlsError::AcmeConfig(_)),
|
||||
"empty domains must surface as TlsError::AcmeConfig, got {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "acme"))]
|
||||
#[tokio::test]
|
||||
async fn new_acme_identity_without_feature_returns_config_error() {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
---
|
||||
id: config-validation-and-trivia
|
||||
name: Config robustness + trivia batch — ALPN dedup, empty-domains validation, packaging excludes, https doc line (C-2, C-3, N-6, N-7)
|
||||
status: pending
|
||||
status: completed
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: low
|
||||
@@ -96,8 +96,47 @@ session):
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills this during implementation.
|
||||
- C-2: implemented the dedup guard (preferred option) in `new_acme` —
|
||||
`if !alpn.contains(&b"acme-tls/1".to_vec())` before the push. Also
|
||||
documented the ACME/non-ACME ALPN asymmetry on
|
||||
`TlsServerConfig::new` (ACME always serves `acme-tls/1`, appended
|
||||
idempotently; non-ACME uses the caller's list verbatim). Pinned by
|
||||
`new_acme_caller_supplied_acme_tls_alpn_is_not_duplicated`
|
||||
(`src/server.rs`, `#[cfg(feature = "acme")]`).
|
||||
- C-3: added the empty-domains validation at the top of `new_acme`
|
||||
(before any `AcmeConfig` construction or task spawn) returning
|
||||
`TlsError::AcmeConfig("TlsIdentity::Acme requires a non-empty domain
|
||||
list")`. `contact` left unvalidated per the finding (RFC 8555 §7.3
|
||||
zero-contact accounts are legal). Test
|
||||
`new_acme_empty_domains_returns_config_error` is
|
||||
`#[cfg(feature = "acme")]`-gated per the task's mechanics note.
|
||||
- N-6: `exclude` now also lists `"tasks/"` and `"docs/architecture/"`.
|
||||
Verified `cargo package --list --allow-dirty` contains neither path
|
||||
and `cargo publish --dry-run --allow-dirty` passes.
|
||||
- N-7: doc line added on `AcmeDirectory::Custom` (`src/identity.rs`):
|
||||
the URL goes to rustls-acme verbatim and must be `https://` (an
|
||||
`http://` URL would run ACME token-bearing over plaintext); explicitly
|
||||
notes no runtime validation is applied so non-https test directories
|
||||
stay usable. No runtime check added, per the finding.
|
||||
|
||||
## Summary
|
||||
|
||||
> Agent fills this on completion.
|
||||
All four findings resolved. Changes:
|
||||
|
||||
1. `src/server.rs` — `new_acme` dedups the `acme-tls/1` ALPN append
|
||||
(idempotent construction) and rejects an empty `domains` list with
|
||||
`TlsError::AcmeConfig` before spawning the order loop; the
|
||||
ACME/non-ACME ALPN asymmetry is documented on `TlsServerConfig::new`.
|
||||
2. `src/identity.rs` — `AcmeDirectory::Custom` documents the
|
||||
https-only caller contract (no runtime validation, per finding).
|
||||
3. `Cargo.toml` — `exclude` gains `"tasks/"` and `"docs/architecture/"`.
|
||||
|
||||
New tests (both `#[cfg(feature = "acme")]`): caller-supplied
|
||||
`acme-tls/1` yields exactly one entry; empty `domains` constructs to
|
||||
`TlsError::AcmeConfig`.
|
||||
|
||||
Verification: `cargo test` (default) and `cargo test --all-features`
|
||||
(77 lib tests, both new tests pass), `cargo clippy --all-targets -- -D
|
||||
warnings`, `cargo fmt --check`, `cargo doc --no-deps`,
|
||||
`cargo package --list --allow-dirty` (no `tasks/` / `docs/architecture/`
|
||||
entries), `cargo publish --dry-run --allow-dirty` — all green.
|
||||
Reference in New Issue
Block a user