Skip to content

Adds TLS SNI to ghidra client connections #8

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.

Sign up for GitHub

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: master
Choose a base branch
from
+55 −6
Open

Adds TLS SNI to ghidra client connections #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions Ghidra/Features/GhidraServer/src/main/java/ghidra/server/stream/RemoteBlockStreamHandle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
import java.io.*;
import java.net.Socket;
import java.security.SecureRandom;
import java.util.*;

import javax.net.SocketFactory;
import javax.net.ssl.SSLSocketFactory;
import javax.net.*;
import javax.net.ssl.*;

import db.buffers.BlockStream;
import db.buffers.DataBuffer;
Expand Down Expand Up @@ -278,7 +279,12 @@ protected Socket connect() throws IOException {
}

SocketFactory socketFactory = SSLSocketFactory.getDefault();
Socket socket = socketFactory.createSocket(streamServerIPAddress, streamServerPort);
SSLSocket socket = (SSLSocket)socketFactory.createSocket(streamServerIPAddress, streamServerPort);

List<SNIServerName> serverNames = Arrays.asList(new SNIHostName(streamServerIPAddress));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Consider using the hostname rather than IP address for SNI.

SNI is designed to indicate the hostname being accessed, not the IP address. While using the IP address will work technically, it may not provide the expected behavior with certain proxy configurations that rely on hostname-based routing.

-		List<SNIServerName> serverNames = Arrays.asList(new SNIHostName(streamServerIPAddress));
+		// Use the original hostname if available, falling back to IP address if necessary
+		String sniHostname = streamServerIPAddress;
+		List<SNIServerName> serverNames = Arrays.asList(new SNIHostName(sniHostname));

Note: This implementation would require tracking and passing the original hostname through the API. The current implementation using IP address will work for basic SNI functionality, but might not provide the optimal behavior with hostname-based proxy routing.

🌐 Web query:

Is using an IP address with SNI (Server Name Indication) valid?

💡 Result:

Using an IP address with SNI (Server Name Indication) is not valid according to the SNI specification and its intended purpose.

SNI is an extension to the TLS protocol that allows a client to specify the hostname (domain name) it wants to connect to during the TLS handshake, enabling servers to present the correct certificate for that hostname even when multiple domains are hosted on the same IP address[2][3][7]. The key point is that SNI is designed for hostnames, not IP addresses.

If a client attempts to use an IP address in the SNI field:

  • Most servers will ignore the SNI field or be unable to present the correct certificate, because the certificate is typically issued for a domain name, not an IP address[3][9].
  • The SNI specification requires a "hostname" (per RFC 3546 and RFC 6066) in the SNI extension, not an IP address. Using an IP address does not comply with the standard and may result in errors or a fallback to a default certificate, leading to browser security warnings about certificate mismatches[2][3][9].

In summary:
SNI is designed to be used with domain names, not IP addresses. Using an IP address in the SNI field is not valid, and is not supported by the protocol or by major web servers. If you use an IP address in SNI, you will likely encounter connection errors or certificate warnings.[2][3][9]

Citations:

Use DNS hostname for SNI instead of IP address

SNI (per RFC 6066) must carry the server’s DNS name. Supplying an IP address will be ignored by many servers or cause a default‐certificate fallback, leading to TLS failures or certificate mismatches.

Please update the code at:

  • Ghidra/Features/GhidraServer/src/main/java/ghidra/server/stream/RemoteBlockStreamHandle.java: line 284

Suggested changes:

  • Propagate the original hostname (e.g. streamServerHostname) through your API.
  • Construct the SNI value from the hostname, falling back to the IP only if the hostname is unavailable.

Example diff:

-   List<SNIServerName> serverNames = Arrays.asList(new SNIHostName(streamServerIPAddress));
+   // Use DNS hostname for SNI, fallback to IP if necessary
+   String sniHostname = (streamServerHostname != null && !streamServerHostname.isEmpty())
+           ? streamServerHostname
+           : streamServerIPAddress;
+   List<SNIServerName> serverNames = Arrays.asList(new SNIHostName(sniHostname));

You’ll need to update the surrounding API to accept and pass along streamServerHostname accordingly.

Committable suggestion skipped: line range outside the PR's diff.

SSLParameters params = socket.getSSLParameters();
params.setServerNames(serverNames);
socket.setSSLParameters(params);

// TODO: set socket options ?

Expand Down
44 changes: 44 additions & 0 deletions Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/client/GhidraSSLClientSocket.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/* ###
* IP: GHIDRA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package ghidra.framework.client;

import java.io.IOException;
import java.net.*;
import java.util.*;

import javax.net.ssl.*;
import javax.rmi.ssl.SslRMIClientSocketFactory;

/**
* <code>GhidraSSLClientSocket</code> facilitates ability to impose client authentication
* for SSL server sockets used with Ghidra Server RMI connection.
*/
public class GhidraSSLClientSocket extends SslRMIClientSocketFactory {
/**
* Creates an SSLSocket on a given port
*
* @throws IOException if an error occurs on socket creation.
*/
public Socket createSocket(String host, int port) throws IOException
{
SSLSocket sslSocket = (SSLSocket) super.createSocket(host, port);
List<SNIServerName> serverNames = Arrays.asList(new SNIHostName(host));
SSLParameters params = sslSocket.getSSLParameters();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Improvement Opportunity: Consider adding error handling for SNI configuration in GhidraSSLClientSocket and RemoteBlockStreamHandle. While the current implementation is functionally correct, adding try/catch blocks around the SNI configuration would make the code more robust against unexpected errors during the SNI setup process, ensuring graceful degradation if SNI configuration fails.

Recommendation:

try {
  List<SNIServerName> serverNames = Arrays.asList(new SNIHostName(host));
  SSLParameters params = sslSocket.getSSLParameters();
  params.setServerNames(serverNames);
  sslSocket.setSSLParameters(params);
} catch (IllegalArgumentException e) {
  // Log warning but continue with connection without SNI
  // This could happen if the hostname is invalid for SNI
}

params.setServerNames(serverNames);
sslSocket.setSSLParameters(params);
return sslSocket;
}
}
5 changes: 2 additions & 3 deletions Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/client/ServerConnectTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLSocket;
import javax.rmi.ssl.SslRMIClientSocketFactory;
import javax.security.auth.Subject;
import javax.security.auth.callback.Callback;
import javax.security.auth.callback.PasswordCallback;
Expand Down Expand Up @@ -163,7 +162,7 @@ public static GhidraServerHandle getGhidraServerHandle(ServerInfo server) throws
}
catch (IOException e) {
reg = LocateRegistry.getRegistry(server.getServerName(), server.getPortNumber(),
new SslRMIClientSocketFactory());
new GhidraSSLClientSocket());
checkServerBindNames(reg);
}

Expand Down Expand Up @@ -321,7 +320,7 @@ else if (pkiSignatureCb != null) {
private static void testServerSSLConnection(ServerInfo server) throws IOException {

RMIServerPortFactory portFactory = new RMIServerPortFactory(server.getPortNumber());
SslRMIClientSocketFactory factory = new SslRMIClientSocketFactory();
GhidraSSLClientSocket factory = new GhidraSSLClientSocket();
String serverName = server.getServerName();
int sslRmiPort = portFactory.getRMISSLPort();

Expand Down