Java: Promote LDAP Authentication Query by egregius313 · Pull Request #12458 · github/codeql

egregius313 · 2023-03-09T01:56:13Z

Query promotion of java/insecure-ldap-auth from #4854.

github-actions · 2023-03-09T01:57:08Z

QHelp previews:

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.qhelp

Insecure LDAP authentication

When 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.

Recommendation

Use the ldaps:// protocol to send credentials through SSL or use SASL authentication.

Example

In the following (bad) example, a ldap:// URL is used and credentials will be sent in plaintext.

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 ldaps:// URL is used so credentials will be encrypted with SSL.

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 ldap:// URL is used, but SASL authentication is enabled so that the credentials will be encrypted.

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

Oracle: LDAP and LDAPS URLs
Oracle: Simple authentication
Common Weakness Enumeration: CWE-522.
Common Weakness Enumeration: CWE-319.

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.ql

java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll

java/ql/lib/semmle/code/java/security/InsecureLdapAuth.qll

java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.ql

atorralba

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/InsecureLdapAuth.qll

java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll

saritai

@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/Security/CWE/CWE-522/InsecureLdapAuth.ql

java/ql/src/change-notes/2023-03-08-insecure-ldap-authentication.md

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.qhelp

egregius313 · 2023-03-24T16:40:48Z

@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!

Thanks @saritai I've applied your suggestions.

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>

Co-authored-by: Sarita Iyer <66540150+saritai@users.noreply.github.com>

saritai

Looks good to me from a docs perspective! 👍

atorralba

Happy to approve this, just one minor comment!

java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>

github-actions bot added documentation Java labels Mar 9, 2023

github-advanced-security bot found potential problems Mar 9, 2023

View reviewed changes

java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll Fixed Show fixed Hide fixed

java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll Fixed Show fixed Hide fixed

egregius313 force-pushed the egregius313/promote-insecure-ldap-authentication branch from 46bc904 to 24b5e12 Compare March 9, 2023 03:11

owen-mc changed the title ~~Promote LDAP Authentication Query~~ Java: Promote LDAP Authentication Query Mar 9, 2023

egregius313 force-pushed the egregius313/promote-insecure-ldap-authentication branch from 02f5d06 to 144ec47 Compare March 9, 2023 16:05

github-advanced-security bot found potential problems Mar 9, 2023

View reviewed changes

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.ql Fixed Show fixed Hide fixed

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.ql Fixed Show fixed Hide fixed

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.ql Fixed Show fixed Hide fixed

egregius313 marked this pull request as ready for review March 10, 2023 20:36

egregius313 requested a review from a team as a code owner March 10, 2023 20:36

atorralba reviewed Mar 17, 2023

View reviewed changes

egregius313 added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Mar 17, 2023

saritai reviewed Mar 23, 2023

View reviewed changes

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.ql Outdated Show resolved Hide resolved

java/ql/src/change-notes/2023-03-08-insecure-ldap-authentication.md Outdated Show resolved Hide resolved

java/ql/src/Security/CWE/CWE-522/InsecureLdapAuth.qhelp Outdated Show resolved Hide resolved

egregius313 requested review from a team as code owners March 27, 2023 16:11

egregius313 requested a review from a team March 27, 2023 16:11

egregius313 requested review from a team as code owners March 27, 2023 16:11

github-actions bot added ATM C# C++ DataFlow Library labels Mar 27, 2023

egregius313 and others added 10 commits March 27, 2023 12:16

Change version of PathNode used to appropriate module

efdfc2d

Rename SSL configuration and fix PathGraph

752620a

Documentation changes

cb58936

Change names of configuration to fit new naming convention

658c54a

Make classes/predicates not used outside of query private

151357d

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>

Import changes

24d4859

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>

Add InsecureLdapUrlSink

f28f1af

Move public classes/predicates to top of library file

0eaf222

Apply docs review suggestions

43d79dc

Co-authored-by: Sarita Iyer <66540150+saritai@users.noreply.github.com>

Docs review suggestion

106e5e7

Co-authored-by: Sarita Iyer <66540150+saritai@users.noreply.github.com>

egregius313 force-pushed the egregius313/promote-insecure-ldap-authentication branch from 3282353 to 106e5e7 Compare March 27, 2023 16:16

github-actions bot removed C++ Swift C# JS Kotlin Python ATM Go QL-for-QL DataFlow Library Ruby labels Mar 27, 2023

Update to the Global/flow* api

9bfb13b

saritai previously approved these changes Mar 27, 2023

View reviewed changes

atorralba reviewed Mar 28, 2023

View reviewed changes

java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll Outdated Show resolved Hide resolved

Make configuration public

97ec808

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>

egregius313 dismissed saritai’s stale review via 97ec808 March 28, 2023 14:28

atorralba approved these changes Mar 28, 2023

View reviewed changes

egregius313 merged commit b00104e into github:main Mar 28, 2023

egregius313 deleted the egregius313/promote-insecure-ldap-authentication branch March 28, 2023 14:39

Conversation

egregius313 commented Mar 9, 2023

Uh oh!

github-actions bot commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Insecure LDAP authentication

Recommendation

Example

References

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

atorralba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saritai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egregius313 commented Mar 24, 2023

Uh oh!

saritai left a comment

Choose a reason for hiding this comment

Uh oh!

atorralba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Mar 9, 2023 •

edited

Loading