Extend ServerKeyExchange parsing to work with a signature
authorMatt Caswell <matt@openssl.org>
Thu, 5 Jan 2017 12:32:06 +0000 (12:32 +0000)
committerMatt Caswell <matt@openssl.org>
Tue, 10 Jan 2017 23:02:50 +0000 (23:02 +0000)
Previously SKE in TLSProxy only knew about one anonymous ciphersuite so
there was never a signature. Extend that to include a ciphersuite that is
not anonymous. This also fixes a bug where the existing SKE processing was
checking against the wrong anon ciphersuite value. This has a knock on
impact on the sslskewith0p test. The bug meant the test was working...but
entirely by accident!

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)

test/recipes/70-test_sslskewith0p.t
util/TLSProxy/Message.pm
util/TLSProxy/ServerKeyExchange.pm

index bfdee8b739c9412c6030b07d036119d32e183a22..dc3d9d916555d3003e2956798803f569434048e9 100755 (executable)
@@ -41,6 +41,7 @@ my $proxy = TLSProxy::Proxy->new(
 $proxy->cipherc('ADH-AES128-SHA:@SECLEVEL=0');
 $proxy->ciphers('ADH-AES128-SHA:@SECLEVEL=0');
 
+$proxy->clientflags("-no_tls1_3");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 1;
 ok(TLSProxy::Message->fail, "ServerKeyExchange with 0 p");
index 1b87befe32a09d09e98c630425a3c845ff5e0368..438209fc40ba60333f75340640687e935074061d 100644 (file)
@@ -84,7 +84,8 @@ use constant {
 };
 
 use constant {
-    CIPHER_ADH_AES_128_SHA => 0x03000034
+    CIPHER_DHE_RSA_AES_128_SHA => 0x0033,
+    CIPHER_ADH_AES_128_SHA => 0x0034
 };
 
 my $payload = "";
index 7640b3f55bd53e6300a5a5a414e83926bef018d5..cb4cc7c7625a58946cdaa7cde9e2c090e4ce7e2f 100644 (file)
@@ -33,6 +33,7 @@ sub new
     $self->{p} = "";
     $self->{g} = "";
     $self->{pub_key} = "";
+    $self->{sigalg} = -1;
     $self->{sig} = "";
 
     return $self;
@@ -41,10 +42,13 @@ sub new
 sub parse
 {
     my $self = shift;
+    my $sigalg = -1;
 
     #Minimal SKE parsing. Only supports one known DHE ciphersuite at the moment
-    return if (TLSProxy::Proxy->ciphersuite()
-               != TLSProxy::Message::CIPHER_ADH_AES_128_SHA);
+    return if TLSProxy::Proxy->ciphersuite()
+                 != TLSProxy::Message::CIPHER_ADH_AES_128_SHA
+              && TLSProxy::Proxy->ciphersuite()
+                 != TLSProxy::Message::CIPHER_DHE_RSA_AES_128_SHA;
 
     my $p_len = unpack('n', $self->data);
     my $ptr = 2;
@@ -62,18 +66,28 @@ sub parse
     $ptr += $pub_key_len;
 
     #We assume its signed
-    my $sig_len = unpack('n', substr($self->data, $ptr));
+    my $record = ${$self->records}[0];
+
+    if (TLSProxy::Proxy->is_tls13()
+            || $record->version() == TLSProxy::Record::VERS_TLS_1_2) {
+        $sigalg = unpack('n', substr($self->data, $ptr));
+        $ptr += 2;
+    }
     my $sig = "";
-    if (defined $sig_len) {
-       $ptr += 2;
-       $sig = substr($self->data, $ptr, $sig_len);
-       $ptr += $sig_len;
+    if (defined $sigalg) {
+        my $sig_len = unpack('n', substr($self->data, $ptr));
+        if (defined $sig_len) {
+            $ptr += 2;
+            $sig = substr($self->data, $ptr, $sig_len);
+            $ptr += $sig_len;
+        }
     }
 
     $self->p($p);
     $self->g($g);
     $self->pub_key($pub_key);
-    $self->sig($sig);
+    $self->sigalg($sigalg) if defined $sigalg;
+    $self->signature($sig);
 }
 
 
@@ -89,9 +103,10 @@ sub set_message_contents
     $data .= $self->g;
     $data .= pack('n', length($self->pub_key));
     $data .= $self->pub_key;
-    if (length($self->sig) > 0) {
-        $data .= pack('n', length($self->sig));
-        $data .= $self->sig;
+    $data .= pack('n', $self->sigalg) if ($self->sigalg != -1);
+    if (length($self->signature) > 0) {
+        $data .= pack('n', length($self->signature));
+        $data .= $self->signature;
     }
 
     $self->data($data);
@@ -123,7 +138,15 @@ sub pub_key
     }
     return $self->{pub_key};
 }
-sub sig
+sub sigalg
+{
+    my $self = shift;
+    if (@_) {
+      $self->{sigalg} = shift;
+    }
+    return $self->{sigalg};
+}
+sub signature
 {
     my $self = shift;
     if (@_) {