Closed alfonsrv closed 4 years ago
ViViDboarder
commented
4 years ago Thanks for moving this!
ldap_admin is not required. It's just a convenience tool to manage the LDAP server for testing purposes.
I'm not seeing the backtrace. Could you try setting RUST_BACKTRACE=1 directly in the docker-compose.yml file?
alfonsrv
commented
4 years ago Sure. Sorry, not too familiar with docker yet & thought the .env file would be global to all projects.
cgos@cgos-bitwarden:~/bitwarden_rs_ldap$ docker-compose up ldap_sync
bitwarden_rs_ldap_ldap_sync_1 is up-to-date
Attaching to bitwarden_rs_ldap_ldap_sync_1
ldap_sync_1 | Existing user or invite found with email: user1@cgos.biz
ldap_sync_1 | Existing user or invite found with email: user2@cgos.biz
ldap_sync_1 | Existing user or invite found with email: user3@cgos.biz
ldap_sync_1 | Existing user or invite found with email: user4@cgos.biz
ldap_sync_1 | Existing user or invite found with email: user5@cgos.biz
ldap_sync_1 | Existing user or invite found with email: user6@cgos.biz
ldap_sync_1 | Existing user or invite found with email: user7@cgos.biz
ldap_sync_1 | Existing user or invite found with email: user8@cgos.biz
ldap_sync_1 | thread 'main' panicked at 'rc=32 (noSuchObject), dn: "", text: ""', src/main.rs:21:9
ldap_sync_1 | stack backtrace:
ldap_sync_1 | 0: backtrace::backtrace::libunwind::trace
ldap_sync_1 | at ./cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
ldap_sync_1 | 1: backtrace::backtrace::trace_unsynchronized
ldap_sync_1 | at ./cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
ldap_sync_1 | 2: std::sys_common::backtrace::_print
ldap_sync_1 | at src/libstd/sys_common/backtrace.rs:47
ldap_sync_1 | 3: std::sys_common::backtrace::print
ldap_sync_1 | at src/libstd/sys_common/backtrace.rs:36
ldap_sync_1 | 4: std::panicking::default_hook::{{closure}}
ldap_sync_1 | at src/libstd/panicking.rs:200
ldap_sync_1 | 5: std::panicking::default_hook
ldap_sync_1 | at src/libstd/panicking.rs:214
ldap_sync_1 | 6: std::panicking::rust_panic_with_hook
ldap_sync_1 | at src/libstd/panicking.rs:477
ldap_sync_1 | 7: std::panicking::continue_panic_fmt
ldap_sync_1 | at src/libstd/panicking.rs:384
ldap_sync_1 | 8: std::panicking::begin_panic_fmt
ldap_sync_1 | at src/libstd/panicking.rs:339
ldap_sync_1 | 9: bitwarden_rs_ldap::main
ldap_sync_1 | 10: std::rt::lang_start::{{closure}}
ldap_sync_1 | 11: std::rt::lang_start_internal::{{closure}}
ldap_sync_1 | at src/libstd/rt.rs:49
ldap_sync_1 | 12: std::panicking::try::do_call
ldap_sync_1 | at src/libstd/panicking.rs:296
ldap_sync_1 | 13: __rust_maybe_catch_panic
ldap_sync_1 | at src/libpanic_unwind/lib.rs:82
ldap_sync_1 | 14: std::panicking::try
ldap_sync_1 | at src/libstd/panicking.rs:275
ldap_sync_1 | 15: std::panic::catch_unwind
ldap_sync_1 | at src/libstd/panic.rs:394
ldap_sync_1 | 16: std::rt::lang_start_internal
ldap_sync_1 | at src/libstd/rt.rs:48
ldap_sync_1 | 17: main
bitwarden_rs_ldap_ldap_sync_1 exited with code 101No worries! I'm new to Rust so I was hoping that would be useful but it seems that pretty much the entire stack trace is in the std libs. š¤¦āā
I think it's because I pass the error up the stack and then panic on it in the main function. I kinda expected the error to contain more information. I do see a comment I wrote though, it doesn't seem too helpful
// TODO: Something something error handlingAnyway, the error appears to be when searching. I searched for the error text in the LDAP library I'm using and it appears to actually be an error returned from the LDAP server itself.
Can you verify your bind and search info using another tool? Like ldapsearch?
alfonsrv
commented
4 years ago The everlasting struggle of testing locally and debugging in the wild... I can relate. Generally, what object-type is bitwarden_rs_ldap expecting? I just expected it to consume user objects which this my query is currently receiving exclusively.
Here's the output of ldapsearch - I have the search filter configured like that in multiple other applications, such as Confluence, nextCloud or Zulip - even though they all use a slightly different syntax - e.g. the latter requires to pass the sAMAccountName in the search filter as a variable.
cgos@cgos-bitwarden:~/bitwarden_rs_ldap$ ldapsearch -x -h 192.168.100.5 -D "cn=adquery,ou=serviceaccounts,dc=cgos,dc=loc" -w "adquerypwd" -b "dc=cgos,dc=loc" '(&(objectClass=user)(sAMAccountName=*)(memberOf:1.2.840.113556.1.4.1941:=cn=Bitwarden,ou=APPs,ou=cgos-Gruppen,dc=cgos,dc=loc))' -s sub "(sAMAccountName=*)"
# extended LDIF
#
# LDAPv3
# base <dc=cgos,dc=loc> with scope subtree
# filter: (&(objectClass=user)(sAMAccountName=*)(memberOf:1.2.840.113556.1.4.1941:=cn=Bitwarden,ou=APPs,ou=cgos-Gruppen,dc=cgos,dc=loc))
# requesting: (sAMAccountName=*)
#
# User Name 1, CGos-Users, cgos.loc
dn: CN=User Name1,OU=cgos-users,DC=cgos,DC=loc
# search reference
ref: ldap://DomainDnsZones.cgos.loc/DC=DomainDnsZones,DC=cgos,DC=loc
# search reference
ref: ldap://ForestDnsZones.cgos.loc/DC=ForestDnsZones,DC=cgos,DC=loc
# search reference
ref: ldap://cgos.loc/CN=Configuration,DC=cgos,DC=loc
# search result
search: 2
result: 0 Success
# numResponses: 5
# numEntries: 1
# numReferences: 3edit: Thinking about the question which object you're expecting ā a feature request could be having the ability to define the corresponding LDAP attribute of the login (section 4) to provide a unified login experience across all platform (sAMAccountName everywhere, instead of a sAMAccountName x Email as username mix).
ViViDboarder
commented
4 years ago To be honest, I only have a basic understanding of LDAP queries, so it's quite likely that I overlooked something that should be configurable.
The only things being passed to the LDAP client library are the base dn you provide, a Scope of Subtree, the search filter, and then a set of fields. Fields included are uid, givenname, sn, cn, mail. The last one being configurable. It's possible that one of those doesn't exist and causes this error. For the record, I don't know that any of those are really even necessary. I can't remember why I added them. It may be possible to pass in only the mail field, as that's all that is required for the invite.
I can try to reproduce based on that assumption.
Edit: Well, so much for that. Just tried by referencing an invalid field for the mail field and got:
Warning: Email field, "dne", not found on userSo the search query actually worked just fine and I was able to get an object back and then just fail gracefully when trying to read the field.
I'll keep looking.
ViViDboarder
commented
4 years ago Ok. I was able to reproduce by simply updating my ldap_search_base_dn to something invalid. Eg from dc=example,dc=org to dc=example,dc=com.
But from your comments, it looks like the dn is correct...
alfonsrv
commented
4 years ago When I change my ldap_search_base_dn to sth invalid, the error floods my console, while when everything seems correct the error is only thrown once. We could look at it together in my production environment (MS ADDS) if you want to gain a more differentiated insight.
Otherwise I'll try setting this up as a separateclean install tomorrow and see if the results differ.
alfonsrv
commented
4 years ago I fixed my issue ā multiple things were wrong in my configuration, the typo you mentioned for ldap_search_base_dn indeed did cause the issue. However this was mainly due to my lack of understanding of Docker and not knowing how to properly mount the config.toml file in the first place.
I realized however that this project merely serves as an automation to fill in the invitation field in the admin for users found in LDAP and does not actually in fact sync passwords from sources like Active Directory, which is the main reason I've been implementing LDAP into applications. I don't know if this is actually somehow possible for Bitwarden, or if this is not viable due to security limitations / concerns, but it would certainly be the thing I would love to see.
ViViDboarder
commented
4 years ago Ah. Yea, not exactly possible to do that while maintaining the client side encryption. There was some discussion of it on this thread somewhat recently: https://github.com/dani-garcia/bitwarden_rs/pull/677
It might be possible if we fork the clients as well, but that seems like more effort than anyone seems interested in picking up right now.
Trying to get LDAP to work with Bitwarden, causing the error described below. I'm trying to get all users synced from an existing ADDS. We have previously used Bitwarden without bitwarden_rs_ldap.
On a sidenote: I'm not quite sure if the ldap_admin etc is actually required for my usecase or if I just need the sync submodule after following the docker hub instructions - a clarification would be much appreciated.
Console Log
example.config.toml
bitwarden_rs Docker run
edit: also tried simplifying the search filter to
ou=Users,dc=cgos,dc=locbut results in the same error.