<p>Philip Prindeville has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/19278">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_crypto: handle unsafe private key files<br><br>Note that strstr() takes a "const char *" but returns a "char *"<br>which we're then scribbling on. That's a no-no. We should<br>preserve the argument sent to us in try_load_key() as "fname".<br><br>ASTERISK-30213 #close<br><br>Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a<br>---<br>M res/res_crypto.c<br>1 file changed, 46 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/78/19278/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 82014b6..41daa8d 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,19 +174,22 @@</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(0, 100%, 40%);">- char *c = NULL, ffname[256];</span><br><span style="color: hsl(120, 100%, 40%);">+ int n, ktype = 0, found = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *c = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ char ffname[256];</span><br><span> unsigned char digest[MD5_DIGEST_LENGTH];</span><br><span> unsigned digestlen;</span><br><span> FILE *f;</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 style="color: hsl(120, 100%, 40%);">+ size_t fnamelen = strlen(fname);</span><br><span> </span><br><span> /* Make sure its name is a public or private key */</span><br><span style="color: hsl(0, 100%, 40%);">- if ((c = strstr(fname, ".pub")) && !strcmp(c, ".pub")) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fnamelen > 4 && !strcmp((c = &fname[fnamelen - 4]), ".pub")) {</span><br><span> ktype = AST_KEY_PUBLIC;</span><br><span style="color: hsl(0, 100%, 40%);">- } else if ((c = strstr(fname, ".key")) && !strcmp(c, ".key")) {</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (fnamelen > 4 && !strcmp((c = &fname[fnamelen - 4]), ".key")) {</span><br><span> ktype = AST_KEY_PRIVATE;</span><br><span> } else {</span><br><span> return NULL;</span><br><span>@@ -200,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>@@ -245,7 +270,6 @@</span><br><span> }</span><br><span> </span><br><span> /* Make fname just be the normal name now */</span><br><span style="color: hsl(0, 100%, 40%);">- *c = '\0';</span><br><span> if (!key) {</span><br><span> if (!(key = ast_calloc(1, sizeof(*key)))) {</span><br><span> fclose(f);</span><br><span>@@ -254,8 +278,8 @@</span><br><span> }</span><br><span> /* First the filename */</span><br><span> ast_copy_string(key->fn, ffname, sizeof(key->fn));</span><br><span style="color: hsl(0, 100%, 40%);">- /* Then the name */</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_string(key->name, fname, sizeof(key->name));</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Then the name less suffix */</span><br><span style="color: hsl(120, 100%, 40%);">+ snprintf(key->name, sizeof(key->name), "%.*s", (int)(c - fname), fname);</span><br><span> key->ktype = ktype;</span><br><span> /* Yes, assume we're going to be deleted */</span><br><span> key->delme = 1;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19278">change 19278</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/+/19278"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a </div>
<div style="display:none"> Gerrit-Change-Number: 19278 </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>