[fix] everything: various bugs found in production

.cargo/config.toml

@@ -0,0 +1,2 @@
+[build]
+rustflags = ["-Clink-arg=-fuse-ld=mold", "-Zthreads=16"]

Cargo.lock

@@ -132,6 +132,17 @@ dependencies = [
  "http",
 ]
 
+[[package]]
+name = "axum-client-ip"
+version = "0.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "72188bed20deb981f3a4a9fe674e5980fd9e9c2bd880baa94715ad5d60d64c67"
+dependencies = [
+ "axum",
+ "forwarded-header-value",
+ "serde",
+]
+
 [[package]]
 name = "axum-core"
 version = "0.4.3"
@@ -153,29 +164,6 @@ dependencies = [
  "tracing",
 ]
 
-[[package]]
-name = "axum-extra"
-version = "0.9.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0be6ea09c9b96cb5076af0de2e383bd2bc0c18f827cf1967bdd353e0b910d733"
-dependencies = [
- "axum",
- "axum-core",
- "bytes",
- "futures-util",
- "headers",
- "http",
- "http-body",
- "http-body-util",
- "mime",
- "pin-project-lite",
- "serde",
- "tower",
- "tower-layer",
- "tower-service",
- "tracing",
-]
-
 [[package]]
 name = "backtrace"
 version = "0.3.71"
@@ -218,15 +206,6 @@ version = "2.5.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "cf4b9d6a944f767f8e5e0db018570623c85f3d925ac718db4e06d0187adb21c1"
 
-[[package]]
-name = "block-buffer"
-version = "0.10.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71"
-dependencies = [
- "generic-array",
-]
-
 [[package]]
 name = "bytes"
 version = "1.6.0"
@@ -303,35 +282,6 @@ dependencies = [
  "windows-sys 0.52.0",
 ]
 
-[[package]]
-name = "cpufeatures"
-version = "0.2.12"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "53fe5e26ff1b7aef8bca9c6080520cfb8d9333c7568e1829cef191a9723e5504"
-dependencies = [
- "libc",
-]
-
-[[package]]
-name = "crypto-common"
-version = "0.1.6"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3"
-dependencies = [
- "generic-array",
- "typenum",
-]
-
-[[package]]
-name = "digest"
-version = "0.10.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292"
-dependencies = [
- "block-buffer",
- "crypto-common",
-]
-
 [[package]]
 name = "encode_unicode"
 version = "0.3.6"
@@ -363,6 +313,16 @@ dependencies = [
  "percent-encoding",
 ]
 
+[[package]]
+name = "forwarded-header-value"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8835f84f38484cc86f110a805655697908257fb9a7af005234060891557198e9"
+dependencies = [
+ "nonempty",
+ "thiserror",
+]
+
 [[package]]
 name = "futures-channel"
 version = "0.3.30"
@@ -396,16 +356,6 @@ dependencies = [
  "pin-utils",
 ]
 
-[[package]]
-name = "generic-array"
-version = "0.14.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a"
-dependencies = [
- "typenum",
- "version_check",
-]
-
 [[package]]
 name = "getrandom"
 version = "0.2.14"
@@ -423,30 +373,6 @@ version = "0.28.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4271d37baee1b8c7e4b708028c57d816cf9d2434acb33a549475f78c181f6253"
 
-[[package]]
-name = "headers"
-version = "0.4.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "322106e6bd0cba2d5ead589ddb8150a13d7c4217cf80d7c4f682ca994ccc6aa9"
-dependencies = [
- "base64 0.21.7",
- "bytes",
- "headers-core",
- "http",
- "httpdate",
- "mime",
- "sha1",
-]
-
-[[package]]
-name = "headers-core"
-version = "0.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "54b4a22553d4242c49fddb9ba998a99962b5cc6f22cb5a3482bec22522403ce4"
-dependencies = [
- "http",
-]
-
 [[package]]
 name = "heck"
 version = "0.5.0"
@@ -672,6 +598,12 @@ dependencies = [
  "windows-sys 0.48.0",
 ]
 
+[[package]]
+name = "nonempty"
+version = "0.7.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e9e591e719385e6ebaeb5ce5d3887f7d5676fceca6411d1925ccc95745f3d6f7"
+
 [[package]]
 name = "nu-ansi-term"
 version = "0.46.0"
@@ -908,17 +840,6 @@ dependencies = [
  "serde",
 ]
 
-[[package]]
-name = "sha1"
-version = "0.10.6"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba"
-dependencies = [
- "cfg-if",
- "cpufeatures",
- "digest",
-]
-
 [[package]]
 name = "sharded-slab"
 version = "0.1.7"
@@ -1190,12 +1111,6 @@ dependencies = [
  "tracing-log",
 ]
 
-[[package]]
-name = "typenum"
-version = "1.17.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "42ff0bf0c66b8238c6f3b578df37d0b7848e55df8577b3f74f92a69acceeb825"
-
 [[package]]
 name = "unicode-ident"
 version = "1.0.12"
@@ -1232,12 +1147,6 @@ version = "0.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "830b7e5d4d90034032940e4ace0d9a9a057e7a45cd94e6c007832e39edb82f6d"
 
-[[package]]
-name = "version_check"
-version = "0.9.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
-
 [[package]]
 name = "wasi"
 version = "0.11.0+wasi-snapshot-preview1"
@@ -1246,14 +1155,13 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423"
 
 [[package]]
 name = "webnsupdate"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
  "axum",
  "axum-auth",
- "axum-extra",
+ "axum-client-ip",
  "base64 0.22.1",
  "clap",
- "headers",
  "http",
  "insta",
  "miette",

Cargo.toml

@@ -1,27 +1,31 @@
+cargo-features = ["codegen-backend"]
 [package]
 description = "An HTTP server using HTTP basic auth to make secure calls to nsupdate"
 name = "webnsupdate"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2021"
 
 [dependencies]
 axum = "0.7.5"
-axum-auth = { version = "0.7.0", default-features = false, features = [
+axum-client-ip = "0.6.0"
-  "auth-basic",
-] }
-axum-extra = { version = "0.9.3", features = ["typed-header"] }
 base64 = "0.22.1"
 clap = { version = "4.5.4", features = ["derive", "env"] }
-headers = "0.4.0"
 http = "1.1.0"
 insta = "1.38.0"
 miette = { version = "7.2.0", features = ["fancy"] }
 ring = { version = "0.17.8", features = ["std"] }
-tokio = { version = "1.37.0", features = [
-  "macros",
-  "rt",
-  "process",
-  "io-util",
-] }
 tracing = "0.1.40"
 tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
+
+[dependencies.axum-auth]
+version = "0.7.0"
+default-features = false
+features = ["auth-basic"]
+
+[dependencies.tokio]
+version = "1.37.0"
+features = ["macros", "rt", "process", "io-util"]
+
+[profile.dev]
+debug = 0
+codegen-backend = "cranelift"

flake.lock

@@ -2,11 +2,11 @@
   "nodes": {
     "nixpkgs": {
       "locked": {
-        "lastModified": 1714906307,
+        "lastModified": 1715534503,
-        "narHash": "sha256-UlRZtrCnhPFSJlDQE7M0eyhgvuuHBTe1eJ9N9AQlJQ0=",
+        "narHash": "sha256-5ZSVkFadZbFP1THataCaSf0JH2cAH3S29hU9rrxTEqk=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "25865a40d14b3f9cf19f19b924e2ab4069b09588",
+        "rev": "2057814051972fa1453ddfb0d98badbea9b83c06",
         "type": "github"
       },
       "original": {
@@ -17,7 +17,23 @@
     },
     "root": {
       "inputs": {
-        "nixpkgs": "nixpkgs"
+        "nixpkgs": "nixpkgs",
+        "systems": "systems"
+      }
+    },
+    "systems": {
+      "locked": {
+        "lastModified": 1681028828,
+        "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=",
+        "owner": "nix-systems",
+        "repo": "default",
+        "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e",
+        "type": "github"
+      },
+      "original": {
+        "owner": "nix-systems",
+        "repo": "default",
+        "type": "github"
       }
     }
   },

flake.nix

@@ -1,31 +1,21 @@
 {
   description = "An http server that calls nsupdate internally";
-
+  inputs = {
-  inputs.nixpkgs.url = "nixpkgs/nixos-unstable";
+    nixpkgs.url = "nixpkgs/nixos-unstable";
+    systems.url = "github:nix-systems/default";
+  };
 
   outputs = {
     self,
     nixpkgs,
+    systems,
   }: let
-    supportedSystems = ["x86_64-linux" "aarch64-darwin" "x86_64-darwin" "aarch64-linux"];
+    forEachSupportedSystem = nixpkgs.lib.genAttrs (import systems);
-    forEachSupportedSystem = f:
-      nixpkgs.lib.genAttrs supportedSystems (system:
-        f {
-          inherit system;
-          pkgs = import nixpkgs {inherit system;};
-        });
   in {
-    formatter = forEachSupportedSystem ({pkgs, ...}: pkgs.alejandra);
+    formatter = forEachSupportedSystem (system: nixpkgs.legacyPackages.${system}.alejandra);
 
-    # checks = forEachSupportedSystem ({pkgs, ...}: {
+    packages = forEachSupportedSystem (system: {
-    #   module = pkgs.testers.runNixOSTest {
+      default = nixpkgs.legacyPackages.${system}.callPackage ./default.nix {};
-    #     name = "webnsupdate module test";
-    #     nodes.testMachine = {imports = [self.nixosModules.default];};
-    #   };
-    # });
-
-    packages = forEachSupportedSystem ({pkgs, ...}: {
-      default = pkgs.callPackage ./default.nix {};
     });
 
     overlays.default = final: prev: {
@@ -34,7 +24,9 @@
 
     nixosModules.default = ./module.nix;
 
-    devShells = forEachSupportedSystem ({pkgs, ...}: {
+    devShells = forEachSupportedSystem (system: let
+      pkgs = nixpkgs.legacyPackages.${system};
+    in {
       default = pkgs.mkShell {
         packages = [pkgs.cargo-insta];
       };

module.nix

@@ -13,6 +13,11 @@ in {
     type = types.submodule {
       options = {
         enable = mkEnableOption "webnsupdate";
+        extraArgs = mkOption {
+          description = ''
+            Extra arguments to be passed to the webnsupdate server command.
+          '';
+        };
         bindIp = mkOption {
           description = ''
             IP address to bind to.
@@ -130,6 +135,7 @@ in {
         wantedBy = ["multi-user.target"];
         after = ["network.target" "bind.service"];
         preStart = "${cmd} verify";
+        path = [pkgs.dig];
         startLimitIntervalSec = 60;
         serviceConfig = {
           ExecStart = [cmd];

src/main.rs

@@ -8,19 +8,16 @@ use std::{
     time::Duration,
 };
 
-use axum::{
+use axum::{extract::State, routing::get, Json, Router};
-    extract::{ConnectInfo, State},
-    routing::get,
-    Json, Router,
-};
 use axum_auth::AuthBasic;
+use axum_client_ip::{SecureClientIp, SecureClientIpSource};
 use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
 use clap::{Args, Parser, Subcommand};
 use http::StatusCode;
 use miette::{ensure, miette, Context, IntoDiagnostic, LabeledSpan, NamedSource, Result};
 use ring::digest::Digest;
 use tokio::io::AsyncWriteExt;
-use tracing::{info, level_filters::LevelFilter, warn};
+use tracing::{debug, error, info, level_filters::LevelFilter, trace, warn};
 use tracing_subscriber::EnvFilter;
 
 const DEFAULT_TTL: Duration = Duration::from_secs(60);
@@ -60,9 +57,14 @@ struct Opts {
     /// If specified, then `webnsupdate` must have read access to the file
     #[arg(long)]
     key_file: Option<PathBuf>,
-    /// Allow not setting a password when the server is exposed to the network
+    /// Allow not setting a password
     #[arg(long)]
     insecure: bool,
+    /// Set client IP source
+    ///
+    /// see: https://docs.rs/axum-client-ip/latest/axum_client_ip/enum.SecureClientIpSource.html
+    #[clap(long, default_value = "RightmostXForwardedFor")]
+    ip_source: SecureClientIpSource,
     #[clap(subcommand)]
     subcommand: Option<Cmd>,
 }
@@ -112,6 +114,7 @@ async fn main() -> Result<()> {
         records,
         salt,
         ttl,
+        ip_source,
     } = Opts::parse();
     let subscriber = tracing_subscriber::FmtSubscriber::builder()
         .without_time()
@@ -144,9 +147,14 @@ async fn main() -> Result<()> {
         key_file: None,
         password_hash: None,
     };
-    if let Some(password_file) = password_file {
+    if let Some(path) = password_file {
-        let pass = std::fs::read_to_string(password_file).into_diagnostic()?;
+        let pass = std::fs::read_to_string(&path).into_diagnostic()?;
-        let pass: Box<[u8]> = pass.trim().as_bytes().into();
+
+        let pass: Box<[u8]> = URL_SAFE_NO_PAD
+            .decode(pass.trim().as_bytes())
+            .into_diagnostic()
+            .wrap_err_with(|| format!("failed to decode password from {}", path.display()))?
+            .into();
         state.password_hash = Some(Box::leak(pass));
     } else {
         ensure!(insecure, "a password must be used");
@@ -174,6 +182,7 @@ async fn main() -> Result<()> {
     // Start services
     let app = Router::new()
         .route("/update", get(update_records))
+        .layer(ip_source.into_extension())
         .with_state(state);
     info!("starting listener on {ip}:{port}");
     let listener = tokio::net::TcpListener::bind(SocketAddr::new(ip, port))
@@ -188,29 +197,35 @@ async fn main() -> Result<()> {
     .into_diagnostic()
 }
 
-#[tracing::instrument(skip(state), level = "trace", ret(level = "warn"))]
+#[tracing::instrument(skip(state, pass), level = "trace", ret(level = "info"))]
 async fn update_records(
     State(state): State<AppState<'static>>,
     AuthBasic((username, pass)): AuthBasic,
-    ConnectInfo(client): ConnectInfo<SocketAddr>,
+    SecureClientIp(ip): SecureClientIp,
 ) -> axum::response::Result<&'static str> {
     let Some(pass) = pass else {
         return Err((StatusCode::UNAUTHORIZED, Json::from("no password provided")).into());
     };
     if let Some(stored_pass) = state.password_hash {
         let password = pass.trim().to_string();
-
+        let pass_hash = hash_identity(&username, &password, state.salt);
-        if hash_identity(&username, &password, state.salt).as_ref() != stored_pass {
+        if pass_hash.as_ref() != stored_pass {
-            warn!("rejected update from {username}@{client}");
+            warn!("rejected update");
+            trace!(
+                "mismatched hashes:\n{}\n{}",
+                URL_SAFE_NO_PAD.encode(pass_hash.as_ref()),
+                URL_SAFE_NO_PAD.encode(stored_pass.as_ref()),
+            );
             return Err((StatusCode::UNAUTHORIZED, "invalid identity").into());
         }
     }
-    let ip = client.ip();
+    info!("accepted update");
     match nsupdate(ip, state.ttl, state.key_file, state.records).await {
         Ok(status) => {
             if status.success() {
                 Ok("successful update")
             } else {
+                error!("nsupdate failed");
                 Err((
                     StatusCode::INTERNAL_SERVER_ERROR,
                     "nsupdate failed, check server logs",
@@ -226,6 +241,7 @@ async fn update_records(
     }
 }
 
+#[tracing::instrument(level = "trace", ret(level = "warn"))]
 async fn nsupdate(
     ip: IpAddr,
     ttl: Duration,
@@ -236,13 +252,27 @@ async fn nsupdate(
     if let Some(key_file) = key_file {
         cmd.args([OsStr::new("-k"), key_file.as_os_str()]);
     }
-    cmd.stdin(Stdio::piped());
+    debug!("spawning new process");
-    let mut child = cmd.spawn()?;
+    let mut child = cmd
+        .stdin(Stdio::piped())
+        .spawn()
+        .inspect_err(|err| warn!("failed to spawn child: {err}"))?;
     let mut stdin = child.stdin.take().expect("stdin not present");
+    debug!("sending update request");
     stdin
         .write_all(update_ns_records(ip, ttl, records).as_bytes())
-        .await?;
+        .await
-    child.wait().await
+        .inspect_err(|err| warn!("failed to write to the stdin of nsupdate: {err}"))?;
+    debug!("closing stdin");
+    stdin
+        .shutdown()
+        .await
+        .inspect_err(|err| warn!("failed to close stdin to nsupdate: {err}"))?;
+    debug!("waiting for nsupdate to exit");
+    child
+        .wait()
+        .await
+        .inspect_err(|err| warn!("failed to wait for child: {err}"))
 }
 
 fn update_ns_records(ip: IpAddr, ttl: Duration, records: &[&str]) -> String {
@@ -258,7 +288,7 @@ fn update_ns_records(ip: IpAddr, ttl: Duration, records: &[&str]) -> String {
         writeln!(cmds, "update delete {record} {ttl_s} IN {rec_type}").unwrap();
         writeln!(cmds, "update add    {record} {ttl_s} IN {rec_type} {ip}").unwrap();
     }
-    writeln!(cmds, "send").unwrap();
+    writeln!(cmds, "send\nquit").unwrap();
     cmds
 }
 
@@ -423,6 +453,7 @@ mod test {
         update delete example.net. 60 IN A
         update add    example.net. 60 IN A 127.0.0.1
         send
+        quit
         "###);
     }
 
@@ -442,6 +473,7 @@ mod test {
         update delete example.net. 60 IN AAAA
         update add    example.net. 60 IN AAAA ::1
         send
+        quit
         "###);
     }