On Thu, Apr 24, 2014 at 4:04 PM, Colin Percival <cperc...@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.