Poor rating of trac site on Mozilla Observatory

[Torge Riedel]

Hi,

I've set up a trac via https using latest stable trac (1.2.2).

I've found a nice tool checking site configuration: https://observatory.mozilla.org/

Checking my trac installation I got a poor "D" rating.

Following is the list of tests failed resulting in a negative score:

Test                                   Score     Explanation
Content Security Policy     -25         Content Security Policy (CSP) header not implemented
Contribute.json                 -10         Contribute.json file cannot be parsed
X-Content-Type-Options    -5           X-Content-Type-Options header not implemented
X-Frame-Options               -20         X-Frame-Options (XFO) header not implemented
X-XSS-Protection               -10         X-XSS-Protection header not implemented

Since other sites hosted on my server get better ratings there must be a chance to fix this in the code. Another way is to add such headers to the apache config, but I'm not sure whether I am breaking something in trac and it's less flexible.

Is there a chance to improve the headers trac is sending? Can I help with whatever is helpful?

Regards
Torge

[Ryan Ollos]

Some of all of this may be best addressed through your web server configuration. Are you running Apache?

- Ryan

[Torge Riedel]

--
You received this message because you are subscribed to the Google Groups "Trac Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to trac-dev+u...@googlegroups.com.
To post to this group, send email to trac...@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.

Yes, I am running apache. And I have full access to my server. Others might not have full access to the apache config and are able to add headers or mod_headers is not activated.

That's why I think as much as possible of such headers should be sent by trac.

[RjOllos]

On Thursday, December 14, 2017 at 10:24:10 PM UTC-8, torgeriedel wrote:

Am 14.12.2017 um 21:03 schrieb Ryan Ollos:

On Thu, Dec 14, 2017 at 9:41 AM, Torge Riedel <torge...@gmx.de> wrote:

Hi,

I've set up a trac via https using latest stable trac (1.2.2).

I've found a nice tool checking site configuration: https://observatory.mozilla.org/

Checking my trac installation I got a poor "D" rating.

Following is the list of tests failed resulting in a negative score:

Test                                   Score     Explanation
Content Security Policy     -25         Content Security Policy (CSP) header not implemented
Contribute.json                 -10         Contribute.json file cannot be parsed
X-Content-Type-Options    -5           X-Content-Type-Options header not implemented
X-Frame-Options               -20         X-Frame-Options (XFO) header not implemented
X-XSS-Protection               -10         X-XSS-Protection header not implemented

Since other sites hosted on my server get better ratings there must be a chance to fix this in the code. Another way is to add such headers to the apache config, but I'm not sure whether I am breaking something in trac and it's less flexible.

Is there a chance to improve the headers trac is sending? Can I help with whatever is helpful?

Regards
Torge

Some of all of this may be best addressed through your web server configuration. Are you running Apache?

- Ryan

--
You received this message because you are subscribed to the Google Groups "Trac Development" group.

To unsubscribe from this group and stop receiving emails from it, send an email to trac-dev+unsubscribe@googlegroups.com.

To post to this group, send email to trac...@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.

Yes, I am running apache. And I have full access to my server. Others might not have full access to the apache config and are able to add headers or mod_headers is not activated.

That's why I think as much as possible of such headers should be sent by trac.

Any such headers need to be configurable, but we want to avoid configuration option bloat. What we might be able to do is add an [http-headers] configuration section to trac.ini. We could specify some common configurations to the documentation.

Example configuration:

[http-headers]
X-Frame-Options = DENY

X-XSS-Protection = 1; mode=block

The option names as read by ConfigParser are case-insensitive, but I think that may be okay as it looks like the HTTP headers are also case-insensitive.

I've done a PoC patch against 1.2-stable, but I'll want to hear what Jun has to say before suggesting this is the right solution, since he has much more experience with web server internals and configuration.

diff --git a/trac/web/api.py b/trac/web/api.py
index b2e76f948..521cd59ab 100644
--- a/trac/web/api.py
+++ b/trac/web/api.py
@@ -686,6 +686,8 @@ class Request(object):
         self.send_header('Content-Type', content_type + ';charset=utf-8')
         if isinstance(content, basestring):
             self.send_header('Content-Length', len(content))
+        for name, val in getattr(self, 'configurable_headers', []):
+            self.send_header(name, val)
         self.end_headers()

         if self.method != 'HEAD':
diff --git a/trac/web/main.py b/trac/web/main.py
index 56b493d38..1a54dce82 100644
--- a/trac/web/main.py
+++ b/trac/web/main.py
@@ -38,8 +38,9 @@ from genshi.output import DocType
 from genshi.template import TemplateLoader

 from trac import __version__ as TRAC_VERSION
-from trac.config import BoolOption, ChoiceOption, ConfigurationError, \
-                        ExtensionOption, Option, OrderedExtensionsOption
+from trac.config import (
+    BoolOption, ChoiceOption, ConfigSection, ConfigurationError,
+    ExtensionOption, Option, OrderedExtensionsOption)
 from trac.core import *
 from trac.env import open_environment
 from trac.loader import get_plugin_info, match_plugins_to_frames
@@ -164,6 +165,10 @@ class RequestDispatcher(Component):
         """The header to use if `use_xsendfile` is enabled. If Nginx is used,
         set `X-Accel-Redirect`. (''since 1.0.6'')""")

+    configurable_headers = ConfigSection('http-headers', """
+        Headers to be added to the HTTP request.
+        """)
+
     # Public API

     def authenticate(self, req):
@@ -317,6 +322,7 @@ class RequestDispatcher(Component):
             'tz': self._get_timezone,
             'use_xsendfile': self._get_use_xsendfile,
             'xsendfile_header': self._get_xsendfile_header,
+            'configurable_headers': self._get_configurable_headers,
         })

     @lazy
@@ -426,6 +432,10 @@ class RequestDispatcher(Component):
                               header)
             return None

+    def _get_configurable_headers(self, req):
+        for name, val in self.configurable_headers.options():
+            yield name, val
+
     def _pre_process_request(self, req, chosen_handler):
         for filter_ in self.filters:
             chosen_handler = filter_.pre_process_request(req, chosen_handler)
(pve) ~/Documents/Workspace/trac-dev/teo-rjollos.git$clear
(pve) ~/Documents/Workspace/trac-dev/teo-rjollos.git$git diff
diff --git a/trac/web/api.py b/trac/web/api.py
index b2e76f948..521cd59ab 100644
--- a/trac/web/api.py
+++ b/trac/web/api.py
@@ -686,6 +686,8 @@ class Request(object):
         self.send_header('Content-Type', content_type + ';charset=utf-8')
         if isinstance(content, basestring):
             self.send_header('Content-Length', len(content))
+        for name, val in getattr(self, 'configurable_headers', []):
+            self.send_header(name, val)
         self.end_headers()

         if self.method != 'HEAD':
diff --git a/trac/web/main.py b/trac/web/main.py
index 56b493d38..8f66906e3 100644
--- a/trac/web/main.py
+++ b/trac/web/main.py
@@ -38,8 +38,9 @@ from genshi.output import DocType
 from genshi.template import TemplateLoader

 from trac import __version__ as TRAC_VERSION
-from trac.config import BoolOption, ChoiceOption, ConfigurationError, \
-                        ExtensionOption, Option, OrderedExtensionsOption
+from trac.config import (
+    BoolOption, ChoiceOption, ConfigSection, ConfigurationError,
+    ExtensionOption, Option, OrderedExtensionsOption)
 from trac.core import *
 from trac.env import open_environment
 from trac.loader import get_plugin_info, match_plugins_to_frames
@@ -164,6 +165,10 @@ class RequestDispatcher(Component):
         """The header to use if `use_xsendfile` is enabled. If Nginx is used,
         set `X-Accel-Redirect`. (''since 1.0.6'')""")

+    configurable_headers = ConfigSection('http-headers', """
+        Headers to be added to the HTTP request. (''since 1.2.3'')
+        """)
+
     # Public API

     def authenticate(self, req):
@@ -317,6 +322,7 @@ class RequestDispatcher(Component):
             'tz': self._get_timezone,
             'use_xsendfile': self._get_use_xsendfile,
             'xsendfile_header': self._get_xsendfile_header,
+            'configurable_headers': self._get_configurable_headers,
         })

     @lazy
@@ -426,6 +432,10 @@ class RequestDispatcher(Component):
                               header)
             return None

+    def _get_configurable_headers(self, req):
+        for name, val in self.configurable_headers.options():
+            yield name, val
+
     def _pre_process_request(self, req, chosen_handler):
         for filter_ in self.filters:
             chosen_handler = filter_.pre_process_request(req, chosen_handler)

- Ryan

[RjOllos]

Attaching same patch as a file.

- Ryan

[Jun Omae]

Hi,

<rjo...@gmail.com> wrote on 2017-Dec-15 at 06:02 PM:
> Any such headers need to be configurable, but we want to avoid configuration option bloat. What we might be able to do is add an [http-headers] configuration section to trac.ini. We could specify some common configurations to the documentation.
>
> Example configuration:
>
> [http-headers]
> X-Frame-Options = DENY
> X-XSS-Protection = 1; mode=block
>
> The option names as read by ConfigParser are case-insensitive, but I think that may be okay as it looks like the HTTP headers are also case-insensitive.
>
> I've done a PoC patch against 1.2-stable, but I'll want to hear what Jun has to say before suggesting this is the right solution, since he has much more experience with web server internals and configuration.

Good feature.

My suggestions:

1. Whether http header name is valid like [trac] xsendfile_header option.
2. Whether http header value is valid (the value cannot contain control characters except TAB and SPACE).
3. Ignore some headers, e.g. Content-Type, Content-Length, Location, ETag, Pragma, Cache-Control, Expires
4. I think we should send configured headers for all send_* methods included send_error().

See attached patch.

I thought it might be good to allow to overwrite headers like "set" in mod_headers module but it would not be needed in use-case of Trac.

--
Jun Omae <jun...@gmail.com> (大前 潤)

[Torge Riedel]

Hi,

I created a temporary dev env of Trac 1.2.2 with the patch of Jun applied. I have configured the following headers in trac.ini:

[http-headers]
Content-Security-Policy = frame-ancestors 'none'; default-src 'none'; img-src 'self'; script-src 'self'; style-src 'self'; base-uri 'self'
Referrer-Policy = no-referrer
Strict-Transport-Security = max-age=31536000; includeSubDomains
X-Frame-Options = DENY
X-Content-Type-Options = nosniff
X-XSS-Protection = 1; mode=block

with the headers defined as above trac should get a better rating or something near to it.

Well currently the trac test env I have set up is getting an F rating on observatory.mozilla.org, since https was not in use, a redirect to https is missing and HSTS was set without https.

And there is another import point which needs to be adjusted:

Cookies         -40     Session cookie set without using the Secure flag or set over http

If used on https the score is "just" -10, but I recommend to add the secure flag to the cookie trac is setting.

Is there a chance to get this in a Trac 1.2.3? I recommend setting the headers above in a default trac.ini created by trac-admin initenv.

Regards
Torge

[RjOllos]

https://trac.edgewall.org/wiki/TracIni#trac-secure_cookies-option

 

Is there a chance to get this in a Trac 1.2.3? I recommend setting the headers above in a default trac.ini created by trac-admin initenv.

Yes, 

https://trac.edgewall.org/ticket/12964

- Ryan

[Jun Omae]

On 12/19/2017 1:32 AM, Torge Riedel wrote:
> I created a temporary dev env of Trac 1.2.2 with the patch of Jun applied. I have configured the following headers in trac.ini:
>
> [http-headers]

> ...

>
> Is there a chance to get this in a Trac 1.2.3? I recommend setting the headers above in a default trac.ini created by trac-admin initenv.

> Content-Security-Policy = frame-ancestors 'none'; default-src 'none'; img-src 'self'; script-src 'self'; style-src 'self'; base-uri 'self'

frame-ancestors:
Should be 'self'. The same reason for X-Frame-Options.


> Referrer-Policy = no-referrer

Should be same-origin by default. Trac core and several plugins use Referer header.


> Strict-Transport-Security = max-age=31536000; includeSubDomains

Shouldn't use by default. All Trac sites don't run on HTTPS.
Also, includeSubDomains should be used only when subdomain(s) are used.
It it hard to reset the "includeSubDomains" behavior on user's browser when configuration is wrong.


> X-Frame-Options = DENY

Should be SAMEORIGIN by default. Trac core and several plugins create iframe elements via javascript.

> X-Content-Type-Options = nosniff
> X-XSS-Protection = 1; mode=block

No problem by default.

[Torge Riedel]

--
You received this message because you are subscribed to the Google Groups "Trac Development" group.

To unsubscribe from this group and stop receiving emails from it, send an email to trac-dev+u...@googlegroups.com.

To post to this group, send email to trac...@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.

Hi,

regarding secure cookies: Sorry, I missed that. Changed in my installation and the rating got better.

I will wait for 1.2.3 and will give feedback after deploy and changing configurable headers.

Thanks for your efforts
Torge

[Torge Riedel]

Hi Jun,

thanks for your feedback. I will take this into account when deploying 1.2.3 and configuring the headers. I will give feedback here.

Regards
Torge

[RjOllos]

I'm sorry for the long delay in getting these changes integrated. I think we'll get the patch in #12964 committed soon and I hope to release 1.2.3 by the end of April.

- Ryan