Conversation
|
Leaving this as a draft, as I want to look at it with a fresh set of eyes tomorrow before finalizing. Comments welcome at this stage :) |
|
Overall, this looks good. My main dislike is that it relies on some opaque docker image in the cloud, rather than just having a |
|
Well, any docker solution would involve layers pulled from the docker registry, at the very least up to the osixia/docker-openldap layer. The one I'm using in the script builds on that, but preconfigures the data... |
| // | ||
| // This particular filter allows the user to sign in with either | ||
| // uid or email | ||
| let filter = format!("(|(uid={})(mail={}))", who, who); |
There was a problem hiding this comment.
This is vulnerable to LDAP injection. It's actually a problem of the library since it does not provide the proper tools for escaping, so I opened #9 to track this issue.
There was a problem hiding this comment.
I don’t think the C LDAP library provided this functionality, and since this is a thin wrapper around it it was never present. The C library likely assumed that you escaped your queries yourself, as I did when I was using it for the project the fork was made for.
That being said, I do not think it’s a bad idea to add that ability if there is a standardized way to perform it and it is not required to be done if you can prove that your queries are otherwise ‘safe’.
| if let Ok(fry_dn) = ldap_dn_lookup(&ldap, user_to_authenticate.as_str()) { | ||
| // Now, perform a bind with the DN we found matching the user attempting to sign in | ||
| // and the password provided in the authentication request | ||
| do_simple_bind(&ldap, fry_dn.as_str(), pwd_to_authenticate.as_str()).unwrap(); | ||
|
|
||
| println!("Successfully signed in as fry"); | ||
| } |
There was a problem hiding this comment.
This is vulnerable to a timing side-channel attack. An attacker that does not have a valid set of credentials should not be able to learn which user accounts exist. However, if the attacker tries to authenticate with a bunch of credentials, they may notice that certain authentication attempts take longer than others. That's because the second simple-bind is only performed if ldap_dn_lookup returns Some. Therefore, the attacker can infer that a long request duration means that the username exists, even if the password is still wrong. This allows the attacker to perform a brute-force search for valid credentials much more efficiently.
Since this is a very common problem with authentication code, the example should demonstrate how to do it more safely. The basic idea is that you do the second simple-bind even if ldap_dn_lookup does not return any results. But you set a flag to remind yourself that you have to ignore the result of that second simple-bind. Here's an example from one of my programs how that looks (although in Go, not in Rust): https://github.com/majewsky/alltag/blob/df161b55fa4c7eba0abec82d2cf0df34e49b0ad4/internal/auth/ldap.go#L96-L115
There was a problem hiding this comment.
Good catch! I'll see if I get time to update the example this weekend!
…ping, and to protect against timing attacks for user discovery
4 participants
I created an example on how to authenticate with simple bind. Panning to add one on authorization at a later stage, but for now I think this will serve as a good intro.