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

Adding CORS header 'Access-Control-Allow-Origin' with errors. #112

Open
TafkaMax opened this issue Jan 17, 2023 · 9 comments
Open

Adding CORS header 'Access-Control-Allow-Origin' with errors. #112

TafkaMax opened this issue Jan 17, 2023 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@TafkaMax
Copy link
Contributor

TafkaMax commented Jan 17, 2023

Hi

I am using modsecurity-crs:nginx as a proxy for my backend, which is an API.

On a totally different machine, there is a frontend JS application. Modern JS needs to have a CORS header set.

The proxy and API work fine, when everything is OK, but when a rule triggers the CORS header is not added.


Example:

The API allows users to POST data - up to 25MB. Currently the application checks the file size and also the proxy checks it.

When the proxy intercepts a max_body_size (or similar variable) that is larger than allowed it sends a 403 request, that does not include the CORS header. On the other hand the application sets it, when it encounters that denial.

I guess the easy way is for the application to block it, but it's better if it is intercepted earlier?

EDIT:

Currently my default.conf.template that I map to the container contains the following (which does not work):

# Nginx configuration for both HTTP and SSL

map $http_upgrade $connection_upgrade {
    default upgrade;
    '' close;
}

# https://serverfault.com/questions/958965/nginx-enabling-cors-for-multiple-subdomains

map $http_origin $allow_origin {
    ~^https?://(.*\.)?.mydomain.example(:\d+)?$ $http_origin;
    ~^https?://(.*\.)?localhost(:\d+)?$ $http_origin;
    default "";
}

server {
# http redirect
}

server {
# stuff
    add_header 'Access-Control-Allow-Origin' $allow_origin;
    location / {
# stuff
    }
}
@fzipi fzipi self-assigned this Feb 1, 2023
@fzipi
Copy link
Member

fzipi commented Feb 1, 2023

Thanks for the report @TafkaMax. We'll check this one soon.

@fzipi fzipi added the bug Something isn't working label Feb 1, 2023
@fzipi
Copy link
Member

fzipi commented Feb 6, 2023

BTW, do you have an example that works?

@TafkaMax
Copy link
Contributor Author

TafkaMax commented Feb 8, 2023

Unfortunately, not something that I can share.

  1. When I add something that return 403.
    image

When it is successful I don't get any error messages regarding CORS

@fzipi
Copy link
Member

fzipi commented Feb 8, 2023

Ok, no worries, let me see what we can do.

@fzipi fzipi added enhancement New feature or request and removed awaiting feedback bug Something isn't working labels Feb 8, 2023
@TafkaMax
Copy link
Contributor Author

TafkaMax commented Feb 8, 2023

When sending the request that returns 403, it is intercepted by the modsec-crs-docker container.

image

When sending a successful request, the header from the backend app is forwarded.

image


As you can see, my previous attempts to modifying the functionality to add the header, do not work. (The code in the inital post)

@fzipi
Copy link
Member

fzipi commented Feb 26, 2023

I started playing with this a bit. I don't think we can provide an answer for all cases, but this is what I'm trying:

Adds the specified field to a response header provided that the response code equals 200, 201 (1.3.10), 204, 206, 301, 302, 303, 304, 307 (1.1.16, 1.0.13), or 308 (1.13.0).

So it will never add for errors 403.

load_module modules/ngx_http_headers_more_filter_module.so;
  • adding a new file includes/cors.conf.template with this temporary content:
more_set_headers -s 403 'Content-Type' 'text/plain';
more_set_headers -s 403 'Access-Control-Allow-Origin' '*';
more_set_headers -s 403 'Access-Control-Max-Age' '3600';
more_set_headers -s 403 'Access-Control-Allow-Methods' 'GET, POST, PUT, DELETE, OPTIONS';
more_set_headers -s 403 'Access-Control-Allow-Headers' '*';

We probably need to change the build step of the image to include the headers_more module on nginx and alpine. And also make those ☝️ configurable.

@TafkaMax
Copy link
Contributor Author

Aha, thanks for the answer. I did not know the add headers were limited and not available to all responses without extra modifications.

@fzipi
Copy link
Member

fzipi commented Jan 27, 2024

@TafkaMax Do you want to take a chance on this one?

@TafkaMax
Copy link
Contributor Author

Currently not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants