Commit e08d0253 authored by Ben Boeckel's avatar Ben Boeckel

valid_name: use trust-dns-resolver

Instead of assuming things about the host environment (namely that a
binary named `host` exists on the machine and uses BIND's API), run the
DNS query from Rust directly.

Fixes: #47
parent 24b6c0bd
Pipeline #148658 failed with stages
in 7 minutes and 19 seconds
......@@ -19,13 +19,13 @@ before_script:
- cargo test --frozen --all --no-run --verbose
.cargo_test: &cargo_test
- apt-get install -yqq --no-install-recommends git bind9-host
- apt-get install -yqq --no-install-recommends git
- git config --global user.name "Ghostflow Testing"
- git config --global user.email "ghostflow@example.invalid"
- cargo test --frozen --all --verbose
.rust_minimum: &rust_minimum
image: "rust:1.36.0"
image: "rust:1.39.0"
variables:
CARGO_UPDATE_POLICY: newest
......@@ -204,7 +204,6 @@ test:git-master:
- *cargo_test_job
- *rust_stable
script:
- apt-get install -yqq --no-install-recommends bind9-host
- git config --global user.name "Ghostflow Testing"
- git config --global user.email "ghostflow@example.invalid"
- PATH=$PWD/git/root/bin:$PATH cargo test --frozen --all --verbose
......
......@@ -31,6 +31,9 @@
* The `Changelog` now requires that changelog file contents change. Changing
the mode on a changelog file is no longer sufficient.
* The `InvalidPaths` check now supports enforcing Windows filename rules.
* The `ValidName` check now performs DNS queries using `trust-dns` instead of
calling out to the system `host` binary. A global DNS resolver using the
system configuration is used.
# v3.9.1
......
......@@ -20,6 +20,7 @@ lazy_static = "^1.1"
log = "~0.4"
rayon = "^1.1"
thiserror = "^1.0.2"
trust-dns-resolver = { version = "~0.12", default-features = false, features = ["tokio"] }
ttl_cache = "~0.3"
wait-timeout = "~0.2"
......
......@@ -28,6 +28,7 @@ mod crates {
pub extern crate rayon;
pub extern crate regex;
pub extern crate thiserror;
pub extern crate trust_dns_resolver;
pub extern crate ttl_cache;
pub extern crate wait_timeout;
......
......@@ -8,11 +8,13 @@
use std::collections::hash_set::HashSet;
use std::fmt::{self, Debug};
use std::process::Command;
use std::io;
use std::sync::Mutex;
use std::time::Duration;
use std::time::{Duration, Instant};
use crates::git_checks_core::impl_prelude::*;
use crates::trust_dns_resolver::error::ResolveErrorKind;
use crates::trust_dns_resolver::Resolver;
use crates::ttl_cache::TtlCache;
#[derive(Debug, Clone, Copy)]
......@@ -57,6 +59,18 @@ lazy_static! {
static ref DEFAULT_TTL_CACHE_HIT_DURATION: Duration = Duration::from_secs(24 * 60 * 60);
// 5 minutes
static ref DEFAULT_TTL_CACHE_MISS_DURATION: Duration = Duration::from_secs(5 * 60);
// DNS resolver.
static ref DNS_RESOLVER: io::Result<Resolver> = Resolver::from_system_conf()
.map_err(|err| {
error!(
target: "git-checks/valid_name",
"failed to construct DNS resolver: {}",
err,
);
err
});
}
/// A check which checks for valid identities.
......@@ -103,6 +117,39 @@ impl Clone for ValidName {
}
}
#[derive(Debug, Clone, Copy)]
enum HostLookup {
Hit,
Miss { valid_until: Option<Instant> },
}
impl HostLookup {
fn is_hit(self) -> bool {
if let HostLookup::Hit = self {
true
} else {
false
}
}
fn cache_duration(self) -> Duration {
match self {
HostLookup::Hit => *DEFAULT_TTL_CACHE_HIT_DURATION,
HostLookup::Miss {
valid_until,
} => {
valid_until
.and_then(|inst| {
let now = Instant::now();
// XXX(1.39.0): Stabilized, but requires a bump in our minimum version.
inst.checked_duration_since(now)
})
.unwrap_or_else(|| *DEFAULT_TTL_CACHE_MISS_DURATION)
},
}
}
}
impl ValidName {
/// Create a new check to check identities.
pub fn new() -> Self {
......@@ -131,42 +178,37 @@ impl ValidName {
name.find(' ').is_some()
}
fn check_host(domain: &str) -> Option<bool> {
let dig = Command::new("host")
.args(&["-t", "MX"])
.arg(format!("{}.", domain)) // Search for the absolute domain.
.output();
let dig_output = match dig {
Ok(dig_output) => dig_output,
Err(err) => {
error!(
target: "git-checks/valid_name",
"failed to construct host command: {:?}",
err,
);
return None;
},
};
if dig_output.status.success() {
Some(true)
} else {
// The `host` tool always outputs to stdout
let output = String::from_utf8_lossy(&dig_output.stdout);
fn check_host(domain: &str) -> Option<HostLookup> {
let resolver = DNS_RESOLVER.as_ref().ok()?;
warn!(
target: "git-checks/valid_name",
"failed to look up MX record for domain {}: {}",
domain,
output,
);
// Search for the absolute domain.
let abs_domain = format!("{}.", domain);
let lookup = resolver.mx_lookup(&abs_domain);
if output.contains("connection timed out") {
None
} else {
Some(false)
}
match lookup {
Ok(_) => Some(HostLookup::Hit),
Err(err) => {
match err.kind() {
ResolveErrorKind::NoRecordsFound {
valid_until, ..
} => {
Some(HostLookup::Miss {
valid_until: valid_until.clone(),
})
},
ResolveErrorKind::Timeout => None,
_ => {
warn!(
target: "git-checks/valid_name",
"failed to look up MX record for domain {}: {:?}",
domain,
err,
);
None
},
}
},
}
}
......@@ -184,15 +226,10 @@ impl ValidName {
return *cached_res;
}
Self::check_host(domain).map_or(false, |res| {
let duration = if res {
*DEFAULT_TTL_CACHE_HIT_DURATION
} else {
*DEFAULT_TTL_CACHE_MISS_DURATION
};
cache.insert(domain.into(), res, duration);
res
Self::check_host(domain).map_or(false, |lookup| {
let hit = lookup.is_hit();
cache.insert(domain.into(), hit, lookup.cache_duration());
hit
})
} else {
false
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment