From 7e2aadc60f8269eee4b8a8b9fb072d0c31141a78 Mon Sep 17 00:00:00 2001 From: Steve Youngs Date: Mon, 16 May 2016 10:08:44 +1000 Subject: [PATCH] Implement a blacklist for problem OpenSSL ciphers. Since OpenSSL v1.0.0 was released, a number of ciphers have caused problems for SXEmacs' ossl, up to and including data corruption. This changeset prevents these ciphers from being used. See: `ossl-cipher-blacklist' The blacklisted ciphers can still be used if they are first removed from `ossl-cipher-blacklist', but obviously this is not recommended. * src/openssl.c (ossl_check_cipher): New. Returns 0 if cipher is NOT on our blacklist. (Fossl_available_ciphers): Check cipher with ossl_check_cipher(). (ossl_cipher_fun): Ditto. (Fossl_bytes_to_key): Ditto. (Fossl_encrypt): Ditto. (Fossl_encrypt_file): Ditto. (Fossl_decrypt): Ditto. (Fossl_decrypt_file): Ditto. (Fossl_seal): Ditto. (Fossl_open): Ditto. (Fossl_pem_write_key): Ditto. (Fossl_pem_key): Ditto. (Fossl_digest_size): Typo fix "cipher" -> "digest". (vars_of_openssl): New var, Vossl_cipher_blacklist. A list of ciphers we don't want to use. (syms_of_openssl): Define all of the blacklisted cipher names. * tests/automated/openssl-tests.el: Run tests on all available ciphers and digests. Make sure use of blacklisted ciphers results in an error. Signed-off-by: Steve Youngs --- src/openssl.c | 110 ++++++++++++++++++++++++++++++- tests/automated/openssl-tests.el | 55 +++------------- 2 files changed, 115 insertions(+), 50 deletions(-) diff --git a/src/openssl.c b/src/openssl.c index dc97da1..f34ce8e 100644 --- a/src/openssl.c +++ b/src/openssl.c @@ -422,6 +422,19 @@ int ossl_ssl_inject_cert_file(Lisp_Object, Lisp_Object, Lisp_Object); Lisp_Object Qssl2, Qssl23, Qssl3, Qtls1; +/* Problem Ciphers */ +Lisp_Object QAES_256_XTS, QAES_128_XTS, Qid_aes256_CCM, Qid_aes256_GCM; +Lisp_Object Qid_aes192_CCM, Qid_aes192_GCM, Qid_aes128_CCM; +Lisp_Object Qid_aes128_GCM, Qid_aes256_wrap, Qid_aes192_wrap; +Lisp_Object Qid_aes128_wrap, QCAMELLIA_256_CFB8, QCAMELLIA_192_CFB8; +Lisp_Object QCAMELLIA_128_CFB8, QCAMELLIA_256_CFB1, QCAMELLIA_192_CFB1; +Lisp_Object QCAMELLIA_128_CFB1, QDES_EDE3_CFB8, QDES_EDE3_CFB1, QDES_CFB8; +Lisp_Object QDES_CFB1, QAES_256_CFB8, QAES_192_CFB8, QAES_128_CFB8; +Lisp_Object QAES_256_CFB1, QAES_192_CFB1, QAES_128_CFB1; +Lisp_Object Qid_smime_alg_CMS3DESwrap; +Lisp_Object Vossl_cipher_blacklist; +int ossl_check_cipher(Lisp_Object); + extern Lisp_Object Qfile_readable_p; extern Lisp_Object Qfile_writable_p; @@ -465,6 +478,15 @@ This yields a plain list of symbols. return digests; } +int +ossl_check_cipher(Lisp_Object cipher) +{ + if (!NILP(Fmember(cipher, Vossl_cipher_blacklist))) { + return 1; + } else { + return 0; + } +} DEFUN("ossl-available-ciphers", Fossl_available_ciphers, 0, 0, 0, /* Return a list of cipher algorithms in the underlying crypto library. @@ -482,7 +504,8 @@ This yields a plain list of symbols. /* is there a better way to get the size of the nid list? */ for (nid = 10000; nid >= 0; --nid) { const EVP_CIPHER *cipher = EVP_get_cipherbynid(nid); - if (cipher) { + if (cipher && + (ossl_check_cipher(intern(OBJ_nid2sn(nid))) == 0)) { ciphers = Fcons(intern(OBJ_nid2sn(nid)), ciphers); } } @@ -535,7 +558,7 @@ Return the hash length of DIGEST in bytes. int size = ossl_digest_size(digest); if (size < 0) - error ("no such cipher"); + error ("no such digest"); return make_int(size); } @@ -572,7 +595,10 @@ Return the block size of DIGEST in bytes. do { \ int __kl; \ const EVP_CIPHER *__ciph; \ - \ + \ + if (ossl_check_cipher(var) != 0) \ + error("use of blacklisted cipher prohibited"); \ + \ OpenSSL_add_all_ciphers(); \ \ __ciph = EVP_get_cipherbyname( \ @@ -1114,6 +1140,8 @@ binary string data. CHECK_SYMBOL(digest); CHECK_NATNUM(count); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); if (!XINT(count)) error ("count has to be a non-zero positive integer"); @@ -1214,6 +1242,9 @@ binary string data. CHECK_STRING(key); CHECK_STRING(iv); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + TO_EXTERNAL_FORMAT(LISP_STRING, string, C_STRING_ALLOCA, string_ext, OSSL_CODING); string_len = OSSL_STRING_LENGTH(string); @@ -1344,6 +1375,9 @@ binary string data. CHECK_STRING(key); CHECK_STRING(iv); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + if (!NILP(outfile)) { CHECK_STRING(outfile); outfile = Fexpand_file_name(outfile, Qnil); @@ -1535,6 +1569,9 @@ non-nil, use this IV instead. CHECK_STRING(key); CHECK_STRING(iv); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + TO_EXTERNAL_FORMAT(LISP_STRING, string, C_STRING_ALLOCA, string_ext, OSSL_CODING); string_len = OSSL_STRING_LENGTH(string); @@ -1658,6 +1695,9 @@ encrypted data redirected. CHECK_STRING(key); CHECK_STRING(iv); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + if (!NILP(outfile)) { CHECK_STRING(outfile); outfile = Fexpand_file_name(outfile, Qnil); @@ -2547,6 +2587,9 @@ returns binary string data. CHECK_EVPPKEY(pkey); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + pk[0] = (XEVPPKEY(pkey))->evp_pkey; if (!ossl_pkey_has_public_data(pk[0])) { error ("cannot seal, key has no public key data"); @@ -2656,6 +2699,9 @@ EIV is the encrypted iv CHECK_STRING(ekey); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + pk = (XEVPPKEY(pkey))->evp_pkey; if (!ossl_pkey_has_private_data(pk)) error ("cannot open, key has no private key data"); @@ -3028,6 +3074,9 @@ PASSWORD is ignored in this case. CHECK_SYMBOL(cipher); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + OpenSSL_add_all_algorithms(); if (NILP(cipher)) { @@ -3176,6 +3225,9 @@ PASSWORD is ignored in this case. CHECK_SYMBOL(cipher); + if (ossl_check_cipher(cipher) != 0) + error("use of blacklisted cipher prohibited"); + OpenSSL_add_all_algorithms(); if (NILP(cipher)) { @@ -4895,10 +4947,62 @@ void syms_of_openssl(void) DEFSUBR(Fossl_x509_not_before); DEFSUBR(Fossl_x509_not_after); DEFSUBR(Fossl_x509_signature_type); + +/* Problem ciphers */ + defsymbol(&QAES_256_XTS, "AES-256-XTS"); + defsymbol(&QAES_128_XTS, "AES-128-XTS"); + defsymbol(&Qid_aes256_CCM, "id-aes256-CCM"); + defsymbol(&Qid_aes256_GCM, "id-aes256-GCM"); + defsymbol(&Qid_aes192_CCM, "id-aes192-CCM"); + defsymbol(&Qid_aes192_GCM, "id-aes192-GCM"); + defsymbol(&Qid_aes128_CCM, "id-aes128-CCM"); + defsymbol(&Qid_aes128_GCM, "id-aes128-GCM"); + defsymbol(&Qid_aes256_wrap, "id-aes256-wrap"); + defsymbol(&Qid_aes192_wrap, "id-aes192-wrap"); + defsymbol(&Qid_aes128_wrap, "id-aes128-wrap"); + defsymbol(&QCAMELLIA_256_CFB8, "CAMELLIA-256-CFB8"); + defsymbol(&QCAMELLIA_192_CFB8, "CAMELLIA-192-CFB8"); + defsymbol(&QCAMELLIA_128_CFB8, "CAMELLIA-128-CFB8"); + defsymbol(&QCAMELLIA_256_CFB1, "CAMELLIA-256-CFB1"); + defsymbol(&QCAMELLIA_192_CFB1, "CAMELLIA-192-CFB1"); + defsymbol(&QCAMELLIA_128_CFB1, "CAMELLIA-128-CFB1"); + defsymbol(&QDES_EDE3_CFB8, "DES-EDE3-CFB8"); + defsymbol(&QDES_EDE3_CFB1, "DES-EDE3-CFB1"); + defsymbol(&QDES_CFB8, "DES-CFB8"); + defsymbol(&QDES_CFB1, "DES-CFB1"); + defsymbol(&QAES_256_CFB8, "AES-256-CFB8"); + defsymbol(&QAES_192_CFB8, "AES-192-CFB8"); + defsymbol(&QAES_128_CFB8, "AES-128-CFB8"); + defsymbol(&QAES_256_CFB1, "AES-256-CFB1"); + defsymbol(&QAES_192_CFB1, "AES-192-CFB1"); + defsymbol(&QAES_128_CFB1, "AES-128-CFB1"); + defsymbol(&Qid_smime_alg_CMS3DESwrap, "id-smime-alg-CMS3DESwrap"); } void vars_of_openssl(void) { + DEFVAR_LISP("ossl-cipher-blacklist", &Vossl_cipher_blacklist /* +A list of ciphers that are blacklisted against use. + +These are ciphers that are known to cause problems with the SXEmacs +OpenSSL code that can result in data corruption. If you find that you +need to use one or more of the ciphers on this list, you can do so by +removing it from this list first. Do we need to mention that this is +probably not a good idea and that you are well and truly on your own +here? But hey, it's your data... + */); + + Lisp_Object badCiphers[28] = { + QAES_256_XTS, QAES_128_XTS, Qid_aes256_CCM, Qid_aes256_GCM, + Qid_aes192_CCM, Qid_aes192_GCM, Qid_aes128_CCM, Qid_aes128_GCM, + Qid_aes256_wrap, Qid_aes192_wrap, Qid_aes128_wrap, + QCAMELLIA_256_CFB8, QCAMELLIA_192_CFB8, QCAMELLIA_128_CFB8, + QCAMELLIA_256_CFB1, QCAMELLIA_192_CFB1, QCAMELLIA_128_CFB1, + QDES_EDE3_CFB8, QDES_EDE3_CFB1, QDES_CFB8, QDES_CFB1, + QAES_256_CFB8, QAES_192_CFB8, QAES_128_CFB8, QAES_256_CFB1, + QAES_192_CFB1, QAES_128_CFB1, Qid_smime_alg_CMS3DESwrap }; + Vossl_cipher_blacklist = Flist(28, badCiphers); + Fprovide(Qopenssl); #ifndef OPENSSL_NO_RSA diff --git a/tests/automated/openssl-tests.el b/tests/automated/openssl-tests.el index 75efcaa..c8e0265 100644 --- a/tests/automated/openssl-tests.el +++ b/tests/automated/openssl-tests.el @@ -221,6 +221,13 @@ (Assert (member 'AES-192-CFB (ossl-available-ciphers))) (Assert (member 'AES-192-OFB (ossl-available-ciphers))) + ;; blacklisted ciphers should give us an error + (mapc-internal + #'(lambda (cipher) + (Check-Error-Message error "use of blacklisted cipher prohibited" + (ossl-cipher-mode cipher))) + ossl-cipher-blacklist) + ;; first we check the key generator (let ((encstrs (list "foo string test bar" @@ -228,54 +235,8 @@ "\n")) (salts (list nil "salt" "" "toomuchsalt")) - ;; Ciphers - (ciphers - ;; We seem to have issues with the following ciphers. Not - ;; sure yet if it is SXEmacs bug, or OpenSSL bug. But perhaps - ;; we should prevent them from being used at all with our ssl - ;; code instead of just conveniently ignoring them in the - ;; testsuite? --SY. - (let ((bad-ciphers '(id-smime-alg-CMS3DESwrap - id-aes128-wrap id-aes192-wrap id-aes256-wrap - id-aes128-GCM id-aes128-CCM id-aes192-GCM - id-aes192-CCM id-aes256-GCM id-aes256-CCM - AES-128-XTS AES-256-XTS)) - ciphers) - (mapc-internal - #'(lambda (cipher) - (let ((ciphmode (substring (symbol-name cipher) -2))) - ;; Never use CFB1 and CFB8 modes. - ;; Both modes tend to mangle the result strings which - ;; yields an assertion error. - ;; Bug in openssl? - ;; -hroptatyr - ;; Shouldn't we prevent their use outside the testsuite - ;; as well? --SY. - (unless (or ;(< (ossl-cipher-bits cipher) 128) - (string= "B1" ciphmode) - (string= "B8" ciphmode) - (member cipher bad-ciphers)) - (setq ciphers - (cons cipher ciphers))))) - (ossl-available-ciphers)) - ciphers)) - ;; Digests - ;; Sebastian had initially only used digests that didn't have - ;; a dash in their name, I'm not sure what his reasoning was, - ;; perhaps just to speed up running the testsuite, I dunno. - ;; But I say we should test them ALL. :-) --SY. + (ciphers (ossl-available-ciphers)) (digests (ossl-available-digests)) - ;; (digests - ;; (let (digests) - ;; (mapc-internal - ;; #'(lambda (digest) - ;; (let ((digestname (symbol-name digest))) - ;; ;; only use digests without a dash in their names - ;; (unless (string-match "-" digestname) - ;; (setq digests - ;; (cons digest digests))))) - ;; (ossl-available-digests)) - ;; digests)) key iv enc dec) -- 2.25.1