Re-implement a simple port knocking client (UDP or TCP) in Java

Question

(I have also published this project on GitHub.) I am aware that some people view this as security through obscurity.

The aim is to implement the port knocking technique in Java. I would like to know what you think about it.

Can the parsing of the arguments be simplified? Is the enum class necessary? Should the knock logic use streams if possible? Or have I overlooked anything else?

import java.io.IOException;
import java.net.DatagramSocket;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.util.AbstractMap;
import java.util.LinkedList;
public class Main {
 private enum Mode {
 UDP,
 TCP;
 public static Mode fromString(String s) {
 return switch (s) {
 case "u" -> UDP;
 case "t" -> TCP;
 default -> throw new IllegalArgumentException("Unknown mode: " + s);
 };
 }
 }
 private static final int TCP_TIMEOUT = 1000; // milliseconds
 private static void printUsageAndExit() {
 System.out.println(
 "Usage : java -jar java-port-knock.jar (delay=<delay_ms>) (host=<host_str>) [u|t]=<port_int> ...");
 System.out.println(
 "Example: java -jar java-port-knock.jar delay=250 host=127.0.0.1 t=123 u=345 t=567");
 System.exit(1);
 }
 public static void main(String[] args) {
 if (args.length == 0) {
 printUsageAndExit();
 }
 // Default parameters...
 int delay = 100; // default delay
 String host = "localhost"; // default host
 // Parse arguments...
 LinkedList<AbstractMap.SimpleEntry<Mode, Integer>> knockSequence = new LinkedList<>();
 for (String arg : args) {
 String[] parts = arg.split("=", 2);
 if (parts.length != 2) {
 printUsageAndExit();
 }
 String key = parts[0];
 String value = parts[1];
 switch (key) {
 case "host" -> host = value;
 case "delay" -> {
 try {
 int d = Integer.parseInt(value);
 if (d < 0 || d > 60_000) {
 printUsageAndExit();
 }
 delay = d;
 } catch (NumberFormatException e) {
 printUsageAndExit();
 }
 }
 case "u", "t" -> {
 try {
 int port = Integer.parseInt(value);
 if (port < 1 || port > 65535) {
 printUsageAndExit();
 }
 knockSequence.add(new AbstractMap.SimpleEntry<>(Mode.fromString(key), port));
 } catch (NumberFormatException e) {
 printUsageAndExit();
 }
 }
 default -> printUsageAndExit();
 }
 }
 if (knockSequence.isEmpty()) {
 printUsageAndExit();
 }
 // Perform knocking sequence...
 for (AbstractMap.SimpleEntry<Mode, Integer> entry : knockSequence) {
 Mode mode = entry.getKey();
 int port = entry.getValue();
 System.out.printf("Knocking %s %s:%d ... ", mode.name(), host, port);
 switch (mode) {
 case UDP -> System.out.println(knockUDP(host, port) ? "Success" : "Failed");
 case TCP -> System.out.println(knockTCP(host, port, TCP_TIMEOUT) ? "Success" : "Failed");
 }
 try {
 Thread.sleep(delay);
 } catch (InterruptedException e) {
 System.out.println("Sleep interrupted: " + e.getMessage());
 return;
 }
 }
 }
 /**
 * Knocks on a UDP port by attempting to create a UDP socket, and sending a datagram. The datagram
 * content is not important, it has a fixed size of 2048 bytes.
 *
 * @param host the hostname or IP address to connect to
 * @param port the UDP port to knock on
 * @return true if the datagram was sent successfully, false otherwise
 */
 public static boolean knockUDP(String host, int port) {
 try (DatagramSocket socket = new DatagramSocket()) {
 socket.connect(new InetSocketAddress(host, port));
 socket.send(new java.net.DatagramPacket(new byte[2048], 2048));
 return true;
 } catch (IOException e) {
 return false;
 }
 }
 /**
 * Knocks on a TCP port by attempting to establish a TCP connection. Returning <code>false</code>
 * does not necessarily mean that the port is closed; it may also be filtered by a firewall.
 *
 * @param host the hostname or IP address to connect to
 * @param port the TCP port to knock on
 * @param timeout the connection timeout in milliseconds
 * @return true if the connection was successful, false otherwise
 */
 public static boolean knockTCP(String host, int port, int timeout) {
 try (Socket socket = new Socket()) {
 socket.connect(new InetSocketAddress(host, port), timeout);
 return true;
 } catch (IOException e) {
 return false;
 }
 }
}

Answer 1

Always use a third party library when parsing parameters. I recently had a really good idea about how to parse command line parameters directly into configuration objects (using reflection and Jakarta validation) and when I checked how others have approached the problem I found 83 different command line parsing libraries (the number is probably over 100 by now). There has to be one that fulfills your needs.

If you don't want to do that, at least extract the parameter parsing into a separate method that returns a configuration object.

LinkedList<AbstractMap.SimpleEntry<Mode, Integer>> knockSequence = new LinkedList<>();

This line could be one of the examples on why Java standard libraries don't have a generic pair class. People lot smarter than us decided against it and we should have a really good reason when we start disagreeing with them. So, just create a domain class that contains the port knock information. You could have a common base class and subclasses for TCP and UDP and move the knock function into those classes.

If you want to go full functional programming, your knock would be a "task" and your delay would be a "task" and you would put a delay task between every knock task, store them in a list and execute them one by one in a stream. It's not necessary, you can decide if it is worth it. It would add a lot of flexibility. You could have a different host for each knock operation and different delay between them (look up "scope creep" with your favourite search engine).

Use Duration for timeout and delay. It was made for this exact purpose. Then you don't have to document that the value is a millisecond (which you forgot in the delay). Or name the fields with a unit suffix long delayMillis. A long type also suggests to the reader that the value is milliseconds since it's what System.currentTimeMillis() returns. These are small things that make the code easier to comprehend. I like Duration better, though.

You should call close() for the TCP socket.

Personally I don't like enums to have methods that closely tie them to specific transportation formats (usually JSON or SOAP serialization but argument parsing falls into this category too), but I am often left alone with this opinion.

Mode is a bit generic name for an enum that lists TransportProtocols.

• Tobias Grothe: "You should call close() for the TCP socket." - Why?
• Tobias Grothe: Thank you very much for the review. Based on this, I have modified the application and created a pull request: see here.

Answer 2

Java has record classes since a while back. You could model your port knocking details as a record.

enum Mode {
 UDP,
 TCP;
 public static Mode fromString(String s) {
 return switch (s) {
 case "u" -> UDP;
 case "t" -> TCP;
 default -> throw new IllegalArgumentException("Unknown mode: " + s);
 };
 }
}
record KnockStep(Mode mode, int port) {}
// later
KnockStep step = new KnockStep(Mode.TCP, 444);

Mode could arguably be called Protocol.

Sometimes "program to an interface, not an implementation" is mentioned as a rule of thumb or design principle. In your case, it could be applied to the linked list you store the knock steps in.

// Here, List is an interface, not a specific implementation.
List<KnockStep> knocSteps = new ArrayList<>();

The record-and-interface-approach works with the enhanced for statement, too: for (KnockStep step : steps) { doKnock(step)}

Here's a small program demonstrating these concepts.

import java.util.Arrays;
import java.util.ArrayList;
import java.util.List;
public class Main {
 
 private enum Protocol {
 TCP,
 UDP
 };
 
 private record KnockStep(Protocol protocol, int port) {}
 
 public static void main(String[] args) {
 KnockStep step1 = new KnockStep(Protocol.TCP, 444);
 KnockStep step2 = new KnockStep(Protocol.UDP, 445);
 List<KnockStep> steps = Arrays.asList(step1, step2);
 for (KnockStep step: steps) {
 System.out.println(step);
 }
 }
}