On Thu, 29 Oct 2009, Kees Cook wrote: > Hello! > > Here is a possible fix to the bug, which checks the openssl-vulnkey return > code for more than just non-zero. > > Jamie, does this look okay to you? I haven't actually tested this myself, > as I'm travelling at the moment. >
Actually, I reworked this a bit. First off, openvpn_execve()'s return code must use WEXITSTATUS(). Also, I replaced the switch logic with an 'if' and 'if else'. Along the way, I noticed that when the patch was updated in 2.1~rc9-1, the error reporting for the openssl-vulnkey a) wrongly referenced openvpn-vulnkey and b) derefenced a null pointer (we should be reporting on options->priv_key_file not options->shared_secret_file, since this clearly isn't a PSK). I fixed this and have attached a suggested debdiff as well as the full, updated patch for debian_openssl_vulnkeys.patch. I believe the patch to be good as is and ready for upload based on the following tests: # apt-get install openssl-blacklist-extra TLS (before patch-- notice the warning message): # openvpn --remote 127.0.0.1 --dev tun --tls-client --cert /usr/share/doc/openssl-blacklist/examples/bad_x509_4096.pem --key /usr/share/doc/openssl-blacklist/examples/bad_rsa_4096.pem --ca /usr/share/doc/openssl-blacklist/examples/bad_x509.pem ... Fri Oct 30 09:47:08 2009 ******* WARNING *******: '(null)' is a known vulnerable key. See 'man openvpn-vulnkey' for details. ... TLS (after patch): # openvpn --remote 127.0.0.1 --dev tun --tls-client --cert /usr/share/doc/openssl-blacklist/examples/good_x509.pem --key /usr/share/doc/openssl-blacklist/examples/good_rsa.key --ca /usr/share/doc/openssl-blacklist/examples/good_x509.pem ... Fri Oct 30 10:03:54 2009 /usr/bin/openssl-vulnkey -q -b 2048 -m <modulus omitted> ... # openvpn --remote 127.0.0.1 --dev tun --tls-client --cert /usr/share/doc/openssl-blacklist/examples/bad_x509_4096.pem --key /usr/share/doc/openssl-blacklist/examples/bad_rsa_4096.pem --ca /usr/share/doc/openssl-blacklist/examples/bad_x509.pem ... Fri Oct 30 10:03:09 2009 /usr/bin/openssl-vulnkey -q -b 4096 -m <modulus omitted> Fri Oct 30 10:03:09 2009 ******* WARNING *******: '/usr/share/doc/openssl-blacklist/examplesbad_rsa_4096.pem' is a known vulnerable key. See 'man openssl-vulnkey' for details. ... PSK (after patch): # openvpn --remote 127.0.0.1 --dev tun --secret /usr/share/doc/openvpn-blacklist/examples/bad.key ... Fri Oct 30 10:37:56 2009 /usr/sbin/openvpn-vulnkey -q /usr/share/doc/openvpn-blacklist/examples/bad.key Fri Oct 30 10:37:56 2009 ******* WARNING *******: '/usr/share/doc/openvpn-blacklist/examples/bad.key' is a known vulnerable key. See 'man openvpn-vulnkey' for details. ... # openvpn --genkey --secret /tmp/test.key # openvpn --remote 127.0.0.1 --dev tun --secret /tmp/test.key ... Fri Oct 30 10:39:15 2009 /usr/sbin/openvpn-vulnkey -q /tmp/test.key ... Jamie -- Jamie Strandboge | http://www.canonical.com
diff -u openvpn-2.1~rc20/debian/changelog openvpn-2.1~rc20/debian/changelog
--- openvpn-2.1~rc20/debian/changelog
+++ openvpn-2.1~rc20/debian/changelog
@@ -1,3 +1,13 @@
+openvpn (2.1~rc20-3) unstable; urgency=low
+
+ * debian/patches/debian_openssl_vulnkeys.patch:
+ - don't report key as vulnerable if openssl-vulnkey or openvpn-vulnkey
+ exit with error (Closes: #483139)
+ - fix warning string referencing '(null)' key when openssl-vulnkey returns
+ non-zero as introduced in 2.1~rc9-1
+
+ -- Jamie Strandboge <ja...@ubuntu.com> Fri, 30 Oct 2009 08:43:16 -0500
+
openvpn (2.1~rc20-2) unstable; urgency=low
* init.d script: Added X-Interactive header. (Closes: #549424)
diff -u openvpn-2.1~rc20/debian/patches/debian_openssl_vulnkeys.patch
openvpn-2.1~rc20/debian/patches/debian_openssl_vulnkeys.patch
--- openvpn-2.1~rc20/debian/patches/debian_openssl_vulnkeys.patch
+++ openvpn-2.1~rc20/debian/patches/debian_openssl_vulnkeys.patch
@@ -1,8 +1,8 @@
-Index: openvpn-2.1_rc20/init.c
+Index: openvpn-2.1~rc20/init.c
===================================================================
---- openvpn-2.1_rc20.orig/init.c 2009-10-01 20:02:18.000000000 +0200
-+++ openvpn-2.1_rc20/init.c 2009-10-05 13:03:33.082970061 +0200
-@@ -1564,6 +1564,23 @@
+--- openvpn-2.1~rc20.orig/init.c 2009-10-01 13:02:18.000000000 -0500
++++ openvpn-2.1~rc20/init.c 2009-10-30 10:22:41.000000000 -0500
+@@ -1564,6 +1564,29 @@
const struct options *options = &c->options;
ASSERT (options->shared_secret_file);
@@ -14,19 +14,25 @@
+ if (access (options->shared_secret_file, R_OK) == 0)
+ {
+ struct argv argv = argv_new ();
++ int ret;
+ argv_printf (&argv, "/usr/sbin/openvpn-vulnkey -q %s",
options->shared_secret_file);
+ argv_msg (M_INFO, &argv);
-+ if (openvpn_execve (&argv, c->c2.es, 0) != 0)
++ ret = openvpn_execve (&argv, c->c2.es, 0);
++ if (WEXITSTATUS (ret) == 1)
+ {
+ msg (M_WARN, "******* WARNING *******: '%s' is a known vulnerable
key. See 'man openvpn-vulnkey' for details.", options->shared_secret_file);
+ }
++ else if (WEXITSTATUS (ret) != 0)
++ {
++ msg (M_WARN, "******* WARNING *******: '%s' cannot be verified as a
non-vulnerable key. See 'man openvpn-vulnkey' for details.",
options->shared_secret_file);
++ }
+ argv_reset (&argv);
+ }
+
init_crypto_pre (c, flags);
/* Initialize packet ID tracking */
-@@ -1649,6 +1666,7 @@
+@@ -1649,6 +1672,7 @@
do_init_crypto_tls_c1 (struct context *c)
{
const struct options *options = &c->options;
@@ -34,7 +40,7 @@
if (!c->c1.ks.ssl_ctx)
{
-@@ -1688,6 +1706,53 @@
+@@ -1688,6 +1712,59 @@
/* Initialize PRNG with config-specified digest */
prng_init (options->prng_hash, options->prng_nonce_secret_len);
@@ -69,13 +75,19 @@
+ }
+ if (bn != NULL)
+ {
++ int ret;
+ struct argv argv = argv_new ();
+ argv_printf (&argv, "/usr/bin/openssl-vulnkey -q -b %d
-m %s", bits, bn);
+ OPENSSL_free(bn);
+ msg (M_INFO, "/usr/bin/openssl-vulnkey -q -b %d -m
<modulus omitted>", bits);
-+ if (openvpn_execve (&argv, NULL, 0) != 0)
++ ret = openvpn_execve (&argv, NULL, 0);
++ if (WEXITSTATUS (ret) == 1)
++ {
++ msg (M_WARN, "******* WARNING *******: '%s' is a
known vulnerable key. See 'man openssl-vulnkey' for details.",
options->priv_key_file);
++ }
++ else if (WEXITSTATUS (ret) != 0)
+ {
-+ msg (M_WARN, "******* WARNING *******: '%s' is a
known vulnerable key. See 'man openvpn-vulnkey' for details.",
options->shared_secret_file);
++ msg (M_WARN, "******* WARNING *******: '%s' cannot
be verified as a non-vulnerable key. See 'man openssl-vulnkey' for details.",
options->priv_key_file);
+ }
+ argv_reset (&argv);
+ }
Index: openvpn-2.1~rc20/init.c
===================================================================
--- openvpn-2.1~rc20.orig/init.c 2009-10-01 13:02:18.000000000 -0500
+++ openvpn-2.1~rc20/init.c 2009-10-30 10:22:41.000000000 -0500
@@ -1564,6 +1564,29 @@
const struct options *options = &c->options;
ASSERT (options->shared_secret_file);
+ /* CVE-2008-0166 (Debian weak key checks) */
+ /* Only check if we can actually read the key file. Unless the file does not
+ * exist in the first place, this should never happen (since static keys do
+ * not work with multi-client mode), but we test it anyway to be on the safe
+ * side and avoid wrong -vulnkey alerts. */
+ if (access (options->shared_secret_file, R_OK) == 0)
+ {
+ struct argv argv = argv_new ();
+ int ret;
+ argv_printf (&argv, "/usr/sbin/openvpn-vulnkey -q %s", options->shared_secret_file);
+ argv_msg (M_INFO, &argv);
+ ret = openvpn_execve (&argv, c->c2.es, 0);
+ if (WEXITSTATUS (ret) == 1)
+ {
+ msg (M_WARN, "******* WARNING *******: '%s' is a known vulnerable key. See 'man openvpn-vulnkey' for details.", options->shared_secret_file);
+ }
+ else if (WEXITSTATUS (ret) != 0)
+ {
+ msg (M_WARN, "******* WARNING *******: '%s' cannot be verified as a non-vulnerable key. See 'man openvpn-vulnkey' for details.", options->shared_secret_file);
+ }
+ argv_reset (&argv);
+ }
+
init_crypto_pre (c, flags);
/* Initialize packet ID tracking */
@@ -1649,6 +1672,7 @@
do_init_crypto_tls_c1 (struct context *c)
{
const struct options *options = &c->options;
+ SSL *ssl;
if (!c->c1.ks.ssl_ctx)
{
@@ -1688,6 +1712,59 @@
/* Initialize PRNG with config-specified digest */
prng_init (options->prng_hash, options->prng_nonce_secret_len);
+ /* CVE-2008-0166 (Debian weak key checks)
+ * Obtain the modulus and bits from the certificate that was initialized,
+ * and send that to openssl-vulnkey.
+ */
+ ssl = SSL_new(c->c1.ks.ssl_ctx);
+ if (ssl != NULL)
+ {
+ X509* cert = NULL;
+ char *bn;
+ int bits;
+
+ cert = SSL_get_certificate(ssl);
+ if (cert != NULL)
+ {
+ EVP_PKEY *pkey = X509_get_pubkey (cert);
+ if (pkey != NULL)
+ {
+ if (pkey->type == EVP_PKEY_RSA && pkey->pkey.rsa != NULL
+ && pkey->pkey.rsa->n != NULL)
+ {
+ bits = BN_num_bits(pkey->pkey.rsa->n);
+ bn = BN_bn2hex(pkey->pkey.rsa->n);
+ }
+ else if (pkey->type == EVP_PKEY_DSA && pkey->pkey.dsa != NULL
+ && pkey->pkey.dsa->p != NULL)
+ {
+ bits = BN_num_bits(pkey->pkey.dsa->p);
+ bn = BN_bn2hex(pkey->pkey.dsa->p);
+ }
+ if (bn != NULL)
+ {
+ int ret;
+ struct argv argv = argv_new ();
+ argv_printf (&argv, "/usr/bin/openssl-vulnkey -q -b %d -m %s", bits, bn);
+ OPENSSL_free(bn);
+ msg (M_INFO, "/usr/bin/openssl-vulnkey -q -b %d -m <modulus omitted>", bits);
+ ret = openvpn_execve (&argv, NULL, 0);
+ if (WEXITSTATUS (ret) == 1)
+ {
+ msg (M_WARN, "******* WARNING *******: '%s' is a known vulnerable key. See 'man openssl-vulnkey' for details.", options->priv_key_file);
+ }
+ else if (WEXITSTATUS (ret) != 0)
+ {
+ msg (M_WARN, "******* WARNING *******: '%s' cannot be verified as a non-vulnerable key. See 'man openssl-vulnkey' for details.", options->priv_key_file);
+ }
+ argv_reset (&argv);
+ }
+ EVP_PKEY_free (pkey);
+ }
+ }
+ SSL_free(ssl);
+ }
+
/* TLS handshake authentication (--tls-auth) */
if (options->tls_auth_file)
{
signature.asc
Description: Digital signature
