From 98336fdafe639294dfe083713b4c8b6d88dacf13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= Date: Tue, 16 Dec 2025 14:02:45 +0100 Subject: [PATCH] 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 | 3 +- bin/proxy/osh-http-proxy-worker | 9 +++ .../configuration/osh-http-proxy_conf.rst | 13 ++++ etc/bastion/osh-http-proxy.conf.dist | 7 +++ lib/perl/OVH/Bastion/ProxyHTTP.pm | 19 ++++-- tests/functional/docker/target_role.sh | 2 + tests/functional/launch_tests_on_instance.sh | 9 ++- tests/functional/proxy/remote-daemon | 11 +++- tests/functional/tests.d/500-http-proxy.sh | 61 +++++++++++++++++-- 9 files changed, 120 insertions(+), 14 deletions(-) diff --git a/bin/proxy/osh-http-proxy-daemon b/bin/proxy/osh-http-proxy-daemon index 181fe7f..f1ca06d 100755 --- a/bin/proxy/osh-http-proxy-daemon +++ b/bin/proxy/osh-http-proxy-daemon @@ -90,10 +90,11 @@ OVH::Bastion::ProxyHTTP->new()->run( timeout_idle => 3600, proxy_config => { insecure => $config->{'insecure'} ? 1 : 0, - timeout => $config->{'timeout'}, # our worker will wait for up to this amount of time for the egress connection to complete + timeout => $config->{'timeout'}, # our worker will wait for up to this amount of time for the egress connection to complete 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!"; diff --git a/bin/proxy/osh-http-proxy-worker b/bin/proxy/osh-http-proxy-worker index f66d4d2..7942348 100755 --- a/bin/proxy/osh-http-proxy-worker +++ b/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'); diff --git a/doc/sphinx/administration/configuration/osh-http-proxy_conf.rst b/doc/sphinx/administration/configuration/osh-http-proxy_conf.rst index 7b3536a..c4056bd 100644 --- a/doc/sphinx/administration/configuration/osh-http-proxy_conf.rst +++ b/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 ************************ diff --git a/etc/bastion/osh-http-proxy.conf.dist b/etc/bastion/osh-http-proxy.conf.dist index 7f9ce52..05582d6 100644 --- a/etc/bastion/osh-http-proxy.conf.dist +++ b/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, diff --git a/lib/perl/OVH/Bastion/ProxyHTTP.pm b/lib/perl/OVH/Bastion/ProxyHTTP.pm index 9d08a0f..2feb1d9 100644 --- a/lib/perl/OVH/Bastion/ProxyHTTP.pm +++ b/lib/perl/OVH/Bastion/ProxyHTTP.pm @@ -281,12 +281,15 @@ sub process_http_request { ); } - # only GET or POST are allowed - if (not grep { uc($self->{'request_info'}{'request_method'}) eq $_ } qw{ GET POST }) { + # default set by the daemon and adjusted by the config, just ensure it's not undef + $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); diff --git a/tests/functional/docker/target_role.sh b/tests/functional/docker/target_role.sh index 5daa53c..ce41ac1 100755 --- a/tests/functional/docker/target_role.sh +++ b/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 diff --git a/tests/functional/launch_tests_on_instance.sh b/tests/functional/launch_tests_on_instance.sh index c1c30f5..9ca3b71 100755 --- a/tests/functional/launch_tests_on_instance.sh +++ b/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 diff --git a/tests/functional/proxy/remote-daemon b/tests/functional/proxy/remote-daemon index 92b8fd2..dbee1b3 100755 --- a/tests/functional/proxy/remote-daemon +++ b/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'); - $ENV{'CONTENT_TYPE'} = $real_content_type; + my %verb2param = (POST=>'XForms:Model', PUT=>'PUTDATA', PATCH=>'PATCHDATA'); + 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"); diff --git a/tests/functional/tests.d/500-http-proxy.sh b/tests/functional/tests.d/500-http-proxy.sh index a922f8b..9ecda9a 100644 --- a/tests/functional/tests.d/500-http-proxy.sh +++ b/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 - 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]}" + # use a disallowed method + 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'