Support LDAP authentication between nodes by anusudarsan · Pull Request #10317 · prestodb/presto

anusudarsan · 2018-04-03T12:13:56Z

No description provided.

dain · 2018-04-04T02:14:10Z

When JTW authentication (a.k.a bearer tokens) lands, I think that would be another good (and super easy) way to setup authentication between nodes.... basically everything can be secured with a shared secret, or pre-signed authentication tokens.

arhimondr · 2018-04-05T22:47:55Z

LDAP authentication for the inner communication was rather a hack to overcome the same security for all endpoints problem. Without this patch it is not possible to enable both, LDAP client authentication and encrypted communication, because there is no concept of separate security settings for client and server.

The better way of implementing this is to segregate Presto endpoints to presto client -> presto server and presto server -> presto server. So than LDAP client authentication could be enabled independently with internal communication settings.

arhimondr · 2018-04-05T23:02:12Z

...word-authenticators/src/main/java/com/facebook/presto/password/LdapAuthenticatorFactory.java

It will bind the filter only to an app context you are trying to bootstrap here. To make it work you should bind this filter to every single http client in Presto core. This is totally different Guice context. And i'm not sure if it is available from inside of a plugin.

arhimondr · 2018-04-05T23:07:51Z

presto-product-tests/conf/presto/etc/multinode-tls-ldap-master.properties

once you set it to false the tests are gonna start to fail due to the thing i explained above. But when the scheduling to a coordinator is enabled, despite it cannot communicate with workers the test will still pass, as there is a one node to run the queries

arhimondr · 2018-04-05T23:13:22Z

...word-authenticators/src/main/java/com/facebook/presto/password/LdapAuthenticatorFactory.java

It seems like an overkill to use Guice here. Just go with a PropertyManager and create a LdapAuthenticator manually. Make sure you check all the properties are being used. That is the functionallity that comes with a Bootstrap

anusudarsan · 2018-04-06T22:13:42Z

presto-main/src/main/java/com/facebook/presto/server/ServerMainModule.java

This can be moved to edca727 and refactored once the Module is merged

arhimondr · 2018-04-11T18:11:50Z

presto-product-tests/conf/presto/etc/log.properties

Why do we change that?

The travis log was getting bigger at times, and the travis stage failed due to https://stackoverflow.com/questions/26082444/how-to-work-around-travis-cis-4mb-output-limit. There is no known good workaround for this travis issue. Now that we run multinode tests in a new stage maybe this is not necessary. But having it in WARN level is not too bad, I think.

Did you try to at least keep it at a default INFO level? INFO is a pretty commonly used logging level for printing out not so spammy important information. If INFO messages are over-verbose we should probably remove unnecessary logging places. In a separate PR of course =)

yup, changed it to INFO and it looks fine. DEBUG was logging all the jars used in the plugins. I dont think we kept it at DEBUG on purpose.

arhimondr · 2018-04-11T18:14:48Z

LGTM

Add hidden -P presto-cli parameter that allows to specify password in non-interactive way. The parameter is hidden and supposed to be used for testing purposes only.

anusudarsan · 2018-07-20T22:39:26Z

@arhimondr I rebased this on master. Can you merge this too?

Add a new test profile for multinode tests. Make PluginManager log at INFO level. Tests were failing intermittently with error log > 4MB.

anusudarsan · 2018-07-25T23:10:52Z

Now that we have support for multiple auth mechanisms, this hack is not needed. The solution is to have both LDAP and SSL authentication mechanisms enabled and have server-to-server communication authenticated with SSL client certificates.

ghost · 2018-09-13T20:38:48Z

My question is: How do I use LDAP and SSL authentication. After some research (2 weeks) on my version 0.198 I added this to the config.properties to make it work. Good luck.

http-server.authentication.type=PASSWORD,CERTIFICATE

anusudarsan requested review from arhimondr and electrum April 3, 2018 12:13

facebook-github-bot added the CLA Signed label Apr 3, 2018

anusudarsan mentioned this pull request Apr 3, 2018

Support Kerberos authentication between nodes #9683

Merged

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch from eb4aaf3 to f2d8bb3 Compare April 3, 2018 12:31

findepi changed the title ~~Support ldap authentication between nodes~~ Support LDAP authentication between nodes Apr 3, 2018

arhimondr requested changes Apr 5, 2018

View reviewed changes

arhimondr reviewed Apr 5, 2018

View reviewed changes

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch from f2d8bb3 to 2f1a848 Compare April 6, 2018 22:12

anusudarsan commented Apr 6, 2018

View reviewed changes

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch 4 times, most recently from 9db5908 to 9089ff9 Compare April 7, 2018 03:29

arhimondr reviewed Apr 11, 2018

View reviewed changes

arhimondr approved these changes Apr 11, 2018

View reviewed changes

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch 3 times, most recently from 3306485 to b507e74 Compare April 12, 2018 19:38

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch 2 times, most recently from 8945544 to 9f72d15 Compare June 15, 2018 15:51

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch from 9f72d15 to 8f93357 Compare July 5, 2018 14:29

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch from 8f93357 to b216721 Compare July 18, 2018 00:10

Andrii Rosa added 4 commits July 20, 2018 18:35

Non interactive presto-cli password

c27f477

Add hidden -P presto-cli parameter that allows to specify password in non-interactive way. The parameter is hidden and supposed to be used for testing purposes only.

Implement LDAP authentication for internal communication

ea1b99d

Document LDAP secured internal communication

f5b5bb2

Secured internal communication with LDAP product tests

5bdc561

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch from b216721 to 9a78541 Compare July 20, 2018 22:37

Reduce product-tests log size to fix travis failure

c8caf12

Add a new test profile for multinode tests. Make PluginManager log at INFO level. Tests were failing intermittently with error log > 4MB.

anusudarsan force-pushed the support_ldap_authentication_between_nodes branch from 9a78541 to c8caf12 Compare July 20, 2018 23:47

anusudarsan closed this Jul 25, 2018

Conversation

anusudarsan commented Apr 3, 2018

Uh oh!

dain commented Apr 4, 2018

Uh oh!

arhimondr commented Apr 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arhimondr commented Apr 11, 2018

Uh oh!

anusudarsan commented Jul 20, 2018

Uh oh!

anusudarsan commented Jul 25, 2018

Uh oh!

ghost commented Sep 13, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghost commented Sep 13, 2018 •

edited by ghost

Loading