From ad0549dfa0396f495cddbc8c83b37013c0d9ce15 Mon Sep 17 00:00:00 2001 From: william Date: Fri, 19 Sep 2014 20:02:38 -0700 Subject: fix issue with loading public and private keys concurrently, fix issue in pusherror when no OpenSSL errors are actually defined, and because OpenSSL doesn't define its FOO_free routines to accept NULL--even though most or all do handle NULL properly--don't pass them NULL --- src/openssl.c | 196 +++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 140 insertions(+), 56 deletions(-) (limited to 'src') diff --git a/src/openssl.c b/src/openssl.c index 8efb348..1aab2ca 100644 --- a/src/openssl.c +++ b/src/openssl.c @@ -236,6 +236,9 @@ static const char *pusherror(lua_State *L, const char *fun) { int line; char txt[256]; + if (!ERR_peek_error()) + return lua_pushstring(L, "oops: no OpenSSL errors set"); + code = ERR_get_error_line(&path, &line); if ((file = strrchr(path, '/'))) @@ -686,8 +689,10 @@ static void bn_prepops(lua_State *L, BIGNUM **r, BIGNUM **a, BIGNUM **b, _Bool c static int ctx__gc(lua_State *L) { BN_CTX **ctx = lua_touserdata(L, 1); - BN_CTX_free(*ctx); - *ctx = NULL; + if (*ctx) { + BN_CTX_free(*ctx); + *ctx = NULL; + } return 0; } /* ctx__gc() */ @@ -837,8 +842,10 @@ static int bn__le(lua_State *L) { static int bn__gc(lua_State *L) { BIGNUM **ud = luaL_checkudata(L, 1, BIGNUM_CLASS); - BN_free(*ud); - *ud = NULL; + if (*ud) { + BN_free(*ud); + *ud = NULL; + } return 0; } /* bn__gc() */ @@ -901,8 +908,10 @@ int luaopen__openssl_bignum(lua_State *L) { static int bio__gc(lua_State *L) { BIO **bio = lua_touserdata(L, 1); - BIO_free(*bio); - *bio = NULL; + if (*bio) { + BIO_free(*bio); + *bio = NULL; + } return 0; } /* bio__gc() */ @@ -1080,18 +1089,19 @@ creat: } /* switch() */ } else if (lua_isstring(L, 1)) { int type = optencoding(L, 2, "*", X509_ANY|X509_PEM|X509_DER); - int ispub = -1; + int pubonly = 0, prvtonly = 0; const char *opt, *data; size_t len; BIO *bio; - int ok = 0; + EVP_PKEY *pub = NULL, *prvt = NULL; + int goterr = 0; /* check if specified publickey or privatekey */ if ((opt = luaL_optstring(L, 3, NULL))) { if (xtolower(opt[0]) == 'p' && xtolower(opt[1]) == 'u') { - ispub = 1; + pubonly = 1; } else if (xtolower(opt[0]) == 'p' && xtolower(opt[1]) == 'r') { - ispub = 0; + prvtonly = 1; } else { return luaL_argerror(L, 3, lua_pushfstring(L, "invalid option %s", opt)); } @@ -1103,42 +1113,85 @@ creat: return throwssl(L, "pkey.new"); if (type == X509_PEM || type == X509_ANY) { - if (ispub == 1 || ispub == -1) { - ok = !!(*ud = PEM_read_bio_PUBKEY(bio, NULL, 0, "")); - - if (ok || (type == X509_PEM && ispub == 1)) - goto done; + if (!prvtonly && !pub) { + /* + * BIO_reset is a rewind for read-only + * memory buffers. See mem_ctrl in + * crypto/bio/bss_mem.c of OpenSSL source. + */ + BIO_reset(bio); + + if (!(pub = PEM_read_bio_PUBKEY(bio, NULL, 0, ""))) + goterr = 1; } - if (ispub == 0 || ispub == -1) { - ok = !!(*ud = PEM_read_bio_PrivateKey(bio, NULL, 0, "")); + if (!pubonly && !prvt) { + BIO_reset(bio); - if (ok || (type == X509_PEM && ispub == 0)) - goto done; + if (!(prvt = PEM_read_bio_PrivateKey(bio, NULL, 0, ""))) + goterr = 1; } } if (type == X509_DER || type == X509_ANY) { - if (ispub == 1 || ispub == -1) { - ok = !!(*ud = d2i_PUBKEY_bio(bio, NULL)); + if (!prvtonly && !pub) { + BIO_reset(bio); - if (ok || (type == X509_DER && ispub == 1)) - goto done; + if (!(pub = d2i_PUBKEY_bio(bio, NULL))) + goterr = 1; } - if (ispub == 0 || ispub == -1) { - ok = !!(*ud = d2i_PrivateKey_bio(bio, NULL)); + if (!pubonly && !prvt) { + BIO_reset(bio); - if (ok || (type == X509_DER && ispub == 0)) - goto done; + if (!(prvt = d2i_PrivateKey_bio(bio, NULL))) + goterr = 1; } } + if (prvt) { +#if 0 + /* TODO: Determine if this is necessary. */ + if (pub && EVP_PKEY_missing_parameters(prvt)) { + if (!EVP_PKEY_copy_parameters(prvt, pub)) { + /* + * NOTE: It's not necessarily true + * that any internal errors were + * set. But we fixed pusherror() to + * handle that situation. + */ + goterr = 1; + + goto done; + } + } +#endif + + *ud = prvt; + prvt = NULL; + } else if (pub) { + *ud = pub; + pub = NULL; + } done: BIO_free(bio); - if (!ok) - return throwssl(L, "pkey.new"); + if (pub) + EVP_PKEY_free(pub); + + if (prvt) + EVP_PKEY_free(prvt); + + if (!*ud) { + if (goterr) + return throwssl(L, "pkey.new"); + + /* we should never get here */ + return luaL_error(L, "failed to load key for some unexpected reason"); + } else if (goterr) { + /* clean up our mess from testing input formats */ + ERR_clear_error(); + } } else { return luaL_error(L, "%s: unknown key initializer", lua_typename(L, lua_type(L, 1))); } @@ -1223,7 +1276,7 @@ static int pk_setPrivateKey(lua_State *L) { lua_pushboolean(L, 1); return 1; -} /* pk_setPrivateKEY() */ +} /* pk_setPrivateKey() */ static int pk_sign(lua_State *L) { @@ -1408,8 +1461,10 @@ static int pk__tostring(lua_State *L) { static int pk__gc(lua_State *L) { EVP_PKEY **ud = luaL_checkudata(L, 1, PKEY_CLASS); - EVP_PKEY_free(*ud); - *ud = NULL; + if (*ud) { + EVP_PKEY_free(*ud); + *ud = NULL; + } return 0; } /* pk__gc() */ @@ -1614,8 +1669,10 @@ static int xn__pairs(lua_State *L) { static int xn__gc(lua_State *L) { X509_NAME **ud = luaL_checkudata(L, 1, X509_NAME_CLASS); - X509_NAME_free(*ud); - *ud = NULL; + if (*ud) { + X509_NAME_free(*ud); + *ud = NULL; + } return 0; } /* xn__gc() */ @@ -1881,8 +1938,10 @@ static int gn__pairs(lua_State *L) { static int gn__gc(lua_State *L) { GENERAL_NAMES **ud = luaL_checkudata(L, 1, X509_GENS_CLASS); - sk_GENERAL_NAME_pop_free(*ud, GENERAL_NAME_free); - *ud = NULL; + if (*ud) { + sk_GENERAL_NAME_pop_free(*ud, GENERAL_NAME_free); + *ud = NULL; + } return 0; } /* gn__gc() */ @@ -1979,8 +2038,10 @@ static int xe_interpose(lua_State *L) { static int xe__gc(lua_State *L) { X509_EXTENSION **ud = luaL_checkudata(L, 1, X509_EXT_CLASS); - X509_EXTENSION_free(*ud); - *ud = NULL; + if (*ud) { + X509_EXTENSION_free(*ud); + *ud = NULL; + } return 0; } /* xe__gc() */ @@ -2899,8 +2960,10 @@ static int xc__tostring(lua_State *L) { static int xc__gc(lua_State *L) { X509 **ud = luaL_checkudata(L, 1, X509_CERT_CLASS); - X509_free(*ud); - *ud = NULL; + if (*ud) { + X509_free(*ud); + *ud = NULL; + } return 0; } /* xc__gc() */ @@ -3130,8 +3193,10 @@ static int xr__tostring(lua_State *L) { static int xr__gc(lua_State *L) { X509_REQ **ud = luaL_checkudata(L, 1, X509_CSR_CLASS); - X509_REQ_free(*ud); - *ud = NULL; + if (*ud) { + X509_REQ_free(*ud); + *ud = NULL; + } return 0; } /* xr__gc() */ @@ -3459,8 +3524,10 @@ static int xx__tostring(lua_State *L) { static int xx__gc(lua_State *L) { X509_CRL **ud = luaL_checkudata(L, 1, X509_CRL_CLASS); - X509_CRL_free(*ud); - *ud = NULL; + if (*ud) { + X509_CRL_free(*ud); + *ud = NULL; + } return 0; } /* xx__gc() */ @@ -3625,8 +3692,10 @@ static int xl__pairs(lua_State *L) { static int xl__gc(lua_State *L) { STACK_OF(X509) **chain = luaL_checkudata(L, 1, X509_CHAIN_CLASS); - sk_X509_pop_free(*chain, X509_free); - *chain = NULL; + if (*chain) { + sk_X509_pop_free(*chain, X509_free); + *chain = NULL; + } return 0; } /* xl__gc() */ @@ -3788,8 +3857,10 @@ static int xs_verify(lua_State *L) { static int xs__gc(lua_State *L) { X509_STORE **ud = luaL_checkudata(L, 1, X509_STORE_CLASS); - X509_STORE_free(*ud); - *ud = NULL; + if (*ud) { + X509_STORE_free(*ud); + *ud = NULL; + } return 0; } /* xs__gc() */ @@ -3857,8 +3928,10 @@ static int stx_add(lua_State *L) { static int stx__gc(lua_State *L) { X509_STORE **ud = luaL_checkudata(L, 1, X509_STORE_CLASS); - X509_STORE_free(*ud); - *ud = NULL; + if (*ud) { + X509_STORE_free(*ud); + *ud = NULL; + } return 0; } /* stx__gc() */ @@ -3975,8 +4048,10 @@ static int p12__tostring(lua_State *L) { static int p12__gc(lua_State *L) { PKCS12 **ud = luaL_checkudata(L, 1, PKCS12_CLASS); - PKCS12_free(*ud); - *ud = NULL; + if (*ud) { + PKCS12_free(*ud); + *ud = NULL; + } return 0; } /* p12__gc() */ @@ -4153,6 +4228,11 @@ static int sx_setPrivateKey(lua_State *L) { /* * NOTE: No easy way to dup the key, but a shared reference should * be okay as keys are less mutable than certificates. + * + * FIXME: SSL_CTX_use_PrivateKey will return true even if the + * EVP_PKEY object has no private key. Instead, we'll just get a + * segfault during the SSL handshake. We need to check that a + * private key is actually defined in the object. */ if (!SSL_CTX_use_PrivateKey(ctx, key)) return throwssl(L, "ssl.context:setPrivateKey"); @@ -4224,8 +4304,10 @@ static int sx_setEphemeralKey(lua_State *L) { static int sx__gc(lua_State *L) { SSL_CTX **ud = luaL_checkudata(L, 1, SSL_CTX_CLASS); - SSL_CTX_free(*ud); - *ud = NULL; + if (*ud) { + SSL_CTX_free(*ud); + *ud = NULL; + } return 0; } /* sx__gc() */ @@ -4446,8 +4528,10 @@ static int ssl_setHostName(lua_State *L) { static int ssl__gc(lua_State *L) { SSL **ud = luaL_checkudata(L, 1, SSL_CLASS); - SSL_free(*ud); - *ud = NULL; + if (*ud) { + SSL_free(*ud); + *ud = NULL; + } return 0; } /* ssl__gc() */ -- cgit v1.2.3-59-g8ed1b