NIFI-4382: Adding support for Apache Knox SSO by mcgilman · Pull Request #2177 · apache/nifi

mcgilman · 2017-09-26T16:28:49Z

Adding support for KnoxSSO.

High Level Description
This PR facilitates Apache Knox SSO. It can support both cases where the users directly access NiFi and simply use Knox SSO for authentication and where Knox is proxying access to NiFi.

In the direct access case when an unauthenticated user attempts to go to NiFi, NiFi will redirect the user to Knox log in. After successfully logging in, Knox will generate a JWT and put it in an HTTP Cookie and redirect back to NiFi. NiFi will consider the JWT and verify it by validating the signature, audience, and expiration. The signature is validated using a configured public key from Knox. Acceptable audiences can also be configured but are not required. If the JWT is validated, the user identity is extracted from the token.

In the proxying case when an unauthenticated user attempts to go to NiFi through Knox, Knox will redirect the user to Knox log in. After successfully logging in, Knox will generate a JWT and put it in an HTTP Cookie and redirect back to NiFi through Knox. If the requests from Knox to NiFi are made over two-way SSL the JWT in the HTTP Cookie is validated at Knox and the user identity will be placed in the proxied entities chain. When the requests from Knox to NiFi are made over one-way SSL, the Knox Cookie is relayed to the request to NiFi allowing NiFi to validate it with the same logic described above in the direct access case.

For the proxying case, the only new capabilities introduced here are also part of the direct access case (JWT token validation). Because of this, all of the capabilities should be able to be tested using direct NiFi access with Knox SSO.

Testing
To test Knox SSO when directly accessing NiFi, install both Knox and NiFi. Knox SSO configuration is described here [1]. In NiFi four new properties have been introduced:

#nifi properties example
nifi.security.user.knox.url=https://localhost:8443/gateway/knoxsso/api/v1/websso
nifi.security.user.knox.publicKey=<path-to>/gateway-identity.pem
nifi.security.user.knox.cookieName=hadoop-jwt
nifi.security.user.knox.audiences=

These properties assume that Knox is running locally on 8443 and NiFi is secured and running on another port. The publicKey can be exported from Knox using the following command:

<knox-install-dir>/bin/knoxcli.sh export-cert

The cookieName property would need to align with what is configured in Knox. The audience property is used to only accept tokens from a particular audience. The audience value is configured as part of Knox SSO [1].

[1] http://knox.apache.org/books/knox-0-13-0/user-guide.html#KnoxSSO+Configuration+Parameters

- Adding support for KnoxSSO.

jtstorck · 2017-09-26T17:45:33Z

I just ran a test with audiences set in Knox, and if there's a space after one of the commas in the KnoxSSO config, it will end up sending that space as part of the audience value. For example,

        <param>
           <name>knoxsso.token.audiences</name>
           <value>foo,bar, baz</value>
        </param>

With that config, NiFi will see three audiences, "foo", "bar", and " baz". Notice the space in front of baz.

I filed an Apache Knox JIRA for this issue: KNOX-1069

jtstorck · 2017-09-26T17:52:04Z

Add "/access/knox/**" to webSecurity.ignoring().antMatchers in NiFiWebApiSecurityConfiguration.java

alopresto · 2017-09-26T18:37:18Z

nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java

+     */
+    public Set<String> getKnoxAudiences() {
+        final String rawAudiences = getProperty(SECURITY_USER_KNOX_AUDIENCES);
+        if (StringUtils.isBlank(rawAudiences)) {


Does this need to be null or could it be an empty Set to allow for simpler iteration on the consuming end?

While it's not possible to realize the distinction when configuring the properties file and using isBlank, I typically use null for unset while an empty Set would indicate the value is configured with no values. I'm happy to make this change if you want me to.

When I made the comment, I didn't realize that intentionally setting this value to empty was valid. We can leave this as is.

alopresto · 2017-09-26T18:38:23Z

nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java

+     * Returns the path to the Knox public key.
+     *
+     * @return path to the Knox public key
+     */


Could this property name include the word Path so it's clear that the return value is not the key content (if the consumer doesn't read the API docs)?

alopresto · 2017-09-26T18:39:49Z

nifi-docs/src/main/asciidoc/administration-guide.adoc

+During Apache Knox authentication, NiFi will redirect users to login with Apache Knox before returning to NiFi. NiFi will verify the Apache Knox
+token during authentication.
+
+NOTE: NiFi can only be configured for username/password, OpenId Connect, or Apache Knox at a given time. It does not support running each of


Maybe explicitly note that "username/password" includes both LDAP and Kerberos.

I've updated the part in the guide where username/password is associated with the pluggable Login Identity Provider (a couple paragraphs above this NOTE) to include the supported options. Thereafter, its referred to as username/password.

alopresto · 2017-09-26T18:46:49Z

...i-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxAuthenticationFilter.java

+        }
+    }
+
+    public String getJwtFromCookie(final HttpServletRequest request) {


Not a big deal, but I could see this method being reused in the future, so accepting the cookieName as a parameter and providing it from the Knox method might be useful. Not a blocker for this PR though.

alopresto · 2017-09-26T18:48:39Z

...security/src/main/java/org/apache/nifi/web/security/knox/KnoxAuthenticationRequestToken.java

+    }
+
+    @Override
+    public String toString() {


The toString() doesn't contain any unique identifying information, so overriding this removes the hashcode that is usually present. Is this a security concern, or does it simply make debugging more difficult?

This toString() implementation was motivated by a recent change to the logging of the NiFi native JWT tokens here [1].

[1]

nifi/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/jwt/JwtAuthenticationRequestToken.java

Line 56 in 2c1f5b4

return "<JWT token>";

alopresto · 2017-09-26T18:49:39Z

.../nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxService.java

+    }
+
+    /**
+     * Returns whether OpenId Connect is enabled.


Think this might be a copy/paste error.

- Addressing PR review comments.

alopresto · 2017-09-26T19:34:51Z

I did not see the nifi.security.user.knox.* properties in the conf/nifi.properties file when I built this branch.

mcgilman · 2017-09-26T19:34:59Z

A new commit has been pushed that addresses the comments received thus far. Thanks for reviewing @jtstorck and @alopresto!

- Updating the docs for nifi.security.user.knox.audiences.

mcgilman · 2017-09-26T19:47:22Z

@alopresto I just rebuilt and the properties in conf/nifi.properties start on line 167. Can you please double check for them?

alopresto · 2017-09-26T21:42:34Z

@mcgilman the nifi.properties values are there; I was looking at the wrong file. Sorry for that.

I am encountering these checkstyle warnings though (they cause a build failure):

[INFO] --- maven-checkstyle-plugin:2.15:check (check-style) @ nifi-web-security ---
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[103] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[133] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[134] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[149] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[177] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[218] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 07:03 min
[INFO] Finished at: 2017-09-26T13:13:37-07:00
[INFO] Final Memory: 269M/2017M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (check-style) on project nifi-web-security: You have 6 Checkstyle violations. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :nifi-web-security

alopresto · 2017-09-26T22:44:03Z

Is there a way to sign out of NiFi once the Knox SSO cookie is present in the browser?

alopresto · 2017-09-26T23:21:37Z

Ok, here are my thoughts on this PR:

I got test failures on TestListenUDP when running the full build. I am assuming nothing here changed that so it's unrelated and I won't count it against this PR, but making a note here because I had not seen it on master.
There are checkstyle issues I noted above.
I validated the following use cases:
- Normal positive flow
- Bad username/password fails
- The Knox user authenticates but has no access policies on the canvas
- The Knox user JWT has no audiences and NiFi does not require any
- The Knox user JWT has no audiences and NiFi requires one
- The Knox user JWT has the wrong audience (i.e. not one required by NiFi)
- The Knox user JWT has the correct audience as required by NiFi
- The Knox user is logged in with one browser and another user is logged in via client certificate in another
Issues found:
- No way to logout of the Knox user through the NiFi UI (treated same as client certificate)
  - Workaround: Use browser to delete hadoop-jwt cookie
- If Knox SSO is configured to provide the wrong/insufficient audiences, the login UX simply immediately redirects to the Knox login UI with no error message
  - Workaround: the NiFi User Log (logs/nifi-user.log) contains a helpful error message

To me, neither issue is a blocker for this PR but I do think they should be resolved in a future release. +1, LGTM (with the checkstyle violation resolutions pending).

mcgilman · 2017-09-26T23:22:37Z

Do you think that we should support logout in this case? If yes, I’ll need to figure out the best way to handle this as currently logging out is entirely client side and I believe this would require some server side action.

alopresto · 2017-09-26T23:27:45Z

Yeah, I don't have deep enough Knox familiarity to judge the best use case for communicating back that the logout command has occurred. If we treated receiving the hadoop-jwt token from Knox the same way we did the credential check for LDAP or Kerberos, and issued our own JWT, deleting the local JWT would trigger re-validating the hadoop-jwt cookie. If we update the local key store to indicate that that specific JWT is no longer valid, I believe we could trigger a redirect to the Knox service. However, my understanding is that we cannot simply delete the hadoop-jwt cookie because other services rely on it for SSO, and I do not know what the Knox API is like to trigger a logout remotely. At this time, I do not have a good suggestion for this scenario (other than to clearly note it for administrators and users because it may be confusing).

mcgilman · 2017-09-27T16:41:38Z

Thanks again @alopresto for the review. I had observed the same behavior where the user is redirected back to the login page when the token could not be validated. I think a good path forward here may be to support multiple login mechanisms concurrently (ldap, oidc, and knox for instance). When this is the case, the NiFi login page would likely offer buttons for the user to initiate the desired SSO login sequence. The reason this is awkward currently is because the redirection happens automatically. This could also give us an opportunity to provide some sort of error message to the end user. Let's consider this option and the logout concern is separate follow on JIRAs.

alopresto · 2017-09-27T16:57:50Z

Thanks Matt. I agree that a "multiple login option splash page" would be a good feature when necessary -- old Stack Overflow comes to mind for me. Looking forward to this code getting in and the follow-on enhancements.

mcgilman · 2017-09-27T17:11:16Z

JIRAs for logout [1] and login splash screen [2].

[1] https://issues.apache.org/jira/browse/NIFI-4430
[2] https://issues.apache.org/jira/browse/NIFI-4431

- Addressing checkstyle issues. - Ensuring the cookie is removed prior to request replication.

jtstorck · 2017-09-27T20:25:31Z

@mcgilman Merged your PR to master. Thanks for the contribution! Also, thanks to @alopresto for reviewing!

NIFI-4382:

0f2b0ac

- Adding support for KnoxSSO.

alopresto reviewed Sep 26, 2017

View reviewed changes

NIFI-4382:

f2745f6

- Addressing PR review comments.

NIFI-4382:

5d83fc0

- Updating the docs for nifi.security.user.knox.audiences.

NIFI-4382:

d90ba61

- Addressing checkstyle issues. - Ensuring the cookie is removed prior to request replication.

asfgit closed this in 6c798d1 Sep 27, 2017

Conversation

mcgilman commented Sep 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtstorck commented Sep 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtstorck commented Sep 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

alopresto commented Sep 26, 2017

Uh oh!

mcgilman commented Sep 26, 2017

Uh oh!

mcgilman commented Sep 26, 2017

Uh oh!

alopresto commented Sep 26, 2017

Uh oh!

alopresto commented Sep 26, 2017

Uh oh!

alopresto commented Sep 26, 2017

Uh oh!

mcgilman commented Sep 26, 2017

Uh oh!

alopresto commented Sep 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcgilman commented Sep 27, 2017

Uh oh!

alopresto commented Sep 27, 2017

Uh oh!

mcgilman commented Sep 27, 2017

Uh oh!

jtstorck commented Sep 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mcgilman commented Sep 26, 2017 •

edited

Loading

jtstorck commented Sep 26, 2017 •

edited

Loading

jtstorck commented Sep 26, 2017 •

edited

Loading

alopresto commented Sep 26, 2017 •

edited

Loading