feat: httpproxy: add support for more HTTP methods (#601)

By default this stays as before (GET and POST),
but more methods can be allowed through the
HTTP Proxy configuration.

bin/proxy/osh-http-proxy-daemon

@@ -94,6 +94,7 @@ OVH::Bastion::ProxyHTTP->new()->run(
         log_request_response          => $config->{'log_request_response'} ? 1 : 0,
         log_request_response_max_size => $config->{'log_request_response_max_size'},
         allowed_egress_protocols      => $config->{'allowed_egress_protocols'} || ['https'],
+        allowed_methods               => $config->{'allowed_methods'}          || ['GET', 'POST'],
     },
 ) or die "Proxy launch failed!";
 

bin/proxy/osh-http-proxy-worker

@@ -162,6 +162,15 @@ if (not grep { $egress_protocol eq $_ } @{$config->{'allowed_egress_protocols'}}
     );
 }
 
+if (not grep { $method eq $_ } @{$config->{'allowed_methods'} || []}) {
+    log_and_exit(
+        400,
+        "Bad Request (method forbidden)",
+        "The $method method is forbidden by policy",
+        {comment => 'method_forbidden'}
+    );
+}
+
 my $shortGroup;
 if ($group) {
     $fnret = OVH::Bastion::is_valid_group_and_existing(group => $group, groupType => 'key');

doc/sphinx/administration/configuration/osh-http-proxy_conf.rst

@@ -29,6 +29,7 @@ These options modify the behavior of the HTTP Proxy, an optional module of The B
 - `timeout`_
 - `log_request_response`_
 - `log_request_response_max_size`_
+- `allowed_methods`_
 - `allowed_egress_protocols`_
 
 Option Reference
@@ -177,6 +178,18 @@ to the specified size. This is a way to avoid turning off request response loggi
 very busy bastions, by ensuring logs growth don't get out of hand, as some responses to queries can
 take megabytes, with possibly limited added value to traceability.
 
+allowed_methods
+***************
+
+:Type: ``array of strings``
+
+:Default: ``["GET","POST"]``
+
+:Example: ``["GET","POST","PUT","PATCH","DELETE"]``
+
+This array lists the allowed HTTP methods. By default, only GET and POST are allowed.
+If required, other methods can be added such as PUT, PATCH, DELETE, etc.
+
 allowed_egress_protocols
 ************************
 

etc/bastion/osh-http-proxy.conf.dist

@@ -104,6 +104,13 @@
 #  DEFAULT: 65536
 "log_request_response_max_size": 65536,
 #
+# allowed_methods (array of strings)
+#     DESC: This array lists the allowed HTTP methods. By default, only GET and POST are allowed.
+#           If required, other methods can be added such as PUT, PATCH, DELETE, etc.
+#  EXAMPLE: ["GET","POST","PUT","PATCH","DELETE"]
+#  DEFAULT: ["GET","POST"]
+"allowed_methods": ["GET","POST"],
+#
 # allowed_egress_protocols (array of strings)
 #     DESC: This array lists the allowed egress protocols. By default, only https is allowed.
 #           If required, plain http egress can be enabled here, which is of course strongly discouraged,

lib/perl/OVH/Bastion/ProxyHTTP.pm

@@ -281,12 +281,15 @@ sub process_http_request {
         );
     }
 
-    # only GET or POST are allowed
+    # default set by the daemon and adjusted by the config, just ensure it's not undef
-    if (not grep { uc($self->{'request_info'}{'request_method'}) eq $_ } qw{ GET POST }) {
+    $self->{'proxy_config'}{'allowed_methods'} ||= [];
+    # only some methods are allowed
+    if (not grep { uc($self->{'request_info'}{'request_method'}) eq $_ } @{$self->{'proxy_config'}{'allowed_methods'}})
+    {
         return $self->log_and_exit(
             400,
             "Bad Request (method forbidden)",
-            "Only GET and POST methods are allowed",
+            "The " . uc($self->{'request_info'}{'request_method'}) . " method is forbidden by policy",
             {comment => 'method_forbidden'}
         );
     }
@@ -583,10 +586,16 @@ sub process_http_request {
     # fake an application/xml content, this has the effect in the CGI module code
     # to not mess at all with the data, which is what we want. This way we can get the
     # raw unparsed/unmodified data through the special 'XForms:Model' param. Once done,
-    # we simply restore the real content-type
+    # we simply restore the real content-type.
+    # For PUT and PATCH, this is easier,
+    # cf https://metacpan.org/dist/CGI/view/lib/CGI.pod#Handling-non-urlencoded-arguments
+    my $content;
     my $real_content_type = $ENV{'CONTENT_TYPE'};
     $ENV{'CONTENT_TYPE'} = 'application/xml';
-    my $content = CGI->new->param('XForms:Model');
+    my %verb2param = (POST => 'XForms:Model', PUT => 'PUTDATA', PATCH => 'PATCHDATA');
+    if (my $param = $verb2param{$self->{'request_info'}{'request_method'}}) {
+        $content = CGI->new->param($param);
+    }
     $ENV{'CONTENT_TYPE'}    = $real_content_type;
     $ENV{'PROXY_POST_DATA'} = encode_base64($content);
 

tests/functional/docker/target_role.sh

@@ -162,6 +162,8 @@ if [ "$WANT_HTTP_PROXY" = 1 ]; then
     sed -i -re 's="insecure":.+="insecure":true,=' /etc/bastion/osh-http-proxy.conf
     # also build a config with disallowed https egress and allowed http egress
     sed -re 's="allowed_egress_protocols":.+="allowed_egress_protocols":["http"]=' /etc/bastion/osh-http-proxy.conf > /etc/bastion/osh-http-proxy-httponly.conf
+    # and another one with more allowed methods
+    sed -re 's="allowed_methods":.+="allowed_methods":["GET","POST","PUT","PATCH","DELETE"],=' /etc/bastion/osh-http-proxy.conf > /etc/bastion/osh-http-proxy-methods.conf
 
     # ensure the remote daemon is executable
     chmod 0755 "$basedir"/tests/functional/proxy/remote-daemon

tests/functional/launch_tests_on_instance.sh

@@ -478,7 +478,14 @@ run()
         [ "$case" = "sshd_reload" ] && sleep 1
         flock "$outdir/$basename.retval" $screen "$outdir/$basename.cc" -D -m -fn -ln $r0 '
                 /opt/bastion/bin/admin/check-consistency.pl ; echo _RETVAL_CC=$?= ;
-                grep -Fw -e warn -e die -e code-warning /var/log/bastion/bastion.log | grep -Fv -e "'"${code_warn_exclude:-__none__}"'" -e "System does not support IPv6" | sed "s/^/_SYSLOG=/" ;
+                grep -Fw -e warn -e die -e code-warning /var/log/bastion/bastion.log
+                | grep -Fv
+                  -e "'"${code_warn_exclude:-__none__}"'"
+                  -e "System does not support IPv6"
+                  -e "Defaulting to E[UG]ID"
+                  -e "starting!"
+                  -e "Binding to SSL port"
+                | sed "s/^/_SYSLOG=/" ;
                 : > /var/log/bastion/bastion.log
             '
         flock "$outdir/$basename.retval" true

tests/functional/proxy/remote-daemon

@@ -19,10 +19,15 @@ sub process_http_request {
     my $hasContentType;
     my $wantedResponseSize = 64;
 
+    # this is to get the unparsed body of the request, portion of code
+    # taken from lib/perl/OVH/Bastion/ProxyHTTP.pm, more context available over there
+    my $content;
     my $real_content_type = $ENV{'CONTENT_TYPE'};
     $ENV{'CONTENT_TYPE'} = 'application/xml';
-    my $content = CGI->new->param('XForms:Model');
+    my %verb2param = (POST=>'XForms:Model', PUT=>'PUTDATA', PATCH=>'PATCHDATA');
-    $ENV{'CONTENT_TYPE'} = $real_content_type;
+    if (my $param = $verb2param{$self->{'request_info'}{'request_method'}}) {
+       $content =  CGI->new->param($param);
+    }
 
     foreach my $headerTuple (@{ $self->{'request_info'}{'request_headers'} }) {
         if ($headerTuple->[0] =~ /^x-test-add-response-header-(.+)/i) {
@@ -35,11 +40,13 @@ sub process_http_request {
     }
     print "Content-type: text/plain\n" if !$hasContentType;
 
+    # if the request had a body, include it verbatim in our answer
     if ($content) {
         print "Content-Length: ".length($content)."\n\n";
         print $content;
     }
     else {
+        # otherwise, generate some data in our answer
         print "Content-Length: ".$wantedResponseSize."\n\n";
         my @chars = ('0'..'9', 'a'..'z', 'A'..'Z', "\n");
 

tests/functional/tests.d/500-http-proxy.sh

@@ -195,8 +195,8 @@ testsuite_proxy()
     contain "X-Bastion-Egress-Timing: "
     contain "Content-Length: 1000000"
 
-    # use a disallowed verb
+    # use a disallowed method
-    script forbidden_verb "curl -ski -X OPTIONS -u '$account0@test@127.0.0.1%9443:$proxy_password' https://$remote_ip:$remote_proxy_port/test | cat; exit \${PIPESTATUS[0]}"
+    script forbidden_method "curl -ski -X PUT -u '$account0@test@127.0.0.1%9443:$proxy_password' https://$remote_ip:$remote_proxy_port/test | cat; exit \${PIPESTATUS[0]}"
     retvalshouldbe 0
     contain 'HTTP/1.0 400 Bad Request (method forbidden)'
     contain 'Server: The Bastion'
@@ -204,10 +204,22 @@ testsuite_proxy()
     contain 'X-Bastion-ReqID: '
     nocontain 'WWW-Authenticate: '
     contain 'Content-Type: text/plain'
-    contain 'Only GET and POST methods are allowed'
+    contain 'is forbidden by policy'
+
+    # use alternate config to only allow more methods
+    success config_swap $r0 "\"cp /etc/bastion/osh-http-proxy-methods.conf /etc/bastion/osh-http-proxy.conf\""
+
+    # when daemon will restart, it'll log stuff, ignore it
+    ignorecodewarn 'osh-http-proxy-daemon'
+    # pkill doesn't work well under FreeBSD, so do it ourselves for all OSes
+    success force_restart $r0 "\"ps -U proxyhttp -o pid,command | grep -v PID | awk '{print \\\$1}' | xargs -r kill; true\""
+    if [ "$COUNTONLY" != 1 ]; then
+        # wait for target_role.sh to restart the daemon
+        sleep 4
+    fi
 
     # post some data
-    script post_data "curl -ski -d somedata -u '$account0@test@127.0.0.1%9443:$proxy_password' https://$remote_ip:$remote_proxy_port/test | cat; exit \${PIPESTATUS[0]}"
+    script post_data "curl -ski -X POST -d somedata -u '$account0@test@127.0.0.1%9443:$proxy_password' https://$remote_ip:$remote_proxy_port/test | cat; exit \${PIPESTATUS[0]}"
     retvalshouldbe 0
     contain "HTTP/1.0 200 OK"
     testsuite_proxy_check_headers
@@ -226,6 +238,45 @@ testsuite_proxy()
     contain "Content-Length: 8"
     contain "somedata"
 
+    # put some data
+    script patch_data "curl -ski -X PUT -d somedata -u '$account0@test@127.0.0.1%9443:$proxy_password' https://$remote_ip:$remote_proxy_port/test | cat; exit \${PIPESTATUS[0]}"
+    retvalshouldbe 0
+    contain "HTTP/1.0 200 OK"
+    testsuite_proxy_check_headers
+    nocontain 'WWW-Authenticate: '
+    contain 'Content-Type: text/plain'
+    contain 'X-Bastion-Remote-IP: 127.0.0.1'
+    contain 'X-Bastion-Request-Length: 8'
+    contain 'X-Bastion-Auth-Mode: self/default'
+    contain 'X-Bastion-Local-Status: 200 OK'
+    contain "X-Bastion-Remote-Client-SSL-Cert-Subject: "
+    contain "X-Bastion-Remote-Client-SSL-Cipher: "
+    contain "X-Bastion-Remote-Client-SSL-Warning: Peer certificate not verified"
+    contain "X-Bastion-Remote-Status: 200"
+    contain "X-Bastion-Remote-Server: Net::Server::HTTP/"
+    contain "X-Bastion-Egress-Timing: "
+    contain "Content-Length: 8"
+    contain "somedata"
+
+    # put some data with no body
+    script patch_data_no_body "curl -ski -X PUT -u '$account0@test@127.0.0.1%9443:$proxy_password' https://$remote_ip:$remote_proxy_port/test | cat; exit \${PIPESTATUS[0]}"
+    retvalshouldbe 0
+    contain "HTTP/1.0 200 OK"
+    testsuite_proxy_check_headers
+    nocontain 'WWW-Authenticate: '
+    contain 'Content-Type: text/plain'
+    contain 'X-Bastion-Remote-IP: 127.0.0.1'
+    contain 'X-Bastion-Request-Length: 0'
+    contain 'X-Bastion-Auth-Mode: self/default'
+    contain 'X-Bastion-Local-Status: 200 OK'
+    contain "X-Bastion-Remote-Client-SSL-Cert-Subject: "
+    contain "X-Bastion-Remote-Client-SSL-Cipher: "
+    contain "X-Bastion-Remote-Client-SSL-Warning: Peer certificate not verified"
+    contain "X-Bastion-Remote-Status: 200"
+    contain "X-Bastion-Remote-Server: Net::Server::HTTP/"
+    contain "X-Bastion-Egress-Timing: "
+    contain "Content-Length: 64"
+
     # use a disallowed egress method
     script forbidden_egress_protocol "curl -ski -H 'X-Bastion-Egress-Protocol: http' -u '$account0@test@127.0.0.1%9443:$proxy_password' https://$remote_ip:$remote_proxy_port/test | cat; exit \${PIPESTATUS[0]}"
     retvalshouldbe 0
@@ -236,7 +287,7 @@ testsuite_proxy()
     contain 'not allowed by policy'
 
     # use alternate config to only allow http egress
-    success config_swap $r0 "\"mv /etc/bastion/osh-http-proxy.conf /etc/bastion/osh-http-proxy-normal.conf; mv /etc/bastion/osh-http-proxy-httponly.conf /etc/bastion/osh-http-proxy.conf\""
+    success config_swap $r0 "\"cp /etc/bastion/osh-http-proxy-httponly.conf /etc/bastion/osh-http-proxy.conf\""
 
     # when daemon will restart, it'll log stuff, ignore it
     ignorecodewarn 'osh-http-proxy-daemon'