<p>N A has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/17708">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">curl, res_stir_shaken: refactor utility functions<br><br>Refactors curl and temp file utility functions<br>into file.c and utils.c.<br><br>ASTERISK-29809 #close<br><br>Change-Id: Ife478708c8f2b127239cb73c1755ef18c0bf431b<br>---<br>M funcs/func_curl.c<br>M include/asterisk/file.h<br>M include/asterisk/utils.h<br>M main/file.c<br>M main/utils.c<br>M res/res_stir_shaken/curl.c<br>6 files changed, 66 insertions(+), 62 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/08/17708/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/funcs/func_curl.c b/funcs/func_curl.c</span><br><span>index 3b04f12..9b15b2d 100644</span><br><span>--- a/funcs/func_curl.c</span><br><span>+++ b/funcs/func_curl.c</span><br><span>@@ -629,31 +629,6 @@</span><br><span> AST_THREADSTORAGE_CUSTOM(curl_instance, curl_instance_init, curl_instance_cleanup);</span><br><span> AST_THREADSTORAGE(thread_escapebuf);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*!</span><br><span style="color: hsl(0, 100%, 40%);">- * \brief Check for potential HTTP injection risk.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * CVE-2014-8150 brought up the fact that HTTP proxies are subject to injection</span><br><span style="color: hsl(0, 100%, 40%);">- * attacks. An HTTP URL sent to a proxy contains a carriage-return linefeed combination,</span><br><span style="color: hsl(0, 100%, 40%);">- * followed by a complete HTTP request. Proxies will handle this as two separate HTTP</span><br><span style="color: hsl(0, 100%, 40%);">- * requests rather than as a malformed URL.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * libcURL patched this vulnerability in version 7.40.0, but we have no guarantee that</span><br><span style="color: hsl(0, 100%, 40%);">- * Asterisk systems will be using an up-to-date cURL library. Therefore, we implement</span><br><span style="color: hsl(0, 100%, 40%);">- * the same fix as libcURL for determining if a URL is vulnerable to an injection attack.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \param url The URL to check for vulnerability</span><br><span style="color: hsl(0, 100%, 40%);">- * \retval 0 The URL is not vulnerable</span><br><span style="color: hsl(0, 100%, 40%);">- * \retval 1 The URL is vulnerable.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-static int url_is_vulnerable(const char *url)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- if (strpbrk(url, "\r\n")) {</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> struct curl_args {</span><br><span> const char *url;</span><br><span> const char *postdata;</span><br><span>@@ -685,7 +660,7 @@</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (url_is_vulnerable(args->url)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_url_is_vulnerable(args->url)) {</span><br><span> ast_log(LOG_ERROR, "URL '%s' is vulnerable to HTTP injection attacks. Aborting CURL() call.\n", args->url);</span><br><span> return -1;</span><br><span> }</span><br><span>@@ -961,14 +936,14 @@</span><br><span> }</span><br><span> </span><br><span> for (i = 0; i < ARRAY_LEN(bad_urls); ++i) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (!url_is_vulnerable(bad_urls[i])) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ast_url_is_vulnerable(bad_urls[i])) {</span><br><span> ast_test_status_update(test, "String '%s' detected as valid when it should be invalid\n", bad_urls[i]);</span><br><span> res = AST_TEST_FAIL;</span><br><span> }</span><br><span> }</span><br><span> </span><br><span> for (i = 0; i < ARRAY_LEN(good_urls); ++i) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (url_is_vulnerable(good_urls[i])) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_url_is_vulnerable(good_urls[i])) {</span><br><span> ast_test_status_update(test, "String '%s' detected as invalid when it should be valid\n", good_urls[i]);</span><br><span> res = AST_TEST_FAIL;</span><br><span> }</span><br><span>diff --git a/include/asterisk/file.h b/include/asterisk/file.h</span><br><span>index b2da2a2..92b65e7 100644</span><br><span>--- a/include/asterisk/file.h</span><br><span>+++ b/include/asterisk/file.h</span><br><span>@@ -147,6 +147,19 @@</span><br><span> FILE *ast_file_mkftemp(char *template, mode_t mode);</span><br><span> </span><br><span> /*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief Create a temporary file located at path</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \note This function assumes path does not end with a '/'</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param path The directory path to create the file in</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param filename Function allocates memory and stores full filename (including path) here</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval -1 on failure</span><br><span style="color: hsl(120, 100%, 40%);">+ * \return file descriptor on success</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+int ast_create_temp_file(const char *path, char **filename);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*!</span><br><span> * \brief Callback called for each file found when reading directories</span><br><span> * \param dir_name the name of the directory</span><br><span> * \param filename the name of the file</span><br><span>diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h</span><br><span>index 72deaa3..07c5cbb 100644</span><br><span>--- a/include/asterisk/utils.h</span><br><span>+++ b/include/asterisk/utils.h</span><br><span>@@ -396,6 +396,24 @@</span><br><span> */</span><br><span> void ast_uri_decode(char *s, struct ast_flags spec);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief Check for potential HTTP injection risk.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * CVE-2014-8150 brought up the fact that HTTP proxies are subject to injection</span><br><span style="color: hsl(120, 100%, 40%);">+ * attacks. An HTTP URL sent to a proxy contains a carriage-return linefeed combination,</span><br><span style="color: hsl(120, 100%, 40%);">+ * followed by a complete HTTP request. Proxies will handle this as two separate HTTP</span><br><span style="color: hsl(120, 100%, 40%);">+ * requests rather than as a malformed URL.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * libcURL patched this vulnerability in version 7.40.0, but we have no guarantee that</span><br><span style="color: hsl(120, 100%, 40%);">+ * Asterisk systems will be using an up-to-date cURL library. Therefore, we implement</span><br><span style="color: hsl(120, 100%, 40%);">+ * the same fix as libcURL for determining if a URL is vulnerable to an injection attack.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param url The URL to check for vulnerability</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval 0 The URL is not vulnerable</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval 1 The URL is vulnerable.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+int ast_url_is_vulnerable(const char *url);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! ast_xml_escape</span><br><span> \brief Escape reserved characters for use in XML.</span><br><span> </span><br><span>diff --git a/main/file.c b/main/file.c</span><br><span>index 0c1fdd4..d5afd05 100644</span><br><span>--- a/main/file.c</span><br><span>+++ b/main/file.c</span><br><span>@@ -199,6 +199,26 @@</span><br><span> return p;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+int create_temp_file(const char *path, char **filename, const char *template_name)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int fd;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_asprintf(filename, "%s/%s", path, template_name) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_ERROR, "Failed to set up temporary file path\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</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%);">+ ast_mkdir(path, 0644);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((fd = mkstemp(*filename)) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_NOTICE, "Failed to create temporary file\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(*filename);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</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%);">+ return fd;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> int ast_stopstream(struct ast_channel *tmp)</span><br><span> {</span><br><span> ast_channel_lock(tmp);</span><br><span>diff --git a/main/utils.c b/main/utils.c</span><br><span>index dc94c99..cf10167 100644</span><br><span>--- a/main/utils.c</span><br><span>+++ b/main/utils.c</span><br><span>@@ -776,6 +776,15 @@</span><br><span> *o = '\0';</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+int ast_url_is_vulnerable(const char *url)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ if (strpbrk(url, "\r\n")) {</span><br><span style="color: hsl(120, 100%, 40%);">+ return 1;</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%);">+ return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> char *ast_escape_quoted(const char *string, char *outbuf, int buflen)</span><br><span> {</span><br><span> const char *ptr = string;</span><br><span>diff --git a/res/res_stir_shaken/curl.c b/res/res_stir_shaken/curl.c</span><br><span>index ad3adbc..f98608b 100644</span><br><span>--- a/res/res_stir_shaken/curl.c</span><br><span>+++ b/res/res_stir_shaken/curl.c</span><br><span>@@ -151,42 +151,11 @@</span><br><span> return curl;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*!</span><br><span style="color: hsl(0, 100%, 40%);">- * \brief Create a temporary file located at path</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \note This function assumes path does not end with a '/'</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \param path The directory path to create the file in</span><br><span style="color: hsl(0, 100%, 40%);">- * \param filename Function allocates memory and stores full filename (including path) here</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \retval -1 on failure</span><br><span style="color: hsl(0, 100%, 40%);">- * \return file descriptor on success</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-static int create_temp_file(const char *path, char **filename)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- const char *template_name = "certXXXXXX";</span><br><span style="color: hsl(0, 100%, 40%);">- int fd;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (ast_asprintf(filename, "%s/%s", path, template_name) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_ERROR, "Failed to set up temporary file path for CURL\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return -1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- ast_mkdir(path, 0644);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if ((fd = mkstemp(*filename)) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_NOTICE, "Failed to create temporary file for CURL\n");</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(*filename);</span><br><span style="color: hsl(0, 100%, 40%);">- return -1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- return fd;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> char *curl_public_key(const char *public_cert_url, const char *path, struct curl_cb_data *data)</span><br><span> {</span><br><span> FILE *public_key_file;</span><br><span> RAII_VAR(char *, tmp_filename, NULL, ast_free);</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *template_name = "certXXXXXX";</span><br><span> char *filename;</span><br><span> char *serial;</span><br><span> int fd;</span><br><span>@@ -199,9 +168,9 @@</span><br><span> /* For now, it's fine to pass in path as is - it shouldn't end with a '/'. However,</span><br><span> * if we decide to change how certificates are stored in the future (configurable paths),</span><br><span> * then we will need to check to see if path ends with '/', copy everything up to the '/',</span><br><span style="color: hsl(0, 100%, 40%);">- * and use this new variable for create_temp_file as well as for ast_asprintf below.</span><br><span style="color: hsl(120, 100%, 40%);">+ * and use this new variable for ast_create_temp_file as well as for ast_asprintf below.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- fd = create_temp_file(path, &tmp_filename);</span><br><span style="color: hsl(120, 100%, 40%);">+ fd = ast_create_temp_file(path, &tmp_filename, template_name);</span><br><span> if (fd == -1) {</span><br><span> ast_log(LOG_ERROR, "Failed to get temporary file descriptor for CURL\n");</span><br><span> return NULL;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/17708">change 17708</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/+/17708"/><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: Ife478708c8f2b127239cb73c1755ef18c0bf431b </div>
<div style="display:none"> Gerrit-Change-Number: 17708 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>