Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2047#discussion_r130757944
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/oidc/OidcService.java
---
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.oidc;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.nimbusds.oauth2.sdk.AuthorizationGrant;
+import com.nimbusds.oauth2.sdk.Scope;
+import com.nimbusds.oauth2.sdk.id.State;
+import org.apache.nifi.web.security.util.CacheKey;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.net.URI;
+import java.security.SecureRandom;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import static
org.apache.nifi.web.security.oidc.StandardOidcIdentityProvider.OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED;
+
+/**
+ * OidcService is a service for managing the OpenId Connect Authorization
flow.
+ */
+public class OidcService {
+
+ private OidcIdentityProvider identityProvider;
+ private Cache<CacheKey, State> stateLookupForPendingRequests; //
identifier from cookie -> state value
+ private Cache<CacheKey, String> jwtLookupForCompletedRequests; //
identifier from cookie -> jwt or identity (and generate jwt on retrieval)
+
+ /**
+ * Creates a new OtpService with an expiration of 5 minutes.
+ */
+ public OidcService(final OidcIdentityProvider identityProvider) {
+ this(identityProvider, 60, TimeUnit.SECONDS);
+ }
+
+ /**
+ * Creates a new OtpService.
+ *
+ * @param duration The expiration duration
+ * @param units The expiration units
+ * @throws NullPointerException If units is null
+ * @throws IllegalArgumentException If duration is negative
+ */
+ public OidcService(final OidcIdentityProvider identityProvider, final
int duration, final TimeUnit units) {
+ this.identityProvider = identityProvider;
+ this.stateLookupForPendingRequests =
CacheBuilder.newBuilder().expireAfterWrite(duration, units).build();
+ this.jwtLookupForCompletedRequests =
CacheBuilder.newBuilder().expireAfterWrite(duration, units).build();
+ }
+
+ /**
+ * Returns whether OpenId Connect is enabled.
+ *
+ * @return whether OpenId Connect is enabled
+ */
+ public boolean isOidcEnabled() {
+ return identityProvider.isOidcEnabled();
+ }
+
+ /**
+ * Returns the OpenId Connect authorization endpoint.
+ *
+ * @return the authorization endpoint
+ */
+ public URI getAuthorizationEndpoint() {
+ return identityProvider.getAuthorizationEndpoint();
+ }
+
+ /**
+ * Returns the OpenId Connect scope.
+ *
+ * @return scope
+ */
+ public Scope getScope() {
+ return identityProvider.getScope();
+ }
+
+ /**
+ * Returns the OpenId Connect client id.
+ *
+ * @return client id
+ */
+ public String getClientId() {
+ return identityProvider.getClientId().getValue();
+ }
+
+ /**
+ * Initiates an OpenId Connection authorization code flow using the
specified request identifier to maintain state.
+ *
+ * @param oidcRequestIdentifier request identifier
+ * @return state
+ */
+ public State createState(final String oidcRequestIdentifier) {
+ if (!isOidcEnabled()) {
+ throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED);
+ }
+
+ final CacheKey oidcRequestIdentifierKey = new
CacheKey(oidcRequestIdentifier);
+ final State state = new State(new BigInteger(130, new
SecureRandom()).toString(32));
+
+ try {
+ synchronized (stateLookupForPendingRequests) {
+ final State cachedState =
stateLookupForPendingRequests.get(oidcRequestIdentifierKey, () -> state);
+ if (!state.equals(cachedState)) {
+ throw new IllegalStateException("An existing login
request is already in progress.");
+ }
+ }
+ } catch (ExecutionException e) {
+ throw new IllegalStateException("Unable to store the login
request state.");
+ }
+
+ return state;
+ }
+
+ /**
+ * Validates the purposed state with the given request identifier.
Will return false if the
+ * state does not match or if entry for this request identifier has
expired.
+ *
+ * @param oidcRequestIdentifier request identifier
+ * @param purposedState purposed state
+ * @return whether the state is valid or not
+ */
+ public boolean isStateValid(final String oidcRequestIdentifier, final
State purposedState) {
+ if (!isOidcEnabled()) {
+ throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED);
+ }
+
+ final CacheKey oidcRequestIdentifierKey = new
CacheKey(oidcRequestIdentifier);
+
+ synchronized (stateLookupForPendingRequests) {
+ final State state =
stateLookupForPendingRequests.getIfPresent(oidcRequestIdentifierKey);
+ if (state != null) {
+
stateLookupForPendingRequests.invalidate(oidcRequestIdentifierKey);
+ }
+
+ return state != null && state.equals(purposedState);
--- End diff --
I looked at
[`State#equals()`](https://www.javadoc.io/doc/com.nimbusds/oauth2-oidc-sdk/5.34)
and it uses the standard Java `String#equals()` method which short-circuits on
inequality. This leaks information and could lead to a timing attack
vulnerability. We should use a [constant-time equality
comparison](https://codahale.com/a-lesson-in-timing-attacks/) (i.e. iterate
over every byte and use XOR, comparing the final result to `0`). One is readily
available in [`MessageDigest.isEqual(byte[] a, byte[]
b)`](https://docs.oracle.com/javase/8/docs/api/java/security/MessageDigest.html#isEqual-byte:A-byte:A-)
or can be implemented and encapsulated ourselves in either the `CryptoUtils`
or `CipherUtility` class.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
