<p>Philip Prindeville has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/19405">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_crypto: handle unsafe private key files<br><br>ASTERISK-30213 #close<br><br>Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a<br>---<br>M res/res_crypto.c<br>M tests/test_crypto.c<br>2 files changed, 42 insertions(+), 1 deletion(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/05/19405/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_crypto.c b/res/res_crypto.c</span><br><span>index 8d6c536..2f7868c 100644</span><br><span>--- a/res/res_crypto.c</span><br><span>+++ b/res/res_crypto.c</span><br><span>@@ -34,6 +34,7 @@</span><br><span> #include "asterisk.h"</span><br><span> </span><br><span> #include <dirent.h>                 /* for closedir, opendir, readdir, DIR */</span><br><span style="color: hsl(120, 100%, 40%);">+#include <sys/stat.h>               /* for fstat */</span><br><span> </span><br><span> #include <openssl/err.h>            /* for ERR_print_errors_fp */</span><br><span> #include <openssl/ssl.h>            /* for NID_sha1, RSA */</span><br><span>@@ -173,7 +174,7 @@</span><br><span> */</span><br><span> static struct ast_key *try_load_key(const char *dir, const char *fname, int ifd, int ofd, int *not2)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-   int ktype = 0, found = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     int n, ktype = 0, found = 0;</span><br><span>         const char *c = NULL;</span><br><span>        char ffname[256];</span><br><span>    unsigned char digest[MD5_DIGEST_LENGTH];</span><br><span>@@ -182,6 +183,7 @@</span><br><span>       EVP_MD_CTX *ctx = NULL;</span><br><span>      struct ast_key *key;</span><br><span>         static int notice = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+        struct stat st;</span><br><span>      size_t fnamelen = strlen(fname);</span><br><span> </span><br><span>         /* Make sure its name is a public or private key */</span><br><span>@@ -202,6 +204,27 @@</span><br><span>           return NULL;</span><br><span>         }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ n = fstat(fileno(f), &st);</span><br><span style="color: hsl(120, 100%, 40%);">+        if (n != 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+         ast_log(LOG_ERROR, "Unable to stat key file: %s: %s\n", ffname, strerror(errno));</span><br><span style="color: hsl(120, 100%, 40%);">+           fclose(f);</span><br><span style="color: hsl(120, 100%, 40%);">+            return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+  }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   if (!S_ISREG(st.st_mode)) {</span><br><span style="color: hsl(120, 100%, 40%);">+           ast_log(LOG_ERROR, "Key file is not a regular file: %s\n", ffname);</span><br><span style="color: hsl(120, 100%, 40%);">+         fclose(f);</span><br><span style="color: hsl(120, 100%, 40%);">+            return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+  }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* only user read or read/write modes allowed */</span><br><span style="color: hsl(120, 100%, 40%);">+      if (ktype == AST_KEY_PRIVATE &&</span><br><span style="color: hsl(120, 100%, 40%);">+           ((st.st_mode & ALLPERMS) & ~(S_IRUSR | S_IWUSR)) != 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+          ast_log(LOG_ERROR, "Private key file has bad permissions: %s: %#4o\n", ffname, st.st_mode & ALLPERMS);</span><br><span style="color: hsl(120, 100%, 40%);">+          fclose(f);</span><br><span style="color: hsl(120, 100%, 40%);">+            return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+  }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  ctx = EVP_MD_CTX_create();</span><br><span>   if (ctx == NULL) {</span><br><span>           ast_log(LOG_ERROR, "Out of memory\n");</span><br><span>diff --git a/tests/test_crypto.c b/tests/test_crypto.c</span><br><span>index 1eec181..848a562 100644</span><br><span>--- a/tests/test_crypto.c</span><br><span>+++ b/tests/test_crypto.c</span><br><span>@@ -40,6 +40,7 @@</span><br><span> #include "asterisk/file.h"</span><br><span> </span><br><span> #include <assert.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <sys/stat.h></span><br><span> #include <linux/limits.h></span><br><span> #include <openssl/evp.h></span><br><span> </span><br><span>@@ -117,6 +118,9 @@</span><br><span>   push_key_dir((const char *)key_dir);</span><br><span>         snprintf(priv, sizeof(priv), "%s/%s.key", key_dir, keypair1);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+   /* because git doesn't preserve permissions */</span><br><span style="color: hsl(120, 100%, 40%);">+    (void)chmod(priv, 0400);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>   if (ast_crypto_reload() != 1) {</span><br><span>              ast_test_status_update(test, "Couldn't force crypto reload\n");</span><br><span>                goto cleanup;</span><br><span>@@ -414,6 +418,9 @@</span><br><span>  push_key_dir((const char *)key_dir);</span><br><span>         snprintf(priv, sizeof(priv), "%s/%s.key", key_dir, keypair1);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+   /* because git doesn't preserve permissions */</span><br><span style="color: hsl(120, 100%, 40%);">+    (void)chmod(priv, 0400);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>   if (ast_crypto_reload() != 1) {</span><br><span>              ast_test_status_update(test, "Couldn't force crypto reload\n");</span><br><span>                goto cleanup;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19405">change 19405</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/19405"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a </div>
<div style="display:none"> Gerrit-Change-Number: 19405 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Philip Prindeville <philipp@redfish-solutions.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>