Skip to content

Commit

Permalink
ftp: fix MLSC support for medium-length directories
Browse files Browse the repository at this point in the history
Motivation:

When the response to an FTP command become large enough that response
must be sent over multple TLS frames (multiple calls to the 'wrap'
method).  Almost all command responses are short enough that this is not
an issue.  Only the MLSC command (which returns a directory listing over
the control channel) triggers this problem.  The MLSC command is
currently only used by the Globus transfer service.

A complication arises because Globus couples the GSI/GSS encoding layer
with the application layer.  This results in the requirement that a TLS
frame must encode the information about an exact number of directory
elements; information about a file or directory must not be split over
multiple TLS frames.

SSLEngine assumes there is no coupling between the TLS layer and the
application layer.  As a result, it will wrap as much application data
as possible.  The resulting TLS frame is emitted and the process is
repeated until all application data is wrapped.  For Globus, this
approach does not work, since such wrapping would (very likely) result
in information about a file or directory being split over subsequent TLS
frames.

Commit 99ddbf5 solves this problem by allowing the application layer
(GssFtpDoorV1) to know how much application data will fit in a TLS
frame.  This allows GssFtpDoorV1 to ensure SSLEngine will always wrap
all application data it is given.

Unfortunately, that commit used the output from a similar sounding but
ultimately unrelated getter method to provide GssFtpDoorV1 with the
maximum application data size (for a single TLS frame).  The supplied
value is too large.

Directories that are large enough will work correctly because if
GssFtpDoorV1 believes the response is too large for a single TLS frame,
it splits the response into single lines and wraps each individually.
Small directories will also work, since the output fits within a single
TLS frame.  However "medium" sized directories fail as the response is
small enough for GssFtpDoorV1 to believe (incorrectly) that the
application data will fit within a single TLS frame.

Modification:

Switch to a hard-coded 16 KiB value for the maximum application data
that will fit within the TLS frame.

Add some additional diagnostic information should the problem come back.

Result:

Fix Globus transfer service directory listing for "medium" size
directories.

Target: master
Request: 7.0
Request: 6.2
Request: 6.1
Request: 6.0
Request: 5.2
Requires-notes: yes
Requires-book: no
Patch: https://rb.dcache.org/r/12765/
Acked-by: Tigran Mkrtchyan
Acked-by: Lea Morschel
  • Loading branch information
paulmillar committed Jan 19, 2021
1 parent 7eaf3a7 commit 912bfb7
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
Expand Up @@ -17,6 +17,9 @@
*/
package org.dcache.dss;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLPeerUnverifiedException;
Expand All @@ -37,9 +40,11 @@
import static java.util.Arrays.asList;
import static java.util.Collections.emptySet;
import static java.util.Collections.singleton;
import static org.dcache.util.ByteUnit.KiB;

public class SslEngineDssContext implements DssContext
{
private static final Logger LOGGER = LoggerFactory.getLogger(SslEngineDssContext.class);
private static final ByteBuffer EMPTY = ByteBuffer.wrap(new byte[0]);

private final SSLEngine engine;
Expand Down Expand Up @@ -116,9 +121,16 @@ private void handshake() throws IOException

private void wrap(ByteBuffer data) throws IOException
{
int count=0;
SSLEngineResult result;
do {
count++;
LOGGER.debug("Wrapping: buffer posn={}, limit={}, capacity={}, remaining={}",
data.position(), data.limit(), data.capacity(), data.remaining());
result = engine.wrap(data, outToken);
LOGGER.debug("Result: status={}, seq={}, consumed={}, produced={}, remaining={}",
result.getStatus(), result.sequenceNumber(), result.bytesConsumed(),
result.bytesProduced(), data.remaining());
switch (result.getStatus()) {
case BUFFER_UNDERFLOW:
throw new RuntimeException();
Expand All @@ -137,15 +149,19 @@ private void wrap(ByteBuffer data) throws IOException
throw new EOFException();
}
} while (result.getStatus() != SSLEngineResult.Status.OK);

if (data.hasRemaining()) {
throw new RuntimeException("SSLEngine did not wrap all data.");
throw new RuntimeException("SSLEngine did not wrap all data:"
+ " c=" + count + ", in=" + data.limit() + " r=" + data.remaining()
+ " out=" + outToken.position());
}
}

@Override
public long maxApplicationSize()
{
return engine.getSession().getApplicationBufferSize();
// Maximum TLS frame encodes 16 KiB application data.
return KiB.toBytes(16);
}

private boolean unwrap() throws IOException
Expand Down
Expand Up @@ -101,9 +101,15 @@ protected void secure_reply(CommandRequest request, String answer, String code)
wrapAndSend(code, ' ', allData);
} else {
List<String> lines = Splitter.on("\r\n").splitToList(answer);
LOGGER.debug("Command \"{}\" response is too large, splitting it into {} lines",
request, lines.size());
for (int i = 0; i < lines.size(); i++) {
boolean isLastLine = i == lines.size()-1;
byte[] lineData = (lines.get(i) + "\r\n").getBytes(UTF_8);
if (lineData.length > context.maxApplicationSize()) {
LOGGER.error("Line {} of {} is too large ({} > {})", i+1,
lines.size(), lineData.length, context.maxApplicationSize());
}
wrapAndSend(code, isLastLine ? ' ' : '-', lineData);
}
}
Expand Down

0 comments on commit 912bfb7

Please sign in to comment.