TLS validation of wildcard subjectAltName domain + SSLKEYLOGFILE support by nbanb · Pull Request #187 · masneyb/gftp

nbanb · 2025-04-14T13:57:11Z

20250414:
First commit add support of TLS validation for wildcard domain names (*.domain.tld) present in certificate subjectAltName field
See 20250414 comments

EDIT 20250417:
The second commit add support for OpenSSL SSLKEYLOGFILE to be able to decrypt TLS1.2+ with elliptic curves (ECDHE_*)
See 20250417 comments

Adding support of TLS validation for wildcard domain names (*.domain.tld) present in certificate subjectAltName field

nbanb · 2025-04-14T14:06:39Z

Hi

This pull request to solve issue #186:
gFTP don't match wildcard domains in subjectAltName of TLS certificate

It add support of TLS validation of <host>.domain.tld when *.domain.tld is included in TLS certificate subjectAltName

Kind regards
nbanba

nbanb · 2025-04-15T08:13:02Z

Hi

@masneyb
After some search, it appears that this evolution makes sense in 2025 regarding how TLS certificate, wildcard domain name and subjectAltName are used.

From RFC6125#section-6.4.3:
Wildcard SANs are allowed, and clients may implement matching against them

A client employing this specification's verification procedures MAY
match the reference identifier against a presented identifier whose
DNS domain name portion contains the wildcard character '*'.

From RFC2818#section-3.1 (Following RFC2818 is recommended by RFC4217):
SubjectAltName must be preferred versus CommonName

If a subjectAltName extension of type dNSName is present, that MUST
be used as the identity.

Except a mistake from me, adding support of TLS validation for wildcard domains names present in subjectAltName is not just a practical enhancement, it also seems to be year 2025 usage and standards-compliant

Kind regards
nbanba

nbanb · 2025-04-17T17:22:29Z

Hi

This new commit add support of OpenSSL SSLKEYLOGFILE to be able to decrypt FTPS TLSv1.2+ traffic using Diffie Hellman and Elliptic Curves like cipher TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384

To create SSLKEYLOGFILE, simply run from your shell :

export SSLKEYLOGFILE=~/gftp-tls.log

And launch gftp.

If you capture traffic with a command like:

sudo tcpdump -nnei any -vvvttttXXX 'host <server_ip> and host <client_ip> and (port 21 or port <passive_port>)' -w ~/gftp.pcap

You can pass gftp-tls.log and gftp.pcap to a software like wireshark and you will be able to decrypt the TLS traffic and to see FTP command and reply.

Enjoy!

Kind regards,
nbanba

masneyb · 2025-04-28T11:07:08Z

lib/sslcommon.c

+	* 20250414: Nicolas Baranger  
+	* Adding support of TLS validation for wildcard domain names (*.domain.tld)
+	* present in certificate subjectAltName field  
+	****************************************************************************/


Please drop the comments with the names / dates for consistency with the rest of the code. This information is stored in the git commit log. git blame is a useful tool to show exactly what you want here.

masneyb · 2025-04-28T11:07:24Z

lib/sslcommon.c

+	* Adding support of TLS validation for wildcard domain names (*.domain.tld)
+	* present in certificate subjectAltName field  
+	****************************************************************************/
+  	/* Commenting existing code */ 	


Drop this comment as well.

lib/sslcommon.c

masneyb · 2025-04-28T11:09:17Z

lib/sslcommon.c

+		}
+	/**************************************************************************
+	* END OF MODIFICATIONS  
+	****************************************************************************/


Drop unnecessary comment

lib/sslcommon.c

nbanb · 2025-04-28T12:17:53Z

Hi Brian

Thanks for answer and help !
Ok I will make all suggested corrections today and provide a new commit with these changes.
Going back here when done.

Kind regards
Nicolas

masneyb · 2025-04-28T12:54:01Z

Please squash your changes into your previous commits. Here's an article that I found that shows how to do this: https://medium.com/@slamflipstrom/a-beginners-guide-to-squashYou can do git commit --amend or create new commits, and then squash them with `git reing-commits-with-git-rebase-8185cf6e62ec

masneyb · 2025-04-28T12:54:27Z

You'll also need to do git push --force to update the commits in the merge request.

nbanb · 2025-04-28T17:24:20Z

Hi Brian

After modifying my local repo with suggested changes here is what I did:

git add .
git commit --amend
git push origin master --force

Here is a diff summarizing all asked changes:

diff --git a/gftp-old/lib/sslcommon.c b/gftp/lib/sslcommon.c
index a22a785..e1302a8 100644
--- a/../sslcommon.c_gftp
+++ b/lib/sslcommon.c
@@ -200,23 +200,7 @@ static int gftp_ssl_post_connection_check (gftp_request * request)
               ext_str = meth->d2i(NULL, &mutable_ptr, len);
 #endif
               val = meth->i2v(meth, ext_str, NULL);
-       /**************************************************************************
-       * 20250414: Nicolas Baranger  
-       * Adding support of TLS validation for wildcard domain names (*.domain.tld)
-       * present in certificate subjectAltName field  
-       ****************************************************************************/
-       /* Commenting existing code */  
-             /* for (j = 0; j < sk_CONF_VALUE_num(val); j++)
-                {
-                  nval = sk_CONF_VALUE_value (val, j);
-                  if (strcmp (nval->name, "DNS") == 0 && 
-                      strcmp (nval->value, request->hostname) == 0)
-                    {
-                      ok = 1;
-                      break;
-                    }
-                } */
-       /* Rewriting 'for' loop with wildcard support (*.domain.tld) in subjectAltName */       
+       /* 'for' loop with wildcard support (*.domain.tld) in subjectAltName */         
              for (j = 0; j < sk_CONF_VALUE_num(val); j++)
                {
                  nval = sk_CONF_VALUE_value(val, j);
@@ -242,9 +226,6 @@ static int gftp_ssl_post_connection_check (gftp_request * request)
                    }
                  }
                }
-       /**************************************************************************
-       * END OF MODIFICATIONS  
-       ****************************************************************************/
             }
 
           if (ok)
@@ -352,11 +333,11 @@ static void _gftp_ssl_thread_setup (void)
   CRYPTO_set_dynlock_destroy_callback (_gftp_ssl_destroy_dyn_mutex);
 } 
 
-/****************************************************************************/
-/* 20250417 Nicolas Baranger */
-/* Adding OpenSSL SSLKEYLOGFILE support */
-/****************************************************************************/
-static void gftp_ssl_keylog_callback(const SSL *ssl, const char *line) 
+
+#ifndef __maybe_unused
+#define __maybe_unused __attribute__((__unused__))
+#endif
+static __maybe_unused void gftp_ssl_keylog_callback(const SSL *ssl, const char *line) 
 {
   const char *gftp_keylog_file = getenv("SSLKEYLOGFILE");
   if (!gftp_keylog_file)
@@ -369,9 +350,6 @@ static void gftp_ssl_keylog_callback(const SSL *ssl, const char *line)
   fprintf(gftp_fp, "%s\n", line);
   fclose(gftp_fp);
 }
-/***************************************************************************/
-/* END ADDITIONS  */
-/****************************************************************************/
 
 
 int gftp_ssl_startup (gftp_request * request)

I let this comment at line 203 in lib/sslcommon.c::

        /* 'for' loop with wildcard support (*.domain.tld) in subjectAltName */

Let me know if it's ok, I stay available if needed

Thanks again
King regards
nbanba

masneyb · 2025-05-04T22:25:01Z

lib/sslcommon.c

-                      break;
-                    }
-                }
+  	/* 'for' loop with wildcard support (*.domain.tld) in subjectAltName */ 	


Drop this comment

Hi Brian

Ok I drop it

lib/sslcommon.c

masneyb · 2025-05-04T22:34:42Z

lib/sslcommon.c


+#ifndef __maybe_unused
+#define __maybe_unused __attribute__((__unused__))
+#endif


I know I suggested this earlier, however it can be dropped. Sorry. I didn't have time when I first did a review for a more thorough review.

Ok I drop it and replace line 337:

static __maybe_unused void gftp_ssl_keylog_callback(const SSL *ssl, const char *line)

by

static void gftp_ssl_keylog_callback(const SSL *ssl, const char *line)

masneyb · 2025-05-04T22:36:13Z

lib/sslcommon.c

+  	/* 'for' loop with wildcard support (*.domain.tld) in subjectAltName */ 	
+	      for (j = 0; j < sk_CONF_VALUE_num(val); j++)
+		{
+		  nval = sk_CONF_VALUE_value(val, j);


So these first 3 lines are the same as what was there before, and it's showing up as lines added/removed. I suspect it's most likely a difference in tabs or spaces between the two. Can you clean this up so the diff is smaller? The logic of the function in your changes looks good to me.

Hi Brian

You're right, the first 8 spaces had been replaced by 1 tab

I will replace all tab by 8 spaces even in new code parts
Nicolas

masneyb · 2025-05-04T22:37:00Z

Hi @nbanb : I had time for a more thorough review. Sorry it took me longer than I would have liked this week to get back to you.

…x version check Adding support of OpenSSL SSLKEYLOGFILE to be able to decrypt FTPS TLSv1.2+ traffic using Diffie Hellman and Elliptic Curves like cipher TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 To create SSLKEYLOGFILE, simply run from your shell : ```bash export SSLKEYLOGFILE=~/gftp-tls.log ``` And launch `gftp`. If you capture traffic with a command like: ```bash sudo tcpdump -nnei any -vvvttttXXX 'host <server_ip> and host <client_ip> and (port 21 or port <passive_port>)' -w ~/gftp.pcap ``` You can pass `gftp-tls.log` and `gftp.pcap` to a software like `wireshark` and you will be able to decrypt the TLS traffic and to see FTP command and reply. Example of live FTPS traffic decryption using `tshark` (`Wireshark` command line tool) in a `tcpdump` style: ```bash sudo setcap CAP_NET_RAW,CAP_NET_ADMIN,CAP_DAC_OVERRIDE+eip /usr/bin/dumpcap tshark -i <network_interface> -n -o tls.keylog_file:~/gftp-tls.log -o tls.desegment_ssl_records:TRUE -o tls.desegment_ssl_application_data:TRUE -t ad -l -f 'host <server_ip> and host <client_ip> and (port 21 or port <passive_port>)' -T fields -e frame.number -e frame.time_relative -e ip.src -e ip.dst -e tcp.srcport -e tcp.dstport -e tcp.flags.str -e tcp.len -e _ws.col.Protocol -e _ws.col.Info ```

nbanb · 2025-05-05T15:09:44Z

Hi Brian

Thanks for your help and time reviewing the code !

All asked changes had been made:

removing unecessary leading comments
correcting typos issues(to reduce diff)
suppressing ALL remaining OpenSSL old versions conditional test (including those I didn't add)
suppressing the recent __maybe_unused addition and related definitions

Also I git commit --amend and git push origin master --force on my github repo to update this PR (don't know if it's OK but you asked to proceed rhis way last week)

Last thing, I did update the last commit comment to:

Adding OpenSSL SSLKEYLOGFILE support in gFTP and removing OpenSSL <3.x version check

Adding support of OpenSSL SSLKEYLOGFILE to be able to decrypt FTPS TLSv1.2+ traffic using Diffie Hellman and Elliptic Curves like cipher TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384

To create SSLKEYLOGFILE, simply run from your shell :
export SSLKEYLOGFILE=~/gftp-tls.log
And launch gftp.

If you capture traffic with a command like:
sudo tcpdump -nnei any -vvvttttXXX 'host <server_ip> and host <client_ip> and (port 21 or port <passive_port>)' -w ~/gftp.pcap
You can pass gftp-tls.log and gftp.pcap to a software like wireshark and you will be able to decrypt the TLS traffic and to see FTP command and reply.

Example of live FTPS traffic decryption using tshark (Wireshark command line tool) in a tcpdump style:
sudo setcap CAP_NET_RAW,CAP_NET_ADMIN,CAP_DAC_OVERRIDE+eip /usr/bin/dumpcap
tshark -i <network_interface> -n -o tls.keylog_file:~/gftp-tls.log -o tls.desegment_ssl_records:TRUE -o tls.desegment_ssl_application_data:TRUE -t ad -l -f 'host <server_ip> and host <client_ip> and (port 21 or port <passive_port>)' -T fields -e frame.number -e frame.time_relative -e ip.src -e ip.dst -e tcp.srcport -e tcp.dstport -e tcp.flags.str -e tcp.len -e _ws.col.Protocol -e _ws.col.Info

Maybe part of this comment should be added to documentation. Thus writing somewhere (in doc) that only versions of OpenSSL supported by OpenSSL itself can be used and supported by gFTP may be an idea

Please let me know if all OK now or if further changes are needed.

Thanks and kind regards
nbanba

masneyb · 2025-05-05T21:43:21Z

Merged. Thank you for your contribution!

nbanb · 2025-05-06T06:04:49Z

Hi Brian
I closed issue #186 (solved by PR #187)
Thanks for your valuable help !

TLS validation of wildcard subjectAltName domain

4ba37c6

Adding support of TLS validation for wildcard domain names (*.domain.tld) present in certificate subjectAltName field

nbanb mentioned this pull request Apr 14, 2025

gFTP don't match wildcard domains in subjectAltName of TLS certificate #186

Closed

nbanb changed the title ~~TLS validation of wildcard subjectAltName domain~~ TLS validation of wildcard subjectAltName domain + SSLKEYLOGFILE support Apr 17, 2025

masneyb reviewed Apr 28, 2025

View reviewed changes

lib/sslcommon.c Show resolved Hide resolved

masneyb reviewed Apr 28, 2025

View reviewed changes

lib/sslcommon.c Show resolved Hide resolved

masneyb reviewed Apr 28, 2025

View reviewed changes

lib/sslcommon.c Show resolved Hide resolved

nbanb force-pushed the master branch from a7ed926 to 24295f0 Compare April 28, 2025 17:18

masneyb reviewed May 4, 2025

View reviewed changes

lib/sslcommon.c Outdated Show resolved Hide resolved

masneyb reviewed May 4, 2025

View reviewed changes

nbanb force-pushed the master branch from 24295f0 to 2b9ac2c Compare May 5, 2025 14:16

masneyb merged commit df0a5d0 into masneyb:master May 5, 2025

nbanb mentioned this pull request Jan 19, 2026

gFTP now support SSLKEYLOGFILE and tshark is decrypting pocc/tshark.dev#66

Closed

Conversation

nbanb commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbanb commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbanb commented Apr 15, 2025

Uh oh!

nbanb commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nbanb commented Apr 28, 2025

Uh oh!

masneyb commented Apr 28, 2025

Uh oh!

masneyb commented Apr 28, 2025

Uh oh!

nbanb commented Apr 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

masneyb commented May 4, 2025

Uh oh!

nbanb commented May 5, 2025

Uh oh!

masneyb commented May 5, 2025

Uh oh!

nbanb commented May 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nbanb commented Apr 14, 2025 •

edited

Loading

nbanb commented Apr 14, 2025 •

edited

Loading

nbanb commented Apr 17, 2025 •

edited

Loading