[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/534 VOTE: +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 ok @vtslab i won't do the work on merge then. i'm fine with letting it happen with the gremlin-python work. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user vtslab commented on the issue: https://github.com/apache/tinkerpop/pull/534 A good catch by @robertdale indeed, about the bytecode requests. I was not even aware those were human readable. A proposal: include the audit logging of bytecode requests in a new Jira ticket that addresses authentication using gremlin-python. This, because the integrate test setup will look similar. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 ooo - well noted @robertdale - i'm fine with getting that added to `TraversalOpProcessor` at a later date, but it shouldn't be forgotten. maybe i'll just add that in when i go to merge this. thanks for the review. just one more vote needed! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/534 Tested -secure and non-secure configurations with http/console and server logs audit trail. Note that the string-based requests are logged but bytecode is not. Possibly a future enhancement. VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user vtslab commented on the issue: https://github.com/apache/tinkerpop/pull/534 Thanks, Stephen, for guiding me through this so far. Once this gets merged, I'd like to take a look at authentication with the python driver. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 I sense that there could be some changes to LICENSE/NOTICE as a result of this work. I will make changes to those files as necessary on merge. In the mean time, the rest of this looks pretty nice. Good tests, nice documentation updates. All tests pass with `docker/build.sh -t -n -i` VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user mike-tr-adamson commented on the issue: https://github.com/apache/tinkerpop/pull/534 I'm good with what's now there. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 sorry - i was good with whatever @mike-tr-adamson was down for. he originally brought in the sasl stuff for tinkerpop, so i was glad he could take a moment to review and comment on this. if he's good with what's here now, we can have committers start to review things. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user vtslab commented on the issue: https://github.com/apache/tinkerpop/pull/534 I interpreted almosta week of silence as consensus on the gremlin-driver behavior, so I made the following changes: - restored gremlin-driver's Handler (checkout from master) - added a ToDo to gremlin-driver's Handler regarding receiving allowed mechanisms from gremlin-server - added GSSException fail options to three tests that required so - corrected the changelog --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user mike-tr-adamson commented on the issue: https://github.com/apache/tinkerpop/pull/534 Hi @vtslab, the majority of SASL mechanisms that are suitable for this form of authentication require some form of credential, be it token or certificate, at the client end. I agree that these are best approached separately. > In your case, with javax.security.auth.useSubjectCredsOnly=false configured, you would have a Gremlin-> Server with a Krb5Authenticator configured, the server would provide the GSSAPI token in its > authentication request and gremlin-driver would know to select the GSSAPI mechanism. The ideal solution, in my mind anyway, would be for the server to announce which mechanism it wants the client to use. This would allow for a quick fail on the client side if any credentials required by the mechanism weren't available. It ought to be quite straightforward to add this to the authenticate response from the server. It would certainly take away some of the guessing. I'm not suggesting that this change is done here but may be something for a future enhancement to the authentication. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user vtslab commented on the issue: https://github.com/apache/tinkerpop/pull/534 Hi @mike-tr-adamson, I am glad you entered the discussion. I think your main point is valid, namely that there are circumstances, pointed out by you, when gremlin-driver should select the GSSAPI mechanism even though no JAAS_ENTRY is specified (ToDo: make a test for this to safeguard the desired behavior). Having said this, the old behavior (select GSSAPI out of the blue if no username/password is supplied) also has its risks and problems given the multitude of SASL mechanisms that people could want to use, see [http://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml](http://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml). Ideally, one would want gremlin-server to provide a token with the mechanism(s) it supports, so that gremlin-driver can use this to instantiate the SaslClient properly. In your case, with `javax.security.auth.useSubjectCredsOnly=false` configured, you would have a Gremlin-Server with a Krb5Authenticator configured, the server would provide the GSSAPI token in its authentication request and gremlin-driver would know to select the GSSAPI mechanism. However, this ideal situation requires more changes to the gremlin-driver and gremlin-server code. I could live now with adding the GSSException as an option to the tests with your explanation how it could be a valid option. This solves the current challenge and we can add this discussion as comments to the code for future reference, when requirements for other SASL mechanisms pop up. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user mike-tr-adamson commented on the issue: https://github.com/apache/tinkerpop/pull/534 I'm concerned about changing the `getMechanism` logic based on the failing tests. The tests can legitimately fail both ways. It would be valid to fix those tests so that they can fail on either a `ResponseException` or a `GSSException`. As has already been said the difference in error being whether the test finds any kerberos configuration that it can use. The tests were failing on my machine for exactly the same reason - I had an `/etc/krb5.conf` file. I'm not that hung up on the change because you have to provide a `JAAS_ENTRY` in order to use kerberos. It's more of a principal thing. The tests were failing on my machine prior to this PR so the failure was not related to any of these changes. As such they could be fixed outside of this PR in which case the `getMechanism` change wouldn't be needed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user vtslab commented on the issue: https://github.com/apache/tinkerpop/pull/534 OK, thanks, I'll see to it. The downside of the third option was implicit: it changes the driver code which is in production everywhere. But that´s why we do this work in the 3.3.x line and I think it will be an imrpovement. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 it sounds like the third option comes without downsides (you had detracting statements on the other two) so I guess I would go with that. While i haven't thought it all through, this behavior: > now chooses GSSAPI if no credentials are supplied sounds like it might not be a sensible default either, so changing that sounds like the right thing to do. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user vtslab commented on the issue: https://github.com/apache/tinkerpop/pull/534 Hi, I am working on the two failing integration tests. It can be fixed in three ways: 1. just hide the symptoms and also allow a GSSException for a test that should fail anyway: ugly! 2. configure false for the failsafe plugin: fine, but may impact test performance 3. adapt the gremlin-driver handler code, which now chooses GSSAPI if no credentials are supplied. This should be a test on an available JaasEntry. Which of the ones do you like to see committed on the PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 Well - looks like they are still broken. My favorite situation - they fail with maven but pass in intellij. I suppose that has something to do with your explanation regarding the classpath. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 I'm not sure if TINKERPOP-1600 fixed it or if you did something in rebase, but the two failing tests dont' seem to be a problem anymore when I run the tests in intellij. As a result, I'm running the full integration tests now to make sure things are solid. After that I'll give a more detailed review of the code and hopefully we can get this merged soon. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/534 Sorry - I'd forgotten you'd mentioned the broken tests. Ok - we'll figure out what to do there once you rebase. Thanks for renaming the PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...
Github user vtslab commented on the issue: https://github.com/apache/tinkerpop/pull/534 Yes, I mentioned these failing tests in the PR text above. I suspect that the java security libs just pick up the krb5.conf file from the test resources "without asking". It means the test fails faster and tries to find credentials elsewhere when not provided. To resolve it, we either have to adapt the test or we have to try to configure Kerberos without a krb5.conf file (if that is the cause). Either way, it does not file nice if test outcomes depend on unnecessary components being on the classpath or not, so maybe we should look for a third way. PR title should be OK now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
