AST-2022-001 - res_stir_shaken/curl: Limit file size and check start.

Put checks in place to limit how much we will actually download, as well
as a check for the data we receive at the start to ensure it begins with
what we would expect a certificate to begin with.

ASTERISK-29872

Change-Id: Ifd3c6b8bd52b8b6192a04166ccce4fc8a8000b46

res/res_stir_shaken/curl.c

@@ -31,12 +31,27 @@
 /* Used to check CURL headers */
 #define MAX_HEADER_LENGTH 1023
 
+/* Used to limit download size */
+#define MAX_DOWNLOAD_SIZE 8192
+
+/* Used to limit how many bytes we get from CURL per write */
+#define MAX_BUF_SIZE_PER_WRITE 1024
+
+/* Certificates should begin with this */
+#define BEGIN_CERTIFICATE_STR "-----BEGIN CERTIFICATE-----"
+
 /* CURL callback data to avoid storing useless info in AstDB */
 struct curl_cb_data {
 	char *cache_control;
 	char *expires;
 };
 
+struct curl_cb_write_buf {
+	char buf[MAX_DOWNLOAD_SIZE + 1];
+	size_t size;
+	const char *url;
+};
+
 struct curl_cb_data *curl_cb_data_create(void)
 {
 	struct curl_cb_data *data;
@@ -149,94 +164,132 @@ static CURL *get_curl_instance(struct curl_cb_data *data)
 	return curl;
 }
 
+/*!
+ * \brief Write callback passed to libcurl
+ *
+ * \note If this function returns anything other than the size of the data
+ * libcurl expected us to process, the request will cancel. That's why we return
+ * 0 on error, otherwise the amount of data we were given
+ *
+ * \param curl_data The data from libcurl
+ * \param size Always 1 according to libcurl
+ * \param actual_size The actual size of the data
+ * \param our_data The data we passed to libcurl
+ *
+ * \retval The size of the data we processed
+ * \retval 0 if there was an error
+ */
+static size_t curl_write_cb(void *curl_data, size_t size, size_t actual_size, void *our_data)
+{
+	/* Just in case size is NOT always 1 or if it's changed in the future, let's go ahead
+	 * and do the math for the actual size */
+	size_t real_size = size * actual_size;
+	struct curl_cb_write_buf *buf = our_data;
+	size_t new_size = buf->size + real_size;
+
+	if (new_size > MAX_DOWNLOAD_SIZE) {
+		ast_log(LOG_WARNING, "Attempted to retrieve certificate from %s failed "
+			"because it's size exceeds the maximum %d bytes\n", buf->url, MAX_DOWNLOAD_SIZE);
+		return 0;
+	}
+
+	memcpy(&(buf->buf[buf->size]), curl_data, real_size);
+	buf->size += real_size;
+	buf->buf[buf->size] = 0;
+
+	return real_size;
+}
+
 char *curl_public_key(const char *public_cert_url, const char *path, struct curl_cb_data *data)
 {
 	FILE *public_key_file;
-	RAII_VAR(char *, tmp_filename, NULL, ast_free);
-	const char *template_name = "certXXXXXX";
 	char *filename;
 	char *serial;
-	int fd;
 	long http_code;
 	CURL *curl;
 	char curl_errbuf[CURL_ERROR_SIZE + 1];
+	struct curl_cb_write_buf *buf;
 
+	buf = ast_calloc(1, sizeof(*buf));
+	if (!buf) {
+		ast_log(LOG_ERROR, "Failed to allocate memory for CURL write buffer for %s\n", public_cert_url);
+		return NULL;
+	}
+
+	buf->url = public_cert_url;
 	curl_errbuf[CURL_ERROR_SIZE] = '\0';
 
-	/* For now, it's fine to pass in path as is - it shouldn't end with a '/'. However,
-	 * if we decide to change how certificates are stored in the future (configurable paths),
-	 * then we will need to check to see if path ends with '/', copy everything up to the '/',
-	 * and use this new variable for ast_create_temp_file as well as for ast_asprintf below.
-	 */
-	fd = ast_file_fdtemp(path, &tmp_filename, template_name);
-	if (fd == -1) {
-		ast_log(LOG_ERROR, "Failed to get temporary file descriptor for CURL\n");
-		return NULL;
-	}
-
-	public_key_file = fdopen(fd, "wb");
-	if (!public_key_file) {
-		ast_log(LOG_ERROR, "Failed to open file '%s' to write public key from '%s': %s (%d)\n",
-			tmp_filename, public_cert_url, strerror(errno), errno);
-		close(fd);
-		remove(tmp_filename);
-		return NULL;
-	}
-
 	curl = get_curl_instance(data);
 	if (!curl) {
 		ast_log(LOG_ERROR, "Failed to set up CURL instance for '%s'\n", public_cert_url);
-		fclose(public_key_file);
+		ast_free(buf);
-		remove(tmp_filename);
 		return NULL;
 	}
 
 	curl_easy_setopt(curl, CURLOPT_URL, public_cert_url);
-	curl_easy_setopt(curl, CURLOPT_WRITEDATA, public_key_file);
+	curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, curl_write_cb);
+	curl_easy_setopt(curl, CURLOPT_WRITEDATA, buf);
 	curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, curl_errbuf);
+	curl_easy_setopt(curl, CURLOPT_BUFFERSIZE, MAX_BUF_SIZE_PER_WRITE);
 
 	if (curl_easy_perform(curl)) {
 		ast_log(LOG_ERROR, "%s\n", curl_errbuf);
 		curl_easy_cleanup(curl);
-		fclose(public_key_file);
+		ast_free(buf);
-		remove(tmp_filename);
 		return NULL;
 	}
 
 	curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
 
 	curl_easy_cleanup(curl);
-	fclose(public_key_file);
 
 	if (http_code / 100 != 2) {
 		ast_log(LOG_ERROR, "Failed to retrieve URL '%s': code %ld\n", public_cert_url, http_code);
-		remove(tmp_filename);
+		ast_free(buf);
 		return NULL;
 	}
 
-	serial = stir_shaken_get_serial_number_x509(tmp_filename);
+	if (!ast_begins_with(buf->buf, BEGIN_CERTIFICATE_STR)) {
+		ast_log(LOG_WARNING, "Certificate from %s does not begin with what we expect\n", public_cert_url);
+		ast_free(buf);
+		return NULL;
+	}
+
+	serial = stir_shaken_get_serial_number_x509(buf->buf, buf->size);
 	if (!serial) {
-		ast_log(LOG_ERROR, "Failed to get serial from cert %s\n", tmp_filename);
+		ast_log(LOG_ERROR, "Failed to get serial from CURL buffer from %s\n", public_cert_url);
-		remove(tmp_filename);
+		ast_free(buf);
 		return NULL;
 	}
 
 	if (ast_asprintf(&filename, "%s/%s.pem", path, serial) < 0) {
-		ast_log(LOG_ERROR, "Failed to allocate memory for new filename for temporary "
+		ast_log(LOG_ERROR, "Failed to allocate memory for filename after CURL from %s\n", public_cert_url);
-			"file %s after CURL\n", tmp_filename);
 		ast_free(serial);
-		remove(tmp_filename);
+		ast_free(buf);
 		return NULL;
 	}
 
 	ast_free(serial);
 
-	if (rename(tmp_filename, filename)) {
+	public_key_file = fopen(filename, "w");
-		ast_log(LOG_ERROR, "Failed to rename temporary file %s to %s after CURL\n", tmp_filename, filename);
+	if (!public_key_file) {
+		ast_log(LOG_ERROR, "Failed to open file '%s' to write public key from '%s': %s (%d)\n",
+			filename, public_cert_url, strerror(errno), errno);
+		ast_free(buf);
 		ast_free(filename);
-		remove(tmp_filename);
 		return NULL;
 	}
 
+	if (fputs(buf->buf, public_key_file) == EOF) {
+		ast_log(LOG_ERROR, "Failed to write string to file from URL %s\n", public_cert_url);
+		fclose(public_key_file);
+		ast_free(buf);
+		ast_free(filename);
+		return NULL;
+	}
+
+	fclose(public_key_file);
+	ast_free(buf);
+
 	return filename;
 }

res/res_stir_shaken/stir_shaken.c

@@ -137,41 +137,35 @@ EVP_PKEY *stir_shaken_read_key(const char *path, int priv)
 	return key;
 }
 
-char *stir_shaken_get_serial_number_x509(const char *path)
+char *stir_shaken_get_serial_number_x509(const char *buf, size_t buf_size)
 {
-	FILE *fp;
+	BIO *certBIO;
 	X509 *cert;
 	ASN1_INTEGER *serial;
 	BIGNUM *bignum;
 	char *serial_hex;
 	char *ret;
 
-	fp = fopen(path, "r");
+	certBIO = BIO_new(BIO_s_mem());
-	if (!fp) {
+	BIO_write(certBIO, buf, buf_size);
-		ast_log(LOG_ERROR, "Failed to open file %s\n", path);
+	cert = PEM_read_bio_X509(certBIO, NULL, NULL, NULL);
-		return NULL;
+	BIO_free(certBIO);
-	}
-
-	cert = PEM_read_X509(fp, NULL, NULL, NULL);
 	if (!cert) {
-		ast_log(LOG_ERROR, "Failed to read X.509 cert from file %s\n", path);
+		ast_log(LOG_ERROR, "Failed to read X.509 cert from buffer\n");
-		fclose(fp);
 		return NULL;
 	}
 
 	serial = X509_get_serialNumber(cert);
 	if (!serial) {
-		ast_log(LOG_ERROR, "Failed to get serial number from certificate %s\n", path);
+		ast_log(LOG_ERROR, "Failed to get serial number from certificate\n");
 		X509_free(cert);
-		fclose(fp);
 		return NULL;
 	}
 
 	bignum = ASN1_INTEGER_to_BN(serial, NULL);
 	if (bignum == NULL) {
-		ast_log(LOG_ERROR, "Failed to convert serial to bignum for certificate %s\n", path);
+		ast_log(LOG_ERROR, "Failed to convert serial to bignum for certificate\n");
 		X509_free(cert);
-		fclose(fp);
 		return NULL;
 	}
 
@@ -181,18 +175,17 @@ char *stir_shaken_get_serial_number_x509(const char *path)
 	 */
 	serial_hex = BN_bn2hex(bignum);
 	X509_free(cert);
-	fclose(fp);
 	BN_free(bignum);
 
 	if (!serial_hex) {
-		ast_log(LOG_ERROR, "Failed to convert bignum to hex for certificate %s\n", path);
+		ast_log(LOG_ERROR, "Failed to convert bignum to hex for certificate\n");
 		return NULL;
 	}
 
 	ret = ast_strdup(serial_hex);
 	OPENSSL_free(serial_hex);
 	if (!ret) {
-		ast_log(LOG_ERROR, "Failed to dup serial from openssl for certificate %s\n", path);
+		ast_log(LOG_ERROR, "Failed to dup serial from openssl for certificate\n");
 		return NULL;
 	}
 

res/res_stir_shaken/stir_shaken.h

@@ -53,15 +53,16 @@ char *stir_shaken_tab_complete_name(const char *word, struct ao2_container *cont
 EVP_PKEY *stir_shaken_read_key(const char *path, int priv);
 
 /*!
- * \brief Gets the serial number in hex form from the X509 certificate at path
+ * \brief Gets the serial number in hex form from the buffer (for X509)
  *
  * \note The returned string will need to be freed by the caller
  *
- * \param path The full path of the X509 certificate
+ * \param buf The BASE64 encoded buffer
+ * \param buf_size The size of the data in buf
  *
  * \retval NULL on failure
  * \retval serial number on success
  */
-char *stir_shaken_get_serial_number_x509(const char *path);
+char *stir_shaken_get_serial_number_x509(const char *buf, size_t buf_size);
 
 #endif /* _STIR_SHAKEN_H */