Re: Forward secrecy in spiped

[Frederick Akalin <akalin@gmail.com>]

On Thu, Apr 24, 2014 at 4:04 PM, Colin Percival <cperciva@tarsnap.com> wrote:
>
> Want to send me a patch?


Okay, here is my first attempt. I found it to be simplest to mandate
that both sides use -f to turn off forward secrecy, but I don't know
if that rules out valid use cases. Thoughts?

(Also, let me know if the diff below doesn't work. Gmail's handling of
plain text content is...special.)

-- Fred

diff --git a/proto/proto_crypt.c b/proto/proto_crypt.c
index d674bfc..092f57b 100644
--- a/proto/proto_crypt.c
+++ b/proto/proto_crypt.c
@@ -145,14 +145,29 @@ void proto_crypt_dhmac(const struct proto_secret * K,
 }

 /**
- * proto_crypt_dh_validate(yh_r, dhmac_r):
+ * is_zero_or_one(x, len):
+ * Returns non-zero if the big-endian value stored at (${x}, ${len}) is equal
+ * to either 0 or 1.
+ */
+static int is_zero_or_one(const uint8_t *x, size_t len) {
+	for (size_t i = 0; i < len - 1; i++) {
+		if (x[i] != 0)
+			return (0);
+	}
+
+	return ((x[len - 1] == 0) || (x[len - 1] == 1));
+}
+
+/**
+ * proto_crypt_dh_validate(yh_r, dhmac_r, nofps):
  * Return non-zero if the value ${yh_r} received from the remote party is not
  * correctly MACed using the diffie-hellman parameter MAC key ${dhmac_r}, or
- * if the included y value is >= the diffie-hellman group modulus.
+ * if the included y value is >= the diffie-hellman group modulus, or if
+ * "${nofps} is zero" does not equal "the included value is > 1".
  */
 int
 proto_crypt_dh_validate(const uint8_t yh_r[PCRYPT_YH_LEN],
-    const uint8_t dhmac_r[PCRYPT_DHMAC_LEN])
+    const uint8_t dhmac_r[PCRYPT_DHMAC_LEN], int nofps)
 {
 	uint8_t hbuf[32];

@@ -165,7 +180,15 @@ proto_crypt_dh_validate(const uint8_t yh_r[PCRYPT_YH_LEN],
 		return (1);

 	/* Sanity-check the diffie-hellman value. */
-	return (crypto_dh_sanitycheck(&yh_r[0]));
+	if (crypto_dh_sanitycheck(&yh_r[0]))
+		return (1);
+
+        /* Check that forward perfect secrecy is used if and only if the
+         * diffie-hellman value is > 1. */
+	if ((nofps != 0) != (is_zero_or_one(&yh_r[0], CRYPTO_DH_PUBLEN) != 0))
+		return (1);
+
+	return (0);
 }

 /**
diff --git a/proto/proto_crypt.h b/proto/proto_crypt.h
index 8801aa7..cd8fbcb 100644
--- a/proto/proto_crypt.h
+++ b/proto/proto_crypt.h
@@ -39,13 +39,14 @@ void proto_crypt_dhmac(const struct proto_secret *,
     uint8_t[PCRYPT_DHMAC_LEN], uint8_t[PCRYPT_DHMAC_LEN], int);

 /**
- * proto_crypt_dh_validate(yh_r, dhmac_r):
+ * proto_crypt_dh_validate(yh_r, dhmac_r, nofps):
  * Return non-zero if the value ${yh_r} received from the remote party is not
  * correctly MACed using the diffie-hellman parameter MAC key ${dhmac_r}, or
- * if the included y value is >= the diffie-hellman group modulus.
+ * if the included y value is >= the diffie-hellman group modulus, or if
+ * "${nofps} is zero" does not equal "the included value is > 1".
  */
 int proto_crypt_dh_validate(const uint8_t[PCRYPT_YH_LEN],
-    const uint8_t[PCRYPT_DHMAC_LEN]);
+    const uint8_t[PCRYPT_DHMAC_LEN], int);

 /**
  * proto_crypt_dh_generate(yh_l, x, dhmac_l, nofps):
diff --git a/proto/proto_handshake.c b/proto/proto_handshake.c
index 0c87caa..c677e07 100644
--- a/proto/proto_handshake.c
+++ b/proto/proto_handshake.c
@@ -209,7 +209,7 @@ callback_dh_read(void * cookie, ssize_t len)
 		return (handshakefail(H));

 	/* Is the value we read valid? */
-	if (proto_crypt_dh_validate(H->yh_remote, H->dhmac_remote))
+	if (proto_crypt_dh_validate(H->yh_remote, H->dhmac_remote, H->nofps))
 		return (handshakefail(H));

 	/*
diff --git a/spipe/spipe.1 b/spipe/spipe.1
index 5086d6b..c39f2bf 100644
--- a/spipe/spipe.1
+++ b/spipe/spipe.1
@@ -41,7 +41,7 @@ Use the provided key file to authenticate and encrypt.
 .B \-f
 Use fast/weak handshaking: This reduces the CPU time spent in the
 initial connection setup, at the expense of losing perfect forward
-secrecy.
+secrecy. The other party must also use this flag.
 .TP
 .B \-j
 Disable transport layer keep-alives.
diff --git a/spiped/spiped.1 b/spiped/spiped.1
index 09d5abe..82ca7b3 100644
--- a/spiped/spiped.1
+++ b/spiped/spiped.1
@@ -73,7 +73,7 @@ finished launching it will be ready to create pipes.
 .B \-f
 Use fast/weak handshaking: This reduces the CPU time spent in the
 initial connection setup, at the expense of losing perfect forward
-secrecy.
+secrecy. The other party must also use this flag.
 .TP
 .B \-F
 Run in foreground.  This can be useful with systems like daemontools.