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

alertFilter documentation could use some clarification #88

Open
mdem99 opened this issue Jul 13, 2021 · 29 comments
Open

alertFilter documentation could use some clarification #88

mdem99 opened this issue Jul 13, 2021 · 29 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@mdem99
Copy link

mdem99 commented Jul 13, 2021

Describe the bug
The ZAP daemon analyzes traffic from contract tests in the pipeline and produces a report. We are getting some "false positives" or defects that are already in WIP and would like to declassify them to informational threats.
The alert filter gives a "request: ok" but the filter is not afting the report in any way, ergo, it is not working.

To Reproduce
Steps to reproduce the behavior:

  1. Have in a repository a gitlab-ci file with a stage for ZAP, and ZAP running as a GitLab service
  2. Set up an infrastructure where you run contract tests or e2e tests, and use a proxy variable to divert traffic to ZAP Daemon
  3. After everything is set up, add a script file, before the tests, to configure the filter
  4. Add a script with CURL request to save the report
  5. Check the report to see that there is no change or affection due to filter alerting

Expected behavior
The alert filter should impact the results of the report and reclassify a threat. In my case, I wanted to classify this threat as False Positive.
The request (with fake URL):
curl -X -4 --retry 2 --retry-connrefused --retry-delay 3 http://zap:8090/JSON/alertFilter/action/addGlobalAlertFilter/?ruleId=10098\&newLevel=-1\&url=https%3A%2F%2Famazonaws.com%2Fapi%2Fv1%2Fpings\&parameter=Cache-Control\&enabled=true\&evidence=
Screenshots
125438254-80bb7454-5f73-430d-a9d3-5789a4bb737a
125438278-8923f42c-8e59-4003-a89b-6cd2b11716fd

Software versions

  • ZAP: Docker stable:latest and/or weekly:latest images (same problem for both)
  • Add-on: standard
  • GitLab - Pipeline
  • Usage: Docker as Gitlab service (reference)

Errors from zap.log file
The zap.log file is not accessible. Reason. I don't have a personal Docker coitaniner running, but I'm pulling a new image every time (since I'm using it a service).
For debugging I am using config "api.incerrordetails=true".

Additional context
My findings:

  1. with the error reporting option I only got: missing parameter "newLevel". After hours of troubleshooting I found that actually the problem was escaping (the parameters were concatenated with &, I just added "\ &")
  2. I first passed the affected url with the escape, no result. I passed it in "clear" text, no result. So it seems to accept both (or not correctly interpret both)
  3. In the local daemon, with my pc, i can choose between get and post methods. So I tried sending a CURL POST request, as a url and as a JSON. Both returned an error ({"code": "content_type_not_supported", "message":"!api.error.content_type_not_supported!"}
  4. I'm working with the global filter. I also tried context-based but it requires a real context id which I don't know.
  5. From the code of ZAP, it seems that only 2 parameters are actually required: ruleID and newLevel
  6. With the Weekly-Relase the report looks different. There is a new field called "parameters" and a new section for false positives. Due to the loss of documentation, I can't figure out if the alert filter is based on the stable image or the weekly image and how to properly set the new parameters.

**Would you like to help solve this problem?
This seems to be a really strange behavior and it would be very important for me to get it working, so yes, I would also like to help if I can.

@kingthorin
Copy link
Member

kingthorin commented Jul 13, 2021

The URL in the command doesn't match the URL is the screenshot. Note the "new2" portion, also the blacked out portion seems to be much longer than simply amazonaws.com/

@mdem99
Copy link
Author

mdem99 commented Jul 13, 2021

As i wrote, i had to mask it because it's a real url that i'm testing here. So i can ensure that the URL is matching ;)

@thc202
Copy link
Member

thc202 commented Jul 13, 2021

Did you check what is ZAP receiving?

@kingthorin
Copy link
Member

kingthorin commented Jul 13, 2021

I was able to recreate this. I added a Global Filter via the API (with ZAP running in GUI mode) and it didn't apply (even after refreshing the Alerts tree). If I went into Options and Modify > Test it would match 1 alert (as I expected). When I applied from the Modify screen then the alert was actually updated.

Edit: I didn't test reporting.

@mdem99
Copy link
Author

mdem99 commented Jul 13, 2021

The problem is how i can access options etc from docker, should i set something with -config?

@mdem99
Copy link
Author

mdem99 commented Jul 13, 2021

Did you check what is ZAP receiving?

I didn't quite understood the question but the connection ci-zap is working. I get the report and i can send the curl request to set the filter and by doing a query, i got the filter back with my inputs. So ZAP is receiving my commands and traffic correctly

@kingthorin
Copy link
Member

You can use the API view globalAlertFilterList to see what's actually been set.

@mdem99
Copy link
Author

mdem99 commented Jul 13, 2021

You can use the API view globalAlertFilterList to see what's actually been set.

as you see from the second screen, the filter is set

@kingthorin
Copy link
Member

kingthorin commented Jul 13, 2021

@psiinon @thc202 I believe in this block: https://github.com/zaproxy/zap-extensions/blob/54417192ee6ea0d92cd60fdf7c1ebc1c90bac2fe/addOns/alertFilters/src/main/java/org/zaproxy/zap/extension/alertFilters/AlertFilterAPI.java#L249-L274 at either 266/267 or 268/269 this needs to do a extension.applyAlertFilter(af, false); (probably the same for the non-Global additions as well).

Do you agree?

@psiinon
Copy link
Member

psiinon commented Jul 13, 2021

Yeh, def worth trying - good spot

@thc202
Copy link
Member

thc202 commented Jul 13, 2021

Aren't the alert filters supposed to be applied to new alerts?

Maybe add a parameter or a new endpoint, to keep the expected behaviour.

@psiinon
Copy link
Member

psiinon commented Jul 13, 2021

thats true, the API wasnt intended to support applying them to existing alerts, although it could be extended to do so

@kingthorin
Copy link
Member

Okay I'll add an apply endpoint.

@psiinon
Copy link
Member

psiinon commented Jul 13, 2021

But that wont solve this issue will it? If the filter is being correctly applied before the alerts are raised then it will make no difference...

@kingthorin
Copy link
Member

Will have to test further.

@kingthorin
Copy link
Member

Okay so yeah, I'm not able to confirm the original report. If the details actually match properly the filter is applied if it's in place first.

@kingthorin
Copy link
Member

kingthorin commented Jul 13, 2021

@mdem99 I only see a few options

  1. Try testing without setting the "parameter" value (just the URL).
  2. Try listing alerts or an alerts summary before generating the report (to see if the filter did apply but the report somehow isn't picking it up).
  3. Install ZAP locally and go through the same steps while you can see in the GUI exactly what's going on.

@mdem99
Copy link
Author

mdem99 commented Jul 13, 2021

I will try asap.
I've already tried with just URL and no parameters (expected behaviour: everything should be whitelisted for that url), but without any impact. Also, with the GUI, as i said, it works fine.
What are exaclty your findings? And how is this "apply" api endpoint affect exactly the filter and the report?
Thanks

@kingthorin
Copy link
Member

kingthorin commented Jul 13, 2021

I've split the apply thing into another ticket. It's a nice to have it's not really applicable here. If you're creating the filters before the alerts they should apply just fine.

To be clear when I was saying run ZAP in GUI mode you can still "drive" it via the API. So that way you'd see how your curl command(s) are working out.

Also, with the GUI, as i said, it works fine.

Sorry I'm working and checking in here, so apparently I fail at multitasking and retaining details today 🤷

@mdem99
Copy link
Author

mdem99 commented Jul 14, 2021

No prob :)

I'm also working in parallel with GUI (ZAP application for Windows). There i can apply the filter after the "report" creation, or let's say after we got the alerts listed down.
If you meant the ZAP API UI, is the same story as in the Daemon in headless mode. First the filter, then the report.

Again, working with for example with the GUI (ZAP application for Windows), works.
Daemon in Pipeline won't use the filter even if no errors are displayed. This makes me think that the filter was applied correctly but the same alert is still shown afterwards in the report.

Just to be sure, i'm running ZAP as a Service in the Pipeline, could this have any impact to the filter/report creation?

Is there any other kind of information that i can provide you?

Thank you again!

@mdem99
Copy link
Author

mdem99 commented Jul 19, 2021

Hello everyone,

I wanted to know if there are any news regarding this topic. I performed, with my team, all the necessary tests (Daemon in pipeline, API UI and GUI) without any result. As far as I'm concerned, the filter should work as it is.

I noticed a guy had the same problem a while ago and it was never cleared up. I also noticed that he solved the problem himself by writing a parser for the html-report file and applying a filter himself.

Can we find another solution or will I have to find another workaround too?

Thank you very much!

@thc202
Copy link
Member

thc202 commented Jul 19, 2021

That's not the same problem unless you are also adding the filters after the alerts are raised. I'd suggest checking with an API request that there are no alerts already raised before adding the filter.

@mdem99
Copy link
Author

mdem99 commented Jul 19, 2021

I agree with you but i think the similarity is interesting. In that case the filter was also not working and his solutions shows that he had to find another workaround.
In any case, i tested it and there are actually no alerts raised before adding the filter.

@mdem99
Copy link
Author

mdem99 commented Jul 19, 2021

I managed to get the filter working :)

So, a couple of very important things that in my opinion, should be added to the documentation:

  1. The CURL request needs to be escaped. This means to separate each parameter with a backslash ( \ & without space)
  2. "enable" parameter must be set to true.
  3. evidence parameter requires escaping like url (example: Access-Control-Allow-Origin:* becomes Access-Control-Allow-Origin%3A%20%2A)
  4. the "ruleID" parameter must correspond to the ID of the alerts from the JSON/HTML file. Example 10098 corresponds to the Cross-Domain Misconfiguration category, needed to filter an Access-Control-Allow-Origin:*)
  5. not clear, but I think that to be able to set a threat as false positive (-1) it will be necessary to use the new ZAP weekly image, because it has the new layout containing the false positives, not present in the stable image (which is therefore outdated)

These findigs have required weeks of research and testing, because of the absence of documentation and/or examples. I hope this can help new users to solve the problem and I ask instead to you developers the possibility to report everything in the documentation.

Thanks anyway for your assistance and your time!

ZAP is and remains the best tool :)

@mdem99 mdem99 closed this as completed Jul 19, 2021
@kingthorin
Copy link
Member

@mdem99 thanks for following up. Glad you got it sorted out.

@kingthorin kingthorin transferred this issue from zaproxy/zaproxy Jul 19, 2021
@kingthorin kingthorin added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 19, 2021
@kingthorin
Copy link
Member

kingthorin commented Jul 19, 2021

Transferred issue to api docs repo, re-opening.

In particular #88 (comment) should be addressed.

@kingthorin kingthorin reopened this Jul 19, 2021
@kingthorin kingthorin changed the title Alert Filter not working in GitLab-CI alertFilter documentation could use some clarification Jul 19, 2021
@thc202
Copy link
Member

thc202 commented Jul 20, 2021

Shouldn't the changes (if any) be done in the Alert Filters add-on?

@kingthorin
Copy link
Member

Oh, I guess. I was thinking here mainly because of the comments around curl examples needing to be more clear/specific

@thc202
Copy link
Member

thc202 commented Jul 20, 2021

OK, thought this was specifically about the Alert Filters. In that case it applies to all (cURL) examples (e.g. need to escape/encode, enable, use proper values).

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

No branches or pull requests

4 participants