Do not unset Remote-User on the frontend

  The Remote User is managed on the backend apache which will unset it anyway.

component/apache/buildout.hash.cfg

@@ -14,5 +14,5 @@
 # not need these here).
 [template-apache-backend-conf]
 filename = apache-backend.conf.in
-md5sum = 84d43d3535ffc67f677710b1d97e19aa
+md5sum = bb8c175a93336f0e1838fd47225426f9
 

software/apache-frontend/buildout.hash.cfg

@@ -18,7 +18,7 @@ md5sum = f686f765e55d1dce2e55a400f0714b3e
 
 [template-apache-frontend]
 filename = instance-apache-frontend.cfg
-md5sum = a6b566a29f1b5021d0f1f3c4fa20d749
+md5sum = d6398d727eb1e1bc3df1768a9b9a7e0c
 
 [template-apache-replicate]
 filename = instance-apache-replicate.cfg.in
@@ -38,7 +38,7 @@ md5sum = 665e83d660c9b779249b2179d7ce4b4e
 
 [template-apache-frontend-configuration]
 filename = templates/apache.conf.in
-md5sum = 05239181f4d5d0e3fe6bccda587fa9a5
+md5sum = b666d7c4a5c1fd8020713aa53b44a386
 
 [template-custom-slave-list]
 filename = templates/apache-custom-slave-list.cfg.in

software/apache-frontend/templates/apache.conf.in

@@ -20,9 +20,6 @@ TypesConfig {{ httpd_home }}/conf/mime.types
 AddType application/x-compress .Z
 AddType application/x-gzip .gz .tgz
 
-# As backend is trusting Remote-User header unset it always
-RequestHeader unset Remote-User
-
 ServerTokens Prod
 
 # Disable TRACE Method

software/caddy-frontend/buildout.hash.cfg

@@ -54,11 +54,11 @@ md5sum = f20d6c3d2d94fb685f8d26dfca1e822b
 
 [template-default-slave-virtualhost]
 filename = templates/default-virtualhost.conf.in
-md5sum = 2694992850b565edebf11dae62f032c7
+md5sum = 8198e3ad06a4d6c750d22d8cf854fa41
 
 [template-cached-slave-virtualhost]
 filename = templates/cached-virtualhost.conf.in
-md5sum = 434ff5db37b6b980713b03a37eed928a
+md5sum = db68c015f1ac06d74f9373f6f846577d
 
 [template-log-access]
 filename = templates/template-log-access.conf.in

software/caddy-frontend/templates/cached-virtualhost.conf.in

@@ -21,8 +21,6 @@
   proxy / {{ slave_parameter.get('backend_url', '') }} {
     try_duration {{ slave_parameter['proxy_try_duration'] }}s
     try_interval {{ slave_parameter['proxy_try_interval'] }}ms
-    # As backend is trusting Remote-User header unset it always
-    header_upstream -Remote-User
 
     transparent
     timeout 600s
@@ -49,8 +47,6 @@
   proxy / {{ slave_parameter.get('https_backend_url', '') }} {
     try_duration {{ slave_parameter['proxy_try_duration'] }}s
     try_interval {{ slave_parameter['proxy_try_interval'] }}ms
-    # As backend is trusting Remote-User header unset it always
-    header_upstream -Remote-User
     transparent
     timeout 600s
 {%- if ssl_proxy_verify %}

software/caddy-frontend/templates/default-virtualhost.conf.in

@@ -108,8 +108,6 @@
     without /prefer-gzip
     header_upstream Accept-Encoding gzip
 {%-     endif %} {#-     if proxy_name == 'prefer-gzip' #}
-    # As backend is trusting Remote-User header unset it always
-    header_upstream -Remote-User
 {%- for disabled_cookie in disabled_cookie_list %}
     # Remove cookie {{ disabled_cookie }} from client Cookies
     header_upstream Cookie "(.*)(^{{ disabled_cookie }}=[^;]*; |; {{ disabled_cookie }}=[^;]*|^{{ disabled_cookie }}=[^;]*$)(.*)" "$1 $3"
@@ -245,8 +243,6 @@
     without /prefer-gzip
     header_upstream Accept-Encoding gzip
 {%-     endif %} {#-     if proxy_name == 'prefer-gzip' #}
-    # As backend is trusting Remote-User header unset it always
-    header_upstream -Remote-User
 {%- for disabled_cookie in disabled_cookie_list %}
     # Remove cookie {{ disabled_cookie }} from client Cookies
     header_upstream Cookie "(.*)(^{{ disabled_cookie }}=[^;]*; |; {{ disabled_cookie }}=[^;]*|^{{ disabled_cookie }}=[^;]*$)(.*)" "$1 $3"

software/caddy-frontend/test/test.py

@@ -883,7 +883,6 @@ class SlaveHttpFrontendTestCase(HttpFrontendTestCase):
                       headers=None, cookies=None, source_ip=None):
     if headers is None:
       headers = {}
-    headers.setdefault('Remote-User', 'SOME_REMOTE_USER')
     # workaround request problem of setting Accept-Encoding
     # https://github.com/requests/requests/issues/2234
     headers.setdefault('Accept-Encoding', 'dummy')
@@ -908,7 +907,6 @@ class SlaveHttpFrontendTestCase(HttpFrontendTestCase):
                      headers=None):
     if headers is None:
       headers = {}
-    headers.setdefault('Remote-User', 'SOME_REMOTE_USER')
     # workaround request problem of setting Accept-Encoding
     # https://github.com/requests/requests/issues/2234
     headers.setdefault('Accept-Encoding', 'dummy')
@@ -1475,7 +1473,7 @@ http://apachecustomhttpsaccepted.example.com:%%(http_port)s {
         self.instance_path, '*', 'var', 'log', 'httpd', '_empty_access_log'
       ))[0]
 
-    log_regexp = r'^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3} - SOME_REMOTE_USER ' \
+    log_regexp = r'^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3} - - ' \
                  r'\[\d{2}\/.{3}\/\d{4}\:\d{2}\:\d{2}\:\d{2} \+\d{4}\] ' \
                  r'"GET \/test-path HTTP\/1.1" 404 \d+ "-" '\
                  r'"python-requests.*" \d+'

software/slapos-master/buildout.hash.cfg

@@ -22,4 +22,4 @@ md5sum = e8033d4fd7b6348b525a6148762ccdb4
 
 [template-apache-backend-conf]
 filename = apache-backend.conf.in
-md5sum = aff99c44ccf16eaa2ca25430d76d3bd6
+md5sum = 48f086ce1acffca7bab942b43d856fb7

[Vincent Pelletier]

Are you sure ?

Ony the frontend sees client certificate, so if the remote-user is set using values from the certificate it needs to be unset for the case where client does not provide such certificate but sets this header.

/cc @luke

[Rafael Monnerat]

This feature, where it is used (single place), it is only implemented via direct access to the apache backend. ERP5 backend already unset (and set) Remote-User based on SSL Certificate Information, so this unset on this commit is just userless noise for now. (Apache backend keeps unseting Remote-User as usual).

However, once we can authenticate via frontend (relying on caucase and whatever else required), the unset might be reintroduced along with all the code to implement the feature.

[Vincent Pelletier]

So you are knowingly removing something which is not used yet but which is here to prepare future extension that we want to eventually reach. Why ? Is it causing issues ?

Forgetting to later reintroduce this would open a critial authentication bypass vulnerability and "normal" usage will appear working just fine (so this issue, as many security vulnerabilities, will never be discovered on its own). I'm not fond of "let's re-add it later" approach for security.

[Rafael Monnerat]

It is not really possible to forget this one, specially because "Remote-User" is an arbitrary value, it is not writen in ERP5 code base, so I'm not even sure the name we end up using will be really "Remote-User". In any case, it will be required to port code from apache backend to frontend, which implies we are going to port the unset and set in the same way.

[Vincent Pelletier]

It is not really possible to forget this one

I still completely disagree, but whatever.

"Remote-User" is an arbitrary value [...] I'm not even sure the name we end up using will be really "Remote-User"

Fair enough, this means this will need to be specified by frontend (and documented in frontend doc). Frontends will anyway need to provide more than one value from seen client certificate (out the top of my head: serial, subject, some CA identifier, and ideally any arbitrary certificate extension...). Then it needs to implement this kind of protection for all headers it will reserve, and only once all that is done the backends may use these values.

So I agree with removing this cleanup now for this reason.