Skip to content

Commit 709af67

Browse files
committed
DTLS: Set server record layer version(s) earlier
- enable several DTLS tests that can now work cleanly
1 parent 12d7e77 commit 709af67

File tree

3 files changed

+18
-36
lines changed

3 files changed

+18
-36
lines changed

tls/src/main/java/org/bouncycastle/tls/DTLSServerProtocol.java

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,6 @@ protected DTLSTransport serverHandshake(ServerHandshakeState state, DTLSRequest
117117
{
118118
clientMessage = handshake.receiveMessage();
119119

120-
// NOTE: DTLSRecordLayer requires any DTLS version, we don't otherwise constrain this
121-
// ProtocolVersion recordLayerVersion = recordLayer.getReadVersion();
122-
123120
if (clientMessage.getType() == HandshakeType.client_hello)
124121
{
125122
processClientHello(state, clientMessage.getBody());
@@ -141,13 +138,6 @@ protected DTLSTransport serverHandshake(ServerHandshakeState state, DTLSRequest
141138
{
142139
byte[] serverHelloBody = generateServerHello(state);
143140

144-
// TODO[dtls13] Ideally, move this into generateServerHello once legacy_record_version clarified
145-
{
146-
ProtocolVersion recordLayerVersion = serverContext.getServerVersion();
147-
recordLayer.setReadVersion(recordLayerVersion);
148-
recordLayer.setWriteVersion(recordLayerVersion);
149-
}
150-
151141
handshake.sendMessage(HandshakeType.server_hello, serverHelloBody);
152142
}
153143

@@ -460,8 +450,6 @@ protected byte[] generateServerHello(ServerHandshakeState state)
460450
TlsServerContextImpl serverContext = state.serverContext;
461451
SecurityParameters securityParameters = serverContext.getSecurityParametersHandshake();
462452

463-
// TODO[dtls13] Negotiate cipher suite first?
464-
465453
ProtocolVersion serverVersion;
466454

467455
// NOT renegotiating
@@ -477,22 +465,24 @@ protected byte[] generateServerHello(ServerHandshakeState state)
477465
// ? ProtocolVersion.DTLSv12
478466
// : server_version;
479467
//
480-
// recordLayer.setWriteVersion(legacy_record_version);
468+
// state.recordLayer.setWriteVersion(legacy_record_version);
481469
securityParameters.negotiatedVersion = serverVersion;
482470
}
483471

484472
// TODO[dtls13]
485473
// if (ProtocolVersion.DTLSv13.isEqualOrEarlierVersionOf(serverVersion))
486474
// {
487475
// // See RFC 8446 D.4.
488-
// recordStream.setIgnoreChangeCipherSpec(true);
476+
// state.recordLayer.setIgnoreChangeCipherSpec(true);
489477
//
490-
// recordStream.setWriteVersion(ProtocolVersion.DTLSv12);
478+
// state.recordLayer.setReadVersion(ProtocolVersion.DTLSv12);
479+
// state.recordLayer.setWriteVersion(ProtocolVersion.DTLSv12);
491480
//
492481
// return generate13ServerHello(clientHello, clientHelloMessage, false);
493482
// }
494-
//
495-
// recordStream.setWriteVersion(serverVersion);
483+
484+
state.recordLayer.setReadVersion(serverVersion);
485+
state.recordLayer.setWriteVersion(serverVersion);
496486

497487
{
498488
boolean useGMTUnixTime = server.shouldUseGMTUnixTime();
@@ -845,6 +835,8 @@ protected void processClientHello(ServerHandshakeState state, byte[] body)
845835
protected void processClientHello(ServerHandshakeState state, ClientHello clientHello)
846836
throws IOException
847837
{
838+
state.recordLayer.setWriteVersion(ProtocolVersion.DTLSv10);
839+
848840
state.clientHello = clientHello;
849841

850842
// TODO Read RFCs for guidance on the expected record layer version number

tls/src/test/java/org/bouncycastle/tls/test/DTLSRawKeysProtocolTest.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.security.SecureRandom;
44

55
import org.bouncycastle.crypto.params.Ed25519PrivateKeyParameters;
6+
import org.bouncycastle.tls.AlertDescription;
67
import org.bouncycastle.tls.CertificateType;
78
import org.bouncycastle.tls.DTLSClientProtocol;
89
import org.bouncycastle.tls.DTLSRequest;
@@ -13,6 +14,7 @@
1314
import org.bouncycastle.tls.ProtocolVersion;
1415
import org.bouncycastle.tls.TlsClient;
1516
import org.bouncycastle.tls.TlsExtensionsUtils;
17+
import org.bouncycastle.tls.TlsFatalAlertReceived;
1618
import org.bouncycastle.tls.TlsServer;
1719
import org.bouncycastle.tls.crypto.TlsCrypto;
1820
import org.bouncycastle.util.Arrays;
@@ -206,8 +208,6 @@ private void testServerUsesX509AndClientUsesRawKey(ProtocolVersion tlsVersion) t
206208
pumpData(client, server);
207209
}
208210

209-
// NOTE: Test disabled because of problems getting a clean exit of the DTLS server after a fatal alert.
210-
/*
211211
public void testClientSendsClientCertExtensionButServerHasNoCommonTypes() throws Exception
212212
{
213213
testClientSendsClientCertExtensionButServerHasNoCommonTypes(ProtocolVersion.DTLSv12);
@@ -244,10 +244,7 @@ private void testClientSendsClientCertExtensionButServerHasNoCommonTypes(Protoco
244244
assertEquals("Should have caused unsupported_certificate alert", alert.getAlertDescription(), AlertDescription.unsupported_certificate);
245245
}
246246
}
247-
*/
248247

249-
// NOTE: Test disabled because of problems getting a clean exit of the DTLS server after a fatal alert.
250-
/*
251248
public void testClientSendsServerCertExtensionButServerHasNoCommonTypes() throws Exception
252249
{
253250
testClientSendsServerCertExtensionButServerHasNoCommonTypes(ProtocolVersion.DTLSv12);
@@ -284,7 +281,6 @@ private void testClientSendsServerCertExtensionButServerHasNoCommonTypes(Protoco
284281
assertEquals("Should have caused unsupported_certificate alert", alert.getAlertDescription(), AlertDescription.unsupported_certificate);
285282
}
286283
}
287-
*/
288284

289285
private Ed25519PrivateKeyParameters generateKeyPair()
290286
{

tls/src/test/java/org/bouncycastle/tls/test/DTLSTestSuite.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,14 @@ private static void addFallbackTests(TestSuite testSuite)
4343
addTestCase(testSuite, c, "FallbackGood");
4444
}
4545

46-
/*
47-
* NOTE: Temporarily disabled automatic test runs because of problems getting a clean exit
48-
* of the DTLS server after a fatal alert. As of writing, manual runs show the correct
49-
* alerts being raised
50-
*/
46+
{
47+
TlsTestConfig c = createDTLSTestConfig(ProtocolVersion.DTLSv12);
48+
c.clientFallback = true;
49+
c.clientSupportedVersions = ProtocolVersion.DTLSv10.only();
50+
c.expectServerFatalAlert(AlertDescription.inappropriate_fallback);
5151

52-
// {
53-
// TlsTestConfig c = createDTLSTestConfig(ProtocolVersion.DTLSv12);
54-
// c.clientFallback = true;
55-
// c.clientSupportedVersions = ProtocolVersion.DTLSv10.only();
56-
// c.expectServerFatalAlert(AlertDescription.inappropriate_fallback);
57-
//
58-
// addTestCase(testSuite, c, "FallbackBad");
59-
// }
52+
addTestCase(testSuite, c, "FallbackBad");
53+
}
6054

6155
{
6256
TlsTestConfig c = createDTLSTestConfig(ProtocolVersion.DTLSv12);

0 commit comments

Comments
 (0)