Indicate that this is an ip list, not a range (#3626)

* Indicate that this is an ip list, not a range

We mistakenly commented that this was a range, not a list. This
has led to some confusion. Be clear it's a list of ip's, not a range.

* Support Ip Ranges instead of Ip Addresses in X-Forward-For

* Docs feedback

Cargo.lock

@@ -695,6 +695,15 @@ dependencies = [
  "windows-link",
 ]
 
+[[package]]
+name = "cidr"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bd1b64030216239a2e7c364b13cd96a2097ebf0dfe5025f2dedee14a23f2ab60"
+dependencies = [
+ "serde",
+]
+
 [[package]]
 name = "clang-sys"
 version = "1.8.1"
@@ -3274,6 +3283,7 @@ dependencies = [
  "axum-macros",
  "bytes",
  "chrono",
+ "cidr",
  "compact_jwt",
  "cron",
  "filetime",
@@ -3383,6 +3393,7 @@ dependencies = [
 name = "kanidmd_testkit"
 version = "1.7.0-dev"
 dependencies = [
+ "cidr",
  "compact_jwt",
  "escargot",
  "fantoccini",

Cargo.toml

@@ -166,6 +166,7 @@ base64 = "^0.22.1"
 base64urlsafedata = "0.5.1"
 bitflags = "^2.8.0"
 bytes = "^1.9.0"
+cidr = "0.3.1"
 clap = { version = "4.5.38", features = ["derive", "env"] }
 clap_complete = "^4.5.50"
 # Forced by saffron/cron

Makefile

@@ -368,3 +368,11 @@ publish:
 	cargo publish -p kanidm_client
 	cargo publish -p kanidm_tools
 
+.PHONY: rust_container
+rust_container: # Build and run a container based on the Linux rust base container, with our requirements included
+rust_container:
+	docker build --pull -t kanidm_rust -f scripts/Dockerfile.devcontainer .
+	docker run \
+		--rm -it \
+		--name kanidm \
+		--mount type=bind,source=$(PWD),target=/kanidm -w /kanidm kanidm_rust:latest

examples/server.toml

@@ -82,25 +82,25 @@ origin = "https://idm.example.com:8443"
 #   will often add a header such as "Forwarded" or
 #   "X-Forwarded-For". Some other proxies can use the PROXY
 #   protocol v2 header.
-#   This setting allows configuration of the range of trusted
-#   IPs which can supply this header information, and which
-#   format the information is provided in.
+#   This setting allows configuration of the list of trusted
+#   IPs or IP ranges which can supply this header information,
+#   and which format the information is provided in.
 #   Defaults to "none" (no trusted sources)
 #   Only one option can be used at a time.
 # [http_client_address_info]
-# proxy-v2 = ["127.0.0.1"]
+# proxy-v2 = ["127.0.0.1", "127.0.0.0/8"]
 #   # OR
-# x-forward-for = ["127.0.0.1"]
+# x-forward-for = ["127.0.0.1", "127.0.0.0/8"]
 
 #   LDAPS requests can be reverse proxied by a loadbalancer.
 #   To preserve the original IP of the caller, these systems
 #   can add a header such as the PROXY protocol v2 header.
-#   This setting allows configuration of the range of trusted
-#   IPs which can supply this header information, and which
-#   format the information is provided in.
+#   This setting allows configuration of the list of trusted
+#   IPs or IP ranges which can supply this header information,
+#   and which format the information is provided in.
 #   Defaults to "none" (no trusted sources)
 # [ldap_client_address_info]
-# proxy-v2 = ["127.0.0.1"]
+# proxy-v2 = ["127.0.0.1", "127.0.0.0/8"]
 
 [online_backup]
 #   The path to the output folder for online backups

examples/server_container.toml

@@ -81,25 +81,25 @@ origin = "https://idm.example.com:8443"
 #   will often add a header such as "Forwarded" or
 #   "X-Forwarded-For". Some other proxies can use the PROXY
 #   protocol v2 header.
-#   This setting allows configuration of the range of trusted
-#   IPs which can supply this header information, and which
-#   format the information is provided in.
+#   This setting allows configuration of the list of trusted
+#   IPs or IP ranges which can supply this header information,
+#   and which format the information is provided in.
 #   Defaults to "none" (no trusted sources)
 #   Only one option can be used at a time.
 # [http_client_address_info]
-# proxy-v2 = ["127.0.0.1"]
+# proxy-v2 = ["127.0.0.1", "127.0.0.0/8"]
 #   # OR
-# x-forward-for = ["127.0.0.1"]
+# x-forward-for = ["127.0.0.1", "127.0.0.0/8"]
 
 #   LDAPS requests can be reverse proxied by a loadbalancer.
 #   To preserve the original IP of the caller, these systems
 #   can add a header such as the PROXY protocol v2 header.
-#   This setting allows configuration of the range of trusted
-#   IPs which can supply this header information, and which
-#   format the information is provided in.
+#   This setting allows configuration of the list of trusted
+#   IPs or IP ranges which can supply this header information,
+#   and which format the information is provided in.
 #   Defaults to "none" (no trusted sources)
 # [ldap_client_address_info]
-# proxy-v2 = ["127.0.0.1"]
+# proxy-v2 = ["127.0.0.1", "127.0.0.0/8"]
 
 [online_backup]
 #   The path to the output folder for online backups

scripts/Dockerfile.devcontainer

@@ -0,0 +1,6 @@
+FROM rust:latest
+
+COPY ./scripts/install_ubuntu_dependencies.sh /tmp/
+RUN /tmp/install_ubuntu_dependencies.sh
+
+WORKDIR /kanidm

scripts/setup_dev_environment.sh

@@ -45,7 +45,7 @@ fi
 
 # defaults
 KANIDM_CONFIG_FILE="./insecure_server.toml"
-KANIDM_URL="$(rg origin "${KANIDM_CONFIG_FILE}" | awk '{print $NF}' | tr -d '"')"
+KANIDM_URL="$(grep origin "${KANIDM_CONFIG_FILE}" | awk '{print $NF}' | tr -d '"')"
 KANIDM_CA_PATH="/tmp/kanidm/ca.pem"
 
 # wait for them to shut down the server if it's running...
@@ -89,7 +89,7 @@ IDM_ADMIN_USER="idm_admin@localhost"
 echo "Resetting the idm_admin user..."
 IDM_ADMIN_PASS_RAW="$(${KANIDMD} recover-account idm_admin -o json 2>&1)"
 IDM_ADMIN_PASS="$(echo "${IDM_ADMIN_PASS_RAW}" | grep password | jq -r .password)"
-if [ -z "${IDM_ADMIN_PASS}" ] || [ "${IDM_ADMIN_PASS}" == "null " ]; then
+if [ -z "${IDM_ADMIN_PASS}" ] || [ "${IDM_ADMIN_PASS}" == "null" ]; then
     echo "Failed to reset idm_admin password!"
     echo "Raw output:"
     echo "${IDM_ADMIN_PASS_RAW}"

scripts/test_run_release_server.sh

@@ -40,7 +40,6 @@ cargo run --bin kanidmd --release server  &
 KANIDMD_PID=$!
 echo "Kanidm PID: ${KANIDMD_PID}"
 
-
 if [ "$(jobs -p | wc -l)" -eq 0 ]; then
     echo "Kanidmd failed to start!"
     exit 1
@@ -49,12 +48,18 @@ fi
 ATTEMPT=0
 
 KANIDM_CONFIG_FILE="./insecure_server.toml"
-KANIDM_URL="$(rg origin "${KANIDM_CONFIG_FILE}" | awk '{print $NF}' | tr -d '"')"
+if [ -f "${KANIDM_CONFIG_FILE}" ]; then
+    echo "Found config file ${KANIDM_CONFIG_FILE}"
+else
+    echo "Config file ${KANIDM_CONFIG_FILE} not found!"
+    exit 1
+fi
+KANIDM_URL="$(grep origin "${KANIDM_CONFIG_FILE}" | awk '{print $NF}' | tr -d '"')"
 KANIDM_CA_PATH="/tmp/kanidm/ca.pem"
 
 while true; do
-    echo "Waiting for the server to start... testing ${KANIDM_URL}"
-    curl --cacert "${KANIDM_CA_PATH}" -fs "${KANIDM_URL}/status" >/dev/null && break
+    echo "Waiting for the server to start... testing url '${KANIDM_URL}'"
+    curl --cacert "${KANIDM_CA_PATH}" -f "${KANIDM_URL}/status" >/dev/null && break
     sleep 2
     ATTEMPT="$((ATTEMPT + 1))"
     if [ "${ATTEMPT}" -gt 3 ]; then

server/core/Cargo.toml

@@ -27,6 +27,7 @@ axum-htmx = { workspace = true }
 axum-extra = { workspace = true }
 axum-macros = { workspace = true }
 bytes = { workspace = true }
+cidr = { workspace = true, features = ["serde"] }
 chrono = { workspace = true }
 compact_jwt = { workspace = true }
 cron = { workspace = true }

server/core/src/config.rs

@@ -4,7 +4,7 @@
 //! These components should be "per server". Any "per domain" config should be in the system
 //! or domain entries that are able to be replicated.
 
-use hashbrown::HashSet;
+use cidr::IpCidr;
 use kanidm_proto::constants::DEFAULT_SERVER_ADDRESS;
 use kanidm_proto::internal::FsType;
 use kanidm_proto::messages::ConsoleOutputMode;
@@ -110,11 +110,11 @@ pub enum LdapAddressInfo {
     #[default]
     None,
     #[serde(rename = "proxy-v2")]
-    ProxyV2(HashSet<IpAddr>),
+    ProxyV2(Vec<IpCidr>),
 }
 
 impl LdapAddressInfo {
-    pub fn trusted_proxy_v2(&self) -> Option<HashSet<IpAddr>> {
+    pub fn trusted_proxy_v2(&self) -> Option<Vec<IpCidr>> {
         if let Self::ProxyV2(trusted) = self {
             Some(trusted.clone())
         } else {
@@ -139,7 +139,7 @@ impl Display for LdapAddressInfo {
 }
 
 pub(crate) enum AddressSet {
-    NonContiguousIpSet(HashSet<IpAddr>),
+    NonContiguousIpSet(Vec<IpCidr>),
     All,
 }
 
@@ -147,7 +147,9 @@ impl AddressSet {
     pub(crate) fn contains(&self, ip_addr: &IpAddr) -> bool {
         match self {
             Self::All => true,
-            Self::NonContiguousIpSet(range) => range.contains(ip_addr),
+            Self::NonContiguousIpSet(range) => {
+                range.iter().any(|ip_cidr| ip_cidr.contains(ip_addr))
+            }
         }
     }
 }
@@ -157,13 +159,13 @@ pub enum HttpAddressInfo {
     #[default]
     None,
     #[serde(rename = "x-forward-for")]
-    XForwardFor(HashSet<IpAddr>),
+    XForwardFor(Vec<IpCidr>),
     // IMPORTANT: This is undocumented, and only exists for backwards compat
     // with config v1 which has a boolean toggle for this option.
     #[serde(rename = "x-forward-for-all-source-trusted")]
     XForwardForAllSourcesTrusted,
     #[serde(rename = "proxy-v2")]
-    ProxyV2(HashSet<IpAddr>),
+    ProxyV2(Vec<IpCidr>),
 }
 
 impl HttpAddressInfo {
@@ -175,7 +177,7 @@ impl HttpAddressInfo {
         }
     }
 
-    pub(crate) fn trusted_proxy_v2(&self) -> Option<HashSet<IpAddr>> {
+    pub(crate) fn trusted_proxy_v2(&self) -> Option<Vec<IpCidr>> {
         if let Self::ProxyV2(trusted) = self {
             Some(trusted.clone())
         } else {
@@ -1170,3 +1172,32 @@ impl ConfigurationBuilder {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use cidr::{IpCidr, Ipv4Cidr, Ipv6Cidr};
+    use std::net::{Ipv4Addr, Ipv6Addr};
+
+    #[test]
+    fn assert_cidr_parsing_behaviour() {
+        // Assert that we can parse individual hosts, and ranges
+        let parsed_ip_cidr: IpCidr = serde_json::from_str("\"127.0.0.1\"").unwrap();
+        let expect_ip_cidr = IpCidr::from(Ipv4Addr::new(127, 0, 0, 1));
+        assert_eq!(parsed_ip_cidr, expect_ip_cidr);
+
+        let parsed_ip_cidr: IpCidr = serde_json::from_str("\"127.0.0.0/8\"").unwrap();
+        let expect_ip_cidr = IpCidr::from(Ipv4Cidr::new(Ipv4Addr::new(127, 0, 0, 0), 8).unwrap());
+        assert_eq!(parsed_ip_cidr, expect_ip_cidr);
+
+        // Same for ipv6
+        let parsed_ip_cidr: IpCidr = serde_json::from_str("\"2001:0db8::1\"").unwrap();
+        let expect_ip_cidr = IpCidr::from(Ipv6Addr::new(0x2001, 0x0db8, 0, 0, 0, 0, 0, 0x0001));
+        assert_eq!(parsed_ip_cidr, expect_ip_cidr);
+
+        let parsed_ip_cidr: IpCidr = serde_json::from_str("\"2001:0db8::/64\"").unwrap();
+        let expect_ip_cidr = IpCidr::from(
+            Ipv6Cidr::new(Ipv6Addr::new(0x2001, 0x0db8, 0, 0, 0, 0, 0, 0), 64).unwrap(),
+        );
+        assert_eq!(parsed_ip_cidr, expect_ip_cidr);
+    }
+}

server/core/src/https/apidocs/mod.rs

@@ -211,7 +211,6 @@ impl Modify for SecurityAddon {
         schemas(
             attribute::Attribute,
 
-
             scim_v1::ScimSyncState,
             scim_v1::ScimSyncRequest,
             scim_v1::ScimSyncRetentionMode,

server/core/src/https/mod.rs

@@ -29,10 +29,10 @@ use axum::{
     Router,
 };
 use axum_extra::extract::cookie::CookieJar;
+use cidr::IpCidr;
 use compact_jwt::{error::JwtError, JwsCompact, JwsHs256Signer, JwsVerifier};
 use futures::pin_mut;
 use haproxy_protocol::{ProxyHdrV2, RemoteAddress};
-use hashbrown::HashSet;
 use hyper::body::Incoming;
 use hyper_util::rt::{TokioExecutor, TokioIo};
 use kanidm_lib_crypto::x509_cert::{der::Decode, x509_public_key_s256, Certificate};
@@ -43,7 +43,6 @@ use serde::de::DeserializeOwned;
 use sketching::*;
 use std::fmt::Write;
 use std::io::ErrorKind;
-use std::net::IpAddr;
 use std::path::PathBuf;
 use std::pin::Pin;
 use std::sync::Arc;
@@ -363,7 +362,7 @@ async fn server_tls_loop(
     mut rx: broadcast::Receiver<CoreAction>,
     server_message_tx: broadcast::Sender<CoreAction>,
     mut tls_acceptor_reload_rx: mpsc::Receiver<SslAcceptor>,
-    trusted_proxy_v2_ips: Option<Arc<HashSet<IpAddr>>>,
+    trusted_proxy_v2_ips: Option<Arc<Vec<IpCidr>>>,
 ) {
     pin_mut!(listener);
 
@@ -404,7 +403,7 @@ async fn server_plaintext_loop(
     listener: TcpListener,
     app: IntoMakeServiceWithConnectInfo<Router, ClientConnInfo>,
     mut rx: broadcast::Receiver<CoreAction>,
-    trusted_proxy_v2_ips: Option<Arc<HashSet<IpAddr>>>,
+    trusted_proxy_v2_ips: Option<Arc<Vec<IpCidr>>>,
 ) {
     pin_mut!(listener);
 
@@ -438,7 +437,7 @@ pub(crate) async fn handle_conn(
     stream: TcpStream,
     app: IntoMakeServiceWithConnectInfo<Router, ClientConnInfo>,
     connection_addr: SocketAddr,
-    trusted_proxy_v2_ips: Option<Arc<HashSet<IpAddr>>>,
+    trusted_proxy_v2_ips: Option<Arc<Vec<IpCidr>>>,
 ) -> Result<(), std::io::Error> {
     let (stream, client_addr) =
         process_client_addr(stream, connection_addr, trusted_proxy_v2_ips).await?;
@@ -462,7 +461,7 @@ pub(crate) async fn handle_tls_conn(
     stream: TcpStream,
     app: IntoMakeServiceWithConnectInfo<Router, ClientConnInfo>,
     connection_addr: SocketAddr,
-    trusted_proxy_v2_ips: Option<Arc<HashSet<IpAddr>>>,
+    trusted_proxy_v2_ips: Option<Arc<Vec<IpCidr>>>,
 ) -> Result<(), std::io::Error> {
     let (stream, client_addr) =
         process_client_addr(stream, connection_addr, trusted_proxy_v2_ips).await?;
@@ -531,10 +530,14 @@ pub(crate) async fn handle_tls_conn(
 async fn process_client_addr(
     stream: TcpStream,
     connection_addr: SocketAddr,
-    trusted_proxy_v2_ips: Option<Arc<HashSet<IpAddr>>>,
+    trusted_proxy_v2_ips: Option<Arc<Vec<IpCidr>>>,
 ) -> Result<(TcpStream, SocketAddr), std::io::Error> {
     let enable_proxy_v2_hdr = trusted_proxy_v2_ips
-        .map(|trusted| trusted.contains(&connection_addr.ip()))
+        .map(|trusted| {
+            trusted
+                .iter()
+                .any(|ip_cidr| ip_cidr.contains(&connection_addr.ip()))
+        })
         .unwrap_or_default();
 
     let (stream, client_addr) = if enable_proxy_v2_hdr {

server/core/src/https/views/reset.rs

@@ -119,9 +119,10 @@ struct SetUnixCredPartial {
 #[derive(Template)]
 #[template(path = "credential_update_add_ssh_publickey_partial.html")]
 struct AddSshPublicKeyPartial {
+    key_title: Option<String>,
     title_error: Option<String>,
-    key_error: Option<String>,
     key_value: Option<String>,
+    key_error: Option<String>,
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -901,9 +902,10 @@ pub(crate) async fn view_add_ssh_publickey(
     let new_key = match opt_form {
         None => {
             return Ok((AddSshPublicKeyPartial {
+                key_title: None,
                 title_error: None,
-                key_error: None,
                 key_value: None,
+                key_error: None,
             },)
                 .into_response());
         }
@@ -920,9 +922,10 @@ pub(crate) async fn view_add_ssh_publickey(
         let publickey = match SshPublicKey::from_string(&new_key.key) {
             Err(_) => {
                 return Ok((AddSshPublicKeyPartial {
+                    key_title: Some(new_key.title),
                     title_error: None,
-                    key_error: Some("Key cannot be parsed".to_string()),
                     key_value: Some(new_key.key),
+                    key_error: Some("Key cannot be parsed".to_string()),
                 },)
                     .into_response());
             }
@@ -932,7 +935,7 @@ pub(crate) async fn view_add_ssh_publickey(
             .qe_r_ref
             .handle_idmcredentialupdate(
                 cu_session_token,
-                CURequest::SshPublicKey(new_key.title, publickey),
+                CURequest::SshPublicKey(new_key.title.clone(), publickey),
                 kopid.eventid,
             )
             .await;
@@ -966,6 +969,7 @@ pub(crate) async fn view_add_ssh_publickey(
         status,
         HxPushUrl(Uri::from_static("/ui/reset/add_ssh_publickey")),
         AddSshPublicKeyPartial {
+            key_title: Some(new_key.title),
             title_error,
             key_error,
             key_value: Some(new_key.key),

server/core/src/ldaps.rs

@@ -1,15 +1,15 @@
 use crate::actors::QueryServerReadV1;
 use crate::CoreAction;
+use cidr::IpCidr;
 use futures_util::sink::SinkExt;
 use futures_util::stream::StreamExt;
 use haproxy_protocol::{ProxyHdrV2, RemoteAddress};
-use hashbrown::HashSet;
 use kanidmd_lib::idm::ldap::{LdapBoundToken, LdapResponseState};
 use kanidmd_lib::prelude::*;
 use ldap3_proto::proto::LdapMsg;
 use ldap3_proto::LdapCodec;
 use openssl::ssl::{Ssl, SslAcceptor};
-use std::net::{IpAddr, SocketAddr};
+use std::net::SocketAddr;
 use std::pin::Pin;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -122,10 +122,14 @@ async fn client_tls_accept(
     tls_acceptor: SslAcceptor,
     connection_addr: SocketAddr,
     qe_r_ref: &'static QueryServerReadV1,
-    trusted_proxy_v2_ips: Option<Arc<HashSet<IpAddr>>>,
+    trusted_proxy_v2_ips: Option<Arc<Vec<IpCidr>>>,
 ) {
     let enable_proxy_v2_hdr = trusted_proxy_v2_ips
-        .map(|trusted| trusted.contains(&connection_addr.ip()))
+        .map(|trusted| {
+            trusted
+                .iter()
+                .any(|ip_cidr| ip_cidr.contains(&connection_addr.ip()))
+        })
         .unwrap_or_default();
 
     let (stream, client_addr) = if enable_proxy_v2_hdr {
@@ -186,7 +190,7 @@ async fn ldap_tls_acceptor(
     qe_r_ref: &'static QueryServerReadV1,
     mut rx: broadcast::Receiver<CoreAction>,
     mut tls_acceptor_reload_rx: mpsc::Receiver<SslAcceptor>,
-    trusted_proxy_v2_ips: Option<Arc<HashSet<IpAddr>>>,
+    trusted_proxy_v2_ips: Option<Arc<Vec<IpCidr>>>,
 ) {
     loop {
         tokio::select! {
@@ -249,7 +253,7 @@ pub(crate) async fn create_ldap_server(
     qe_r_ref: &'static QueryServerReadV1,
     rx: broadcast::Receiver<CoreAction>,
     tls_acceptor_reload_rx: mpsc::Receiver<SslAcceptor>,
-    trusted_proxy_v2_ips: Option<HashSet<IpAddr>>,
+    trusted_proxy_v2_ips: Option<Vec<IpCidr>>,
 ) -> Result<tokio::task::JoinHandle<()>, ()> {
     if address.starts_with(":::") {
         // takes :::xxxx to xxxx

server/core/src/repl/mod.rs

@@ -92,7 +92,7 @@ pub(crate) async fn create_repl_server(
     Ok((repl_handle, ctrl_tx))
 }
 
-#[instrument(level = "info", skip_all)]
+#[instrument(level = "debug", skip_all)]
 /// This returns the remote address that worked, so you can try that first next time
 async fn repl_consumer_connect_supplier(
     domain: &str,
@@ -116,7 +116,10 @@ async fn repl_consumer_connect_supplier(
         )
         .await
         {
-            Ok(Ok(tc)) => tc,
+            Ok(Ok(tc)) => {
+                trace!("Connection established to peer on {:?}", sock_addr);
+                tc
+            }
             Ok(Err(err)) => {
                 debug!(?err, "Failed to connect to {}", sock_addr);
                 continue;
@@ -127,8 +130,6 @@ async fn repl_consumer_connect_supplier(
             }
         };
 
-        trace!("Connection established to peer on {:?}", sock_addr);
-
         let mut tlsstream = match Ssl::new(tls_connector.context())
             .and_then(|tls_obj| SslStream::new(tls_obj, tcpstream))
         {
@@ -236,7 +237,7 @@ async fn repl_run_consumer_refresh(
     Ok(Some(addr))
 }
 
-#[instrument(level="info", skip(tls_connector, idms), fields(eventid=Uuid::new_v4().to_string()))]
+#[instrument(level="debug", skip(tls_connector, idms), fields(eventid=Uuid::new_v4().to_string()))]
 async fn repl_run_consumer(
     domain: &str,
     sock_addrs: &[SocketAddr],
@@ -282,11 +283,11 @@ async fn repl_run_consumer(
                 changes
             }
             Ok(SupplierResponse::Pong) | Ok(SupplierResponse::Refresh(_)) => {
-                error!("Supplier Response contains invalid State");
+                error!("Supplier Response contains invalid state");
                 return None;
             }
             Err(err) => {
-                error!(?err, "consumer decode error, unable to continue.");
+                error!(?err, "Consumer decode error, unable to continue.");
                 return None;
             }
         }
@@ -306,7 +307,7 @@ async fn repl_run_consumer(
         }) {
             Ok(state) => state,
             Err(err) => {
-                error!(?err, "consumer was not able to apply changes.");
+                error!(?err, "Consumer was not able to apply changes.");
                 return None;
             }
         }
@@ -365,7 +366,7 @@ async fn repl_run_consumer(
         return None;
     }
 
-    warn!("Replication refresh was successful.");
+    info!("Replication refresh was successful.");
     Some(socket_addr)
 }
 
@@ -544,7 +545,7 @@ async fn repl_task(
     info!("Replica task for {} has stopped.", origin);
 }
 
-#[instrument(level = "info", skip_all)]
+#[instrument(level = "debug", skip_all)]
 async fn handle_repl_conn(
     max_frame_bytes: usize,
     tcpstream: TcpStream,
@@ -626,6 +627,7 @@ async fn handle_repl_conn(
     debug!(?client_address, "replication client disconnected 🛬");
 }
 
+/// This is the main acceptor for the replication server.
 async fn repl_acceptor(
     listener: TcpListener,
     idms: Arc<IdmServer>,

server/core/templates/credential_update_add_ssh_publickey_partial.html

@@ -6,26 +6,38 @@
         hx-post="/ui/reset/add_ssh_publickey">
         <div>
             <label for="key-title" class="form-label">Title</label>
-            <input type="text" class="form-control(% if let Some(_) = title_error %) is-invalid(% endif %)" id="key-title" name="title" aria-describedby="title-validation-feedback">
+            <input type="text"
+                class="form-control(% if let Some(_) = title_error %) is-invalid(% endif %)"
+                id="key-title" name="title"
+                aria-describedby="title-validation-feedback"
+                autocapitalize="off" autocomplete="off" required
+                value="(% if let Some(key_title) = key_title %)(( key_title ))(% endif %)">
+            (% if let Some(title_error) = title_error %)
             <div id="title-validation-feedback" class="invalid-feedback">
-                (% if let Some(title_error) = title_error %)(( title_error ))(% endif %)
+                (( title_error ))
             </div>
+            (% endif %)
         </div>
         <div>
             <label for="key-content" class="form-label">Key</label>
-            <textarea class="form-control(% if let Some(_) = key_error %) is-invalid(% endif %)" id="key-content" rows="5" name="key"
+            <textarea
+                class="form-control(% if let Some(_) = key_error %) is-invalid(% endif %)"
+                id="key-content" rows="5" name="key"
                 aria-describedby="key-validation-feedback"
-                placeholder="Begins with 'ssh-rsa', 'ecdsa-sha2-nistp256', 'ecdsa-sha2-nistp384', 'ecdsa-sha2-nistp521', 'ssh-ed25519', 'sk-ecdsa-sha2-nistp256@openssh.com', or 'sk-ssh-ed25519@openssh.com'"
-            >(% if let Some(key_value) = key_value %)(( key_value ))(% endif %)</textarea>
+                autocapitalize="off" autocomplete="off" required
+                placeholder="Begins with 'ssh-rsa', 'ecdsa-sha2-nistp256', 'ecdsa-sha2-nistp384', 'ecdsa-sha2-nistp521', 'ssh-ed25519', 'sk-ecdsa-sha2-nistp256@openssh.com', or 'sk-ssh-ed25519@openssh.com'">(% if let Some(key_value) = key_value %)(( key_value ))(% endif %)</textarea>
+            (% if let Some(key_error) = key_error %)
             <div id="key-validation-feedback" class="invalid-feedback">
-                (% if let Some(key_error) = key_error %)(( key_error ))(% endif %)
+                (( key_error ))
             </div>
-        </div>        
+            (% endif%)
+        </div>
 
-        <div class="column-gap-2 d-flex justify-content-end mt-2" hx-target="#credentialUpdateDynamicSection">
-            <button type="button" class="btn btn-danger" hx-get=((Urls::CredReset)) hx-target="body">Cancel</button>
+        <div class="column-gap-2 d-flex justify-content-end mt-2"
+            hx-target="#credentialUpdateDynamicSection">
+            <button type="button" class="btn btn-danger"
+                hx-get=((Urls::CredReset)) hx-target="body">Cancel</button>
             <button type="submit" class="btn btn-primary">Submit</button>
         </div>
     </form>
 </div>
-

server/lib/src/repl/consumer.rs

@@ -343,7 +343,7 @@ impl QueryServerWriteTransaction<'_> {
         }
     }
 
-    #[instrument(level = "info", skip_all)]
+    #[instrument(level = "debug", skip_all)]
     fn consumer_apply_changes_v1(
         &mut self,
         ctx_domain_version: DomainVersion,
@@ -394,7 +394,7 @@ impl QueryServerWriteTransaction<'_> {
             })?;
 
         // == ⚠️  Below this point we begin to make changes! ==
-        info!(
+        debug!(
             "Proceeding to apply incremental from domain {:?} at level {}",
             ctx_domain_uuid, ctx_domain_version
         );

server/lib/src/repl/supplier.rs

@@ -148,14 +148,14 @@ impl QueryServerReadTransaction<'_> {
             RangeDiffStatus::Ok(ranges) => ranges,
             RangeDiffStatus::Refresh { lag_range } => {
                 error!("Replication - Consumer is lagging and must be refreshed.");
-                info!(?lag_range);
+                debug!(?lag_range);
                 debug!(consumer_ranges = ?ctx_ranges);
                 debug!(supplier_ranges = ?our_ranges);
                 return Ok(ReplIncrementalContext::RefreshRequired);
             }
             RangeDiffStatus::Unwilling { adv_range } => {
                 error!("Replication - Supplier is lagging and must be investigated.");
-                info!(?adv_range);
+                debug!(?adv_range);
                 debug!(consumer_ranges = ?ctx_ranges);
                 debug!(supplier_ranges = ?our_ranges);
                 return Ok(ReplIncrementalContext::UnwillingToSupply);
@@ -164,9 +164,7 @@ impl QueryServerReadTransaction<'_> {
                 lag_range,
                 adv_range,
             } => {
-                error!("Replication Critical - Consumers are advanced of us, and also lagging! This must be immediately investigated!");
-                info!(?lag_range);
-                info!(?adv_range);
+                error!(?adv_range, ?lag_range, "Replication Critical - Consumers are advanced of us, and also lagging! This must be immediately investigated!");
                 debug!(consumer_ranges = ?ctx_ranges);
                 debug!(supplier_ranges = ?our_ranges);
                 return Ok(ReplIncrementalContext::UnwillingToSupply);

server/lib/src/valueset/ssh.rs

@@ -252,4 +252,30 @@ mod tests {
         // Test that we can parse json values into a valueset.
         crate::valueset::scim_json_put_reflexive::<ValueSetSshKey>(&vs, &[])
     }
+
+    #[test]
+    /// this is a test case for bad characters in SSH keys
+    fn test_invalid_character() {
+        let ecdsa = concat!("ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEÀAAAIbmlzdHA1MjEAAACFBAGyIY7o3B",
+        //                             ^ note the À here
+        "tOzRiJ9vvjj96bRImwmyy5GvFSIUPlK00HitiAWGhiO1jGZKmK7220Oe4rqU3uAwA00a0758UODs+0OQHLMDRtl81l",
+        "zPrVSdrYEDldxH9+a86dBZhdm0è15+ODDts2LHUknsJCRRldO4o9R9VrohlF7cbyBlnhJQrR4S+Oag== william@a",
+        "methyst");
+        println!("bytes of À {:?}", "À".as_bytes());
+        let found_index = ecdsa.find("À").expect("Failed to find è in string");
+        assert_eq!(found_index, 51, "Expected index 51");
+        let bad_ssh_error = SshPublicKey::from_string(ecdsa);
+
+        assert!(
+            bad_ssh_error.is_err(),
+            "Expected error, but got: {:?}",
+            bad_ssh_error
+        );
+        if let Err(err) = bad_ssh_error {
+            assert_eq!(
+                err.to_string(),
+                format!("Invalid symbol 195, offset {}.", found_index - 20)
+            ); // the offset is 31 because the string has a 20 character leading key type plus the space
+        }
+    }
 }

server/testkit/Cargo.toml

@@ -48,6 +48,7 @@ url = { workspace = true, features = ["serde"] }
 kanidm_build_profiles = { workspace = true }
 
 [dev-dependencies]
+cidr = { workspace = true }
 compact_jwt = { workspace = true }
 escargot = "0.5.13"
 # used for webdriver testing

server/testkit/tests/testkit/ip_addr_extractors.rs

@@ -1,3 +1,4 @@
+use cidr::IpCidr;
 use kanidm_client::KanidmClient;
 use kanidm_proto::constants::X_FORWARDED_FOR;
 use kanidmd_core::config::HttpAddressInfo;
@@ -51,7 +52,7 @@ async fn dont_trust_xff_send_header(rsclient: &KanidmClient) {
 // =====================================================
 // *test where we do trust the x-forwarded-for header
 
-#[kanidmd_testkit::test(http_client_address_info = HttpAddressInfo::XForwardFor ( [DEFAULT_IP_ADDRESS].into() ))]
+#[kanidmd_testkit::test(http_client_address_info = HttpAddressInfo::XForwardFor ( [IpCidr::from(DEFAULT_IP_ADDRESS)].into() ))]
 async fn trust_xff_address_set(rsclient: &KanidmClient) {
     inner_test_trust_xff(rsclient).await;
 }
@@ -284,7 +285,7 @@ async fn proxy_v2_make_request(
     Ok(ip_res)
 }
 
-#[kanidmd_testkit::test(with_test_env = true, http_client_address_info = HttpAddressInfo::ProxyV2 ( [DEFAULT_IP_ADDRESS].into() ))]
+#[kanidmd_testkit::test(with_test_env = true, http_client_address_info = HttpAddressInfo::ProxyV2 ( [IpCidr::from(DEFAULT_IP_ADDRESS)].into() ))]
 async fn trust_proxy_v2_address_set(test_env: &AsyncTestEnvironment) {
     // Send with no header - with proxy v2, a header is ALWAYS required
     let proxy_hdr: [u8; 0] = [];
@@ -308,7 +309,7 @@ async fn trust_proxy_v2_address_set(test_env: &AsyncTestEnvironment) {
     assert_eq!(res, IpAddr::V4(Ipv4Addr::new(172, 24, 12, 118)));
 }
 
-#[kanidmd_testkit::test(with_test_env = true, http_client_address_info = HttpAddressInfo::ProxyV2 ( [ IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)) ].into() ))]
+#[kanidmd_testkit::test(with_test_env = true, http_client_address_info = HttpAddressInfo::ProxyV2 ( [ IpCidr::from(Ipv4Addr::new(10, 0, 0, 1)) ].into() ))]
 async fn trust_proxy_v2_untrusted(test_env: &AsyncTestEnvironment) {
     // Send with a valid header, but we aren't a trusted source.
     let proxy_hdr =