Merge pull request #49 from yaleman/max_ber_size

Allow setting max_ber_size when creating an ldap client

.github/workflows/rust_build.yml

@@ -0,0 +1,40 @@
+---
+name: "Linux Build and Test"
+# Trigger the workflow on push to master or pull request
+"on":
+  push:
+    branches:
+      - master
+  pull_request:
+
+env:
+  SCCACHE_GHA_ENABLED: "true"
+  RUSTC_WRAPPER: "sccache"
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+jobs:
+  rust_build:
+    runs-on: ubuntu-latest
+    env:
+      SCCACHE_GHA_ENABLED: true
+      RUSTC_WRAPPER: sccache
+      CARGO_INCREMENTAL: 0
+      CARGO_TERM_COLOR: always
+    steps:
+      - uses: actions/checkout@v4
+      - name: Install Rust
+        uses: dtolnay/rust-toolchain@stable
+      - name: Setup sccache
+        uses: mozilla-actions/sccache-action@v0.0.3
+        with:
+          version: "v0.4.2"
+      - name: "Build the workspace"
+        run: cargo build --workspace
+
+      - name: "Run cargo test"
+        run: cargo test
+
+      - name: "Run cargo test --release"
+        run: cargo test --release

CONTRIBUTORS.md

@@ -1,10 +1,12 @@
+# Contributors
+
 ## Author
 
-* William Brown (Firstyear): william@blackhats.net.au
+* William Brown (Firstyear): <william@blackhats.net.au>
 
-## Contributors
+## Thanks to the contributors
 
+* Hiroyuki Wada (wadahiro)
+* James Hodgkinson (yaleman)
 * Jiegec
 * Nitnelave
-* Hiroyuki Wada (wadahiro)
-

Cargo.toml

@@ -8,7 +8,7 @@ members = ["proto", "client", "cli"]
 
 
 [workspace.package]
-version = "0.4.2"
+version = "0.4.3"
 authors = ["William Brown <william@blackhats.net.au>"]
 rust-version = "1.74"
 edition = "2021"

cli/src/ldap_debug_opt.rs

@@ -1,4 +1,3 @@
-
 use std::fmt;
 use std::str::FromStr;
 
@@ -39,7 +38,7 @@ struct BerDumpOptions {
 
     #[clap(short, long)]
     /// the path to the dump
-    path: PathBuf
+    path: PathBuf,
 }
 
 #[derive(Debug, clap::Subcommand)]
@@ -48,16 +47,14 @@ enum LdapDebugAction {
     BerDump(BerDumpOptions),
 }
 
-
 #[derive(Debug, clap::Parser)]
 #[clap(about = "Ldap Debugging Utility")]
 struct LdapDebugOpt {
     #[clap(short, long)]
-    /// Display extended infomation during runtime.
+    /// Display extended information during runtime.
     verbose: bool,
 
     #[clap(subcommand)]
     /// The ldap action to perform
-    action: LdapDebugAction
+    action: LdapDebugAction,
 }
-

cli/src/ldap_opt.rs

@@ -1,4 +1,3 @@
-
 /*
 #[derive(clap::ValueEnum, Debug, Clone)]
 enum SyncRequestMode {
@@ -22,7 +21,6 @@ enum LdapAction {
 
         // /// scope
         // scope
-
         /// Execute this query
         filter: String,
     },
@@ -56,7 +54,7 @@ enum LdapAction {
 #[clap(about = "Ldap Client Utility")]
 struct LdapOpt {
     #[structopt(short, long)]
-    /// Display extended infomation during runtime.
+    /// Display extended information during runtime.
     verbose: bool,
 
     #[clap(short = 'H', long = "url")]
@@ -76,6 +74,5 @@ struct LdapOpt {
 
     #[clap(subcommand)]
     /// The ldap action to perform
-    action: LdapAction
+    action: LdapAction,
 }
-

client/src/lib.rs

@@ -4,7 +4,6 @@
 #![deny(clippy::unimplemented)]
 #![deny(clippy::unwrap_used)]
 #![deny(clippy::panic)]
-#![deny(clippy::unreachable)]
 #![deny(clippy::await_holding_lock)]
 #![deny(clippy::needless_pass_by_value)]
 #![deny(clippy::trivially_copy_pass_by_ref)]
@@ -105,17 +104,17 @@ impl fmt::Display for LdapError {
                 write!(f, "The LDAP server sent a response we did not expect")
             }
             LdapError::FileIOError => {
-                write!(f, "An error occured while accessing a file")
+                write!(f, "An error occurred while accessing a file")
             }
             LdapError::TransportReadError => {
-                write!(f, "An error occured reading from the transport")
+                write!(f, "An error occurred reading from the transport")
             }
             LdapError::TransportWriteError => {
-                write!(f, "An error occured writing to the transport")
+                write!(f, "An error occurred writing to the transport")
             }
             LdapError::UnavailableCriticalExtension => write!(f, "An extension marked as critical was not available"),
             LdapError::InvalidCredentials => write!(f, "Invalid DN or Password"),
-            LdapError::InsufficentAccessRights => write!(f, "Insufficent Access"),
+            LdapError::InsufficentAccessRights => write!(f, "Insufficient Access"),
             LdapError::UnwillingToPerform => write!(f, "Too many failures, server is unwilling to perform the operation."),
             LdapError::EsyncRefreshRequired => write!(f, "An initial content sync is required. The current cookie should be considered invalid."),
             LdapError::NotImplemented => write!(f, "An error occurred, but we haven't implemented code to handle this error yet.")
@@ -261,6 +260,8 @@ pub struct LdapClientBuilder<'a> {
     timeout: Duration,
     cas: Vec<&'a Path>,
     verify: bool,
+    /// The maximum LDAP packet size parsed during decoding.
+    max_ber_size: Option<usize>,
 }
 
 impl<'a> LdapClientBuilder<'a> {
@@ -270,12 +271,12 @@ impl<'a> LdapClientBuilder<'a> {
             timeout: Duration::from_secs(30),
             cas: Vec::new(),
             verify: true,
+            max_ber_size: None,
         }
     }
 
-    pub fn set_timeout(mut self, timeout: Duration) -> Self {
-        self.timeout = timeout;
-        self
+    pub fn set_timeout(self, timeout: Duration) -> Self {
+        Self { timeout, ..self }
     }
 
     pub fn add_tls_ca<T>(mut self, ca: &'a T) -> Self
@@ -286,9 +287,19 @@ impl<'a> LdapClientBuilder<'a> {
         self
     }
 
-    pub fn danger_accept_invalid_certs(mut self, accept_invalid_certs: bool) -> Self {
-        self.verify = accept_invalid_certs;
-        self
+    pub fn danger_accept_invalid_certs(self, accept_invalid_certs: bool) -> Self {
+        Self {
+            verify: !accept_invalid_certs,
+            ..self
+        }
+    }
+
+    /// Set the maximum size of a decoded message
+    pub fn max_ber_size(self, max_ber_size: Option<usize>) -> Self {
+        Self {
+            max_ber_size,
+            ..self
+        }
     }
 
     #[tracing::instrument(level = "debug", skip_all)]
@@ -298,6 +309,7 @@ impl<'a> LdapClientBuilder<'a> {
             timeout,
             cas,
             verify,
+            max_ber_size,
         } = self;
 
         info!(%url);
@@ -363,6 +375,9 @@ impl<'a> LdapClientBuilder<'a> {
             }
         };
 
+        // If they didn't set it in the builder then set it to the default
+        let max_ber_size = max_ber_size.unwrap_or(ldap3_proto::DEFAULT_MAX_BER_SIZE);
+
         // If ldaps - start openssl
         let (write_transport, read_transport) = if need_tls {
             let mut tls_parms = SslConnector::builder(SslMethod::tls_client()).map_err(|e| {
@@ -420,16 +435,17 @@ impl<'a> LdapClientBuilder<'a> {
                 })?;
 
             info!("tls configured");
+
             let (r, w) = tokio::io::split(tlsstream);
             (
                 LdapWriteTransport::Tls(FramedWrite::new(w, LdapCodec::default())),
-                LdapReadTransport::Tls(FramedRead::new(r, LdapCodec::new(Some(32768)))),
+                LdapReadTransport::Tls(FramedRead::new(r, LdapCodec::new(Some(max_ber_size)))),
             )
         } else {
             let (r, w) = tokio::io::split(tcpstream);
             (
                 LdapWriteTransport::Plain(FramedWrite::new(w, LdapCodec::default())),
-                LdapReadTransport::Plain(FramedRead::new(r, LdapCodec::new(Some(32768)))),
+                LdapReadTransport::Plain(FramedRead::new(r, LdapCodec::new(Some(max_ber_size)))),
             )
         };
 
@@ -521,3 +537,23 @@ impl LdapClient {
             })
     }
 }
+
+/// Doesn't test the actual *build* step because that requires a live LDAP server.
+#[test]
+fn test_ldapclient_builder() {
+    let url = Url::parse("ldap://ldap.example.com:389").unwrap();
+    let client = LdapClientBuilder::new(&url).max_ber_size(Some(1234567));
+    assert_eq!(client.timeout, Duration::from_secs(30));
+    let client = client.set_timeout(Duration::from_secs(60));
+    assert_eq!(client.timeout, Duration::from_secs(60));
+    assert_eq!(client.cas.len(), 0);
+    assert_eq!(client.max_ber_size, Some(1234567));
+    assert_eq!(client.verify, true);
+
+    let ca_path = "test.pem".to_string();
+    let client = client.add_tls_ca(&ca_path);
+    assert_eq!(client.cas.len(), 1);
+
+    let badssl_client = client.danger_accept_invalid_certs(true);
+    assert_eq!(badssl_client.verify, false);
+}

client/src/syncrepl.rs

@@ -146,7 +146,7 @@ impl LdapClient {
                     done: false,
                 }) => {
                     // These are no-ops that are skipped for our purposes
-                    // They are intended to deliniate the seperate phases, but we actually don't
+                    // They are intended to deliniate the separate phases, but we actually don't
                     // care until we get the search result done.
                     let _d_uuids = delete_uuids.get_or_insert_with(Vec::default);
                 }
@@ -157,7 +157,7 @@ impl LdapClient {
                     },
                 ) => {
                     // These are no-ops that are skipped for our purposes
-                    // They are intended to deliniate the seperate phases, but we actually don't
+                    // They are intended to deliniate the separate phases, but we actually don't
                     // care until we get the search result done.
                     let _p_uuids = present_uuids.get_or_insert_with(Vec::default);
                 }

proto/examples/search/main.rs

@@ -78,7 +78,7 @@ pub async fn main() -> Result<(), Box<dyn std::error::Error>> {
     loop {
         let res = framed.next().await.expect("no meg")?;
         if let LdapOp::SearchResultDone(..) = &res.op {
-            info!("search sucessfull");
+            info!("search successful");
             break;
         }
     }

proto/src/error.rs

@@ -93,7 +93,7 @@ pub enum LdapProtoError {
 
     #[error("Intermediate response tag is invalid")]
     IntermediateResponseTag,
-    #[error("Invalid intermediat response id")]
+    #[error("Invalid intermediate response id")]
     IntermediateResponseId,
     #[error("Invalid BER in intermediate response")]
     IntermediateResponseBer,

proto/src/lib.rs

@@ -36,9 +36,10 @@ pub use crate::filter::parse_ldap_filter_str;
 pub use crate::simple::*;
 
 const KILOBYTES: usize = 1024;
-const DEFAULT_MAX_BER_SIZE: usize = 8 * KILOBYTES;
+pub const DEFAULT_MAX_BER_SIZE: usize = 32 * KILOBYTES;
 
 pub struct LdapCodec {
+    /// Default is [DEFAULT_MAX_BER_SIZE]
     max_ber_size: usize,
 }
 
@@ -325,7 +326,7 @@ mod tests {
             msgid: 23333,
             op: LdapOp::AddResponse(LdapResult {
                 code: LdapResultCode::Success,
-                matcheddn: "dc=exmaple,dc=com".to_string(),
+                matcheddn: "dc=example,dc=com".to_string(),
                 message: "msg".to_string(),
                 referral: vec![],
             }),
@@ -348,7 +349,7 @@ mod tests {
             msgid: 23333,
             op: LdapOp::DelResponse(LdapResult {
                 code: LdapResultCode::Success,
-                matcheddn: "dc=exmaple,dc=com".to_string(),
+                matcheddn: "dc=example,dc=com".to_string(),
                 message: "msg".to_string(),
                 referral: vec![],
             }),
@@ -486,7 +487,7 @@ mod tests {
         let mrs = LdapPasswordModifyResponse {
             res: LdapResult {
                 code: LdapResultCode::Success,
-                matcheddn: "uid=william,dc=exmaple,dc=com".to_string(),
+                matcheddn: "uid=william,dc=example,dc=com".to_string(),
                 message: "msg".to_string(),
                 referral: vec![],
             },
@@ -586,7 +587,7 @@ mod tests {
 
             let mut substring_filter_tags = match sequence.inner.pop().unwrap() {
                 Tag::Sequence(sequence) => sequence,
-                _ => panic!("substring_filter_tags sould be squence"),
+                _ => panic!("substring_filter_tags should be sequence"),
             }
             .inner;
             let cn = sequence.inner.pop().unwrap();

proto/src/proto.rs

@@ -1476,7 +1476,7 @@ impl TryFrom<Vec<StructureTag>> for LdapBindResponse {
 
         // Now with the remaining tags, as per rfc4511#section-4.2.2, we extract the optional sasl creds. Class Context, id 7. OctetString.
         let saslcreds = tags
-            .get(0)
+            .first()
             .map(|tag| {
                 debug!(?tag);
                 let vec = tag
@@ -2051,7 +2051,7 @@ impl TryFrom<Vec<StructureTag>> for LdapSearchRequest {
             .pop()
             .and_then(|t| t.match_class(TagClass::Universal))
             .and_then(|t|
-                // Some non-complient clients will not tag this as enum.
+                // Some non-compliant clients will not tag this as enum.
                 if cfg!(feature = "strict") {
                     t.match_id(Types::Enumerated as u64)
                 } else {