]> jfr.im git - irc/atheme/atheme.git/commitdiff
saslserv: drop support for DH-AES and DH-BLOWFISH
authorWilliam Pitcock <redacted>
Tue, 19 Aug 2014 21:25:41 +0000 (16:25 -0500)
committerWilliam Pitcock <redacted>
Tue, 19 Aug 2014 21:29:24 +0000 (16:29 -0500)
Both mechanisms have serious design flaws which make them insecure.  To fix the
design flaws would make both mechanisms incompatible with current implementations,
and provide little-to-no gain over using PLAIN in a TLS channel instead.

Thusly, we drop support for them.

contrib/cap_sasl.pl
dist/atheme.conf.example
modules/saslserv/Makefile
modules/saslserv/dh-aes.c [deleted file]
modules/saslserv/dh-blowfish.c [deleted file]

index 090493fdbe80d36941047c6e7a5823d34dbee1bd..6e5cfffd5a71a1d9c5d1535982541a0790e7a537 100644 (file)
@@ -243,109 +243,6 @@ $mech{EXTERNAL} = sub {
        "";
 };
 
-eval {
-       require Crypt::OpenSSL::Bignum;
-       my $compute_secret;
-       eval {
-               require Crypt::DH;
-               $compute_secret = sub { (shift)->compute_secret(@_); };
-       };
-       if ($@) {
-               # Crypt::DH probably not found. Try Crypt::DH::GMP instead
-               # Reportedly Ubuntu has dropped Crypt::DH
-               require Crypt::DH::GMP;
-               require Crypt::DH::GMP::Compat;
-               $compute_secret = sub { Math::BigInt->new((shift)->compute_secret(@_)) };
-       }
-       require Crypt::Blowfish;
-       require Math::BigInt;
-       sub bin2bi { return Crypt::OpenSSL::Bignum->new_from_bin(shift)->to_decimal } # binary to BigInt
-       sub bi2bin { return Crypt::OpenSSL::Bignum->new_from_decimal((shift)->bstr)->to_bin } # BigInt to binary
-       $mech{'DH-BLOWFISH'} = sub {
-               my($sasl, $data) = @_;
-               my $u = $sasl->{user};
-               my $pass = $sasl->{password};
-
-               # Generate private key and compute secret key
-               my($p, $g, $y) = unpack("(n/a*)3", $data);
-               my $dh = Crypt::DH->new(p => bin2bi($p), g => bin2bi($g));
-               $dh->generate_keys;
-
-               my $secret = bi2bin($compute_secret->($dh, bin2bi($y)));
-               my $pubkey = bi2bin($dh->pub_key);
-
-               # Pad the password to the nearest multiple of blocksize and encrypt
-               $pass .= "\0";
-               $pass .= chr(rand(256)) while length($pass) % 8;
-
-               my $cipher = Crypt::Blowfish->new($secret);
-               my $crypted = '';
-               while(length $pass) {
-                       my $clear = substr($pass, 0, 8, '');
-                       $crypted .= $cipher->encrypt($clear);
-               }
-
-               pack("n/a*Z*a*", $pubkey, $u, $crypted);
-       };
-};
-
-eval {
-       require Crypt::OpenSSL::Bignum;
-       my $compute_secret;
-       eval {
-               require Crypt::DH;
-               $compute_secret = sub { (shift)->compute_secret(@_); };
-       };
-       if ($@) {
-               # Crypt::DH probably not found. Try Crypt::DH::GMP instead
-               # Reportedly Ubuntu has dropped Crypt::DH
-               require Crypt::DH::GMP;
-               require Crypt::DH::GMP::Compat;
-               $compute_secret = sub { Math::BigInt->new((shift)->compute_secret(@_)) };
-       }
-       require Math::BigInt;
-       require Crypt::Rijndael;
-       require Crypt::CBC;
-       sub bin2bi { return Crypt::OpenSSL::Bignum->new_from_bin(shift)->to_decimal } # binary to BigInt
-       sub bi2bin { return Crypt::OpenSSL::Bignum->new_from_decimal((shift)->bstr)->to_bin } # BigInt to binary
-       $mech{'DH-AES'} = sub {
-               my($sasl, $data) = @_;
-               my $u = $sasl->{user};
-               my $pass = $sasl->{password};
-
-               # Same as DH-BLOWFISH for the DH part.
-               my($p, $g, $y) = unpack("(n/a*)3", $data);
-               my $dh = Crypt::DH->new(p => bin2bi($p), g => bin2bi($g));
-               $dh->generate_keys;
-
-               my $secret = bi2bin($compute_secret->($dh, bin2bi($y)));
-               my $pubkey = bi2bin($dh->pub_key);
-
-               # Padding is different. Multiple of 16 instead of 8
-               # Pad the username too.
-               $u .= "\0";
-               $pass .= "\0";
-               $pass .= chr(rand(256)) while length($pass) % 16;
-
-               my $userpass = $u . $pass;
-
-               # Hum... this is a CBC mode cipher. We need an IV :P
-               my $iv = Crypt::CBC->random_bytes(16);
-               my $cipher = Crypt::CBC->new(
-                       -literal_key => 1,
-                       -key    => $secret,
-                       -cipher => "Crypt::Rijndael",
-                       -iv     => $iv,
-                       -header => 'none',
-               );
-
-               my $crypted = $cipher->encrypt($userpass);
-
-               # Packing is different from DH-BLOWFISH.
-               pack("n/a*a*a*", $pubkey, $iv, $crypted);
-       };
-};
-
 sub in_path {
        my $exe = shift;
        return grep {-x "$_/$exe"}
index 1d61d05f1dca5a53858903372e5dc9770e5b2df9..47653fa34d9de1b151f246901304da7ed3709828 100644 (file)
@@ -534,25 +534,18 @@ loadmodule "modules/infoserv/main";
  *
  * Core components                              modules/saslserv/main
  * PLAIN mechanism                              modules/saslserv/plain
- * DH-BLOWFISH mechanism (DEPRECATED)           modules/saslserv/dh-blowfish
- * DH-AES mechanism                             modules/saslserv/dh-aes
  * ECDSA-NIST256p-CHALLENGE                     modules/saslserv/ecdsa-nist256p-challenge
  * AUTHCOOKIE mechanism (for IRIS)              modules/saslserv/authcookie
  * EXTERNAL mechanism (IRCv3.1+)                modules/saslserv/external
  *
- * DH-BLOWFISH has potential issues with weak keys unless the DH parameters
- * are selected to avoid them. Its use is discouraged. Use DH-AES and/or
- * ECDSA-NIST256p-CHALLENGE if at all possible.
- *
- * DH-BLOWFISH, DH-AES, and ECDSA-NIST256p-CHALLENGE all require SSL.
+ * ECDSA-NIST256p-CHALLENGE support requires that Atheme be compiled against OpenSSL.
  */
 loadmodule "modules/saslserv/main";
 loadmodule "modules/saslserv/plain";
 loadmodule "modules/saslserv/authcookie";
 #loadmodule "modules/saslserv/external";
-#loadmodule "modules/saslserv/dh-blowfish"; /* requires SSL (DEPRECATED) */
-#loadmodule "modules/saslserv/dh-aes"; /* requires SSL */
 #loadmodule "modules/saslserv/ecdsa-nist256p-challenge"; /* requires SSL */
+
 /* GameServ modules.
  *
  * Here you can disable or enable certain features of GameServ, by
index 0ae78f39173d0e7c75d0743c4ef89ea94e517b29..75046ff49a99b2acf363f980c5c7aefa005978de 100644 (file)
@@ -10,8 +10,6 @@ MODULE = saslserv
 
 SRCS = \
        authcookie.c    \
-       dh-blowfish.c   \
-       dh-aes.c        \
        ecdsa-nist256p-challenge.c \
        external.c      \
        main.c          \
@@ -22,7 +20,6 @@ include ../../buildsys.mk
 include ../../buildsys.module.mk
 
 CPPFLAGS       += -I../../include
-# Only dh-blowfish.so needs this, oh well.
 LIBS           += ${SSL_LIBS}
 LIBS           += -L../../libathemecore -lathemecore ${LDFLAGS_RPATH}
 
diff --git a/modules/saslserv/dh-aes.c b/modules/saslserv/dh-aes.c
deleted file mode 100644 (file)
index 78a5492..0000000
+++ /dev/null
@@ -1,213 +0,0 @@
-/*
- * Copyright (c) 2013 Atheme Development Group
- * Rights to this code are as documented in doc/LICENSE.
- *
- * DH-AES mechanism provider
- * Written by Elizabeth Myers, 14 Apr 2013.
- *
- * Basically the same as DH-BLOWFISH, with a couple alterations:
- * 1) AES instead of blowfish. Blowfish suffers from weak keys, and the author
- *    of it (Bruce Schneier) recommends not using it.
- * 2) Username is encrypted with password
- * 3) CBC mode is used with AES. The IV is sent with the key by the client.
- *
- * Why this over ECDSA-NIST256p-CHALLENGE? Not all langs have good support for
- * ECDSA. DH and AES support, on the other hand, are relatively ubiqutious. It
- * may also be usable in environments where the convenience of a password
- * outweighs the benefits of using challenge-response.
- *
- * Padding structure (sizes in big-endian/network byte order):
- * Server sends: <size p><p><size g><g><size pubkey><pubkey>
- * Client sends: <size pubkey><pubkey><iv (always 16 bytes)><ciphertext>
- *
- * Ciphertext is to be the following encrypted:
- * <username>\0<password>\0<padding if necessary>
- *
- * The DH secret is used to encrypt and decrypt blocks as the key.
- *
- * Note: NEVER reuse IV's. I believe this goes without saying.
- * Also ensure your padding is random. This is just good practise.
- */
-
-#include "atheme.h"
-
-#ifdef HAVE_OPENSSL
-
-#include <openssl/bn.h>
-#include <openssl/dh.h>
-#include <openssl/aes.h>
-
-DECLARE_MODULE_V1
-(
-       "saslserv/dh-aes", false, _modinit, _moddeinit,
-       PACKAGE_STRING,
-       "Atheme Development Group <http://www.atheme.org>"
-);
-
-static DH *base_dhparams;
-
-sasl_mech_register_func_t *regfuncs;
-
-static int mech_start(sasl_session_t *p, char **out, size_t *out_len);
-static int mech_step(sasl_session_t *p, char *message, size_t len, char **out, size_t *out_len);
-static void mech_finish(sasl_session_t *p);
-sasl_mechanism_t mech = {"DH-AES", &mech_start, &mech_step, &mech_finish};
-
-void _modinit(module_t *m)
-{
-       MODULE_TRY_REQUEST_SYMBOL(m, regfuncs, "saslserv/main", "sasl_mech_register_funcs");
-
-       if ((base_dhparams = DH_generate_parameters(256, 5, NULL, NULL)) == NULL)
-               return;
-
-       regfuncs->mech_register(&mech);
-}
-
-void _moddeinit(module_unload_intent_t intent)
-{
-       DH_free(base_dhparams);
-
-       regfuncs->mech_unregister(&mech);
-}
-
-static inline DH *DH_clone(DH *dh)
-{
-       DH *out = DH_new();
-
-       out->p = BN_dup(dh->p);
-       out->g = BN_dup(dh->g);
-
-       if (!DH_generate_key(out))
-       {
-               DH_free(out);
-               return NULL;
-       }
-
-       return out;
-}
-
-static int mech_start(sasl_session_t *p, char **out, size_t *out_len)
-{
-       char *ptr;
-       DH *dh;
-
-       if ((dh = DH_clone(base_dhparams)) == NULL)
-               return ASASL_FAIL;
-
-       /* Serialize p, g, and pub_key */
-       *out_len = BN_num_bytes(dh->p) + BN_num_bytes(dh->g) + BN_num_bytes(dh->pub_key) + 6;
-       *out = malloc((size_t)*out_len);
-       ptr = *out;
-
-       /* p */
-       *((unsigned int *)ptr) = htons(BN_num_bytes(dh->p));
-       BN_bn2bin(dh->p, (unsigned char *)ptr + 2);
-       ptr += 2 + BN_num_bytes(dh->p);
-
-       /* g */
-       *((unsigned int *)ptr) = htons(BN_num_bytes(dh->g));
-       BN_bn2bin(dh->g, (unsigned char *)ptr + 2);
-       ptr += 2 + BN_num_bytes(dh->g);
-
-       /* pub_key */
-       *((unsigned int *)ptr) = htons(BN_num_bytes(dh->pub_key));
-       BN_bn2bin(dh->pub_key, (unsigned char *)ptr + 2);
-       ptr += 2 + BN_num_bytes(dh->pub_key);
-
-       p->mechdata = dh;
-       return ASASL_MORE;
-}
-
-static int mech_step(sasl_session_t *p, char *message, size_t len, char **out, size_t *out_len)
-{
-       DH *dh = NULL;
-       AES_KEY key;
-       BIGNUM *their_key = NULL;
-       myuser_t *mu;
-       char *secret = NULL, *userpw = NULL, *ptr = NULL;
-       char iv[AES_BLOCK_SIZE];
-       int ret = ASASL_FAIL;
-       uint16_t size;
-       int secret_size;
-
-       if (!p->mechdata)
-               return ASASL_FAIL;
-       dh = (DH*)p->mechdata;
-
-       /* Their pub_key */
-       if (len <= 2)
-               goto end;
-
-       size = ntohs(*(uint16_t *)message);
-       message += 2;
-       len -= 2;
-
-       if (size >= len)
-               goto end;
-
-       if ((their_key = BN_bin2bn(message, size, NULL)) == NULL)
-               goto end;
-
-       message += size;
-       len -= size;
-
-       /* Data must be a multiple of the AES block size. (16)
-        * Verify we also have an IV and at least one block of data.
-        * Cap at a rather arbitrary limit of 272 (IV + 16 blocks of 16 each).
-        */
-       if (len < sizeof(iv) + AES_BLOCK_SIZE || len % AES_BLOCK_SIZE || len > 272)
-               goto end;
-
-       /* Extract the IV */
-       memcpy(iv, message, sizeof(iv));
-       message += sizeof(iv);
-       len -= sizeof(iv);
-
-       /* Compute shared secret */
-       secret = malloc(DH_size(dh));
-       secret_size = DH_compute_key(secret, their_key, dh);
-       if (secret_size <= 0)
-               goto end;
-
-       /* Decrypt! (AES_set_decrypt_key takes bits not bytes, hence multiply
-        * by 8) */
-       AES_set_decrypt_key(secret, secret_size * 8, &key);
-
-       ptr = userpw = malloc(len + 1);
-       userpw[len] = '\0';
-       AES_cbc_encrypt(message, userpw, len, &key, iv, AES_DECRYPT);
-
-       /* Username */
-       size = strlen(ptr);
-       if (size++ >= NICKLEN) /* our base64 routines null-terminate - how polite */
-               goto end;
-       p->username = strdup(ptr);
-       ptr += size;
-       len -= size;
-       if ((mu = myuser_find_by_nick(p->username)) == NULL)
-               goto end;
-
-       /* Password remains */
-       if (verify_password(mu, ptr))
-               ret = ASASL_DONE;
-end:
-       if (their_key)
-               BN_free(their_key);
-       free(secret);
-       free(userpw);
-       return ret;
-}
-
-static void mech_finish(sasl_session_t *p)
-{
-       DH_free((DH *) p->mechdata);
-       p->mechdata = NULL;
-}
-
-#endif /* HAVE_OPENSSL */
-
-/* vim:cinoptions=>s,e0,n0,f0,{0,}0,^0,=s,ps,t0,c3,+s,(2s,us,)20,*30,gs,hs
- * vim:ts=8
- * vim:sw=8
- * vim:noexpandtab
- */
diff --git a/modules/saslserv/dh-blowfish.c b/modules/saslserv/dh-blowfish.c
deleted file mode 100644 (file)
index afd55c9..0000000
+++ /dev/null
@@ -1,181 +0,0 @@
-/*
- * Copyright (c) 2006 Atheme Development Group
- * Rights to this code are as documented in doc/LICENSE.
- *
- * DH-BLOWFISH mechanism provider
- */
-
-#include "atheme.h"
-
-#ifdef HAVE_OPENSSL
-
-#include <openssl/bn.h>
-#include <openssl/dh.h>
-#include <openssl/blowfish.h>
-
-DECLARE_MODULE_V1
-(
-       "saslserv/dh-blowfish", false, _modinit, _moddeinit,
-       PACKAGE_STRING,
-       "Atheme Development Group <http://www.atheme.org>"
-);
-
-static DH *base_dhparams;
-
-sasl_mech_register_func_t *regfuncs;
-
-static int mech_start(sasl_session_t *p, char **out, size_t *out_len);
-static int mech_step(sasl_session_t *p, char *message, size_t len, char **out, size_t *out_len);
-static void mech_finish(sasl_session_t *p);
-sasl_mechanism_t mech = {"DH-BLOWFISH", &mech_start, &mech_step, &mech_finish};
-
-void _modinit(module_t *m)
-{
-       MODULE_TRY_REQUEST_SYMBOL(m, regfuncs, "saslserv/main", "sasl_mech_register_funcs");
-
-       if ((base_dhparams = DH_generate_parameters(256, 5, NULL, NULL)) == NULL)
-               return;
-
-       regfuncs->mech_register(&mech);
-}
-
-void _moddeinit(module_unload_intent_t intent)
-{
-       DH_free(base_dhparams);
-
-       regfuncs->mech_unregister(&mech);
-}
-
-static inline DH *DH_clone(DH *dh)
-{
-       DH *out = DH_new();
-
-       out->p = BN_dup(dh->p);
-       out->g = BN_dup(dh->g);
-
-       if (!DH_generate_key(out))
-       {
-               DH_free(out);
-               return NULL;
-       }
-
-       return out;
-}
-
-static int mech_start(sasl_session_t *p, char **out, size_t *out_len)
-{
-       char *ptr;
-       DH *dh;
-
-       if ((dh = DH_clone(base_dhparams)) == NULL)
-               return ASASL_FAIL;
-
-       /* Serialize p, g, and pub_key */
-       *out_len = BN_num_bytes(dh->p) + BN_num_bytes(dh->g) + BN_num_bytes(dh->pub_key) + 6;
-       *out = malloc((size_t)*out_len);
-       ptr = *out;
-
-       /* p */
-       *((unsigned int *)ptr) = htons(BN_num_bytes(dh->p));
-       BN_bn2bin(dh->p, (unsigned char *)ptr + 2);
-       ptr += 2 + BN_num_bytes(dh->p);
-
-       /* g */
-       *((unsigned int *)ptr) = htons(BN_num_bytes(dh->g));
-       BN_bn2bin(dh->g, (unsigned char *)ptr + 2);
-       ptr += 2 + BN_num_bytes(dh->g);
-
-       /* pub_key */
-       *((unsigned int *)ptr) = htons(BN_num_bytes(dh->pub_key));
-       BN_bn2bin(dh->pub_key, (unsigned char *)ptr + 2);
-       ptr += 2 + BN_num_bytes(dh->pub_key);
-
-       p->mechdata = dh;
-       return ASASL_MORE;
-}
-
-static int mech_step(sasl_session_t *p, char *message, size_t len, char **out, size_t *out_len)
-{
-       DH *dh = NULL;
-       BF_KEY key;
-       BIGNUM *their_key = NULL;
-       myuser_t *mu;
-       char *ptr, *secret = NULL, *password = NULL;
-       int ret = ASASL_FAIL;
-       uint16_t size;
-       int secret_size;
-
-       if (!p->mechdata)
-               return ASASL_FAIL;
-       dh = (DH*)p->mechdata;
-
-       /* Their pub_key */
-       if (len < 2)
-               goto end;
-       size = ntohs(*(uint16_t *)message);
-       message += 2;
-       len -= 2;
-       if (size > len)
-               goto end;
-       if ((their_key = BN_bin2bn((unsigned char *)message, size, NULL)) == NULL)
-               goto end;
-       message += size;
-       len -= size;
-
-       /* Username */
-       size = strlen(message);
-       if (size >= NICKLEN) /* our base64 routines null-terminate - how polite */
-               goto end;
-       p->username = strdup(message);
-       message += size + 1;
-       len -= size + 1;
-       if ((mu = myuser_find_by_nick(p->username)) == NULL)
-               goto end;
-       /* AES-encrypted password remains */
-
-       /* Compute shared secret */
-       secret = (char*)malloc(DH_size(dh));
-       secret_size = DH_compute_key((unsigned char *)secret, their_key, dh);
-       if (secret_size <= 0)
-               goto end;
-
-       /* Data must be multiple of block size, and let's be reasonable about size */
-       if (len == 0 || len % 8 || len > 128)
-               goto end;
-
-       /* Decrypt! */
-       BF_set_key(&key, secret_size, (unsigned char *)secret);
-       ptr = password = (char*)malloc(len + 1);
-       password[len] = '\0';
-       while (len)
-       {
-               BF_ecb_encrypt((unsigned char *)message, (unsigned char *)ptr, &key, BF_DECRYPT);
-               message += 8;
-               ptr += 8;
-               len -= 8;
-       }
-
-       if (verify_password(mu, password))
-               ret = ASASL_DONE;
-
-end:
-       if (their_key)
-               BN_free(their_key);
-       free(secret);
-       free(password);
-       return ret;
-}
-
-static void mech_finish(sasl_session_t *p)
-{
-       DH_free((DH *) p->mechdata);
-       p->mechdata = NULL;
-}
-
-#endif /* HAVE_OPENSSL */
-
-/* vim:cinoptions=>s,e0,n0,f0,{0,}0,^0,=s,ps,t0,c3,+s,(2s,us,)20,*30,gs,hs
- * vim:ts=8
- * vim:sw=8
- * vim:noexpandtab
- */