Skip to content
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

Security headers not present in responses #7398

Open
2 tasks done
paulywog80 opened this issue Jan 24, 2024 · 7 comments
Open
2 tasks done

Security headers not present in responses #7398

paulywog80 opened this issue Jan 24, 2024 · 7 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@paulywog80
Copy link

Actions before raising this issue

  • I searched the existing issues and did not find anything similar.
  • I read/searched the docs

Steps to Reproduce

  1. open a command prompt
  2. enter the following: curl --head https://cvat.reconyx.com

Expected Behavior

I would expect response headers related to security would be present. It seems these three used to be in previous versions, but the switch to using Traefik dropped them. These headers include (taking information from https://github.com/opencv/cvat/blob/v1.2.0/cvat/apps/documentation/installation.md#serving-over-https):

  • Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
  • X-Content-Type-Options "nosniff" always;
  • Content-Security-Policy "default-src 'self' http: https: data: blob: 'unsafe-inline'" always;

Possible Solution

Add these headers into the Traefik config. See the FILE (YAML) tabs here: https://doc.traefik.io/traefik/v2.9/middlewares/http/headers/

Context

We have a third party which rates our security score, and not having these headers on responses results in a minor deduction.

Environment

- Linux VM
- most recent changelog tag:
<a id='changelog-2.9.1'></a>
## \[2.9.1\] - 2023-11-23

This release has changes only in the Enterprise version.
@paulywog80 paulywog80 added the bug Something isn't working label Jan 24, 2024
@bsekachev
Copy link
Member

Hello,
Why do you consider it to be a bug?

@paulywog80
Copy link
Author

Hi there, thanks for the quick response!

These headers were previously implemented, presumably as a feature to make this project more secure. They were removed because they conflicted with some service called "kibana" in commit 2e9b17a. I guess I would consider that commit to be the commit that introduced the intended behavior of "fixing interactions with kibana" with the unintended behavior of "reducing the security of this project", as there doesn't seem to be something that accounts for the lost behavior that those security headers provided. For what it's worth, it doesn't seem like kibana is used in this project anymore, but I might be missing something.

I realize there is a separate route I could have taken to address this issue by emailing secure@cvat.ai, but the impact of these changes didn't seem to warrant a non-github approach. If you would prefer I do that, please let me know.

@nmanovic nmanovic added the good first issue Good for newcomers label Mar 5, 2024
@mach-12
Copy link
Contributor

mach-12 commented Mar 6, 2024

I'll be taking this up.

@mach-12
Copy link
Contributor

mach-12 commented Mar 8, 2024

Hello @nmanovic requesting for assignment of this issue. I recreated this issue in the dev envorinment. The solution is to add the security headers removed during the Kibana migration back into the docker-compose.yaml.

security headers

  • add_header X-Frame-Options "SAMEORIGIN" always;
  • add_header X-XSS-Protection "1; mode=block" always;
  • add_header X-Content-Type-Options "nosniff" always;
  • add_header Referrer-Policy "no-referrer-when-downgrade" always;
  • add_header Content-Security-Policy "default-src 'self' http: https: data: blob: 'unsafe-inline'" always;
  • add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;

@nmanovic
Copy link
Contributor

nmanovic commented Mar 8, 2024

Hello @nmanovic requesting for assignment of this issue. I recreated this issue in the dev envorinment. The solution is to add the security headers removed during the Kibana migration back into the docker-compose.yaml.

security headers

  • add_header X-Frame-Options "SAMEORIGIN" always;
  • add_header X-XSS-Protection "1; mode=block" always;
  • add_header X-Content-Type-Options "nosniff" always;
  • add_header Referrer-Policy "no-referrer-when-downgrade" always;
  • add_header Content-Security-Policy "default-src 'self' http: https: data: blob: 'unsafe-inline'" always;
  • add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;

@SpecLad , could you please comment on the proposed solution?

@SpecLad
Copy link
Contributor

SpecLad commented Mar 8, 2024

A few comments:

  1. We already use the X-Frame-Options header - see https://github.com/opencv/cvat/blob/bfb902fca4c44c1aa44be490193ba87a282aafc0/cvat/nginx.conf#L49 and https://github.com/opencv/cvat/blob/bfb902fca4c44c1aa44be490193ba87a282aafc0/cvat-ui/react_nginx.conf#L23.
  2. X-XSS-Protection is useless. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-XSS-Protection.
  3. For Referrer-Policy, strict-origin-when-cross-origin is probably the better option.
  4. Your proposed Content-Security-Policy seems to basically allow everything. Is there even any point in adding it?
  5. Strict-Transport-Security is dangerous and should not be added by default. We don't want to lock users into using HTTPS without their knowledge.
  6. It's not clear to me what you mean by adding those headers "back into the docker-compose.yaml", as they were never in there in the first place. However, note that any solution should cover both Compose and Helm-based deployments.

@mach-12
Copy link
Contributor

mach-12 commented Mar 17, 2024

I have done changes. Would appreciate reviews @SpecLad @nmanovic

SpecLad added a commit that referenced this issue May 20, 2024
Added security headers for Referrer-Policy, X-Content-Type-Options.

Referring to Issue #7398, Added
additional security headers. Added to address the deduction in security
score rating third party scanners.

- Referrer-Policy "strict-origin-when-cross-origin";: Limit the referrer
information sent when a user navigates away from the website

- X-Content-Type-Options "nosniff";: Prevent browsers from attempting to
MIME-sniff the content type of a response to reduce risk of XSS and
Content Injection

Co-authored-by: Roman Donchenko <rdonchen@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants