Update sample-keys

PATCH In review Steffan Karger 2 participants 7 messages May 11, 2017 – Jun 13, 2017

Share & tools

Steffan Karger

author

2017-05-11 21:13 UTC #0 lore raw

PATCH 1/3 Update sample-keys

So 'make check' works again - the old keys were expired. These are now the same keys as we use in release/2.3, release/2.4 and master.

Signed-off-by Steffan Karger <[email protected]...>

---
 sample-keys/ca.crt     |  48 ++++++++++++------
 sample-keys/ca.key     |  67 +++++++++++++++++++------
 sample-keys/client.crt | 126 ++++++++++++++++++++++++++++++-----------------
 sample-keys/client.key |  43 ++++++++++------
 sample-keys/server.crt | 130 +++++++++++++++++++++++++++++++++----------------
 sample-keys/server.key |  43 ++++++++++------
 6 files changed, 310 insertions(+), 147 deletions(-)

sample-keys/ca.crt +32 ~~-16~~

index e063ccc..a11bafa 100644
@@ -1,19 +1,35 @@
 -----BEGIN CERTIFICATE-----
-MIIDBjCCAm+gAwIBAgIBADANBgkqhkiG9w0BAQQFADBmMQswCQYDVQQGEwJLRzEL
+MIIGKDCCBBCgAwIBAgIJAKFO3vqQ8q6BMA0GCSqGSIb3DQEBCwUAMGYxCzAJBgNV
+BAYTAktHMQswCQYDVQQIEwJOQTEQMA4GA1UEBxMHQklTSEtFSzEVMBMGA1UEChMM
+T3BlblZQTi1URVNUMSEwHwYJKoZIhvcNAQkBFhJtZUBteWhvc3QubXlkb21haW4w
+HhcNMTQxMDIyMjE1OTUyWhcNMjQxMDE5MjE1OTUyWjBmMQswCQYDVQQGEwJLRzEL
 MAkGA1UECBMCTkExEDAOBgNVBAcTB0JJU0hLRUsxFTATBgNVBAoTDE9wZW5WUE4t
-VEVTVDEhMB8GCSqGSIb3DQEJARYSbWVAbXlob3N0Lm15ZG9tYWluMB4XDTA0MTEy
-NTE0NDA1NVoXDTE0MTEyMzE0NDA1NVowZjELMAkGA1UEBhMCS0cxCzAJBgNVBAgT
-Ak5BMRAwDgYDVQQHEwdCSVNIS0VLMRUwEwYDVQQKEwxPcGVuVlBOLVRFU1QxITAf
-BgkqhkiG9w0BCQEWEm1lQG15aG9zdC5teWRvbWFpbjCBnzANBgkqhkiG9w0BAQEF
-AAOBjQAwgYkCgYEAqPjWJnesPu6bR/iec4FMz3opVaPdBHxg+ORKNmrnVZPh0t8/
-ZT34KXkYoI9B82scurp8UlZVXG8JdUsz+yai8ti9+g7vcuyKUtcCIjn0HLgmdPu5
-gFX25lB0pXw+XIU031dOfPvtROdG5YZN5yCErgCy7TE7zntLnkEDuRmyU6cCAwEA
-AaOBwzCBwDAdBgNVHQ4EFgQUiaZg47rqPq/8ZH9MvYzSSI3gzEYwgZAGA1UdIwSB
-iDCBhYAUiaZg47rqPq/8ZH9MvYzSSI3gzEahaqRoMGYxCzAJBgNVBAYTAktHMQsw
-CQYDVQQIEwJOQTEQMA4GA1UEBxMHQklTSEtFSzEVMBMGA1UEChMMT3BlblZQTi1U
-RVNUMSEwHwYJKoZIhvcNAQkBFhJtZUBteWhvc3QubXlkb21haW6CAQAwDAYDVR0T
-BAUwAwEB/zANBgkqhkiG9w0BAQQFAAOBgQBfJoiWYrYdjM0mKPEzUQk0nLYTovBP
-I0es/2rfGrin1zbcFY+4dhVBd1E/StebnG+CP8r7QeEIwu7x8gYDdOLLsZn+2vBL
-e4jNU1ClI6Q0L7jrzhhunQ5mAaZztVyYwFB15odYcdN2iO0tP7jtEsvrRqxICNy3
-8itzViPTf5W4sA==
+VEVTVDEhMB8GCSqGSIb3DQEJARYSbWVAbXlob3N0Lm15ZG9tYWluMIICIjANBgkq
+hkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAsJVPCqt3vtoDW2U0DII1QIh2Qs0dqh88
+8nivxAIm2LTq93e9fJhsq3P/UVYAYSeCIrekXypR0EQgSgcNTvGBMe20BoHO5yvb
+GjKPmjfLj6XRotCOGy8EDl/hLgRY9efiA8wsVfuvF2q/FblyJQPR/gPiDtTmUiqF
+qXa7AJmMrqFsnWppOuGd7Qc6aTsae4TF1e/gUTCTraa7NeHowDaKhdyFmEEnCYR5
+CeUsx2JlFWAH8PCrxBpHYbmGyvS0kH3+rQkaSM/Pzc2bS4ayHaOYRK5XsGq8XiNG
+KTTLnSaCdPeHsI+3xMHmEh+u5Og2DFGgvyD22gde6W2ezvEKCUDrzR7bsnYqqyUy
+n7LxnkPXGyvR52T06G8KzLKQRmDlPIXhzKMO07qkHmIonXTdF7YI1azwHpAtN4dS
+rUe1bvjiTSoEsQPfOAyvD0RMK/CBfgEZUzAB50e/IlbZ84c0DJfUMOm4xCyft1HF
+YpYeyCf5dxoIjweCPOoP426+aTXM7kqq0ieIr6YxnKV6OGGLKEY+VNZh1DS7enqV
+HP5i8eimyuUYPoQhbK9xtDGMgghnc6Hn8BldPMcvz98HdTEH4rBfA3yNuCxLSNow
+4jJuLjNXh2QeiUtWtkXja7ec+P7VqKTduJoRaX7cs+8E3ImigiRnvmK+npk7Nt1y
+YE9hBRhSoLsCAwEAAaOB2DCB1TAdBgNVHQ4EFgQUK0DlyX319JY46S/jL9lAZMmO
+BZswgZgGA1UdIwSBkDCBjYAUK0DlyX319JY46S/jL9lAZMmOBZuhaqRoMGYxCzAJ
+BgNVBAYTAktHMQswCQYDVQQIEwJOQTEQMA4GA1UEBxMHQklTSEtFSzEVMBMGA1UE
+ChMMT3BlblZQTi1URVNUMSEwHwYJKoZIhvcNAQkBFhJtZUBteWhvc3QubXlkb21h
+aW6CCQChTt76kPKugTAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIBBjANBgkqhkiG
+9w0BAQsFAAOCAgEABc77f4C4P8fIS+V8qCJmVNSDU44UZBc+D+J6ZTgW8JeOHUIj
+Bh++XDg3gwat7pIWQ8AU5R7h+fpBI9n3dadyIsMHGwSogHY9Gw7di2RVtSFajEth
+rvrq0JbzpwoYedMh84sJ2qI/DGKW9/Is9+O52fR+3z3dY3gNRDPQ5675BQ5CQW9I
+AJgLOqzD8Q0qrXYi7HaEqzNx6p7RDTuhFgvTd+vS5d5+28Z5fm2umnq+GKHF8W5P
+ylp2Js119FTVO7brusAMKPe5emc7tC2ov8OFFemQvfHR41PLryap2VD81IOgmt/J
+kX/j/y5KGux5HZ3lxXqdJbKcAq4NKYQT0mCkRD4l6szaCEJ+k0SiM9DdTcBDefhR
+9q+pCOyMh7d8QjQ1075mF7T+PGkZQUW1DUjEfrZhICnKgq+iEoUmM0Ee5WtRqcnu
+5BTGQ2mSfc6rV+Vr+eYXqcg7Nxb3vFXYSTod1UhefonVqwdmyJ2sC79zp36Tbo2+
+65NW2WJK7KzPUyOJU0U9bcu0utvDOvGWmG+aHbymJgcoFzvZmlXqMXn97pSFn4jV
+y3SLRgJXOw1QLXL2Y5abcuoBVr4gCOxxk2vBeVxOMRXNqSWZOFIF1bu/PxuDA+Sa
+hEi44aHbPXt9opdssz/hdGfd8Wo7vEJrbg7c6zR6C/Akav1Rzy9oohIdgOw=
 -----END CERTIFICATE-----

sample-keys/ca.key +52 ~~-15~~

67 lines not on this page · raw message

sample-keys/client.crt +82 ~~-44~~

146 lines not on this page · raw message

sample-keys/client.key +28 ~~-15~~

43 lines not on this page · raw message

sample-keys/server.crt +88 ~~-42~~

151 lines not on this page · raw message

sample-keys/server.key +28 ~~-15~~

43 lines not on this page · raw message

Signature · 1 line

2.7.4

Steffan Karger

author

2017-05-11 21:13 UTC #1 lore raw

PATCH 2/3 cleanup: merge packet_id_alloc_outgoing() into packet_id_write()

From: Steffan Karger <[email protected]...>

The functions packet_id_alloc_outgoing() and packet_id_write() were always called in tandem. Instead of forcing the caller to allocate a packet_id_net to do so, merge the two functions. This simplifies the API and reduces the chance on mistakes in the future.

This patch was cherry-picked from 5d747770 (release/2.3), with the unit tests removed because release/2.2 does not have unit tests.

Signed-off-by Steffan Karger <[email protected]...>

---
 crypto.c    | 12 +++---------
 packet_id.c | 24 +++++++++++++++++++++---
 packet_id.h | 34 +++++++++++++---------------------
 3 files changed, 37 insertions(+), 33 deletions(-)

crypto.c +3 -9

index 6dd6a5f..b7b2f01 100644
@@ -119,23 +119,19 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
 	      /* Put packet ID in plaintext buffer or IV, depending on cipher mode */
 	      if (opt->packet_id)
 		{
-		  struct packet_id_net pin;
-		  packet_id_alloc_outgoing (&opt->packet_id->send, &pin, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM));
-		  ASSERT (packet_id_write (&pin, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true));
+		  ASSERT (packet_id_write (&opt->packet_id->send, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true));
 		}
 	    }
 	  else if (mode == EVP_CIPH_CFB_MODE || mode == EVP_CIPH_OFB_MODE)
 	    {
-	      struct packet_id_net pin;
 	      struct buffer b;
 
 	      ASSERT (opt->flags & CO_USE_IV);    /* IV and packet-ID required */
 	      ASSERT (opt->packet_id); /*  for this mode. */
 
-	      packet_id_alloc_outgoing (&opt->packet_id->send, &pin, true);
 	      memset (iv_buf, 0, iv_size);
 	      buf_set_write (&b, iv_buf, iv_size);
-	      ASSERT (packet_id_write (&pin, &b, true, false));
+	      ASSERT (packet_id_write (&opt->packet_id->send, &b, true, false));
 	    }
 	  else /* We only support CBC, CFB, or OFB modes right now */
 	    {

@@ -193,9 +189,7 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
 	{
 	  if (opt->packet_id)
 	    {
-	      struct packet_id_net pin;
-	      packet_id_alloc_outgoing (&opt->packet_id->send, &pin, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM));
-	      ASSERT (packet_id_write (&pin, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true));
+	      ASSERT (packet_id_write (&opt->packet_id->send, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true));
 	    }
 	  work = *buf;
 	}

packet_id.c +21 -3

index b11e71f..d190074 100644
@@ -248,12 +248,30 @@ packet_id_read (struct packet_id_net *pin, struct buffer *buf, bool long_form)
   return true;
 }
 
+static void
+packet_id_send_update(struct packet_id_send *p, bool long_form)
+{
+  if (!p->time)
+    {
+      p->time = now;
+    }
+  p->id++;
+  if (!p->id)
+    {
+      ASSERT(long_form);
+      p->time = now;
+      p->id = 1;
+    }
+}
+
 bool
-packet_id_write (const struct packet_id_net *pin, struct buffer *buf, bool long_form, bool prepend)
+packet_id_write (struct packet_id_send *p, struct buffer *buf, bool long_form,
+        bool prepend)
 {
-  packet_id_type net_id = htonpid (pin->id);
-  net_time_t net_time = htontime (pin->time);
+  packet_id_send_update(p, long_form);
 
+  const packet_id_type net_id = htonpid(p->id);
+  const net_time_t net_time = htontime(p->time);
   if (prepend)
     {
       if (long_form)

packet_id.h +13 ~~-21~~

index 12c1df3..59b7a7b 100644
@@ -249,7 +249,19 @@ const char *packet_id_persist_print (const struct packet_id_persist *p, struct g
  */
 
 bool packet_id_read (struct packet_id_net *pin, struct buffer *buf, bool long_form);
-bool packet_id_write (const struct packet_id_net *pin, struct buffer *buf, bool long_form, bool prepend);
+
+/**
+ * Write a packet ID to buf, and update the packet ID state.
+ *
+ * @param p             Packet ID state.
+ * @param buf           Buffer to write the packet ID too
+ * @param long_form     If true, also update and write time_t to buf
+ * @param prepend       If true, prepend to buffer, otherwise apppend.
+ *
+ * @return true if successful, false otherwise.
+ */
+bool packet_id_write (struct packet_id_send *p, struct buffer *buf,
+        bool long_form, bool prepend);
 
 /*
  * Inline functions.

@@ -291,26 +303,6 @@ packet_id_close_to_wrapping (const struct packet_id_send *p)
   return p->id >= PACKET_ID_WRAP_TRIGGER;
 }
 
-/*
- * Allocate an outgoing packet id.
- * Sequence number ranges from 1 to 2^32-1.
- * In long_form, a time_t is added as well.
- */
-static inline void
-packet_id_alloc_outgoing (struct packet_id_send *p, struct packet_id_net *pin, bool long_form)
-{
-  if (!p->time)
-    p->time = now;
-  pin->id = ++p->id;
-  if (!pin->id)
-    {
-      ASSERT (long_form);
-      p->time = now;
-      pin->id = p->id = 1;
-    }
-  pin->time = p->time;
-}
-
 static inline bool
 check_timestamp_delta (time_t remote, unsigned int max_delta)
 {

Signature · 1 line

2.7.4

Gert Doering

2017-06-13 07:11 UTC #2 lore raw

[Openvpn-devel] [PATCH applied] Re: cleanup: merge packet_id_alloc_outgoing() into packet_id_write()

ACK. Same patch as in 2.3

Diffing the two *patches* shows differences like this one:

- else if (cipher_kt_mode_ofb_cfb(cipher_kt))
+ else if (mode == EVP_CIPH_CFB_MODE || mode == EVP_CIPH_OFB_MODE)

.. but's not a patch change, but different context (old 2.2 crypto style).

Your patch has been applied to the release/2.2 branch.

commit aded94168651ffc4346e73604b62cfcee4ef7297

Author: Steffan Karger
Date:   Thu May 11 23:13:40 2017 +0200

     cleanup: merge packet_id_alloc_outgoing() into packet_id_write()

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14644.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

Steffan Karger

author

2017-05-11 21:13 UTC #3 lore raw

PATCH 3/3 Drop packets instead of assert out if packet id rolls over (CVE-2017-7479)

In reply to Steffan Karger

From: Steffan Karger <[email protected]...>

Previously, if a mode was selected where packet ids are not allowed to roll over, but renegotiation does not succeed for some reason (e.g. no password entered in time, certificate expired or a malicious peer that refuses the renegotiaion on purpose) we would continue to use the old keys. Until the packet ID would roll over and we would ASSERT() out.

Given that this can be triggered on purpose by an authenticated peer, this is a fix for an authenticated remote DoS vulnerability. An attack is rather inefficient though; a peer would need to get us to send 2^32 packets (min-size packet is IP+UDP+OPCODE+PID+TAG (no payload), results in (20+8+1+4+16)*2^32 bytes, or approx. 196 GB).

This is a fix for finding 5.2 from the OSTIF / Quarkslab audit.

This patch was cherry-picked from b727643c (release/2.3).

CVE: 2017-7479

Signed-off-by Steffan Karger <[email protected]...>

---
 crypto.c    | 18 +++++++++++++-----
 packet_id.c | 22 ++++++++++++++++------
 packet_id.h |  1 +
 3 files changed, 30 insertions(+), 11 deletions(-)

crypto.c +13 -5

index b7b2f01..b1a8a02 100644
@@ -117,9 +117,11 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
 		prng_bytes (iv_buf, iv_size);
 
 	      /* Put packet ID in plaintext buffer or IV, depending on cipher mode */
-	      if (opt->packet_id)
+	      if (opt->packet_id
+		  && !packet_id_write (&opt->packet_id->send, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true))
 		{
-		  ASSERT (packet_id_write (&opt->packet_id->send, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true));
+		  msg (D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
+		  goto err;
 		}
 	    }
 	  else if (mode == EVP_CIPH_CFB_MODE || mode == EVP_CIPH_OFB_MODE)

@@ -131,7 +133,11 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
 
 	      memset (iv_buf, 0, iv_size);
 	      buf_set_write (&b, iv_buf, iv_size);
-	      ASSERT (packet_id_write (&opt->packet_id->send, &b, true, false));
+	      if (!packet_id_write (&opt->packet_id->send, &b, true, false))
+		{
+		  msg (D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
+		  goto err;
+		}
 	    }
 	  else /* We only support CBC, CFB, or OFB modes right now */
 	    {

@@ -187,9 +193,11 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
 	}
       else				/* No Encryption */
 	{
-	  if (opt->packet_id)
+	  if (opt->packet_id
+	      && !packet_id_write (&opt->packet_id->send, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true))
 	    {
-	      ASSERT (packet_id_write (&opt->packet_id->send, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true));
+	      msg (D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
+	      goto err;
 	    }
 	  work = *buf;
 	}

packet_id.c +16 -6

index d190074..a579726 100644
@@ -248,27 +248,37 @@ packet_id_read (struct packet_id_net *pin, struct buffer *buf, bool long_form)
   return true;
 }
 
-static void
+static bool
 packet_id_send_update(struct packet_id_send *p, bool long_form)
 {
   if (!p->time)
     {
       p->time = now;
     }
-  p->id++;
-  if (!p->id)
+  if (p->id == PACKET_ID_MAX)
     {
-      ASSERT(long_form);
+      /* Packet ID only allowed to roll over if using long form and time has
+       * moved forward since last roll over.
+       */
+      if (!long_form || now <= p->time)
+        {
+            return false;
+        }
       p->time = now;
-      p->id = 1;
+      p->id = 0;
     }
+    p->id++;
+    return true;
 }
 
 bool
 packet_id_write (struct packet_id_send *p, struct buffer *buf, bool long_form,
         bool prepend)
 {
-  packet_id_send_update(p, long_form);
+  if (!packet_id_send_update(p, long_form))
+    {
+      return false;
+    }
 
   const packet_id_type net_id = htonpid(p->id);
   const net_time_t net_time = htontime(p->time);

packet_id.h +1 -0

index 59b7a7b..7d70840 100644
@@ -50,6 +50,7 @@
  * to for network transmission.
  */
 typedef uint32_t packet_id_type;
+#define PACKET_ID_MAX UINT32_MAX
 typedef uint32_t net_time_t;
 
 /*

Signature · 1 line

2.7.4

Gert Doering

2017-06-13 07:11 UTC #4 lore raw

[Openvpn-devel] [PATCH applied] Re: Drop packets instead of assert out if packet id rolls over (CVE-2017-7479)

ACK, same patch as in 2.3 (minus the obvious differences in context, Changes.rst, cmocka).

Your patch has been applied to the release/2.2 branch.

commit 66576f9903cc166072901673ff1e673fa2e29611

Author: Steffan Karger
Date:   Thu May 11 23:13:41 2017 +0200

     Drop packets instead of assert out if packet id rolls over (CVE-2017-7479)

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14645.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

2017-06-13 07:11 UTC #5 lore raw

[Openvpn-devel] [PATCH applied] Re: Drop packets instead of assert out if packet id rolls over (CVE-2017-7479)

In reply to Steffan Karger

ACK, same patch as in 2.3 (minus the obvious differences in context, Changes.rst, cmocka).

Your patch has been applied to the release/2.2 branch.

commit 66576f9903cc166072901673ff1e673fa2e29611

Author: Steffan Karger
Date:   Thu May 11 23:13:41 2017 +0200

     Drop packets instead of assert out if packet id rolls over (CVE-2017-7479)

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14645.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

2017-06-13 06:56 UTC #6 lore raw

In reply to Steffan Karger

ACK "because it makes sense" - tested, works.

Your patch has been applied to the release/2.2 branch.

commit 9162db3d2850f2edf2db5bc3a47220d899370c9d

Author: Steffan Karger
Date:   Thu May 11 23:13:39 2017 +0200

     Update sample-keys

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14643.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering