inejge / ldap3

A pure-Rust LDAP library using the Tokio stack
Apache License 2.0
220 stars 38 forks source link

crash in SearchEntry::construct #2

Closed clux closed 7 years ago

clux commented 7 years ago

Hello again 😄 Some more adventures in AD land. Perhaps some more different parsing issues?

extern crate ldap3;
use ldap3::{LdapConn, Scope, SearchEntry};

fn main() {
    let ldap = LdapConn::new("ldaps://myadhost.com").unwrap();

    let (res, ctrls) = ldap.simple_bind("bindacc", "bindpw").expect("bind");
    println!("got bind res {:?} with ctrls {:?}", res, ctrls);

    let (result_set, result, _controls) = ldap.search(
        "dc=some,dc=thing,dc=com", // search base
        Scope::Subtree,
        "(sAMAccountName=ealbrigt)", // search filter
        vec![] as Vec<&str>
    ).unwrap();

    println!("{:?}", result);
    for entry in result_set {
        println!("entry: {:?}", SearchEntry::construct(entry));
    }
}

produces a crash in SearchEntry::construct

got bind res LdapResult { rc: 0, matched: "", text: "", refs: [] } with ctrls []
LdapResult { rc: 0, matched: "", text: "", refs: [{"ldaps://redactd"}, {"ldaps://redactd"}, {"ldaps://redactd"}] }
thread 'main' panicked at 'value: FromUtf8Error { bytes: [228, 65, 186, 133, 183, 227, 249, 74, 177, 26, 213, 226, 204, 116, 167, 109], error: Utf8Error { valid_up_to: 0, error_len: Some(1) } }', /checkout/src/libcore/result.rs:859
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: <core::result::Result<T, E>>::expect
             at /checkout/src/libcore/result.rs:761
   1: ldap3::search::SearchEntry::construct::{{closure}}
             at /home/clux/.cargo/git/checkouts/ldap3-3b291c3a502bdb2a/9582a6c/src/search.rs:144
   2: core::ops::impls::<impl core::ops::FnOnce<A> for &'a mut F>::call_once
             at /checkout/src/libcore/ops.rs:2654
   3: <core::option::Option<T>>::map
             at /checkout/src/libcore/option.rs:392
   4: <core::iter::Map<I, F> as core::iter::iterator::Iterator>::next
             at /checkout/src/libcore/iter/mod.rs:999
   5: <collections::vec::Vec<T> as collections::vec::SpecExtend<T, I>>::spec_extend
             at /checkout/src/libcollections/vec.rs:1696
   6: <collections::vec::Vec<T> as collections::vec::SpecExtend<T, I>>::from_iter
             at /checkout/src/libcollections/vec.rs:1679
   7: <collections::vec::Vec<T> as core::iter::traits::FromIterator<T>>::from_iter
             at /checkout/src/libcollections/vec.rs:1566
   8: core::iter::iterator::Iterator::collect
             at /checkout/src/libcore/iter/iterator.rs:1221
   9: ldap3::search::SearchEntry::construct
             at /home/clux/.cargo/git/checkouts/ldap3-3b291c3a502bdb2a/9582a6c/src/search.rs:141
  10: ldap::main
             at ./tests/ldap.rs:19

I can replace the last print with a debug entry print: println!("entry {:?}", entry); and the executable runs to completion then. The debug data looks like a bunch of sensible AD binary data to me, but I don't know enough about AD to see what's going wrong here (if you would like to see it, I'm happy to send that information along).

Any ideas?

inejge commented 7 years ago

Hmm, yes, the problem is that the decoder blindly tries to convert the binary value to a UTF-8 string and fails. It's difficult to solve without knowing the type of the attribute before decoding. As a workaround, you could retrieve just the attributes which you know are not binary. I'll think about the ways to make it work without making the user go through major contortions in order to retrieve values.

Meanwhile, could you paste the output of that last println!("entry {:?}", entry); here? I'd like to see if AD provides any type hints.

clux commented 7 years ago

Ah, yes, it does actually work when I specify only the sAMAccountName in attrs rather than an empty vec. I'm sending you an email with the output.

clux commented 7 years ago

I have at any rate gotten my auth setup to work with AD now, and will try to move this function into my web server somehow. Relatively painless even with my limited understanding of this stuff. Leaving my final test code here:

extern crate ldap3;
use ldap3::{LdapConn, Scope, SearchEntry};
use std::process;
use std::env;

fn main() {
    // You need to have set these first:
    let user = env::var("DOMAIN_USER").unwrap();
    let pass = env::var("DOMAIN_PASS").unwrap();

    // Make an ldap connection:
    let ldap = LdapConn::new("ldaps://myldaphost.com").unwrap();

    // Bind with a read-only account (required to be allowed to search with our AD)
    let (res, ctrls) = ldap.simple_bind("acc1", "pw1").expect("bind");
    println!("got bind res {:?} with ctrls {:?}", res, ctrls);

    // Search for your user:
    let (result_set, result, _controls) = ldap.search(
        "dc=some,dc=stuff,dc=com", // search base
        Scope::Subtree,
        &format!("sAMAccountName={}", user), // search filter
        vec!["sAMAccountName"] // what we want returned
    ).expect("search");

    // Find the `dn` entry of your user
    println!("{:?}", result);
    let entry = result_set.into_iter().next().expect("result first");
    let se = SearchEntry::construct(entry);
    println!("entry: {:?}", se);

    // Try to bind to that `dn` entry with your password:
    let (authres, ctrls2) = ldap.simple_bind(&se.dn, &pass).expect("bind");
    println!("got authres {:?}, ctlrs  {:?}", authres, ctrls2);
    if authres.rc == 0 {
        println!("authenticated");
        process::exit(0);
    }
    else {
        println!("failed to authenticate: {}", authres.text);
        process::exit(1);
    }
}
clux commented 7 years ago

Thanks a lot for your help :)

inejge commented 7 years ago

NP, thank you for putting up with freshly-minted code. In that spirit, here's a tentative fix for binary values. Could you try it out?

[dependencies.ldap3]
git = "https://github.com/inejge/ldap3"
rev = "c0565a89ad2221d43ef213d69b03ef31fdfd7415"
clux commented 7 years ago

NP!

Have checked and it is able to construct the SearchEntry now even with an empty attr vec, so looks good on my end :)

Seems the weird data in AD now appears wrapped in a double array inside bin_attrs on the return here, but those were never what I needed anyway. Excerpt from debug print:

bin_attrs: {"objectGUID": [[228, ..., 109]], "objectSid": [[1, ..., 0]]}
inejge commented 7 years ago

Yes, what you're seeing for each attribute is a Vec of Vec<u8>, with a single element (an attribute could have multiple values).

Thanks for the report and testing!