-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add X-Frame-Options header to .htaccess #16179
Conversation
There is a setupcheck that tests |
#4605 (comment) |
I see the point of the comment but how should an app overwrite the x-frame-options with the current code? cc @georgehrke because some calendar sharing/embedding. |
I think meanwhile this is done by the |
Docs are already merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me. Altough my apache fu is rather limited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, also seems to work fine to still set different headers on the Nextcloud side. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, also seems to work fine to still set different headers on the Nextcloud side. 👍
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
e042c89
to
1b074f4
Compare
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 |
+ DietPi-Software | Nextcloud: Add X-Frame-Options header to Nginx config, which is now required webserver-config wise. Switch to "always" send headers and update some regex to match official docs: nextcloud/server#16179, https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html + DietPi-Software | Nextcloud: Remove HSTS header completely from the config, since this is simply the wrong place. It is and should be added to the vhost configs when HTTPS is enabled.
@J0WI |
Why would you disable X-Frame-Options, XSS protection or any other security header on non 2xx responses?
Security through obscurity is no good practice. Admins should just keep their services secure.
I don't think that this change has a measurable effect on performance. |
As said, because it does not have any practical effect that I am aware of, security-wise. No content is sent by the webserver with 30x/40x answers, so nothing can be done by the client where any of the headers could kick in.
Of course, but sending out information for no reason is as well no good practice.
I agree, however I did not say that there are measurable or strong reasons to remove "always", I was just wondering what was your reason to explicitly do add it. As of above, I just see disadvantages, probably not measurable ones or deal-breakers, but I think you added it for a certain reason, any practical advantage that comes with it, and this is what I was asking for 😉. IMO the question, before changing something, should be "why" and not "why not" 😉? |
This addresses the some major points of #8207.
This should not change the behavior of a normal request to NextCloud. But it changes the following:
X-Frame-Options
is only sent by PHP responsesX-Frame-Options
is sent on all responsesX-Frame-Options
header is added to the nginx example, it's sent twice for PHP responsesX-Frame-Options
header should be added to the nginx exampleThis should not affect installations without
modHeadersAvailable
.