tls-attacker / TLS-Attacker

TLS-Attacker is a Java-based framework for analyzing TLS libraries. It can be used to manually test TLS clients and servers or as as a software library for more advanced tools.
Apache License 2.0
803 stars 138 forks source link

Issue with defaultServerConnection and finishWithCloseNotify Settings in DTLS #182

Open hamma96 opened 1 week ago

hamma96 commented 1 week ago

I noticed that when I provide a random value inside defaultServerConnection, it appears to be ignored and does not take effect as expected.

Additionally, I attempted to set false, but my server still sends the CloseNotify alert. Despite the configuration, the server continues to send the alert.

For context, I am currently testing DTLS

Could you please investigate this issue? Let me know if you need any additional information. image

ic0ns commented 1 week ago

I am not sure I get what you are trying to do. Can you show me exactly what you did and what you expect that should happen?

hamma96 commented 1 week ago

I’ve encountered a couple of issues when testing DTLS with the configuration specified in workflow_config.xml. Here are the details:

  1. defaultServerConnection Values Are Ignored:
    I noticed that when I provide a specific value for the defaultServerConnection in my configuration (e.g., ip and hostname), these values seem to be ignored by the server, and the connection does not reflect the expected IP address. Despite configuring 127.1.11.15 as the IP, another address is assigned for both the source and destination. Is there any reason for this, or does TLS-Attacker assign IP addresses dynamically?

  2. finishWithCloseNotify Not Working as Expected:
    I also attempted to set <finishWithCloseNotify>false</finishWithCloseNotify>, but despite this configuration, my server still sends the CloseNotify alert. It appears the configuration is being ignored, and the server continues to send the alert.

Configuration and Command:

I am running the following command to start the server:

java -jar TLS-Server.jar -port 4433 -config workflow_config.xml -version DTLS12 -debug

My workflow_config.xml file contains the following:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<config>
    <defaultPSKKey>1a2b3c4d5e6f</defaultPSKKey>
    <defaultPSKIdentity>333333</defaultPSKIdentity>
    <defaultPSKIdentityHint>333333</defaultPSKIdentityHint>

    <defaultServerConnection>
        <alias>server</alias>
        <ip>127.1.11.15</ip>
        <hostname>127.1.11.15</hostname>
        <timeout>4000</timeout>
    </defaultServerConnection>
    <finishWithCloseNotify>false</finishWithCloseNotify>
    <useFreshRandom>true</useFreshRandom>
</config>

Expected Behavior:

Actual Behavior:

Testing with OpenSSL Client:

I am testing the server with the following OpenSSL client command:

openssl s_client -dtls1_2 -connect 127.0.0.1:4433 -psk 1a2b3c4d5e6f -psk_identity 333 -cipher PSK-AES256-CBC-SHA -msg -debug

Could you please investigate these issues? I’m especially puzzled by the IP address behavior and the unexpected CloseNotify alert. If you need more information or additional logs, feel free to let me know.

ic0ns commented 1 week ago

Ok now I get you. The ServerConnection part is related to the fact that we do not support binding to specific network interfaces but simply attach to whatever the OS hands us. We just call: new DatagramSocket(port); in the ServerUdpTransportHandler.java so we do not control where you are connecting to. The fact that the IP appears in the XML is related to the Connection. We should maybe clear that up to avoid this confusion.

Regarding the Close Notifies: This confused me as well. The reason for this is that the example evaluates the provided parameters in a specific order. Basically it first reads in your config and then changes the config according to your other provided parameters. One of them is -version DTLS12 which is does "more" than just setting the version to DTLS 1.2. Concretly it changes the following things:


        config.setHighestProtocolVersion(protocolVersion);
        config.setDefaultSelectedProtocolVersion(protocolVersion);
        TransportHandlerType th = TransportHandlerType.TCP;
        if (config.getHighestProtocolVersion().isDTLS()) {
            th = TransportHandlerType.UDP;
            config.setDefaultLayerConfiguration(StackConfiguration.DTLS);
            config.setWorkflowExecutorType(WorkflowExecutorType.DTLS);
            config.setFinishWithCloseNotify(true);
            config.setIgnoreRetransmittedCssInDtls(true);
            config.setInitialRecordVersion(ProtocolVersion.DTLS10);
        }

        if (config.getDefaultClientConnection() == null) {
            config.setDefaultClientConnection(new OutboundConnection());
        }
        if (config.getDefaultServerConnection() == null) {
            config.setDefaultServerConnection(new InboundConnection());
        }
        config.getDefaultClientConnection().setTransportHandlerType(th);
        config.getDefaultServerConnection().setTransportHandlerType(th);

As you can see - it basically overwrite the FinishWithCloseNotify to true. So you can either comment out that line - or if that does not work for you you can change the config accordingly and not use the -version parameter.

hamma96 commented 1 week ago

Thanks for the detailed explanation—it helps clarify things!

Regarding the first point on binding to specific network interfaces, would it be possible to modify the code to add a binding option, such as specifying the network interface directly in DatagramSocket? If so, I’d like to explore implementing that change to target a particular interface.

As for the CloseNotify behavior, I see how the -version DTLS12 flag impacts the configuration beyond just setting the protocol version. I’ll try adjusting config.setFinishWithCloseNotify(false) in my setup and avoid the -version parameter to see if that achieves the expected behavior.

Let me know if you have any other suggestions or best practices for these adjustments. Thanks again!

ic0ns commented 1 week ago

We have a prototype implementation for binding to specific network interfaces but it hasn't been touched in 2 years - we plan to polish it at some point and merge but I would not hold my breath

hamma96 commented 1 week ago

Thank you for the update! Based on the existing prototype and its context, I’ve implemented a solution to bind the server to a specific network interface using the provided srcIp and port values. This is achieved through the ServerUdpTransportHandler class, which extends the existing UdpTransportHandler class and overrides the relevant methods for binding the UDP socket to the desired IP address. do you think this is will solve the problem or it is more complicated than this?

Technical Details:

Code Implementation:

public class ServerUdpTransportHandler extends UdpTransportHandler {

    private String srcIp;
    private int port;

    public ServerUdpTransportHandler(Connection con) {
        super(con);
        this.port = con.getPort();
        this.srcIp = con.getIp();
    }

    public ServerUdpTransportHandler(long timeout, int port) {
        super(timeout, ConnectionEndType.SERVER);
        this.port = port;
    }

    @Override
    public void initialize() throws IOException {
        // Ensure the socket is initialized before proceeding
        if (socket == null) {
            throw new IOException("TransportHandler not pre-initialized");
        }
        this.initialized = true;
    }

    @Override
    public void preInitialize() throws IOException {
        // Bind the socket to the provided source IP address and port
        InetAddress tempBindAddr = InetAddress.getByName(this.srcIp);
        socket = new DatagramSocket(port, tempBindAddr);
        cachedSocketState = null;
    }

    @Override
    public void closeClientConnection() throws IOException {
        // Properly close the client connection and socket
        closeConnection();
    }
}

This implementation should provide the functionality to bind the server to a specific network interface (IP address) and port combination. The preInitialize() method ensures that the socket is bound to the correct IP address before use, and initialize() guarantees that the socket is properly set up before handling any traffic.

Please let me know if you have any further feedback or requirements, and I’d be happy to refine the solution based on your input.

ic0ns commented 1 week ago

well - for your specific case it can work like this maybe - but for the project as a whole its a bit more complicated.

hamma96 commented 1 week ago

Thanks for the feedback! I understand that there may be more complexity for the broader project. Just to stay aligned, is there an existing issue or improvement ticket for this functionality? If there are any plans to address it in the future, I’d be happy to contribute or support the improvements where needed.

Let me know how I can assist!

ic0ns commented 1 week ago

If you want to help with this: I pushed the branch with the prototype to the public repository: https://github.com/tls-attacker/TLS-Attacker/tree/specifiableInterface You can do so by updating it to the current master and making sure it's actually working