search

cvat-ai / cvat

Annotate better with CVAT, the industry-leading data engine for machine learning. Used and trusted by teams at any scale, for data of any scale.
https://cvat.ai
MIT License
12.13k stars 2.94k forks source link

Security headers not present in responses #7398

Open paulywog80 opened 7 months ago

paulywog80 commented 7 months ago

Actions before raising this issue

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):

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.
bsekachev commented 7 months ago

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

paulywog80 commented 7 months ago

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 2e9b17afadc8e05286214001d7880a62c0579fdb. 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.

mach-12 commented 5 months ago

I'll be taking this up.

mach-12 commented 5 months ago

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

nmanovic commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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