From e474cf3e6f937f0bc26a0f4171bacb468ebd2241 Mon Sep 17 00:00:00 2001 From: Jakub Filak Date: Wed, 21 Oct 2015 14:40:28 +0200 Subject: [PATCH] curl: return URLs without userinfo All clients should work with URLs without userinfo. This commit will ensure that. This commit also changes log messages from: Sending /var/tmp/problem_dir.tar.gz to scp://localhost/tmp/tmp.x5WVgpgUsY/target/ Error while uploading: 'curl_easy_perform: Login denied' Please enter user name for 'scp://localhost/tmp/tmp.x5WVgpgUsY/target/': root Please enter password for 'root': Sending /var/tmp/problem_dir.tar.gz to scp://localhost/tmp/tmp.x5WVgpgUsY/target/ Successfully sent /var/tmp/problem_dir.tar.gz to scp://localhost/tmp/tmp.x5WVgpgUsY/target/ to: Sending /var/tmp/problem_dir.tar.gz to scp://localhost Error while uploading: 'curl_easy_perform: Login denied' Please enter user name for 'scp://localhost': root Please enter password for 'scp://root@localhost': Sending /var/tmp/problem_dir.tar.gz to scp://localhost Successfully created scp://localhost/tmp/tmp.x5WVgpgUsY/target/problem_dir.tar.gz Signed-off-by: Jakub Filak --- src/include/libreport_curl.h | 9 ++++++ src/lib/curl.c | 67 +++++++++++++++++++++++++++++++------------- 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/include/libreport_curl.h b/src/include/libreport_curl.h index 812738c..b9277ad 100644 --- a/src/include/libreport_curl.h +++ b/src/include/libreport_curl.h @@ -128,6 +128,15 @@ enum { #define upload_file libreport_upload_file char *upload_file(const char *url, const char *filename); +/* Uploads filename to url. + * + * If url does not ends with '/', base name of filename will be amended. + * + * Fails if the url does not have scheme or hostname. + * + * @return Resulting URL on success (the URL does not contain userinfo); + * otherwise NULL. + */ #define upload_file_ext libreport_upload_file_ext char *upload_file_ext(post_state_t *post_state, const char *url, diff --git a/src/lib/curl.c b/src/lib/curl.c index 606d9ea..a64c464 100644 --- a/src/lib/curl.c +++ b/src/lib/curl.c @@ -600,27 +600,48 @@ char *upload_file_ext(post_state_t *state, const char *url, const char *filename { /* we don't want to print the whole url as it may contain password * rhbz#856960 - * there can be '@' in the login or password so let's try to find the - * first '@' from the end + * + * jfilak: + * We want to print valid URLs in useful messages. + * + * The old code had this approach: + * there can be '@' in the login or password so let's try to find the + * first '@' from the end + * + * The new implementation decomposes URI to its base elements and uses only + * scheme and hostname for the logging purpose. These elements should not + * contain any sensitive information. */ - const char *clean_url = strrchr(url, '@'); - if (clean_url) - clean_url++; - else - clean_url = url; - - char *whole_url; - unsigned len = strlen(url); - if (len > 0 && url[len-1] == '/') - whole_url = concat_path_file(url, strrchr(filename, '/') ? : filename); - else - whole_url = xstrdup(url); - - const char *username_bck = state->username; const char *password_bck = state->password; + + char *whole_url = NULL; + char *scheme = NULL; + char *hostname = NULL; char *username = NULL; char *password = NULL; + char *clean_url = NULL; + + if (uri_userinfo_remove(url, &clean_url, &scheme, &hostname, &username, &password, NULL) != 0) + goto finito; + + if (scheme == NULL || hostname == NULL) + { + log_warning(_("Ingoring URL without scheme and hostname")); + goto finito; + } + + if (username && (state->username == NULL || state->username[0] == '\0')) + { + state->username = username; + state->password = password; + } + + unsigned len = strlen(clean_url); + if (len > 0 && clean_url[len-1] == '/') + whole_url = concat_path_file(clean_url, strrchr(filename, '/') ? : filename); + else + whole_url = xstrdup(clean_url); /* work around bug in libssh2(curl with scp://) * libssh2_aget_disconnect() calls close(0) @@ -634,7 +655,9 @@ char *upload_file_ext(post_state_t *state, const char *url, const char *filename */ do_post: - log(_("Sending %s to %s"), filename, clean_url); + /* Do not include the path part of the URL as it can contain sensitive data + * in case of typos */ + log(_("Sending %s to %s//%s"), filename, scheme, hostname); post(state, whole_url, /*content_type:*/ "application/octet-stream", @@ -658,13 +681,13 @@ char *upload_file_ext(post_state_t *state, const char *url, const char *filename (state->curl_result == CURLE_LOGIN_DENIED || state->curl_result == CURLE_REMOTE_ACCESS_DENIED)) { - char *msg = xasprintf(_("Please enter user name for '%s':"), clean_url); + char *msg = xasprintf(_("Please enter user name for '%s//%s':"), scheme, hostname); free(username); username = ask(msg); free(msg); if (username != NULL && username[0] != '\0') { - msg = xasprintf(_("Please enter password for '%s':"), username); + msg = xasprintf(_("Please enter password for '%s//%s@%s':"), scheme, username, hostname); free(password); password = ask_password(msg); free(msg); @@ -687,13 +710,17 @@ char *upload_file_ext(post_state_t *state, const char *url, const char *filename else { /* This ends up a "reporting status message" in abrtd */ - log(_("Successfully sent %s to %s"), filename, clean_url); + log(_("Successfully created %s"), whole_url); } close(stdin_bck); +finito: free(password); free(username); + free(hostname); + free(scheme); + free(clean_url); state->username = username_bck; state->password = password_bck; -- 1.8.3.1