Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 23 commits into from
Mar 28, 2023
Merged

Java: Promote LDAP Authentication Query #12458

merged 23 commits into from
Mar 28, 2023

Conversation

egregius313
Copy link
Contributor

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023 •
edited
Loading

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

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 9, 2023
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 9, 2023
View reviewed changes
@egregius313 egregius313 force-pushed the egregius313/promote-insecure-ldap-authentication branch from 46bc904 to 24b5e12 Compare March 9, 2023 03:11
@owen-mc owen-mc changed the title Promote LDAP Authentication Query Java: Promote LDAP Authentication Query Mar 9, 2023
@egregius313 egregius313 force-pushed the egregius313/promote-insecure-ldap-authentication branch from 02f5d06 to 144ec47 Compare March 9, 2023 16:05
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 9, 2023
View reviewed changes
@egregius313 egregius313 marked this pull request as ready for review March 10, 2023 20:36
@egregius313 egregius313 requested a review from a team as a code owner March 10, 2023 20:36
atorralba
atorralba reviewed Mar 17, 2023
View reviewed changes
Copy link
Contributor

@atorralba atorralba left a 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.

@egregius313 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
saritai reviewed Mar 23, 2023
View reviewed changes
Copy link
Contributor

@saritai saritai left a 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!

@egregius313
Copy link
Contributor Author

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

@egregius313 egregius313 requested review from a team as code owners March 27, 2023 16:11
@egregius313 egregius313 requested a review from a team March 27, 2023 16:11
@egregius313 egregius313 requested review from a team as code owners March 27, 2023 16:11
egregius313 and others added 10 commits March 27, 2023 12:16
@egregius313 egregius313 force-pushed the egregius313/promote-insecure-ldap-authentication branch from 3282353 to 106e5e7 Compare March 27, 2023 16:16
saritai
saritai previously approved these changes Mar 27, 2023
View reviewed changes
Copy link
Contributor

@saritai saritai left a 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! 👍

atorralba
atorralba reviewed Mar 28, 2023
View reviewed changes
Copy link
Contributor

@atorralba atorralba left a 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!

@egregius313 @atorralba
Make configuration public
97ec808 
Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>
@egregius313 egregius313 merged commit b00104e into github:main Mar 28, 2023
@egregius313 egregius313 deleted the egregius313/promote-insecure-ldap-authentication branch March 28, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saritai saritai saritai left review comments

+1 more reviewer

@atorralba atorralba atorralba approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
documentation Java ready-for-doc-review This PR requires and is ready for review from the GitHub docs team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@egregius313 @atorralba @saritai