Skip to content

Commit

Permalink
DTLS: Improve record seq. behaviour in HelloVerifyRequest scenarios
Browse files Browse the repository at this point in the history
  • Loading branch information
peterdettman committed Mar 9, 2019
1 parent fd83b9c commit 98cf757
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ protected DTLSTransport clientHandshake(ClientHandshakeState state, DTLSRecordLa
byte[] cookie = processHelloVerifyRequest(state, serverMessage.getBody());
byte[] patched = patchClientHelloWithCookie(clientHelloBody, cookie);

handshake.resetAfterHelloVerifyRequest();
handshake.resetAfterHelloVerifyRequestClient();
handshake.sendMessage(HandshakeType.client_hello, patched);

serverMessage = handshake.receiveMessage();
Expand Down
5 changes: 5 additions & 0 deletions tls/src/main/java/org/bouncycastle/tls/DTLSEpoch.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,9 @@ long getSequenceNumber()
{
return sequenceNumber;
}

void setSequenceNumber(long sequenceNumber)
{
this.sequenceNumber = sequenceNumber;
}
}
20 changes: 14 additions & 6 deletions tls/src/main/java/org/bouncycastle/tls/DTLSRecordLayer.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ static byte[] receiveClientHelloRecord(byte[] data, int dataOff, int dataLen) th
return null;
}

// TODO Are there any constraints on this?
long sequenceNumber = TlsUtils.readUint48(data, dataOff + 5);
System.out.println("Record sequence in ClientHello: " + sequenceNumber);
// long sequenceNumber = TlsUtils.readUint48(data, dataOff + 5);

int length = TlsUtils.readUint16(data, dataOff + 11);
if (dataLen != RECORD_HEADER_LENGTH + length)
Expand All @@ -54,15 +52,15 @@ static byte[] receiveClientHelloRecord(byte[] data, int dataOff, int dataLen) th
return Arrays.copyOfRange(data, dataOff + RECORD_HEADER_LENGTH, dataOff + dataLen);
}

static void sendHelloVerifyRequestRecord(DatagramSender sender, byte[] message) throws IOException
static void sendHelloVerifyRequestRecord(DatagramSender sender, long recordSeq, byte[] message) throws IOException
{
TlsUtils.checkUint16(message.length);

byte[] record = new byte[RECORD_HEADER_LENGTH + message.length];
TlsUtils.writeUint8(ContentType.handshake, record, 0);
TlsUtils.writeVersion(ProtocolVersion.DTLSv10, record, 1);
TlsUtils.writeUint16(0, record, 3);
TlsUtils.writeUint48(0, record, 5);
TlsUtils.writeUint48(recordSeq, record, 5);
TlsUtils.writeUint16(message.length, record, 11);

System.arraycopy(message, 0, record, RECORD_HEADER_LENGTH, message.length);
Expand Down Expand Up @@ -117,11 +115,21 @@ private static void sendDatagram(DatagramSender sender, byte[] record)
setPlaintextLimit(MAX_FRAGMENT_LENGTH);
}

void resetAfterHelloVerifyRequest()
void resetAfterHelloVerifyRequestClient()
{
currentEpoch.getReplayWindow().reset();
}

void resetAfterHelloVerifyRequestServer(long writeRecordSeqNo)
{
currentEpoch.setSequenceNumber(writeRecordSeqNo);

/*
* TODO The sequence number reflects one that the client has already sent, so we could
* initialize the replay window here accordingly.
*/
}

void setPlaintextLimit(int plaintextLimit)
{
this.plaintextLimit = plaintextLimit;
Expand Down
20 changes: 10 additions & 10 deletions tls/src/main/java/org/bouncycastle/tls/DTLSReliableHandshake.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ static DTLSRequest readClientRequest(byte[] data, int dataOff, int dataLen, Outp
return null;
}

long recordSeq = TlsUtils.readUint48(data, dataOff + 5);

short msgType = TlsUtils.readUint8(message, 0);
if (HandshakeType.client_hello != msgType)
{
Expand Down Expand Up @@ -54,10 +56,10 @@ static DTLSRequest readClientRequest(byte[] data, int dataOff, int dataLen, Outp

ClientHello clientHello = ClientHello.parse(new ByteArrayInputStream(message, MESSAGE_HEADER_LENGTH, length), dtlsOutput);

return new DTLSRequest(message, clientHello);
return new DTLSRequest(recordSeq, message, clientHello);
}

static void sendHelloVerifyRequest(DatagramSender sender, int messageSeq, byte[] cookie) throws IOException
static void sendHelloVerifyRequest(DatagramSender sender, long recordSeq, int messageSeq, byte[] cookie) throws IOException
{
TlsUtils.checkUint16(messageSeq);
TlsUtils.checkUint8(cookie.length);
Expand All @@ -75,7 +77,7 @@ static void sendHelloVerifyRequest(DatagramSender sender, int messageSeq, byte[]
TlsUtils.writeVersion(ProtocolVersion.DTLSv10, message, MESSAGE_HEADER_LENGTH + 0);
TlsUtils.writeOpaque8(cookie, message, MESSAGE_HEADER_LENGTH + 2);

DTLSRecordLayer.sendHelloVerifyRequestRecord(sender, message);
DTLSRecordLayer.sendHelloVerifyRequestRecord(sender, recordSeq, message);
}

/*
Expand All @@ -101,9 +103,12 @@ static void sendHelloVerifyRequest(DatagramSender sender, int messageSeq, byte[]
{
sending = false;

long recordSeq = request.getRecordSeq();
int messageSeq = request.getMessageSeq();
byte[] message = request.getMessage();

recordLayer.resetAfterHelloVerifyRequestServer(recordSeq);

// Simulate a previous flight consisting of the request ClientHello
DTLSReassembler reassembler = new DTLSReassembler(HandshakeType.client_hello, message.length - MESSAGE_HEADER_LENGTH);
currentInboundFlight.put(Integers.valueOf(messageSeq), reassembler);
Expand All @@ -112,15 +117,10 @@ static void sendHelloVerifyRequest(DatagramSender sender, int messageSeq, byte[]
next_receive_seq = messageSeq + 1;

handshakeHash.update(message, 0, message.length);

/*
* TODO If we store the (highest) record sequence number of the request, we could
* initialize the replay window here accordingly.
*/
}
}

void resetAfterHelloVerifyRequest()
void resetAfterHelloVerifyRequestClient()
{
currentInboundFlight = new Hashtable();
previousInboundFlight = null;
Expand All @@ -131,7 +131,7 @@ void resetAfterHelloVerifyRequest()

handshakeHash.reset();

recordLayer.resetAfterHelloVerifyRequest();
recordLayer.resetAfterHelloVerifyRequestClient();
}

void notifyHelloComplete()
Expand Down
9 changes: 8 additions & 1 deletion tls/src/main/java/org/bouncycastle/tls/DTLSRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

public class DTLSRequest
{
private final long recordSeq;
private final byte[] message;
private final ClientHello clientHello;

DTLSRequest(byte[] message, ClientHello clientHello)
DTLSRequest(long recordSeq, byte[] message, ClientHello clientHello)
{
this.recordSeq = recordSeq;
this.message = message;
this.clientHello = clientHello;
}
Expand All @@ -25,4 +27,9 @@ int getMessageSeq()
{
return TlsUtils.readUint16(message, 4);
}

long getRecordSeq()
{
return recordSeq;
}
}
2 changes: 1 addition & 1 deletion tls/src/main/java/org/bouncycastle/tls/DTLSVerifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public synchronized DTLSRequest verifyRequest(byte[] clientID, byte[] data, int
return request;
}

DTLSReliableHandshake.sendHelloVerifyRequest(sender, request.getMessageSeq(), expectedCookie);
DTLSReliableHandshake.sendHelloVerifyRequest(sender, request.getRecordSeq(), request.getMessageSeq(), expectedCookie);
}
}
catch (IOException e)
Expand Down

0 comments on commit 98cf757

Please sign in to comment.