-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Java: Promote LDAP Authentication Query #12458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Java: Promote LDAP Authentication Query #12458
Conversation
QHelp previews: java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.qhelpInsecure LDAP authenticationWhen using the Java LDAP API to perform LDAPv3-style extended operations and controls, a context with connection properties including user credentials is started. Transmission of LDAP credentials in cleartext allows remote attackers to obtain sensitive information by sniffing the network. RecommendationUse the ExampleIn the following (bad) example, a String ldapUrl = "ldap://ad.your-server.com:389";
Hashtable<String, String> environment = new Hashtable<String, String>();
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
environment.put(Context.PROVIDER_URL, ldapUrl);
environment.put(Context.REFERRAL, "follow");
environment.put(Context.SECURITY_AUTHENTICATION, "simple");
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
environment.put(Context.SECURITY_CREDENTIALS, password);
DirContext dirContext = new InitialDirContext(environment); In the following (good) example, a String ldapUrl = "ldaps://ad.your-server.com:636";
Hashtable<String, String> environment = new Hashtable<String, String>();
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
environment.put(Context.PROVIDER_URL, ldapUrl);
environment.put(Context.REFERRAL, "follow");
environment.put(Context.SECURITY_AUTHENTICATION, "simple");
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
environment.put(Context.SECURITY_CREDENTIALS, password);
DirContext dirContext = new InitialDirContext(environment); In the following (good) example, a String ldapUrl = "ldap://ad.your-server.com:389";
Hashtable<String, String> environment = new Hashtable<String, String>();
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
environment.put(Context.PROVIDER_URL, ldapUrl);
environment.put(Context.REFERRAL, "follow");
environment.put(Context.SECURITY_AUTHENTICATION, "DIGEST-MD5 GSSAPI");
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
environment.put(Context.SECURITY_CREDENTIALS, password);
DirContext dirContext = new InitialDirContext(environment); References
|
46bc904
to
24b5e12
Compare
02f5d06
to
144ec47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mostly LGTM! 👍
I added a few comments about style only. We can go ahead and request a docs review too.
java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll
Outdated
Show resolved
Hide resolved
java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll
Outdated
Show resolved
Hide resolved
java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll
Outdated
Show resolved
Hide resolved
java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@egregius313 Overall looks good to me from a docs perspective! ✨ I just had a few minor comments bringing it in line with our style guide. Feel free to ping me if any questions or if ready for me to follow up!
java/ql/src/change-notes/2023-03-08-insecure-ldap-authentication.md
Outdated
Show resolved
Hide resolved
Thanks @saritai I've applied your suggestions. |
Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>
Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>
Co-authored-by: Sarita Iyer <66540150+saritai@users.noreply.github.com>
Co-authored-by: Sarita Iyer <66540150+saritai@users.noreply.github.com>
3282353
to
106e5e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me from a docs perspective! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to approve this, just one minor comment!
java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll
Outdated
Show resolved
Hide resolved
Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>
Query promotion of
java/insecure-ldap-auth
from #4854.