ftp: Reuse DssContxtFactory

Motivation:

The DssContextFactory is a heavy weight object to load as it involves
loading credentials and certificates from disk.

Modification:

Moves the creation of the factory into the FtpInterpreterFactory subclass,
thus allowing the DssContextFactory to be reused by all FTP sessions.

Result:

Less per session overhead.

Target: trunk
Require-notes: yes
Require-book: no
Acked-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
Patch: https://rb.dcache.org/r/8638/

modules/dcache-ftp/src/main/java/diskCacheV111/doors/LineBasedDoorProvider.java

@@ -1,7 +1,6 @@
 package diskCacheV111.doors;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import diskCacheV111.util.ConfigurationException;
 
 import dmg.cells.services.login.LoginCellFactory;
 import dmg.cells.services.login.LoginCellProvider;
@@ -10,8 +9,6 @@
 
 public class LineBasedDoorProvider implements LoginCellProvider
 {
-    private static final Logger LOGGER = LoggerFactory.getLogger(LineBasedDoorProvider.class);
-
     @Override
     public int getPriority(String name)
     {
@@ -34,9 +31,9 @@ public LoginCellFactory createFactory(String name, Args args, String parentCellN
                 factory.configure(args);
                 return new LineBasedDoorFactory(factory, args, parentCellName);
             }
-        } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
-            LOGGER.error("Failed to instantiate interpreter factory: {}", e.toString());
+            throw new IllegalArgumentException("Not a LineBasedInterpreterFactory: " + interpreter);
+        } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | ConfigurationException e) {
+            throw new IllegalArgumentException("Failed to instantiate interpreter factory: " + e.toString(), e);
         }
-        throw new IllegalArgumentException();
     }
 }

modules/dcache-ftp/src/main/java/diskCacheV111/doors/LineBasedInterpreterFactory.java

@@ -19,13 +19,15 @@
 
 import java.util.concurrent.Executor;
 
+import diskCacheV111.util.ConfigurationException;
+
 import dmg.cells.nucleus.CellEndpoint;
 import dmg.util.StreamEngine;
 
 import org.dcache.util.Args;
 
 public interface LineBasedInterpreterFactory
 {
-    void configure(Args args);
+    void configure(Args args) throws ConfigurationException;
     LineBasedInterpreter create(CellEndpoint endpoint, StreamEngine engine, Executor executor) throws Exception;
 }

modules/dcache-ftp/src/main/java/org/dcache/ftp/door/FtpInterpreterFactory.java

@@ -22,6 +22,7 @@
 
 import diskCacheV111.doors.LineBasedInterpreter;
 import diskCacheV111.doors.LineBasedInterpreterFactory;
+import diskCacheV111.util.ConfigurationException;
 
 import dmg.cells.nucleus.CellEndpoint;
 import dmg.util.StreamEngine;
@@ -31,12 +32,12 @@
 
 public abstract class FtpInterpreterFactory implements LineBasedInterpreterFactory
 {
-    private OptionParser options;
+    protected OptionParser options;
 
-    protected abstract AbstractFtpDoorV1 createInterpreter();
+    protected abstract AbstractFtpDoorV1 createInterpreter() throws Exception;
 
     @Override
-    public void configure(Args args)
+    public void configure(Args args) throws ConfigurationException
     {
         options = new OptionParser(args);
     }

modules/dcache-ftp/src/main/java/org/dcache/ftp/door/GsiFtpDoorV1.java

@@ -1,57 +1,21 @@
 package org.dcache.ftp.door;
 
-import org.dcache.dss.DssContextFactory;
-
 import javax.security.auth.Subject;
 
 import org.dcache.auth.Subjects;
-import org.dcache.util.Option;
-import org.dcache.util.Crypto;
-import org.dcache.dss.GsiEngineDssContextFactory;
+import org.dcache.dss.DssContextFactory;
 import org.dcache.util.NetLoggerBuilder;
 
 public class GsiFtpDoorV1 extends GssFtpDoorV1
 {
-    @Option(
-        name="service-key",
-        required=true
-    )
-    protected String service_key;
-
-    @Option(
-        name="service-cert",
-        required=true
-    )
-    protected String service_cert;
-
-    @Option(
-        name="service-trusted-certs",
-        required=true
-    )
-    protected String service_trusted_certs;
-
-    @Option(
-            name="gridftp.security.ciphers",
-            required=true
-    )
-    protected String cipherFlags;
-
-    public GsiFtpDoorV1()
+    public GsiFtpDoorV1(DssContextFactory dssContextFactory)
     {
-        super("GSI FTP", "gsiftp", "gsi");
+        super("GSI FTP", "gsiftp", "gsi", dssContextFactory);
     }
 
     @Override
     protected void logSubject(NetLoggerBuilder log, Subject subject)
     {
         log.add("user.dn", Subjects.getDn(subject));
     }
-
-    @Override
-    protected DssContextFactory createFactory() throws Exception
-    {
-        return new GsiEngineDssContextFactory(service_key, service_cert, service_trusted_certs,
-                                              Crypto.getBannedCipherSuitesFromConfigurationValue(cipherFlags));
-    }
-
 }

modules/dcache-ftp/src/main/java/org/dcache/ftp/door/GsiFtpInterpreterFactory.java

@@ -17,11 +17,54 @@
  */
 package org.dcache.ftp.door;
 
+import diskCacheV111.util.ConfigurationException;
+
+import org.dcache.dss.GsiEngineDssContextFactory;
+import org.dcache.util.Args;
+import org.dcache.util.Crypto;
+import org.dcache.util.Option;
+
 public class GsiFtpInterpreterFactory extends FtpInterpreterFactory
 {
+    @Option(name="service-key",
+            required=true)
+    protected String service_key;
+
+    @Option(name="service-cert",
+            required=true)
+    protected String service_cert;
+
+    @Option(name="service-trusted-certs",
+            required=true)
+    protected String service_trusted_certs;
+
+    @Option(name="gridftp.security.ciphers",
+            required=true)
+    protected String cipherFlags;
+
+    private GsiEngineDssContextFactory dssContextFactory;
+
+    @Override
+    public void configure(Args args) throws ConfigurationException
+    {
+        super.configure(args);
+        options.inject(this);
+        try {
+            dssContextFactory = getDssContextFactory();
+        } catch (Exception e) {
+            throw new ConfigurationException("Failed to create security context:" + e.getMessage(), e);
+        }
+    }
+
     @Override
     protected AbstractFtpDoorV1 createInterpreter()
     {
-        return new GsiFtpDoorV1();
+        return new GsiFtpDoorV1(dssContextFactory);
+    }
+
+    protected GsiEngineDssContextFactory getDssContextFactory() throws Exception
+    {
+        return new GsiEngineDssContextFactory(service_key, service_cert, service_trusted_certs,
+                                              Crypto.getBannedCipherSuitesFromConfigurationValue(cipherFlags));
     }
 }

modules/dcache-ftp/src/main/java/org/dcache/ftp/door/GssFtpDoorV1.java

@@ -38,19 +38,13 @@ public abstract class GssFtpDoorV1 extends AbstractFtpDoorV1
     protected String gssFlavor;
 
     protected DssContext context;
-    private DssContextFactory factory;
+    private DssContextFactory dssContextFactory;
 
-    public GssFtpDoorV1(String ftpDoorName, String tlogName, String gssFlavor)
+    public GssFtpDoorV1(String ftpDoorName, String tlogName, String gssFlavor, DssContextFactory dssContextFactory)
     {
         super(ftpDoorName, tlogName);
         this.gssFlavor = gssFlavor;
-    }
-
-    @Override
-    public void init() throws Exception
-    {
-        super.init();
-        factory = createFactory();
+        this.dssContextFactory = dssContextFactory;
     }
 
     @Override
@@ -81,7 +75,7 @@ public void ftp_auth(String arg)
         }
 
         try {
-            context = factory.create(_remoteSocketAddress, _localSocketAddress);
+            context = dssContextFactory.create(_remoteSocketAddress, _localSocketAddress);
         } catch (IOException e) {
             LOGGER.error(e.toString());
             reply("500 Error: " + e.toString());
@@ -267,6 +261,4 @@ public void ftp_pass(String arg)
             reply("500 Send USER first");
         }
     }
-
-    protected abstract DssContextFactory createFactory() throws Exception;
 }

modules/dcache-ftp/src/main/java/org/dcache/ftp/door/KerberosFtpDoorV1.java

@@ -1,61 +1,21 @@
 package org.dcache.ftp.door;
 
-import org.dcache.dss.DssContextFactory;
-import org.dcache.dss.KerberosDssContextFactory;
-import org.ietf.jgss.GSSException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import javax.security.auth.Subject;
 
-import java.io.IOException;
-
 import org.dcache.auth.Subjects;
-import org.dcache.util.Option;
+import org.dcache.dss.DssContextFactory;
 import org.dcache.util.NetLoggerBuilder;
 
 public class KerberosFtpDoorV1 extends GssFtpDoorV1
 {
-    private static final Logger LOGGER = LoggerFactory.getLogger(KerberosFtpDoorV1.class);
-
-    @Option(name = "svc-principal",
-            required = true)
-    private String servicePrincipal;
-
-    @Option(name = "kdc-list")
-    private String kdcList;
-
-    public KerberosFtpDoorV1()
+    public KerberosFtpDoorV1(DssContextFactory dssContextFactory)
     {
-        super("Kerberos FTP", "krbftp", "k5");
+        super("Kerberos FTP", "krbftp", "k5", dssContextFactory);
     }
 
     @Override
     protected void logSubject(NetLoggerBuilder log, Subject subject)
     {
         log.add("user.kerberos", Subjects.getKerberosName(subject));
     }
-
-    @Override
-    protected DssContextFactory createFactory() throws IOException, GSSException
-    {
-        int nretry = 10;
-        String[] kdcList = (this.kdcList != null) ? this.kdcList.split(",") : new String[0];
-        GSSException error;
-        do {
-            if (kdcList.length > 0) {
-                String kdc = kdcList[nretry % kdcList.length];
-                System.getProperties().put("java.security.krb5.kdc", kdc);
-            }
-            try {
-                return new KerberosDssContextFactory(servicePrincipal);
-            } catch (GSSException e) {
-                LOGGER.debug("KerberosFTPDoorV1::getServiceContext: got exception " +
-                             " while looking up credential: {}", e.getMessage());
-                error = e;
-            }
-            --nretry;
-        } while (nretry > 0);
-        throw error;
-   }
 }

modules/dcache-ftp/src/main/java/org/dcache/ftp/door/KerberosFtpInterpreterFactory.java

@@ -17,11 +17,69 @@
  */
 package org.dcache.ftp.door;
 
+import org.ietf.jgss.GSSException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+import diskCacheV111.util.ConfigurationException;
+
+import org.dcache.dss.DssContextFactory;
+import org.dcache.dss.KerberosDssContextFactory;
+import org.dcache.util.Args;
+import org.dcache.util.Option;
+
 public class KerberosFtpInterpreterFactory extends FtpInterpreterFactory
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(KerberosFtpInterpreterFactory.class);
+
+    @Option(name = "svc-principal",
+            required = true)
+    private String servicePrincipal;
+
+    @Option(name = "kdc-list")
+    private String kdcList;
+
+    private DssContextFactory dssContextFactory;
+
+    @Override
+    public void configure(Args args) throws ConfigurationException
+    {
+        super.configure(args);
+        options.inject(this);
+        try {
+            dssContextFactory = createDssContextFactory();
+        } catch (IOException | GSSException e) {
+            throw new ConfigurationException("Failed to create security context:" + e.getMessage(), e);
+        }
+    }
+
     @Override
     protected AbstractFtpDoorV1 createInterpreter()
     {
-        return new KerberosFtpDoorV1();
+        return new KerberosFtpDoorV1(dssContextFactory);
+    }
+
+    protected DssContextFactory createDssContextFactory() throws IOException, GSSException
+    {
+        int nretry = 10;
+        String[] kdcList = (this.kdcList != null) ? this.kdcList.split(",") : new String[0];
+        GSSException error;
+        do {
+            if (kdcList.length > 0) {
+                String kdc = kdcList[nretry % kdcList.length];
+                System.getProperties().put("java.security.krb5.kdc", kdc);
+            }
+            try {
+                return new KerberosDssContextFactory(servicePrincipal);
+            } catch (GSSException e) {
+                LOGGER.debug("KerberosFTPDoorV1::getServiceContext: got exception " +
+                             " while looking up credential: {}", e.getMessage());
+                error = e;
+            }
+            --nretry;
+        } while (nretry > 0);
+        throw error;
     }
 }