Enforce TLS key size minimums (#2145)

* Enforce TLS key size minimums - Fixes #2144 * at some point clippy got mad
2025-02-23 12:37:00 +01:00 · 2023-09-26 09:59:00 +10:00 · 2023-09-26 09:59:00 +10:00 · c7a269575c
parent c998a1eda5
commit c7a269575c
10 changed files with 325 additions and 48 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@ -3011,6 +3011,7 @@ dependencies = [
 "serde_json",
 "serde_with",
 "sketching",
+ "tempfile",
 "time",
 "tokio",
 "tokio-openssl",
@ -5019,7 +5020,6 @@ dependencies = [
 name = "testkit-macros"
 version = "0.1.0"
 dependencies = [
- "kanidmd_core",
 "proc-macro2",
 "quote",
 "syn 2.0.37",
--- a/Cargo.toml
+++ b/Cargo.toml
@ -173,7 +173,8 @@ sketching = { path = "./libs/sketching" }
 smartstring = "^1.0.1"
 smolset = "^1.3.1"
 sshkeys = "^0.3.1"
-syn = { version = "2.0.37", features = ["full"] }
+syn = { version = "2.0.32", features = ["full"] }
+tempfile = "3.8.0"
 testkit-macros = { path = "./server/testkit-macros" }
 time = { version = "^0.3.21", features = ["formatting", "local-offset"] }

--- a/book/src/security_hardening.md
+++ b/book/src/security_hardening.md
@ -158,3 +158,7 @@ docker run --rm -i -t -u 1000:1000 -v kanidmd:/data kanidm/server:latest /sbin/k

 > **HINT** You need to use the UID or GID number with the `-u` argument, as the container can't
 > resolve usernames from the host system.
+
+## Minimum TLS key lengths
+
+We enforce a minimum RSA key length of 2048 bits, and EC keys need 224 bits.
--- a/libs/profiles/Cargo.toml
+++ b/libs/profiles/Cargo.toml
@ -24,4 +24,4 @@ base64 = { workspace = true }

 [build-dependencies]
 base64 = { workspace = true }
-gix = { workspace = true }
+gix = { workspace = true, default-features = false }
--- a/server/core/Cargo.toml
+++ b/server/core/Cargo.toml
@ -2,7 +2,6 @@
 name = "kanidmd_core"
 description = "Kanidm Server Core and Library"
 documentation = "https://docs.rs/kanidm/latest/kanidm/"
-autotests = false

 version = { workspace = true }
 authors = { workspace = true }
@ -14,7 +13,7 @@ repository = { workspace = true }

 [dependencies]
 async-trait = { workspace = true }
-axum = { workspace=true }
+axum = { workspace = true }
 axum-auth = "0.4.0"
 axum-csp = { workspace = true }
 axum-macros = "0.3.8"
@ -28,6 +27,7 @@ futures-util = { workspace = true }
 http = "0.2.9"
 hyper = { workspace = true }
 kanidm_proto = { workspace = true }
+kanidm_utils_users = { workspace = true }
 kanidmd_lib = { workspace = true }
 ldap3_proto = { workspace = true }
 libc = { workspace = true }
@ -36,20 +36,29 @@ rand = { workspace = true }
 regex = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
+serde_with = { workspace = true }
 sketching = { workspace = true }
-time = { workspace = true, features = ["serde", "std","local-offset"] }
+time = { workspace = true, features = ["serde", "std", "local-offset"] }
 tokio = { workspace = true, features = ["net", "sync", "io-util", "macros"] }
 tokio-openssl = { workspace = true }
 tokio-util = { workspace = true, features = ["codec"] }
-toml = {workspace = true}
+toml = { workspace = true }
 tower = { version = "0.4.13", features = ["tokio-stream", "tracing"] }
-tower-http = { version = "0.4.4", features = ["tokio", "tracing", "uuid", "compression-gzip", "compression-zstd", "trace", "fs"] }
+tower-http = { version = "0.4.4", features = [
+    "compression-gzip",
+    "compression-zstd",
+    "fs",
+    "tokio",
+    "trace",
+    "tracing",
+    "uuid",
+] }
 tracing = { workspace = true, features = ["attributes"] }
 tracing-subscriber = { workspace = true, features = ["time", "json"] }
 urlencoding = { workspace = true }
-kanidm_utils_users = { workspace = true }
-uuid = { workspace = true, features = ["serde", "v4" ] }
-serde_with = { workspace = true }
+tempfile = { workspace = true }
+uuid = { workspace = true, features = ["serde", "v4"] }
+

 [build-dependencies]
 kanidm_build_profiles = { workspace = true }
--- a/server/core/src/crypto.rs
+++ b/server/core/src/crypto.rs
@ -4,7 +4,9 @@
 use openssl::ec::{EcGroup, EcKey};
 use openssl::error::ErrorStack;
 use openssl::nid::Nid;
-use openssl::ssl::{SslAcceptor, SslAcceptorBuilder, SslFiletype, SslMethod};
+use openssl::pkey::{PKeyRef, Private};
+use openssl::rsa::Rsa;
+use openssl::ssl::{SslAcceptor, SslFiletype, SslMethod};
 use openssl::x509::{
    extension::{
        AuthorityKeyIdentifier, BasicConstraints, ExtendedKeyUsage, KeyUsage,
@ -13,6 +15,7 @@ use openssl::x509::{
    X509NameBuilder, X509ReqBuilder, X509,
 };
 use openssl::{asn1, bn, hash, pkey};
+use sketching::*;

 use crate::config::Configuration;

@ -23,25 +26,110 @@ use std::path::Path;
 const CA_VALID_DAYS: u32 = 30;
 const CERT_VALID_DAYS: u32 = 5;

+// Basing minimums off https://www.keylength.com setting "year" to 2030 - tested as at 2023-09-25
+//
+// |Method           |Date     |Symmetric| FM      |DL Key| DL Group|Elliptic Curve|Hash|
+// |   ---           |   ---   |   ---   |   ---   | ---  |   ---   |  ---         | ---|
+// |Lenstra / Verheul|2030     |  93     |2493^2016|165   | 2493    |  176         | 186|
+// |Lenstra Updated  |2030     |  88     |1698^2063|176   | 1698    |  176         | 176|
+// |ECRYPT           |2029-2068|  256    |15360    |512   | 15360   |  512         | 512|
+// |NIST             |2019-2030|  112    |2048     |224   | 2048    |  224         | 224|
+// |ANSSI            |> 2030   |  128    |3072     |200   | 3072    |  256         | 256|
+// |NSA              |-        |  256    |3072     |-     | -       |  384         | 384|
+// |RFC3766          |-        |  -      |   -     | -    |   -     |   -          |  - |
+// |BSI              |-        |  -      |   -     | -    |   -     |   -          |  - |
+// DL - Discrete Logarithm
+// FM - Factoring Modulus
+
+const RSA_MIN_KEY_SIZE_BITS: u64 = 2048;
+const EC_MIN_KEY_SIZE_BITS: u64 = 224;
+
+/// returns a signing function that meets a sensible minimum
+fn get_signing_func() -> hash::MessageDigest {
+    hash::MessageDigest::sha256()
+}
+
+/// Ensure we're enforcing safe minimums for TLS keys
+pub fn check_privkey_minimums(privkey: &PKeyRef<Private>) -> Result<(), String> {
+    if let Ok(key) = privkey.rsa() {
+        if key.size() < (RSA_MIN_KEY_SIZE_BITS / 8) as u32 {
+            Err(format!(
+                "TLS RSA key is less than {} bits!",
+                RSA_MIN_KEY_SIZE_BITS
+            ))
+        } else {
+            debug!(
+                "The RSA private key size is: {} bits, that's OK!",
+                key.size() * 8
+            );
+            Ok(())
+        }
+    } else if let Ok(key) = privkey.ec_key() {
+        // allowing this to panic because ... it's an i32 and hopefully we don't have negative bit lengths?
+        #[allow(clippy::panic)]
+        let key_bits: u64 = key.private_key().num_bits().try_into().unwrap_or_else(|_| {
+            panic!(
+                "Failed to convert EC bitlength {} to u64",
+                key.private_key().num_bits()
+            )
+        });
+
+        if key_bits < EC_MIN_KEY_SIZE_BITS {
+            Err(format!(
+                "TLS EC key is less than {} bits! Got: {}",
+                EC_MIN_KEY_SIZE_BITS, key_bits
+            ))
+        } else {
+            #[cfg(any(test, debug_assertions))]
+            println!("The EC private key size is: {} bits, that's OK!", key_bits);
+            debug!("The EC private key size is: {} bits, that's OK!", key_bits);
+            Ok(())
+        }
+    } else {
+        error!("TLS key is not RSA or EC, cannot check minimums!");
+        Ok(())
+    }
+}
+
 /// From the server configuration, generate an OpenSSL acceptor that we can use
-/// to build our sockets for https/ldaps.
-pub fn setup_tls(config: &Configuration) -> Result<Option<SslAcceptorBuilder>, ErrorStack> {
+/// to build our sockets for HTTPS/LDAPS.
+pub fn setup_tls(config: &Configuration) -> Result<Option<SslAcceptor>, ErrorStack> {
    match &config.tls_config {
        Some(tls_config) => {
+            // Signing algorithm minimums are enforced by the SSLAcceptor - it won't start up with a sha1-signed cert.
            let mut ssl_builder = SslAcceptor::mozilla_modern(SslMethod::tls())?;
            ssl_builder.set_certificate_chain_file(&tls_config.chain)?;
+
            ssl_builder.set_private_key_file(&tls_config.key, SslFiletype::PEM)?;
            ssl_builder.check_private_key()?;
-            Ok(Some(ssl_builder))
+
+            let acceptor = ssl_builder.build();
+
+            // let's enforce some TLS minimums!
+            #[allow(clippy::expect_used)]
+            let privkey = acceptor
+                .context()
+                .private_key()
+                .expect("Couldn't pull TLS key after configuring one!");
+
+            check_privkey_minimums(privkey).map_err(|err| {
+                #[cfg(any(test, debug_assertions))]
+                println!("{}", err);
+                admin_error!("{}", err);
+                ErrorStack::get() // this probably should be a real errorstack but... how?
+            })?;
+
+            Ok(Some(acceptor))
        }
        None => Ok(None),
    }
 }

-fn get_group() -> Result<EcGroup, ErrorStack> {
+fn get_ec_group() -> Result<EcGroup, ErrorStack> {
    EcGroup::from_curve_name(Nid::X9_62_PRIME256V1)
 }

+#[derive(Debug)]
 pub(crate) struct CaHandle {
    key: pkey::PKey<pkey::Private>,
    cert: X509,
@ -76,10 +164,108 @@ pub(crate) fn write_ca(
        })
 }

-pub(crate) fn build_ca() -> Result<CaHandle, ErrorStack> {
-    let ecgroup = get_group()?;
-    let eckey = EcKey::generate(&ecgroup)?;
-    let ca_key = pkey::PKey::from_ec_key(eckey)?;
+#[derive(Debug)]
+pub enum KeyType {
+    #[allow(dead_code)]
+    Rsa,
+    Ec,
+}
+impl Default for KeyType {
+    fn default() -> Self {
+        Self::Ec
+    }
+}
+
+#[derive(Debug)]
+pub struct CAConfig {
+    pub key_type: KeyType,
+    pub key_bits: u64,
+    pub skip_enforce_minimums: bool,
+}
+
+impl Default for CAConfig {
+    fn default() -> Self {
+        #[allow(clippy::expect_used)]
+        Self::new(KeyType::Ec, 256, false)
+            .expect("Somehow the defaults failed to pass validation while building a CA Config?")
+    }
+}
+
+impl CAConfig {
+    fn new(key_type: KeyType, key_bits: u64, skip_enforce_minimums: bool) -> Result<Self, String> {
+        let res = Self {
+            key_type,
+            key_bits,
+            skip_enforce_minimums,
+        };
+        if !skip_enforce_minimums {
+            res.enforce_minimums()?;
+        };
+        Ok(res)
+    }
+
+    /// Make sure we're meeting the minimum spec for key length etc
+    fn enforce_minimums(&self) -> Result<(), String> {
+        match self.key_type {
+            KeyType::Rsa => {
+                trace!(
+                    "Generating CA Config for RSA Key with {} bits",
+                    self.key_bits
+                );
+                if self.key_bits < RSA_MIN_KEY_SIZE_BITS {
+                    return Err(format!(
+                        "RSA key size must be at least {} bits",
+                        RSA_MIN_KEY_SIZE_BITS
+                    ));
+                }
+            }
+            KeyType::Ec => {
+                trace!("Generating CA Config for EcKey with {} bits", self.key_bits);
+                if self.key_bits < EC_MIN_KEY_SIZE_BITS {
+                    return Err(format!(
+                        "EC key size must be at least {} bits",
+                        EC_MIN_KEY_SIZE_BITS
+                    ));
+                }
+            }
+        };
+        Ok(())
+    }
+}
+
+pub(crate) fn gen_private_key(
+    key_type: &KeyType,
+    key_bits: Option<u64>,
+) -> Result<pkey::PKey<pkey::Private>, ErrorStack> {
+    match key_type {
+        KeyType::Rsa => {
+            let key_bits = key_bits.unwrap_or(RSA_MIN_KEY_SIZE_BITS);
+            let rsa = Rsa::generate(key_bits as u32)?;
+            pkey::PKey::from_rsa(rsa)
+        }
+        KeyType::Ec => {
+            // TODO: take key bitlength and use it for the curve group, somehow?
+            let ecgroup = get_ec_group()?;
+            let eckey = EcKey::generate(&ecgroup)?;
+            pkey::PKey::from_ec_key(eckey)
+        }
+    }
+}
+
+/// build up a CA certificate and key.
+pub(crate) fn build_ca(ca_config: Option<CAConfig>) -> Result<CaHandle, ErrorStack> {
+    let ca_config = ca_config.unwrap_or(CAConfig::default());
+
+    let ca_key = gen_private_key(&ca_config.key_type, Some(ca_config.key_bits))?;
+
+    if !ca_config.skip_enforce_minimums {
+        check_privkey_minimums(&ca_key).map_err(|err| {
+            admin_error!("failed to build_ca due to privkey minimums {}", err);
+            #[cfg(any(test, debug_assertions))]
+            println!("failed to build_ca due to privkey minimums: {}", err);
+            ErrorStack::get() // this probably should be a real errorstack but... how?
+        })?;
+    }
    let mut x509_name = X509NameBuilder::new()?;

    x509_name.append_entry_by_text("C", "AU")?;
@ -119,7 +305,7 @@ pub(crate) fn build_ca() -> Result<CaHandle, ErrorStack> {

    cert_builder.set_pubkey(&ca_key)?;

-    cert_builder.sign(&ca_key, hash::MessageDigest::sha256())?;
+    cert_builder.sign(&ca_key, get_signing_func())?;
    let ca_cert = cert_builder.build();

    Ok(CaHandle {
@ -153,6 +339,12 @@ pub(crate) fn load_ca(
        error!(err = ?e, "Failed to convert PEM to key");
    })?;

+    check_privkey_minimums(&ca_key).map_err(|err| {
+        #[cfg(any(test, debug_assertions))]
+        println!("{:?}", err);
+        admin_error!("{}", err);
+    })?;
+
    let ca_cert = X509::from_pem(&ca_cert_pem).map_err(|e| {
        error!(err = ?e, "Failed to convert PEM to cert");
    })?;
@ -224,12 +416,12 @@ pub(crate) fn write_cert(
 pub(crate) fn build_cert(
    domain_name: &str,
    ca_handle: &CaHandle,
+    key_type: Option<KeyType>,
+    key_bits: Option<u64>,
 ) -> Result<CertHandle, ErrorStack> {
-    let ecgroup = get_group()?;
-    let eckey = EcKey::generate(&ecgroup)?;
-    let int_key = pkey::PKey::from_ec_key(eckey)?;
+    let key_type = key_type.unwrap_or(KeyType::default());
+    let int_key = gen_private_key(&key_type, key_bits)?;

-    //
    let mut req_builder = X509ReqBuilder::new()?;
    req_builder.set_pubkey(&int_key)?;

@ -243,7 +435,7 @@ pub(crate) fn build_cert(
    let x509_name = x509_name.build();

    req_builder.set_subject_name(&x509_name)?;
-    req_builder.sign(&int_key, hash::MessageDigest::sha256())?;
+    req_builder.sign(&int_key, get_signing_func())?;
    let req = req_builder.build();
    // ==

@ -296,7 +488,7 @@ pub(crate) fn build_cert(
        .build(&cert_builder.x509v3_context(Some(&ca_handle.cert), None))?;
    cert_builder.append_extension(subject_alt_name)?;

-    cert_builder.sign(&ca_handle.key, hash::MessageDigest::sha256())?;
+    cert_builder.sign(&ca_handle.key, get_signing_func())?;
    let int_cert = cert_builder.build();

    Ok(CertHandle {
@ -305,3 +497,77 @@ pub(crate) fn build_cert(
        chain: vec![ca_handle.cert.clone()],
    })
 }
+
+#[test]
+// might as well test my logic
+fn test_enforced_minimums() {
+    let good_ca_configs = vec![
+        // test rsa 4096 (ok)
+        (KeyType::Rsa, 4096, false),
+        // test rsa 2048 (ok)
+        (KeyType::Rsa, 2048, false),
+        // test ec 256 (ok)
+        (KeyType::Ec, 256, false),
+    ];
+    good_ca_configs.into_iter().for_each(|config| {
+        dbg!(&config);
+        assert!(CAConfig::new(config.0, config.1, config.2).is_ok());
+    });
+    let bad_ca_configs = vec![
+        // test rsa 1024 (no)
+        (KeyType::Rsa, 1024, false),
+        // test ec 128 (no)
+        (KeyType::Ec, 128, false),
+    ];
+    bad_ca_configs.into_iter().for_each(|config| {
+        dbg!(&config);
+        assert!(CAConfig::new(config.0, config.1, config.2).is_err());
+    });
+}
+
+#[test]
+fn test_ca_loader() {
+    let ca_key_tempfile = tempfile::NamedTempFile::new().unwrap();
+    let ca_cert_tempfile = tempfile::NamedTempFile::new().unwrap();
+    // let's test the defaults first
+
+    let ca_config = CAConfig::default();
+    if let Ok(ca) = build_ca(Some(ca_config)) {
+        write_ca(&ca_key_tempfile.path(), &ca_cert_tempfile.path(), &ca).unwrap();
+        assert!(load_ca(&ca_key_tempfile.path(), &ca_cert_tempfile.path()).is_ok());
+    };
+
+    let good_ca_configs = vec![
+        // test rsa 4096 (ok)
+        (KeyType::Rsa, 4096, false),
+        // test rsa 2048 (ok)
+        (KeyType::Rsa, 2048, false),
+        // test ec 256 (ok)
+        (KeyType::Ec, 256, false),
+    ];
+    good_ca_configs.into_iter().for_each(|config| {
+        println!("testing good config {:?}", config);
+        let ca_config = CAConfig::new(config.0, config.1, config.2).unwrap();
+        let ca = build_ca(Some(ca_config)).unwrap();
+        write_ca(&ca_key_tempfile.path(), &ca_cert_tempfile.path(), &ca).unwrap();
+        let ca_result = load_ca(&ca_key_tempfile.path(), &ca_cert_tempfile.path());
+        println!("result: {:?}", ca_result);
+        assert!(ca_result.is_ok());
+    });
+    let bad_ca_configs = vec![
+        // test rsa 1024 (bad)
+        (KeyType::Rsa, 1024, true),
+    ];
+    bad_ca_configs.into_iter().for_each(|config| {
+        println!(
+            "\ntesting bad config keytype: {:?} key size: {}, skip_enforce_minimums: {}",
+            config.0, config.1, config.2
+        );
+        let ca_config = CAConfig::new(config.0, config.1, config.2).unwrap();
+        let ca = build_ca(Some(ca_config)).unwrap();
+        write_ca(&ca_key_tempfile.path(), &ca_cert_tempfile.path(), &ca).unwrap();
+        let ca_result = load_ca(&ca_key_tempfile.path(), &ca_cert_tempfile.path());
+        println!("result: {:?}", ca_result);
+        assert!(ca_result.is_err());
+    });
+}
--- a/server/core/src/ldaps.rs
+++ b/server/core/src/ldaps.rs
@ -9,7 +9,7 @@ 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, SslAcceptorBuilder};
+use openssl::ssl::{Ssl, SslAcceptor};
 use tokio::io::{AsyncRead, AsyncWrite};
 use tokio::net::TcpListener;
 use tokio_openssl::SslStream;
@ -108,7 +108,7 @@ async fn client_process<W: AsyncWrite + Unpin, R: AsyncRead + Unpin>(
 /// TLS LDAP Listener, hands off to [client_process]
 async fn tls_acceptor(
    listener: TcpListener,
-    tls_parms: SslAcceptor,
+    ssl_acceptor: SslAcceptor,
    qe_r_ref: &'static QueryServerReadV1,
    mut rx: broadcast::Receiver<CoreAction>,
 ) {
@ -124,7 +124,7 @@ async fn tls_acceptor(
                    Ok((tcpstream, client_socket_addr)) => {
                        // Start the event
                        // From the parameters we need to create an SslContext.
-                        let mut tlsstream = match Ssl::new(tls_parms.context())
+                        let mut tlsstream = match Ssl::new(ssl_acceptor.context())
                            .and_then(|tls_obj| SslStream::new(tls_obj, tcpstream))
                        {
                            Ok(ta) => ta,
@ -154,7 +154,7 @@ async fn tls_acceptor(

 pub(crate) async fn create_ldap_server(
    address: &str,
-    opt_tls_params: Option<SslAcceptorBuilder>,
+    opt_ssl_acceptor: Option<SslAcceptor>,
    qe_r_ref: &'static QueryServerReadV1,
    rx: broadcast::Receiver<CoreAction>,
 ) -> Result<tokio::task::JoinHandle<()>, ()> {
@ -175,11 +175,11 @@ pub(crate) async fn create_ldap_server(
        );
    })?;

-    let ldap_acceptor_handle = match opt_tls_params {
-        Some(tls_params) => {
+    let ldap_acceptor_handle = match opt_ssl_acceptor {
+        Some(ssl_acceptor) => {
            info!("Starting LDAPS interface ldaps://{} ...", address);
-            let tls_parms = tls_params.build();
-            tokio::spawn(tls_acceptor(listener, tls_parms, qe_r_ref, rx))
+
+            tokio::spawn(tls_acceptor(listener, ssl_acceptor, qe_r_ref, rx))
        }
        None => {
            error!("The server won't run without TLS!");
--- a/server/core/src/lib.rs
+++ b/server/core/src/lib.rs
@ -599,7 +599,7 @@ pub fn cert_generate_core(config: &Configuration) {

    let ca_handle = if !ca_cert.exists() || !ca_key.exists() {
        // Generate the CA again.
-        let ca_handle = match crypto::build_ca() {
+        let ca_handle = match crypto::build_ca(None) {
            Ok(ca_handle) => ca_handle,
            Err(e) => {
                error!(err = ?e, "Failed to build CA");
@ -625,7 +625,7 @@ pub fn cert_generate_core(config: &Configuration) {

    if !tls_key_path.exists() || !tls_chain_path.exists() || !tls_cert_path.exists() {
        // Generate the cert from the ca.
-        let cert_handle = match crypto::build_cert(origin_domain, &ca_handle) {
+        let cert_handle = match crypto::build_cert(origin_domain, &ca_handle, None, None) {
            Ok(cert_handle) => cert_handle,
            Err(e) => {
                error!(err = ?e, "Failed to build certificate");
@ -886,7 +886,7 @@ pub async fn create_server_core(
    // If we have been requested to init LDAP, configure it now.
    let maybe_ldap_acceptor_handle = match &config.ldapaddress {
        Some(la) => {
-            let opt_ldap_tls_params = match crypto::setup_tls(&config) {
+            let opt_ldap_ssl_acceptor = match crypto::setup_tls(&config) {
                Ok(t) => t,
                Err(e) => {
                    error!("Failed to configure LDAP TLS parameters -> {:?}", e);
@ -897,7 +897,7 @@ pub async fn create_server_core(
                // ⚠️  only start the sockets and listeners in non-config-test modes.
                let h = ldaps::create_ldap_server(
                    la.as_str(),
-                    opt_ldap_tls_params,
+                    opt_ldap_ssl_acceptor,
                    server_read_ref,
                    broadcast_tx.subscribe(),
                )
--- a/server/lib/src/server/access/search.rs
+++ b/server/lib/src/server/access/search.rs
@ -144,12 +144,12 @@ fn search_oauth2_filter_entry<'a>(
                security_access!(entry = ?entry.get_uuid(), ident = ?iuser.entry.get_uuid2rdn(), "ident is a memberof a group granted an oauth2 scope by this entry");

                return AccessResult::Allow(btreeset!(
-                    ATTR_CLASS.clone(),
-                    ATTR_DISPLAYNAME.clone(),
-                    ATTR_UUID.clone(),
-                    ATTR_OAUTH2_RS_NAME.clone(),
-                    ATTR_OAUTH2_RS_ORIGIN.clone(),
-                    ATTR_OAUTH2_RS_ORIGIN_LANDING.clone()
+                    ATTR_CLASS,
+                    ATTR_DISPLAYNAME,
+                    ATTR_UUID,
+                    ATTR_OAUTH2_RS_NAME,
+                    ATTR_OAUTH2_RS_ORIGIN,
+                    ATTR_OAUTH2_RS_ORIGIN_LANDING
                ));
            }
            AccessResult::Ignore
--- a/server/testkit-macros/Cargo.toml
+++ b/server/testkit-macros/Cargo.toml
@ -7,9 +7,6 @@ edition = "2021"
 proc-macro = true

 [dependencies]
-kanidmd_core.workspace = true
 proc-macro2 = { workspace = true }
 quote = { workspace = true }
 syn = { workspace = true }
-
-