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

Bug: New AWS-WAF Rule Breaks EZID-Related Tests #1625

Closed
artntek opened this issue May 17, 2023 · 5 comments
Closed

Bug: New AWS-WAF Rule Breaks EZID-Related Tests #1625

artntek opened this issue May 17, 2023 · 5 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@artntek
Copy link
Contributor

artntek commented May 17, 2023

Many tests that communicate with the prod website at https://ezid.cdlib.org include a _target url in the POST request body. On 5/11/23, security was updated for the public site, and application of a new AWS-WAF Rule means that requests are denied with a 403 Unauthorized error if the body contains a url that includes localhost or 127.0.0.1. This is populated from the metacat.properties server.name value, which is typically set to localhost, thus causing the test failures.

Here is the new definition:

   {
        "ruleGroupId": "AWS#AWSManagedRulesCommonRuleSet",
        "terminatingRule": {
          "ruleId": "EC2MetaDataSSRF_BODY",
          "action": "BLOCK",
          "overriddenAction": "BLOCK",
          "ruleMatchDetails": null
        },
        "nonTerminatingMatchingRules": [],
        "excludedRules": null,
        "customerConfig": null
   }

Instead of using the properties value, therefore, the tests should be changed to substitute a dummy hostname, without needing to affect all the other tests.

Epic #1608

@artntek artntek added the bug Something isn't working label May 17, 2023
@artntek artntek added this to the 3.0.0 milestone May 17, 2023
@artntek artntek self-assigned this May 17, 2023
@mbjones
Copy link
Member

mbjones commented May 17, 2023

What is _target used for in the request? Does changing it from server.name adversely affect operations, or is it just superfluous? If it's superfluous, can we just eliminate it?

@artntek
Copy link
Contributor Author

artntek commented May 17, 2023

I believe it's a callback link from the ezid site to the metacatui page that would display the dataset being referenced. Are you advocating eliminating it altogether, or just eliminating it in the test calls? (I already have a fix that substitutes a dummy hostname via mock Properties for the tests)

@mbjones
Copy link
Member

mbjones commented May 17, 2023

I'm not advocating anything yet, just trying to understand what is was used for, and why you could change it with impunity and still have tests pass.

@artntek
Copy link
Contributor Author

artntek commented May 17, 2023

got it. Yeah - it's not part of the test, and its content is not validated by the ezid site as part of the POST (apart from the new filtering to exclude localhost references). I don't know if it's a required field or not - but the mock value works OK

artntek added a commit that referenced this issue May 23, 2023
Bug Fix: New AWS-WAF Rule Breaks EZID-Related Tests #1625
@artntek
Copy link
Contributor Author

artntek commented May 25, 2023

merged #1626

@artntek artntek closed this as completed May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants