[Asterisk-code-review] res_crypto: handle unsafe private key files (asterisk[16])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19405 )

Change subject: res_crypto: handle unsafe private key files
......................................................................

res_crypto: handle unsafe private key files

ASTERISK-30213 #close

Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a
---
M res/res_crypto.c
M tests/test_crypto.c
2 files changed, 42 insertions(+), 1 deletion(-)

Approvals:
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/res/res_crypto.c b/res/res_crypto.c
index 8d6c536..2f7868c 100644
--- a/res/res_crypto.c
+++ b/res/res_crypto.c
@@ -34,6 +34,7 @@
 #include "asterisk.h"
 
 #include <dirent.h>                 /* for closedir, opendir, readdir, DIR */
+#include <sys/stat.h>               /* for fstat */
 
 #include <openssl/err.h>            /* for ERR_print_errors_fp */
 #include <openssl/ssl.h>            /* for NID_sha1, RSA */
@@ -173,7 +174,7 @@
 */
 static struct ast_key *try_load_key(const char *dir, const char *fname, int ifd, int ofd, int *not2)
 {
-	int ktype = 0, found = 0;
+	int n, ktype = 0, found = 0;
 	const char *c = NULL;
 	char ffname[256];
 	unsigned char digest[MD5_DIGEST_LENGTH];
@@ -182,6 +183,7 @@
 	EVP_MD_CTX *ctx = NULL;
 	struct ast_key *key;
 	static int notice = 0;
+	struct stat st;
 	size_t fnamelen = strlen(fname);
 
 	/* Make sure its name is a public or private key */
@@ -202,6 +204,27 @@
 		return NULL;
 	}
 
+	n = fstat(fileno(f), &st);
+	if (n != 0) {
+		ast_log(LOG_ERROR, "Unable to stat key file: %s: %s\n", ffname, strerror(errno));
+		fclose(f);
+		return NULL;
+	}
+
+	if (!S_ISREG(st.st_mode)) {
+		ast_log(LOG_ERROR, "Key file is not a regular file: %s\n", ffname);
+		fclose(f);
+		return NULL;
+	}
+
+	/* only user read or read/write modes allowed */
+	if (ktype == AST_KEY_PRIVATE &&
+	    ((st.st_mode & ALLPERMS) & ~(S_IRUSR | S_IWUSR)) != 0) {
+		ast_log(LOG_ERROR, "Private key file has bad permissions: %s: %#4o\n", ffname, st.st_mode & ALLPERMS);
+		fclose(f);
+		return NULL;
+	}
+
 	ctx = EVP_MD_CTX_create();
 	if (ctx == NULL) {
 		ast_log(LOG_ERROR, "Out of memory\n");
diff --git a/tests/test_crypto.c b/tests/test_crypto.c
index 1eec181..848a562 100644
--- a/tests/test_crypto.c
+++ b/tests/test_crypto.c
@@ -40,6 +40,7 @@
 #include "asterisk/file.h"
 
 #include <assert.h>
+#include <sys/stat.h>
 #include <linux/limits.h>
 #include <openssl/evp.h>
 
@@ -117,6 +118,9 @@
 	push_key_dir((const char *)key_dir);
 	snprintf(priv, sizeof(priv), "%s/%s.key", key_dir, keypair1);
 
+	/* because git doesn't preserve permissions */
+	(void)chmod(priv, 0400);
+
 	if (ast_crypto_reload() != 1) {
 		ast_test_status_update(test, "Couldn't force crypto reload\n");
 		goto cleanup;
@@ -414,6 +418,9 @@
 	push_key_dir((const char *)key_dir);
 	snprintf(priv, sizeof(priv), "%s/%s.key", key_dir, keypair1);
 
+	/* because git doesn't preserve permissions */
+	(void)chmod(priv, 0400);
+
 	if (ast_crypto_reload() != 1) {
 		ast_test_status_update(test, "Couldn't force crypto reload\n");
 		goto cleanup;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19405
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a
Gerrit-Change-Number: 19405
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Prindeville <philipp at redfish-solutions.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221014/6381bef9/attachment.html>