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

1 files changed, 140 insertions, 56 deletions

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() */