Skip to content

Commit

Permalink
ftp: prevent execution of most commands when unwrapped
Browse files Browse the repository at this point in the history 
Motivation:

Currently dCache FTP command dispatch does not distinguish between
commands executed directly and those executed via RFC 2228 security
extensions.

Modification:

Annotate those commands allowed to be executed plain-text within the GSS
abstraction that adds RFC 2228 support.  Enforce that only those
commands may be run directly; all others must be wrapped using ENC, CONF
or MIC commands.

Result:

Most commands are protected from being run directly (unencrypted).

Target: master
Request: 3.0
Request: 2.16
Request: 2.15
Request: 2.14
Require-notes: yes
Require-book: no

Conflicts:
	modules/dcache-ftp/src/main/java/org/dcache/ftp/door/AbstractFtpDoorV1.java
	modules/dcache-ftp/src/main/java/org/dcache/ftp/door/GsiFtpDoorV1.java
	modules/dcache-ftp/src/main/java/org/dcache/ftp/door/GssFtpDoorV1.java
	modules/dcache-ftp/src/main/java/org/dcache/ftp/door/KerberosFtpDoorV1.java
	modules/dcache-ftp/src/main/java/org/dcache/ftp/door/WeakFtpDoorV1.java

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

Conflicts:
	modules/dcache-ftp/src/main/java/org/dcache/ftp/door/GssFtpDoorV1.java
  • Loading branch information
@paulmillar
paulmillar committed Mar 16, 2017
1 parent 3e34844 commit 3389013
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 16 deletions.
50 changes: 36 additions & 14 deletions modules/dcache-ftp/src/main/java/org/dcache/ftp/door/AbstractFtpDoorV1.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1094,23 +1094,37 @@ public void getInfo(PrintWriter pw)
}
}

protected interface CommandMethodVisitor
{
void acceptCommand(Method method, String name);
}

protected FtpTransfer _transfer;

public AbstractFtpDoorV1(String ftpDoorName, String tlogName)
{
_ftpDoorName = ftpDoorName;
_tlogName = tlogName;

for (Method method : getClass().getMethods()) {
String name = method.getName();
if (name.startsWith("ftp_")) {
String command = name.substring(4);
visitFtpCommands(new CommandMethodVisitor() {
@Override
public void acceptCommand(Method method, String command) {
_methodDict.put(command, method);
Help help = method.getAnnotation(Help.class);
if (help != null) {
_helpDict.put(command, help);
}
}
});
}

final protected void visitFtpCommands(CommandMethodVisitor visitor)
{
for (Method method : getClass().getMethods()) {
String name = method.getName();
if (name.startsWith("ftp_")) {
visitor.acceptCommand(method, name.substring(4));
}
}
}

Expand Down Expand Up @@ -1289,7 +1303,22 @@ public Object ac_get_door_info(Args args)
}
}

public void ftpcommand(String cmdline)
protected boolean isCommandAllowed(String command, Object commandContext)
{
// If a transfer is in progress, only permit ABORT and a few
// other commands to be processed
if (getTransfer() != null && !(command.equals("abor") ||
command.equals("mic") || command.equals("conf") ||
command.equals("enc") || command.equals("quit") ||
command.equals("bye"))) {
reply("503 Transfer in progress", false);
return false;
}

return true;
}

public void ftpcommand(String cmdline, Object commandContext)
throws CommandExitException
{
int l = 4;
Expand All @@ -1315,13 +1344,7 @@ public void ftpcommand(String cmdline)
_lastCommand = cmdline;
}

// If a transfer is in progress, only permit ABORT and a few
// other commands to be processed
if (getTransfer() != null &&
!(cmd.equals("abor") || cmd.equals("mic")
|| cmd.equals("conf") || cmd.equals("enc")
|| cmd.equals("quit") || cmd.equals("bye"))) {
reply("503 Transfer in progress", false);
if (!isCommandAllowed(cmd, commandContext)) {
return;
}

Expand Down Expand Up @@ -1410,7 +1433,7 @@ public void execute(String command)
reply(err("",""));
} else {
_commandCounter++;
ftpcommand(command);
ftpcommand(command, null);
}
} finally {
_commandLine = null;
Expand Down Expand Up @@ -1808,7 +1831,6 @@ public void ftp_dele(String arg)
}
}


@Help("USER <SP> <name> - Authentication username.")
public abstract void ftp_user(String arg);

Expand Down
62 changes: 60 additions & 2 deletions modules/dcache-ftp/src/main/java/org/dcache/ftp/door/GssFtpDoorV1.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@
import javax.security.auth.Subject;

import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import java.lang.reflect.Method;
import java.nio.charset.Charset;
import java.util.Base64;
import java.util.HashSet;
import java.util.Set;

import diskCacheV111.util.CacheException;
import diskCacheV111.util.PermissionDeniedCacheException;
Expand All @@ -19,10 +24,13 @@

import org.dcache.auth.LoginNamePrincipal;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

public abstract class GssFtpDoorV1 extends AbstractFtpDoorV1
{
private static final Logger LOGGER = LoggerFactory.getLogger(GssFtpDoorV1.class);

private static final GssCommandContext SECURE_COMMAND_CONTEXT = new GssCommandContext();
public static final String GLOBUS_URL_COPY_DEFAULT_USER =
":globus-mapping:";

Expand All @@ -35,11 +43,41 @@ public abstract class GssFtpDoorV1 extends AbstractFtpDoorV1
protected DssContext context;
private DssContextFactory dssContextFactory;

private boolean _hasControlPortCleared;

private final Set<String> _plaintextCommands = new HashSet<>();

/**
* Commands that are annotated @Plaintext are allowed to be sent directly,
* as unencrypted commands. All other commands must be sent indirectly via
* an MIC, ENC or CONF command.
*/
@Retention(RUNTIME)
@Target(METHOD)
@interface Plaintext
{
}

public static class GssCommandContext
{
}

public GssFtpDoorV1(String ftpDoorName, String tlogName, String gssFlavor, DssContextFactory dssContextFactory)
{
super(ftpDoorName, tlogName);

this.gssFlavor = gssFlavor;
this.dssContextFactory = dssContextFactory;

visitFtpCommands(new CommandMethodVisitor() {
@Override
public void acceptCommand(Method method, String command) {
Plaintext plaintext = method.getAnnotation(Plaintext.class);
if (plaintext != null) {
_plaintextCommands.add(command);
}
}
});
}

@Override
Expand All @@ -57,6 +95,7 @@ protected void secure_reply(String answer, String code)
}

@Help("AUTH <SP> <arg> - Initiate secure context negotiation.")
@Plaintext
public void ftp_auth(String arg) throws FTPCommandException
{
LOGGER.info("GssFtpDoorV1::secure_reply: going to authorize using {}", gssFlavor);
Expand Down Expand Up @@ -98,6 +137,7 @@ public void ftp_auth(String arg) throws FTPCommandException
}

@Help("ADAT <SP> <arg> - Supply context negotation data.")
@Plaintext
public void ftp_adat(String arg)
{
if (arg == null || arg.length() <= 0) {
Expand Down Expand Up @@ -148,20 +188,23 @@ public void ftp_ccc(String arg)
}

@Help("MIC <SP> <arg> - Integrity protected command.")
@Plaintext
public void ftp_mic(String arg)
throws CommandExitException
{
secure_command(arg, "mic");
}

@Help("ENC <SP> <arg> - Privacy protected command.")
@Plaintext
public void ftp_enc(String arg)
throws CommandExitException
{
secure_command(arg, "enc");
}

@Help("CONF <SP> <arg> - Confidentiality protection command.")
@Plaintext
public void ftp_conf(String arg)
throws CommandExitException
{
Expand Down Expand Up @@ -210,12 +253,27 @@ public void secure_command(String answer, String sectype)

if (msg.equalsIgnoreCase("CCC")) {
_gReplyType = "clear";
_hasControlPortCleared = true;
reply("200 OK");
} else {
_gReplyType = sectype;
ftpcommand(msg);
ftpcommand(msg, SECURE_COMMAND_CONTEXT);
}

}

@Override
protected boolean isCommandAllowed(String command, Object commandContext)
{
boolean isSecureCommand = commandContext == SECURE_COMMAND_CONTEXT;

if (!_hasControlPortCleared && !isSecureCommand &&
!_plaintextCommands.contains(command)) {
reply("530 Command must be wrapped in MIC, ENC or CONF", false);
return false;
}

return super.isCommandAllowed(command, commandContext);
}

@Override
Expand Down

0 comments on commit 3389013

Please sign in to comment.