editcap: warn when --inject-secrets is given a RSA private key

While the documentation of "editcap --inject-secrets" mentions support
for key log files only, people might misinterpret that and assume
support for RSA private keys. This is explicitly not supported due to
the sensitivity of these files. In order to be helpful, print a warning.

Change-Id: Ia7b464c17f1dfb550729dd35775290ed28e14510
Reviewed-on: https://code.wireshark.org/review/31893
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

2 files changed, 41 insertions, 0 deletions

diff --git a/editcap.c b/editcap.c
index c1616db61e..5d162c31be 100644
--- a/editcap.c
+++ b/editcap.c
@@ -932,6 +932,24 @@ lookup_secrets_type(const char *type)
     return 0;
 }
 
+static void
+validate_secrets_file(const char *filename, guint32 secrets_type, const char *data)
+{
+    if (secrets_type == SECRETS_TYPE_TLS) {
+        /*
+         * A key log file is unlikely going to look like either:
+         * - a PEM-encoded private key file.
+         * - a BER-encoded PKCS #12 file ("PFX file"). (Look for a Constructed
+         *   SEQUENCE tag, e.g. bytes 0x30 which happens to be ASCII '0'.)
+         */
+        if (g_str_has_prefix(data, "-----BEGIN ") || data[0] == 0x30) {
+            fprintf(stderr,
+                    "editcap: Warning: \"%s\" is not a key log file, but an unsupported private key file. Decryption will not work.\n",
+                    filename);
+        }
+    }
+}
+
 static int
 framenum_compare(gconstpointer a, gconstpointer b, gpointer user_data _U_)
 {
@@ -1474,6 +1492,9 @@ main(int argc, char *argv[])
                 continue;
             }
 
+            /* Warn for badly formatted files, but proceed anyway. */
+            validate_secrets_file(secrets_filename, secrets_type_id, data);
+
             block = wtap_block_create(WTAP_BLOCK_DSB);
             dsb = (wtapng_dsb_mandatory_t *)wtap_block_get_mandatory_data(block);
             dsb->secrets_type = secrets_type_id;
diff --git a/test/suite_fileformats.py b/test/suite_fileformats.py
index 30eeb44d7a..a2a9232803 100644
--- a/test/suite_fileformats.py
+++ b/test/suite_fileformats.py
@@ -215,6 +215,26 @@ class case_fileformat_pcapng_dsb(subprocesstest.SubprocessTestCase):
             (0x544c534b, len(dsb2_contents), dsb2_contents),
         ))
 
+    def test_pcapng_dsb_bad_key(self, cmd_editcap, dirs, capture_file, check_pcapng_dsb_fields):
+        '''Insertion of a RSA key file is not very effective.'''
+        rsa_keyfile = os.path.join(dirs.key_dir, 'rsasnakeoil2.key')
+        p12_keyfile = os.path.join(dirs.key_dir, 'key.p12')
+        outfile = self.filename_from_id('rsasnakeoil2-dsb.pcapng')
+        proc = self.assertRun((cmd_editcap,
+            '--inject-secrets', 'tls,%s' % rsa_keyfile,
+            '--inject-secrets', 'tls,%s' % p12_keyfile,
+            capture_file('rsasnakeoil2.pcap'), outfile
+        ))
+        self.assertEqual(proc.stderr_str.count('unsupported private key file'), 2)
+        with open(rsa_keyfile, 'rb') as f:
+            dsb1_contents = f.read()
+        with open(p12_keyfile, 'rb') as f:
+            dsb2_contents = f.read()
+        check_pcapng_dsb_fields(outfile, (
+            (0x544c534b, len(dsb1_contents), dsb1_contents),
+            (0x544c534b, len(dsb2_contents), dsb2_contents),
+        ))
+
 
 @fixtures.mark_usefixtures('test_env')
 @fixtures.uses_fixtures