Skip to content

Commit

Permalink
dcache-xrootd: fix delegation client lifecycle management
Browse files Browse the repository at this point in the history
Motivation:

In GplazmaAwareChannelHandlerFactoryFactoryBean, a single delegation client
is created and injected into LoginAuthenticationHandlerFactory, which in
turn passes it into XrootdAuthenticationHandler.  The XrootdAuthenticationHandler
calls ProxyDelegationClient#close when the handler is removed.

The problem with this is that the same ProxyDelegationClient instance
is used for all xrootd clients connecting to the door.  If no clients
have connected, its close() method is never called (preventing clean
shutdown of the dCache domain). Conversely, all clients see a closed client
after the first connection disconnects.

Modification:

Abstract out and maintain as a separate bean the shared components
of the client, and construct a new client per connection, passing
in the shared components.

This effectively means injecting the client factory, rather than
the client, in the LoginAuthenticationHandlerFactory.

The new component (GSIProxyDelegationProvider) is injected into
the GplazmaAware bean, and then passed into GSI client factory
from there.

Result:

dCache shuts down cleanly, and the client does not use
a closed vomsValidator after the first connection.

Target: master
Requires-book: no
Requires-notes: no
Acked-by: Paul
  • Loading branch information
alrossi committed Jun 4, 2019
1 parent 768ff98 commit 71723b8
Show file tree
Hide file tree
Showing 6 changed files with 237 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,76 @@

import io.netty.channel.ChannelHandler;

import java.util.Properties;

import org.dcache.auth.LoginStrategy;
import org.dcache.xrootd.plugins.AuthenticationFactory;
import org.dcache.xrootd.plugins.ChannelHandlerFactory;
import org.dcache.xrootd.plugins.InvalidHandlerConfigurationException;
import org.dcache.xrootd.plugins.ProxyDelegationClient;
import org.dcache.xrootd.plugins.ProxyDelegationClientFactory;
import org.dcache.xrootd.plugins.authn.none.NoAuthenticationFactory;

/**
* Wraps the AuthenticationHandlerFactory required by the door with login
* capabilities. Is configured by spring initialization and SPI to create
* an instance of the LoginAuthentciationHandler with a particular kind
* of delegation client (if any).
*/
public class LoginAuthenticationHandlerFactory implements ChannelHandlerFactory
{
private final String _name;
private final LoginStrategy _loginStrategy;
private final AuthenticationFactory _authenticationFactory;
private final ProxyDelegationClient _proxyDelegationClient;
private final String _authnPluginName;
private final String _clientName;
private final Properties _properties;
private final LoginStrategy _loginStrategy;
private final AuthenticationFactory _authenticationFactory;
private final ProxyDelegationClientFactory _proxyDelegationFactory;

/**
* Handles logins with no authentication.
* Properties and delegation client are unused.
*
* @param authnPluginName name of the authentication handler plugin
* @param loginStrategy
*/
public LoginAuthenticationHandlerFactory(String authnPluginName,
LoginStrategy loginStrategy)
{
this(authnPluginName,
null,
null,
null,
new NoAuthenticationFactory(),
loginStrategy);
}

public LoginAuthenticationHandlerFactory(String name,
/**
* @param authnPluginName name of the authentication handler plugin
* @param clientName name of the (type of) delegation client
* @param proxyDelegationFactory factory to use to construct delegation client
* @param properties passed in from the factory bean
* @param authenticationFactory used during login
* @param loginStrategy used for login
*/
public LoginAuthenticationHandlerFactory(String authnPluginName,
String clientName,
ProxyDelegationClientFactory proxyDelegationFactory,
Properties properties,
AuthenticationFactory authenticationFactory,
ProxyDelegationClient proxyDelegationClient,
LoginStrategy loginStrategy)
{
_name = name;
_authnPluginName = authnPluginName;
_clientName = clientName;
_proxyDelegationFactory = proxyDelegationFactory;
_properties = properties;
_authenticationFactory = authenticationFactory;
_proxyDelegationClient = proxyDelegationClient;
_loginStrategy = loginStrategy;
}

@Override
public String getName()
{
return _name;
return _authnPluginName;
}

@Override
Expand All @@ -41,7 +84,24 @@ public String getDescription()
public ChannelHandler createHandler()
{
return new LoginAuthenticationHandler(_authenticationFactory,
_proxyDelegationClient,
createClient(),
_loginStrategy);
}

private ProxyDelegationClient createClient()
{
if (_proxyDelegationFactory != null) {
try {
return _proxyDelegationFactory.createClient(_clientName,
_properties);
} catch (InvalidHandlerConfigurationException e) {
throw new IllegalArgumentException("Unable to create delegation "
+ "client "
+ _clientName
+ ": "
+ e.toString());
}
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,11 @@
*/
package org.dcache.xrootd.security;

import eu.emi.security.authn.x509.X509CertChainValidatorExt;
import com.google.common.base.Preconditions;
import eu.emi.security.authn.x509.X509Credential;
import org.globus.gsi.gssapi.jaas.GlobusPrincipal;
import org.italiangrid.voms.VOMSValidators;
import org.italiangrid.voms.ac.VOMSACValidator;
import org.italiangrid.voms.ac.VOMSValidationResult;
import org.italiangrid.voms.store.VOMSTrustStore;
import org.italiangrid.voms.store.VOMSTrustStores;
import org.italiangrid.voms.util.CertificateValidatorBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -38,9 +34,6 @@
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;

import org.dcache.auth.FQANPrincipal;
import org.dcache.gsi.KeyPairCache;
Expand Down Expand Up @@ -71,24 +64,13 @@ public class GSIProxyDelegationClient extends X509ProxyDelegationClient
private final Map<String, X509Delegation> delegations;
private final KeyPairCache keyPairCache;

GSIProxyDelegationClient(Properties properties)
GSIProxyDelegationClient(ProxyDelegationStore store)
{
delegations = new ConcurrentHashMap<>();
keyPairCache = new KeyPairCache(1L, TimeUnit.MINUTES);
String vomsDir = properties.getProperty("xrootd.gsi.vomsdir.dir");
String caCertificatePath = properties.getProperty("xrootd.gsi.ca.path");
long trustAnchorRefreshInterval
= Long.parseLong(properties.getProperty("xrootd.gsi.ca.refresh"));
VOMSTrustStore vomsTrustStore
= VOMSTrustStores.newTrustStore(asList(vomsDir));
X509CertChainValidatorExt certChainValidator
= new CertificateValidatorBuilder()
.lazyAnchorsLoading(false)
.trustAnchorsUpdateInterval(trustAnchorRefreshInterval)
.trustAnchorsDir(caCertificatePath)
.build();
vomsValidator = VOMSValidators.newValidator(vomsTrustStore,
certChainValidator);
Preconditions.checkNotNull(store, "GSIProxyDelegationClient "
+ "cannot be constructed with null store.");
vomsValidator = store.vomsValidator;
delegations = store.delegations;
keyPairCache = store.keyPairCache;
}

@Override
Expand All @@ -102,9 +84,7 @@ public void cancelProxyRequest(ProxyRequest proxyRequest)
@Override
public void close()
{
if (vomsValidator != null) {
vomsValidator.shutdown();
}
// NOP
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,16 @@ public class GSIProxyDelegationClientFactory implements
{
private static final String PROTOCOL = "gsi";

private ProxyDelegationStore gsiDelegationProvider;

@Override
public X509ProxyDelegationClient
createClient(String name, Properties properties) {
return PROTOCOL.equals(name) ?
new GSIProxyDelegationClient(properties) : null;
return PROTOCOL.equals(name) ?
new GSIProxyDelegationClient(gsiDelegationProvider) : null;
}

public void setProvider(ProxyDelegationStore gsiDelegationProvider) {
this.gsiDelegationProvider = gsiDelegationProvider;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/* dCache - http://www.dcache.org/
*
* Copyright (C) 2014 - 2017 Deutsches Elektronen-Synchrotron
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.dcache.xrootd.security;

import eu.emi.security.authn.x509.X509CertChainValidatorExt;
import org.italiangrid.voms.VOMSValidators;
import org.italiangrid.voms.ac.VOMSACValidator;
import org.italiangrid.voms.store.VOMSTrustStore;
import org.italiangrid.voms.store.VOMSTrustStores;
import org.italiangrid.voms.util.CertificateValidatorBuilder;
import org.springframework.beans.factory.annotation.Required;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.dcache.gsi.KeyPairCache;
import org.dcache.gsi.X509Delegation;

import static java.util.Arrays.asList;

public class ProxyDelegationStore
{
/*
* Visible to the client this is used to initialize.
*/
final Map<String, X509Delegation> delegations = new ConcurrentHashMap<>();

VOMSACValidator vomsValidator;
KeyPairCache keyPairCache;

private String vomsDir;
private String caCertificatePath;
private long trustAnchorRefreshInterval;

public void initialize()
{
VOMSTrustStore vomsTrustStore
= VOMSTrustStores.newTrustStore(asList(vomsDir));
X509CertChainValidatorExt certChainValidator
= new CertificateValidatorBuilder()
.lazyAnchorsLoading(false)
.trustAnchorsUpdateInterval(trustAnchorRefreshInterval)
.trustAnchorsDir(caCertificatePath)
.build();
vomsValidator = VOMSValidators.newValidator(vomsTrustStore,
certChainValidator);
}

@Required
public void setVomsDir(String vomsDir)
{
this.vomsDir = vomsDir;
}

@Required
public void setCaCertificatePath(String caCertificatePath)
{
this.caCertificatePath = caCertificatePath;
}

public void setKeyPairCache(KeyPairCache keyPairCache)
{
this.keyPairCache = keyPairCache;
}

@Required
public void setTrustAnchorRefreshInterval(long trustAnchorRefreshInterval)
{
this.trustAnchorRefreshInterval = trustAnchorRefreshInterval;
}

public void shutdown()
{
if (vomsValidator != null) {
vomsValidator.shutdown();
}
}
}
Loading

0 comments on commit 71723b8

Please sign in to comment.