Always set headers

[benrubson]

Hi,

Headers may already be defined in Apache configuration.
This then leads to header values being defined twice, thus triggering such error message :
The "X-Content-Type-Options" HTTP header is not configured to equal to "nosniff". This is a potential security or privacy risk and we recommend adjusting this setting.

This PR then adds the always keyword to Header definitions, which solves the issue.

Thank you 👍

Ben

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #31231 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #31231   +/-   ##
=========================================
  Coverage     62.57%   62.57%           
  Complexity    18234    18234           
=========================================
  Files          1145     1145           
  Lines         68396    68396           
  Branches       1234     1234           
=========================================
  Hits          42799    42799           
  Misses        25236    25236           
  Partials        361      361

Flag	Coverage Δ	Complexity Δ
#javascript	52.05% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	63.77% <ø> (ø)	18234 <ø> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2543f6...5913a84. Read the comment docs.

[mmattel]

In case this gets merged, I will add this to the nginx config / documentation too.

[mmattel]

Related issues:
#17613 (Security & Setup Warnings for X-Content-Type-Options and X-Frame-Options)
https://forum.owncloud.org/viewtopic.php?t=33325

A good explanation:
https://stackoverflow.com/questions/39502968/apache-difference-between-header-always-set-and-header-set
...
What is the difference between Header always set and Header set in Apache?
As the quoted bit from the manual says, without 'always' your additions will only go out on succesful responses.
...

@butonic @DeepDiver1975 @PVince81
I vote for merging.

[butonic]

@benrubson can you clarify which routes / url show this error?

1. oc will set the headers for every response:

https://github.com/owncloud/core/blob/master/lib/private/legacy/response.php#L259-L264

header() will by default replace an existing header. so every request should be returned with the right headers.

2. the .htaccess rules for apache will only add these headers if they haven't been added yet, eg for images:

https://github.com/owncloud/core/blob/master/.htaccess#L18-L24

As you can see php will also only set the headers if the modHeadersAvailable environment variable hasn't been set.

The code goes a long way not to set these headers twice. Which urls are problematic?

[benrubson]

@butonic, error shows in /settings/admin?sectionid=general

I agree with you, but in addition to be set by OwnCloud itself, such headers could also already be set by the server administrator, in Apache configuration (i.e. in httpd.conf).

[benrubson]

Still facing this with 10.0.10.
Could we then think about merging this please ?
Many thanks again 👍

[mmattel]

@benrubson pls correct me if I am wrong...
This PR is about to fix the issue when not only owncloud sets a header, but the header is additionally set by an apache config (header on header). Can happen when the apache admin mandatory adds headers in http.conf. According the apache documentation headers can be "merged" and the always directive would do so in this case eliminating the apache error message thrown.
It is not that owncloud settig headers does something wrong or misses something, it is about setting headers from oC and apache leads to that error which is fixed by this PR.
More info at: https://httpd.apache.org/docs/current/mod/mod_headers.html#page-header

[benrubson]

Yes @mmattel, U're totally right 👍

[benrubson]

Many thanks @DeepDiver1975 👍

[mmattel]

For completeness, the docs have been updated

• with regards to nginx: nginx improvements docs#487 (nginx improvements).
• I will check if we need/shoud add a note to apache and refrence the PR in case

[mmattel]

@benrubson fyi, the backport for stable10 has been filed but will not make it in the upcoming release due to codefreeze. It will be in the release after.

[phil-davis]

Backport stable10 is PR #34089

[benrubson]

Yes, this PR and #35099 should solve #23960.