Changeset 1211

Timestamp:

08/16/08 13:52:44 (4 months ago)

Author:

mbr

Message:

- Bugfix for 'Premature end of base64 data' and 'Premature padding of
base64 data' warning messages from MIME::Base64 errors. Now fwknopd
applies more rigorous checks for base64 encoded characters, and either
of these two messages above will result in the packet data being
discarded before it is sent through any decryption function. Mike
Holzmann reported this issue.

Files:

• fwknop/trunk/CREDITS (modified) (1 diff)
• fwknop/trunk/ChangeLog (modified) (1 diff)
• fwknop/trunk/fwknopd (modified) (10 diffs)
• fwknop/trunk/test/fwknop_test.pl (modified) (6 diffs)

fwknop/trunk/CREDITS

@@ -233 +233 @@
-      the GPG_NO_OPTIONS variable in the access.conf file for the fwknopd
-      daemon.
+    - Reported lots of 'Premature end of base64 data' and 'Premature padding
+      of base64 data' warning messages.  A fix was applied to fwknopd to apply
+      more rigorous checks for base64 encoded characters, and either of the
+      two messages above will result in the packet data being discarded before
+      it is sent through any decryption function.

fwknop/trunk/ChangeLog

@@ -33 +33 @@
-      option in the default options file was causing SPA packets to exceed
-      1500 bytes when encrypted with a 2048-bit GnuPG key.
+    - Bugfix for 'Premature end of base64 data' and 'Premature padding of
+      base64 data' warning messages from MIME::Base64 errors.  Now fwknopd
+      applies more rigorous checks for base64 encoded characters, and either
+      of these two messages above will result in the packet data being
+      discarded before it is sent through any decryption function.  Mike
+      Holzmann reported this issue.
-
-fwknop-1.9.6 (07/18/2008):

fwknop/trunk/fwknopd

@@ -499 +499 @@
-    }
-
+    ### check to make sure the packet data only contains base64 encoded
+    ### characters (see RFC 3548 for why the following negated character
+    ### class checks for all non-base64 encoded characters).
+    if ($transport_data =~ /[^\x30-\x39\x41-\x5a\x61-\x7a\x2b\x2f\x3d]/) {
+        if ($debug) {
+            print STDERR localtime() . " [+] Packet contains non-base64 ",
+                "encoded characters, skipping.\n";
+            &check_packet_limit();
+        }
+        return;
+    }
+
-    ### see if this packet is worthy of being granted access through
-    ### the firewall
@@ -671 +683 @@
-    }
-
+    &check_packet_limit();
+    return;
+}
+
+sub check_packet_limit() {
-    ### see if we need to exit if the packet limit (set with -C on the
-    ### command line) has been reached
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
-    return;
-}
@@ -1220 +1236 @@
-                ### see if we need to exit if the packet limit (set with -C on the
-                ### command line) has been reached
-
-
-
-
-
-
-
-
+
+
-                $rv = 1;
-                last;
@@ -1986 +1996 @@
-    ### see if we need to exit if the packet limit (set with -C on the
-    ### command line) has been reached
-
-
-
-
-
-
-
-
+
-
-    if ($os_fprint_only) {
@@ -2238 +2241 @@
-    my $pid;
-    my $decrypted_msg = '';
+    my $base64_decoded_msg = '';
-    my $found_sig     = 0;
-    my $gpg_sign_id   = '';
@@ -2255 +2259 @@
-    }
-
+    ### base64 decode the packet
+    $base64_decoded_msg = decode_base64($msg);
+
+    ### continue only if decode_base64() had no "Premature end of base64 data"
+    ### errors - we want to minimize code that executes against suspicious
+    ### packet data
+    if ($warn_msg =~ /Premature\s+end/i
+            or $warn_msg =~ /Premature\s+padding/i) {
+        if ($debug) {
+            print STDERR localtime() . " [-] $warn_msg";
+        }
+        return $decrypt_rv, $decrypted_msg, $gpg_sign_id;
+    }
+
+    ### look for the 0x8502 GnuPG prefix
+    unless ($base64_decoded_msg =~ /^\x85\x02/) {
+        if ($debug) {
+            print STDERR localtime() . " [-] base64-decoded data does not begin ",
+                "with 0x8502\n";
+        }
+        return $decrypt_rv, $decrypted_msg, $gpg_sign_id;
+    }
+
-    print STDERR localtime() . " [+] Attempting GnuPG decrypt...\n" if $debug;
-    if ($debug and $verbose) {
-        print STDERR localtime() . "     Decrypting raw data (hex dump):\n";
-decode_base64($msg)
+$base64_decoded_msg
-    }
-
@@ -2316 +2343 @@
-    close $pw;
-
-decode_base64($msg)
+$base64_decoded_msg
-    close $input;
-
@@ -2370 +2397 @@
-    my $decrypted_msg = '';
-    my $decrypt_rv    = 0;
+    my $base64_decoded_msg = '';
-
-    unless ($msg =~ /^U2FsdGVkX1/) {
@@ -2387 +2415 @@
-    }
-
+    ### base64 decode the packet
+    $base64_decoded_msg = decode_base64($msg);
+
+    ### continue only if decode_base64() had no "Premature end of base64 data"
+    ### errors - we want to minimize code that executes against suspicious
+    ### packet data
+    if ($warn_msg =~ /Premature\s+end/i
+            or $warn_msg =~ /Premature\s+padding/i) {
+        if ($debug) {
+            print STDERR localtime() . " [-] $warn_msg";
+        }
+        return $decrypt_rv, $decrypted_msg;
+    }
+
+    ### look for the Salted__ prefix
+    unless ($base64_decoded_msg =~ /^Salted__/) {
+        if ($debug) {
+            print STDERR localtime() . " [-] base64-decoded data does not ",
+                "begin with 'Salted__'\n";
+        }
+        return $decrypt_rv, $decrypted_msg;
+    }
+
-    print STDERR localtime() . " [+] Attempting Rijndael decrypt...\n" if $debug;
-
-    if ($debug and $verbose) {
-        print STDERR localtime() . "     Decrypting raw data (hex dump):\n";
-decode_base64($msg)
+$base64_decoded_msg
-    }
-
@@ -2399 +2450 @@
-    });
-    eval {
-decode_base64($msg)
+$base64_decoded_msg
-    };
-    if ($debug and $verbose) {

fwknop/trunk/test/fwknop_test.pl

@@ -5 +5 @@
-# File: fwknop_test.pl
-#
-ing infrastructure for the fwknop
-Single Packet 
+ suite for the fwknop Single Packet
+
-#
-# Author: Michael Rash (mbr@cipherdyne.org)
@@ -240 +240 @@
-&test_driver("(Replay attacks, broken data) $NUM_RAND random packets",
-    \&packet_randomness);
+
-&test_driver('(Replay attacks, broken data) Truncated SPA packet',
-    \&truncated_SPA_packet);
@@ -251 +252 @@
-&test_driver('(Replay attacks, broken data) Sniffing broken SPA packet',
-    \&bogus_SPA_sniff_decrypt);
+&test_driver('(Replay attacks, broken data) Firewall rules do not exist',
+    \&fw_rules_removed);
+&stop_fwknopd_quiet('(Replay attacks, broken data)');
+
+&test_driver('(Replay attacks, broken data) non-base64 SPA packet',
+    \&non_base64_SPA_packet);
+&test_driver('(Replay attacks, broken data) Sniffing non-base64 SPA packet',
+    \&non_base64_SPA_sniff_decrypt);
-&test_driver('(Replay attacks, broken data) Firewall rules do not exist',
-    \&fw_rules_removed);
@@ -2367 +2376 @@
-sub truncated_SPA_packet() {
-    my $rv = &get_access_packet($default_fwknop_args, $NO_QUIET);
-
-
+
+
+
+
+
+
+
+
+
-    return $rv;
-}
@@ -2393 +2409 @@
-                ### ANY (# 1 in access.conf)
-                $found_err = 1;
-
-
+
-            } elsif (/\[\-\]\s+Decrypted.*not\s+conform/i) {
-                ### [-] Decrypted message does not conform to a valid SPA packet
-                return &print_errors("[-] Invalid SPA packet");
-                $found_err = 1;
- 
+
-            }
-        }
@@ -2407 +2421 @@
-        } else {
-            return &print_errors("[*] fwknopd accepted truncated SPA packet");
+        }
+    }
+    return &print_errors("[-] Sniff alarm ($sniff_alarm seconds) expired");
+}
+
+sub non_base64_SPA_sniff_decrypt() {
+
+    if (&run_fwknopd($cache_encrypted_spa_packet,
+            $default_fwknop_conf, $default_access_conf)) {
+
+        ### now that fwknopd has exited, see if the SPA packet was valid
+        my $found_err = 0;
+        open SE, "< $current_test_file"
+            or die "[*] Could not open $current_test_file: $!";
+        while (<SE>) {
+            if (/Packet\s+contains\s+non-base64\s+encoded\s+characters/) {
+                $found_err = 1;
+                last;
+            }
+        }
+        close SE;
+        if ($found_err) {
+            return 1;
+        } else {
+            return &print_errors("[*] fwknopd accepted non-base64 encoded SPA packet");
-        }
-    }