Skip to content

[AI] Fix DTLS config override issue with finishWithCloseNotify #223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public class ClientCommandConfig extends TLSDelegateConfig {
@ParametersDelegate private ListDelegate listDelegate;
@ParametersDelegate private StarttlsDelegate starttlsDelegate;
@ParametersDelegate private ConnectionDelegate connectionDelegate;
@ParametersDelegate private CloseNotifyDelegate closeNotifyDelegate;

@ParametersDelegate private EchDelegate echDelegate;
@ParametersDelegate private QuicDelegate quicDelegate;
Expand Down Expand Up @@ -71,6 +72,7 @@ public ClientCommandConfig(GeneralDelegate delegate) {
this.echDelegate = new EchDelegate();
this.quicDelegate = new QuicDelegate();
this.connectionDelegate = new ConnectionDelegate();
this.closeNotifyDelegate = new CloseNotifyDelegate();
addDelegate(listDelegate);
addDelegate(heartbeatDelegate);
addDelegate(ciphersuiteDelegate);
Expand All @@ -90,6 +92,7 @@ public ClientCommandConfig(GeneralDelegate delegate) {
addDelegate(echDelegate);
addDelegate(quicDelegate);
addDelegate(connectionDelegate);
addDelegate(closeNotifyDelegate);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* TLS-Attacker - A Modular Penetration Testing Framework for TLS
*
* Copyright 2014-2023 Ruhr University Bochum, Paderborn University, Technology Innovation Institute, and Hackmanit GmbH
*
* Licensed under Apache License, Version 2.0
* http://www.apache.org/licenses/LICENSE-2.0.txt
*/
package de.rub.nds.tlsattacker.core.config.delegate;

import com.beust.jcommander.Parameter;
import de.rub.nds.tlsattacker.core.config.Config;

public class CloseNotifyDelegate extends Delegate {

@Parameter(
names = "-close_notify",
arity = 1,
description = "Send close notify alert when finishing (overrides config file setting)")
private Boolean finishWithCloseNotify = null;

public CloseNotifyDelegate() {}

public Boolean getFinishWithCloseNotify() {
return finishWithCloseNotify;
}

public void setFinishWithCloseNotify(Boolean finishWithCloseNotify) {
this.finishWithCloseNotify = finishWithCloseNotify;
}

@Override
public void applyDelegate(Config config) {
if (finishWithCloseNotify != null) {
config.setFinishWithCloseNotify(finishWithCloseNotify);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ public void applyDelegate(Config config) {
th = TransportHandlerType.UDP;
config.setDefaultLayerConfiguration(StackConfiguration.DTLS);
config.setWorkflowExecutorType(WorkflowExecutorType.DTLS);
config.setFinishWithCloseNotify(true);
// Note: finishWithCloseNotify is no longer set here to avoid overriding
// config file settings. Use -close_notify parameter or config file instead.
config.setIgnoreRetransmittedCssInDtls(true);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* TLS-Attacker - A Modular Penetration Testing Framework for TLS
*
* Copyright 2014-2023 Ruhr University Bochum, Paderborn University, Technology Innovation Institute, and Hackmanit GmbH
*
* Licensed under Apache License, Version 2.0
* http://www.apache.org/licenses/LICENSE-2.0.txt
*/
package de.rub.nds.tlsattacker.core.config.delegate;

import static org.junit.jupiter.api.Assertions.*;

import de.rub.nds.tlsattacker.core.config.Config;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class CloseNotifyDelegateTest extends AbstractDelegateTest<CloseNotifyDelegate> {

@BeforeEach
public void setUp() {
super.setUp(new CloseNotifyDelegate());
}

/** Test of getFinishWithCloseNotify method, of class CloseNotifyDelegate. */
@Test
public void testGetFinishWithCloseNotify() {
args = new String[2];
args[0] = "-close_notify";
args[1] = "true";
assertNull(delegate.getFinishWithCloseNotify());
jcommander.parse(args);
assertEquals(true, delegate.getFinishWithCloseNotify());
}

/** Test of setFinishWithCloseNotify method, of class CloseNotifyDelegate. */
@Test
public void testSetFinishWithCloseNotify() {
assertNull(delegate.getFinishWithCloseNotify());
delegate.setFinishWithCloseNotify(true);
assertEquals(true, delegate.getFinishWithCloseNotify());
}

/** Test of applyDelegate method, of class CloseNotifyDelegate. */
@Test
public void testApplyDelegate() {
Config config = new Config();
// Default value should be false
assertFalse(config.isFinishWithCloseNotify());

args = new String[2];
args[0] = "-close_notify";
args[1] = "true";

jcommander.parse(args);
delegate.applyDelegate(config);

assertTrue(config.isFinishWithCloseNotify());
}

@Test
public void testApplyDelegateFalse() {
Config config = new Config();
config.setFinishWithCloseNotify(true);
assertTrue(config.isFinishWithCloseNotify());

args = new String[2];
args[0] = "-close_notify";
args[1] = "false";

jcommander.parse(args);
delegate.applyDelegate(config);

assertFalse(config.isFinishWithCloseNotify());
}

@Test
public void testNothingSetNothingChanges() {
Config config = new Config();
Config config2 = new Config();
delegate.applyDelegate(config);
assertTrue(
EqualsBuilder.reflectionEquals(
config, config2, "certificateChainConfig")); // little ugly
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,29 @@ public void testNothingSetNothingChanges() {
delegate.applyDelegate(config);
assertTrue(EqualsBuilder.reflectionEquals(config, config2, "certificateChainConfig"));
}

/** Test that DTLS version no longer overrides finishWithCloseNotify setting */
@Test
public void testDtlsDoesNotOverrideFinishWithCloseNotify() {
Config config = new Config();
// Set finishWithCloseNotify to false explicitly
config.setFinishWithCloseNotify(false);

String[] args = new String[2];
args[0] = "-version";
args[1] = "DTLS12";

jcommander.parse(args);
delegate.applyDelegate(config);

// Verify that finishWithCloseNotify remains false and is not overridden
assertFalse(config.isFinishWithCloseNotify());
assertSame(ProtocolVersion.DTLS12, config.getHighestProtocolVersion());
assertSame(
TransportHandlerType.UDP,
config.getDefaultClientConnection().getTransportHandlerType());
assertSame(
TransportHandlerType.UDP,
config.getDefaultServerConnection().getTransportHandlerType());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class ServerCommandConfig extends TLSDelegateConfig {
@ParametersDelegate private ExecutorTypeDelegate executorTypeDelegate;
@ParametersDelegate private StarttlsDelegate starttlsDelegate;
@ParametersDelegate private TimeoutDelegate timeoutDelegate;
@ParametersDelegate private CloseNotifyDelegate closeNotifyDelegate;

@Parameter(
names = "-workflow_input",
Expand Down Expand Up @@ -64,6 +65,7 @@ public ServerCommandConfig(GeneralDelegate delegate) {
this.executorTypeDelegate = new ExecutorTypeDelegate();
this.starttlsDelegate = new StarttlsDelegate();
this.timeoutDelegate = new TimeoutDelegate();
this.closeNotifyDelegate = new CloseNotifyDelegate();
addDelegate(maxFragmentLengthDelegate);
addDelegate(ciphersuiteDelegate);
addDelegate(ellipticCurveDelegate);
Expand All @@ -80,6 +82,7 @@ public ServerCommandConfig(GeneralDelegate delegate) {
addDelegate(executorTypeDelegate);
addDelegate(starttlsDelegate);
addDelegate(timeoutDelegate);
addDelegate(closeNotifyDelegate);
}

@Override
Expand Down