sysown / proxysql

High-performance MySQL proxy with a GPL license.
http://www.proxysql.com
GNU General Public License v3.0
5.97k stars 972 forks source link

Remove absolute paths from HTTP served by the web server #3642

Open mboruta1 opened 3 years ago

mboruta1 commented 3 years ago

Bug Description

ProxySQL seems to use relative paths for all links in the HTML served via its web interface with the exception of / (called Home in the interface). This is problematic when attempting to provide access to the interface via a reverse proxy. Please consider the following example:

If said web interface uses relative links, users can click around the interface and their browser will always request URLs starting with <haproxy ip>:443/<service name>.

However, if the web interface contains links with absolute paths, e.g. /, then, when clicked, the user's browser will connect to paths not prefixed by <service name>, e.g. <haproxy ip>:443:/ and thus break the reverse proxy.

Proposed Solution

Given that most URL paths are already relative, making ProxySQL's home URL called something like home.html instead of / (see this), and referenced in a relative fashion, would allow the web service to be easily reverse proxied.

ProxySQL Version

proxysql_2.2.2-ubuntu18_amd64.deb retrieved from here

OS Version

Ubuntu 18.04

Steps to Reproduce

Please see the bug description above.

ProxySQL Error Log

Not Applicable

mboruta1 commented 2 years ago

I would like to push a fix for this myself, but wanted to run it by you (@renecannao) before writing any additional code.

The issue has a description of what I am trying to achieve, but in short I would like users to be able to access ProxySQL's web interface via an arbitrary URL (useful when the service is sitting behind a reverse proxy).

The current code has the URLs it supports hardcoded (e.g. /stats), which means it only successfully returns the stats page if the requested URL it receives is /stats. If we are exposing the web interface via a reverse proxy at, say, myhostname.com/proxysql, the reverse proxy would need to re-write the user's URLs such that the /proxysql prefix is omitted. This works with the exception of user authentication, which requires that the URI specified in the Authorization request (digest) matches the URL requested by the user. One cannot re-write the requested URL without re-writing the URI, and one cannot re-write the URI without breaking authentication (see section 4.2 of the relevant RFC, modification of this header's contents is forbidden). Thus a different solution is needed.

I propose the following changes:

The changes to ProxySQL_HTTP_Server.cpp would look something like the following, with USER_SPECIFIED_PREFIX containing the value of web_prefix.

What do you think? Can I proceed or am I missing something?

diff --git a/lib/ProxySQL_HTTP_Server.cpp b/lib/ProxySQL_HTTP_Server.cpp
index b1140784..dd8b0c9b 100644
--- a/lib/ProxySQL_HTTP_Server.cpp
+++ b/lib/ProxySQL_HTTP_Server.cpp
@@ -115,7 +115,7 @@ static char *generate_home() {
        string html = "";
        html.append("<div class=\"menu\" style=\"height: auto;\">\n");
        html.append("<span style=\"float: left; width: 50%;\">\n");
-       html.append("<div style=\"margin-top: 15px;\"><a href=\"/\""); html.append(style2); html.append("Home</a>\n");
+       html.append("<div style=\"margin-top: 15px;\"><a href=\"home\""); html.append(style2); html.append("Home</a>\n");
        html.append("<a href=stats?metric=system"); html.append(style2); html.append("System</a>\n");
        html.append("<a href=stats?metric=mysql"); html.append(style2); html.append("MySQL</a>\n");
        html.append("<a href=stats?metric=cache"); html.append(style2); html.append("Query Cache</a></div>\n");
@@ -257,7 +257,7 @@ static char *generate_buttons(char *base) {
        char *s = NULL;
         string html = "<div class=\"menu\" style=\"height: auto;\"><span style=\"float: left; width: 100%;\">\n";
         html.append("<div style=\"margin-top: 15px;\">");
-        html.append("<a href=\"/\""); html.append(style2); html.append("Home</a>\n");
+        html.append("<a href=\"home\""); html.append(style2); html.append("Home</a>\n");
         html.append("<a href=stats?metric=system"); html.append(style2); html.append("System</a>\n");
         html.append("<a href=stats?metric=mysql"); html.append(style2); html.append("MySQL</a>\n");
         html.append("<a href=stats?metric=cache"); html.append(style2); html.append("Query Cache</a>\n");
@@ -362,7 +362,7 @@ void ProxySQL_HTTP_Server::print_version() {
        fprintf(stderr,"Standard ProxySQL HTTP Server Handler rev. %s -- %s -- %s\n", PROXYSQL_HTTP_SERVER_VERSION, __FILE__, __TIMESTAMP__);
 }

-#define EMPTY_PAGE "<html><head><title>File not found</title></head><body>File not found<br/><a href=\"/\">Go back home</a></body></html>"
+#define EMPTY_PAGE "<html><head><title>File not found</title></head><body>File not found<br/><a href=\"home\">Go back home</a></body></html>"

 int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, const char *url, const char *method, const char *version, const char *upload_data, size_t *upload_data_size, void **ptr) {

@@ -370,6 +370,8 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection,
        int ret;

        char *username;
        char *password = NULL;
        const char *realm = "Access to ProxySQL status page";
@@ -381,6 +383,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection,
                MHD_destroy_response(response);
                return ret;
        }
        {
                int default_hostgroup = -1;
                char *default_schema = NULL;
@@ -391,6 +394,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection,
                int max_connections;
                void *sha1_pass = NULL;
                password=GloMyAuth->lookup(username, USERNAME_FRONTEND, &_ret_use_ssl, &default_hostgroup, &default_schema, &schema_locked, &transaction_persistent, &fast_forward, &max_connections, &sha1_pass, NULL);
                if (default_schema) { // unused
                        free(default_schema);
                }
@@ -413,18 +417,22 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection,
                }
        }
        ret = MHD_digest_auth_check(connection, realm, username, password, 300);
        free(username);
        free(password);
        if ( (ret == MHD_INVALID_NONCE) || (ret == MHD_NO) ) {
                response = MHD_create_response_from_buffer(strlen(DENIED), (void *)DENIED, MHD_RESPMEM_PERSISTENT);
                if (NULL == response)
                ret = MHD_queue_auth_fail_response(connection, realm, OPAQUE, response, (ret == MHD_INVALID_NONCE) ? MHD_YES : MHD_NO);
                MHD_destroy_response(response);
                return ret;
     }

@@ -448,7 +456,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection,
        if (0 != strcmp (method, "GET"))
                return MHD_NO;              /* unexpected method */

-       if (strcmp(url,"/stats")==0) {
+       if (strcmp(url,"USER_SPECIFIED_PREFIX/stats")==0) {
                valmetric = (char *)MHD_lookup_connection_value (connection, MHD_GET_ARGUMENT_KIND, (char *)"metric");
                interval_s = (char *)MHD_lookup_connection_value (connection, MHD_GET_ARGUMENT_KIND, (char *)"interval");
                if (valmetric == NULL) {
@@ -800,25 +808,26 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection,
                }
        }

-       if (strcmp(url,"/Chart.bundle.js")==0) {
+       if (strcmp(url,"USER_SPECIFIED_PREFIX/Chart.bundle.js")==0) {
                response = MHD_create_response_from_buffer(strlen(Chart_bundle_js_c), Chart_bundle_js_c, MHD_RESPMEM_PERSISTENT);
                ret = MHD_queue_response (connection, MHD_HTTP_OK, response);
                MHD_destroy_response (response);
                return ret;
        }
-       if (strcmp(url,"/font-awesome.min.css")==0) {
+       if (strcmp(url,"USER_SPECIFIED_PREFIX/font-awesome.min.css")==0) {
                response = MHD_create_response_from_buffer(strlen(font_awesome), font_awesome, MHD_RESPMEM_PERSISTENT);
                ret = MHD_queue_response (connection, MHD_HTTP_OK, response);
                MHD_destroy_response (response);
                return ret;
        }
-       if (strcmp(url,"/main-bundle.min.css")==0) {
+       if (strcmp(url,"USER_SPECIFIED_PREFIX/main-bundle.min.css")==0) {
                response = MHD_create_response_from_buffer(strlen(main_bundle_min_css_c), main_bundle_min_css_c, MHD_RESPMEM_PERSISTENT);
                ret = MHD_queue_response (connection, MHD_HTTP_OK, response);
                MHD_destroy_response (response);
                return ret;
        }
-       if (strcmp(url,"/")==0) {
+       if (strcmp(url,"USER_SPECIFIED_PREFIX/home")==0) {
+               fprintf(stderr, "Loading home\n");
                string *s = generate_header((char *)"ProxySQL Home");
                char *home = generate_home();
                s->append(home);
@@ -843,9 +852,9 @@ string * ProxySQL_HTTP_Server::generate_header(char *s) {
        a->append(s);
        a->append("</title>\n");
        a->append("<link rel=\"stylesheet\" href=\"https://fonts.googleapis.com/css?family=Source+Sans+Pro:300,400,400i,600,900|Fira+Mono\"/>\n");
-       a->append("<link rel=\"stylesheet\" href=\"/main-bundle.min.css\"/>\n");
-       a->append("<link rel=\"stylesheet\" href=\"/font-awesome.min.css\"/>\n");
-       a->append("<script src=\"/Chart.bundle.js\"></script>\n</head>\n<body style=\"background-color: white;\">\n");
+       a->append("<link rel=\"stylesheet\" href=\"main-bundle.min.css\"/>\n");
+       a->append("<link rel=\"stylesheet\" href=\"font-awesome.min.css\"/>\n");
+       a->append("<script src=\"Chart.bundle.js\"></script>\n</head>\n<body style=\"background-color: white;\">\n");
        a->append("<header class=\"header cf \" role=\"banner\">\n<a class=\"brand\" href=\"http://www.proxysql.com\"><span><strong>Proxy</strong>SQL</span>\n</a>");
        return a;
 }
renecannao commented 2 years ago

Assigning this to @jaredev

mboruta1 commented 2 years ago

Hello @jaredev, do you have any comments regarding the proposal I outlined above? Should I go ahead and submit a PR or will you be implementing this or a similar solution?

Thank you for your time!

jaredev commented 2 years ago

@mboruta1 Quick update. I've got the issue branch created "v2.x-3642" and added the new admin variable "web_prefix", which will appear as "admin-web_prefix" in the admin interface. Next is the routes and testing with reverse proxy.

jaredev commented 2 years ago

I've delved more into this issue. A few things to note. The paths in use are all relative paths which are relative to the domain. This was verified by using different domains in the hosts file and still functional. That being said, this isn't a bug in general with using a reverse proxy with the web interface. Rather, it is only an issue if the routing is done as such: <domain>:<port>/<servicename>. Where the requested behavior is that <servicename> be a variable which then has to be prefixed for all links on the site, and either stripped from the routing url, or combined and checked for each route. My consideration here is that, this would be additional overhead for the web server to do, which would make it slower for all users. While only some may desire to route in this way. It seems that a better option is to use subdomains instead of sub-directories. For example, the services in the reverse proxy could be in the form <servicename>.<domain>:<port>, for example proxysql.mydomain.com:6080 rather than mydomain.com:6080/proxysql. This way seems to be inline with best practice AFAIK, which is more compatible with services and sites and doesn't require that each service be provided a custom prefix for its routes. @mboruta1 I did some tentative research and HAProxy can be configured with subdomains. So I'd recommend using a subdomain for each service instead of a sub-directory and that should resolve it.

mboruta1 commented 2 years ago

@jaredev I was not aware that root-relative URLs (/.*) are still considered relative URLs, my apologies. Currently the web interface uses a combination of root-relative and document-relative URLs (e.g. stats?metric=system), my suggestion was to replace the root-relative links with document-relative ones, and add to all document relative links a user-specified prefix that would allow the interface to be proxied via a subdirectory.

What you seem to be suggesting is to allow the reverse proxying of ProxySQL's web interface only via subdomains, not subdirectories, thus requiring no code changes in ProxySQL but forcing end users to allocate a DNS subdomain, make it accessible to all users of the web interface, and either alter their current domain SSL certificate to match the new subdomain or create and maintain a new certificate. This is indeed all possible but, I believe, overkill and probably one of the reasons why services like HAProxy and RabbitMQ support subdirectory-based reverse proxying of their web interfaces (these are monitoring interfaces, not end-user services like mail.google.com). I personally do not want to request new DNS entries and certificates simply to access the web interface of a single component of the OpenStack cloud that I help maintain; the other 4 services we expose are proxied via subdirectories.

There also seems to be a consensus online that both methods (subdomain and subdirectory) have their uses (please see this, this, and this), so I wouldn't say that subdomains are best practice and subdirs shouldn't really be supported.

And in regards to your concern regarding performance, you could pre-compute the content of the webpage and the URLs they are served with (e.g. "USER_SPECIFIED_PREFIX/Chart.bundle.js") only when the user changes said prefix. This way all you will need to do when a URL request comes in is match the URL against the allowed (pre-computed) URLs, and return the (pre-computed) web page containing paths with the desired prefix. In fact, because of the way the interface is currently structured, we wouldn't even need to change a page's HTML on prefix changes because all pages are in the 'root dir', and all links would be relative to said dir. Regardless, performance would be exactly the same with or without a prefix specified, with the pre-computation step being performed once per prefix change.

As mentioned above I would also be happy to submit the changes myself.

I hope I managed to convince you that subdirectory support would improve the product by making it more flexible at virtually no cost in performance.

jaredev commented 2 years ago

@mboruta1 I'm not suggesting only allowing reverse proxy by subdomains, as there may be other options. For example, ports per service, other routing methods through apache, etc. To be honest, I am not sure to what extent users would desire this functionality and would be worth further consideration if more information is acquired. As for your mentioned "pre-computing" idea, this would involve checking for changes to the admin variable, would require additional variables to store these computed strings. There would be some performance cost even in this instance, but maybe acceptable. If you're happy to submit some changes on this yourself, I've already done the work of adding the admin variable "web_prefix" in the branch "v2.x-3642" which you can checkout.

mboruta1 commented 2 years ago

@jaredev There are indeed other ways to reverse proxy the interface, but as far as I can tell they all require DNS, firewall (e.g. open up a new port), or certificate changes. Proxying via subdirectory requires no such changes, and is supported by major services with similar monitoring web interfaces. I agree that it may not be a very popular feature, but its a feature useful for at least one enterprise deployment and I'd be happy to submit a PR based off of your branch.

As for your performance concerns, I'm sorry but I find them rather contrived. Are we really concerned about the performance cost of parsing and comparing an additional admin variable? Something that's done only at configuration time? And if said prefix does change, we simply snprintf() some strings. These are all linear, in-memory operations done iff said prefix changes, which is not expected to happen more than 0-2 times in the lifetime of most deployments. Why would this type of performance 'hit' be "maybe acceptable"?

jaredev commented 2 years ago

@mboruta1 Thanks for the additional information. I'm reopening the issue. I am not concerned, I simply stated that there would be some additional overhead. That is why I said, "maybe acceptable" as it may be an acceptable amount of overhead if done. Also, there may be other developers involved in reviewing code to make such a determination, so it would not necessarily be only my decision on the amount of acceptable overhead, hence the "maybe". Rene messaged me and said he was for the feature as well but it's low priority. So by all means, you may use the branch and do a PR.

mboruta1 commented 2 years ago

Great, happy to hear that Rene is for the feature. Will start work on this in the coming weeks and submit a PR hopefully by the end of this month.

rossifumax commented 6 months ago

Hello @jaredev and @mboruta1 ! :wave:

I know that it's a thread from 2021, but any news on this feature ? It would be very useful to be able to configure the "basePath" of the web UI instead of force "/".

My case is a deployment in k8s with an ingress configuration to have ProxySQL dashboard configured under : https://mydomain.com/_tools/proxysql/.

Thanks in advance !