Repository: nifi Updated Branches: refs/heads/master e916594b6 -> 91f40febe
NIFI-4925: - Addressing memory leak from lingering authorization results that did not represent actual access attempts. This closes #2511. Signed-off-by: Mark Payne <marka...@hotmail.com> Project: http://git-wip-us.apache.org/repos/asf/nifi/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/91f40feb Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/91f40feb Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/91f40feb Branch: refs/heads/master Commit: 91f40febebe7c199272375b02299bf2f678b123b Parents: e916594 Author: Matt Gilman <matt.c.gil...@gmail.com> Authored: Fri Mar 2 16:24:34 2018 -0500 Committer: Mark Payne <marka...@hotmail.com> Committed: Mon Mar 5 15:37:31 2018 -0500 ---------------------------------------------------------------------- .../AuthorizationAuditorInvocationHandler.java | 47 ++++++++++++++++++++ .../nifi/authorization/AuthorizerFactory.java | 20 +++++++-- .../authorization/AuthorizerFactoryBean.java | 4 +- .../authorization/AuthorizerFactoryTest.java | 2 + .../authorization/RangerNiFiAuthorizer.java | 24 +++++----- 5 files changed, 83 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi/blob/91f40feb/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationAuditorInvocationHandler.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationAuditorInvocationHandler.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationAuditorInvocationHandler.java new file mode 100644 index 0000000..7f8d76c --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationAuditorInvocationHandler.java @@ -0,0 +1,47 @@ +/* + * 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.authorization; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +public class AuthorizationAuditorInvocationHandler implements InvocationHandler { + + private final Authorizer authorizer; + private final AuthorizationAuditor auditor; + + public AuthorizationAuditorInvocationHandler(final Authorizer authorizer, final AuthorizationAuditor auditor) { + this.authorizer = authorizer; + this.auditor = auditor; + } + + @Override + public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { + try { + if (AuthorizationAuditor.class.getMethod("auditAccessAttempt", AuthorizationRequest.class, AuthorizationResult.class).equals(method)) { + return method.invoke(auditor, args); + } else { + return method.invoke(authorizer, args); + } + } catch (final InvocationTargetException e) { + // If the proxied instance throws an Exception, it'll be wrapped in an InvocationTargetException. We want + // to instead re-throw what the proxied instance threw, so we pull it out of the InvocationTargetException. + throw e.getCause(); + } + } +} http://git-wip-us.apache.org/repos/asf/nifi/blob/91f40feb/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java index 915bff0..812ea17 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java @@ -101,9 +101,11 @@ public final class AuthorizerFactory { } public static Authorizer installIntegrityChecks(final Authorizer baseAuthorizer) { + Authorizer authorizer; + if (baseAuthorizer instanceof ManagedAuthorizer) { final ManagedAuthorizer baseManagedAuthorizer = (ManagedAuthorizer) baseAuthorizer; - return new ManagedAuthorizer() { + authorizer = new ManagedAuthorizer() { @Override public String getFingerprint() throws AuthorizationAccessException { return baseManagedAuthorizer.getFingerprint(); @@ -395,7 +397,7 @@ public final class AuthorizerFactory { } }; } else { - return new Authorizer() { + authorizer = new Authorizer() { @Override public AuthorizationResult authorize(AuthorizationRequest request) throws AuthorizationAccessException { final AuthorizationResult result = baseAuthorizer.authorize(request); @@ -422,6 +424,19 @@ public final class AuthorizerFactory { } }; } + + // conditionally add support for the audit methods + if (baseAuthorizer instanceof AuthorizationAuditor) { + final AuthorizationAuditorInvocationHandler invocationHandler = new AuthorizationAuditorInvocationHandler(authorizer, (AuthorizationAuditor) baseAuthorizer); + + final List<Class<?>> interfaceList = ClassUtils.getAllInterfaces(authorizer.getClass()); + interfaceList.add(AuthorizationAuditor.class); + final Class<?>[] interfaces = interfaceList.toArray(new Class<?>[interfaceList.size()]); + + authorizer = (Authorizer) Proxy.newProxyInstance(authorizer.getClass().getClassLoader(), interfaces, invocationHandler); + } + + return authorizer; } /** @@ -433,7 +448,6 @@ public final class AuthorizerFactory { public static Authorizer withNarLoader(final Authorizer baseAuthorizer, final ClassLoader classLoader) { final AuthorizerInvocationHandler invocationHandler = new AuthorizerInvocationHandler(baseAuthorizer, classLoader); - // extract all interfaces... baseAuthorizer is non null so getAllInterfaces is non null final List<Class<?>> interfaceList = ClassUtils.getAllInterfaces(baseAuthorizer.getClass()); final Class<?>[] interfaces = interfaceList.toArray(new Class<?>[interfaceList.size()]); http://git-wip-us.apache.org/repos/asf/nifi/blob/91f40feb/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java index 7ece8eb..79d3757 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java @@ -172,6 +172,8 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG // ensure it was found if (authorizer == null) { throw new Exception(String.format("The specified authorizer '%s' could not be found.", authorizerIdentifier)); + } else { + authorizer = AuthorizerFactory.installIntegrityChecks(authorizer); } } } @@ -350,7 +352,7 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG authorizerClassLoader = new URLClassLoader(urls, authorizerClassLoader); } - return AuthorizerFactory.installIntegrityChecks(AuthorizerFactory.withNarLoader(instance, authorizerClassLoader)); + return AuthorizerFactory.withNarLoader(instance, authorizerClassLoader); } private AuthorizerConfigurationContext loadAuthorizerConfiguration(final String identifier, final List<Property> properties) { http://git-wip-us.apache.org/repos/asf/nifi/blob/91f40feb/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java index caae13f..167a7d4 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java @@ -289,6 +289,8 @@ public class AuthorizerFactoryTest { Authorizer authorizer = AuthorizerFactory.installIntegrityChecks(mockAuthorizer); authorizer.onConfigured(context); + assertTrue(authorizer instanceof AuthorizationAuditor); + final AuthorizationRequest accessAttempt = new AuthorizationRequest.Builder() .resource(new MockResource("resource1", "Resource 1")) .identity("user-1") http://git-wip-us.apache.org/repos/asf/nifi/blob/91f40feb/nifi-nar-bundles/nifi-ranger-bundle/nifi-ranger-plugin/src/main/java/org/apache/nifi/ranger/authorization/RangerNiFiAuthorizer.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-ranger-bundle/nifi-ranger-plugin/src/main/java/org/apache/nifi/ranger/authorization/RangerNiFiAuthorizer.java b/nifi-nar-bundles/nifi-ranger-bundle/nifi-ranger-plugin/src/main/java/org/apache/nifi/ranger/authorization/RangerNiFiAuthorizer.java index 8b98d53..a49887b 100644 --- a/nifi-nar-bundles/nifi-ranger-bundle/nifi-ranger-plugin/src/main/java/org/apache/nifi/ranger/authorization/RangerNiFiAuthorizer.java +++ b/nifi-nar-bundles/nifi-ranger-bundle/nifi-ranger-plugin/src/main/java/org/apache/nifi/ranger/authorization/RangerNiFiAuthorizer.java @@ -46,9 +46,9 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.net.MalformedURLException; import java.util.Date; +import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.WeakHashMap; /** * Authorizer implementation that uses Apache Ranger to make authorization decisions. @@ -71,7 +71,7 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor { static final String HADOOP_SECURITY_AUTHENTICATION = "hadoop.security.authentication"; static final String KERBEROS_AUTHENTICATION = "kerberos"; - private final ConcurrentMap<AuthorizationRequest, RangerAccessResult> resultLookup = new ConcurrentHashMap<>(); + private final Map<AuthorizationRequest, RangerAccessResult> resultLookup = new WeakHashMap<>(); private volatile RangerBasePluginWithPolicies nifiPlugin = null; private volatile RangerDefaultAuditHandler defaultAuditHandler = null; @@ -175,10 +175,14 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor { final RangerAccessResult result = nifiPlugin.isAccessAllowed(rangerRequest); - if (result != null && result.getIsAllowed()) { - // store the result for auditing purposes later - resultLookup.put(request, result); + // store the result for auditing purposes later if appropriate + if (request.isAccessAttempt()) { + synchronized (resultLookup) { + resultLookup.put(request, result); + } + } + if (result != null && result.getIsAllowed()) { // return approved return AuthorizationResult.approved(); } else { @@ -192,9 +196,6 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor { logger.debug(String.format("Unable to authorize %s due to %s", identity, reason)); } - // store the result for auditing purposes later - resultLookup.put(request, result); - // a policy does exist for the resource so we were really denied access here return AuthorizationResult.denied(request.getExplanationSupplier().get()); } else { @@ -206,7 +207,10 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor { @Override public void auditAccessAttempt(final AuthorizationRequest request, final AuthorizationResult result) { - final RangerAccessResult rangerResult = resultLookup.remove(request); + final RangerAccessResult rangerResult; + synchronized (resultLookup) { + rangerResult = resultLookup.remove(request); + } if (rangerResult != null && rangerResult.getIsAudited()) { AuthzAuditEvent event = defaultAuditHandler.getAuthzEvents(rangerResult);
