zaproxy icon indicating copy to clipboard operation
zaproxy copied to clipboard

API calls with an incorrect apikey should return 401 Unauthorized

Open sastorsl opened this issue 1 year ago • 7 comments

Describe the bug

Connecting to zaproxy with an api key that is incorrect returns an empty response.

The REST-ful way is to return a 40x HTTP response. This sends a clearer message back to the user what the source of the error is.

401 Unauthorized - The API key is not specified or is wrong, Caller is un-authorized (or rather un-authenticated). 403 Forbidden - Caller is forbidden to request the specified resource (authenticated, but actually unauthorized (not allowed)).

In addition, when proxying into zaproxy, for instance in kubernetes, you can end up getting a 502 Bad Gateway, which can indicate wrongly that zaproxy is down.

Steps to reproduce the behavior

Start ZAP as a daemon with an API key

docker run \
  --rm \
  --name zaproxy-test \
  -u zap \
  -p 80:8080 \
  -i ghcr.io/zaproxy/zaproxy:latest \
    zap.sh \
    -silent \
    -daemon \
    -host 0.0.0.0 \
    -port 8080 \
    -config api.addrs.addr.name=.* \
    -config api.addrs.addr.regex=true \
    -config network.localServers.aliases.alias.name=zaproxy.local \
    -config api.key=CORRECT_KEY

Run queries

Incorrect apikey

curl -I --resolve zaproxy.local:80:127.0.0.1 \
  "http://zaproxy.local/JSON/network/view/getAliases/?apikey=WRONG_KEY"

Response:

curl: (52) Empty reply from server

Correct apikey

curl -I --resolve zaproxy.local:80:127.0.0.1 \
  "http://zaproxy.local/JSON/network/view/getAliases/?apikey=CORRECT_KEY" 2>&1 | grep ^HTTP

Response:

HTTP/1.1 200 OK

Expected behavior

Incorrect apikey

curl -I --resolve zaproxy.local:80:127.0.0.1 \
  "http://zaproxy.local/JSON/network/view/getAliases/?apikey=WRONG_KEY"

Response:

HTTP/1.1 401 Unauthorized
...

Correct apikey BUT an API I'm not authorized for.

curl -I --resolve zaproxy.local:80:127.0.0.1 \
  "http://zaproxy.local/JSON/someVery/restricted/secretApi/?apikey=CORRECT_KEY"

Response:

HTTP/1.1 403 Forbidden
...

Software versions

zaproxy:latest zaproxy:2.14.0

Screenshots

See examples

Errors from the zap.log file

120461 [ZAP-IO-Server-1-1] WARN org.zaproxy.zap.extension.api.API - API key incorrect or not supplied: WRONG_KEY in request from 172.17.0.1

Additional context

No response

Would you like to help fix this issue?

  • [ ] Yes

sastorsl avatar Jan 11 '24 14:01 sastorsl

This is a security feature I'm afraid. If you turn on the "Report permission errors via the API" option then we will return a 400. ZAP is not just an API server its also a proxy, so we have to cope with malicious webservers trying to access the ZAP API directly.

psiinon avatar Jan 11 '24 14:01 psiinon

I see and understand.

As we're running zaproxy in a closed environment I setup a test and verified that indeed you get a 400 Bad Request with that options specified during startup. Namely api.reportpermerrors=true.

The test container was started as follows:

docker run \
  --rm \
  --name zaproxy-test \
  -u zap \
  -p 80:8080 \
  -i ghcr.io/zaproxy/zaproxy:latest \
    zap.sh \
    -silent \
    -daemon \
    -host 0.0.0.0 \
    -port 8080 \
    -config "api.addrs.addr.name=.*" \
    -config api.addrs.addr.regex=true \
    -config network.localServers.aliases.alias.name=zaproxy.local \
    -config api.key=CORRECT_KEY \
    -config api.incerrordetails=true \
    -config api.reportpermerrors=true

Adding Report error details via API - api.incerrordetails=true - for reference, but this should not be turned on in production.

PS! I used the steps described in the docs to find the correct option. That is, running ZAP locally on the desktop and inspect the local config file. Can add that the config file on a Mac is under ~/Library/Application\ Support/ZAP/config.xml

Also both xpath and yq are of good help - yq .config.api config.xml.

sastorsl avatar Jan 12 '24 09:01 sastorsl

@psiinon we could perhaps briefly discuss if the response code should be 401 instead of 400. I still think that is the correct response code, perhaps with a 403 in warranted cases.

sastorsl avatar Jan 12 '24 09:01 sastorsl

@sastorsl thats a tricky one. I'm not disagreeing, I'm just worried about changing an existing API that lots of people are already using. How many of them have enabled API error reporting is another matter of course 😁

psiinon avatar Jan 12 '24 09:01 psiinon

It would of course be good to get some votes on this issue in that case. I have not looked into if the actual code change is a big one.

One option is to close this issue as wontfix. It will still be here as a reference, and can be reopened in the future.

However, changing this will not be easier as time goes by.

Could the old behaviour (400) be opt-in or vice versa, through a config option?

sastorsl avatar Jan 12 '24 11:01 sastorsl

Could add that after adding api.reportpermerrors=true I get 400 Bad Request also when doing API calls to non-existent API's.

This issue could perhaps be expanded to plan REST-ful responses for more cases.

sastorsl avatar Jan 12 '24 11:01 sastorsl

There's another issue where returning status code is wanted: https://github.com/zaproxy/zaproxy/issues/3951 not exactly the same but related.

kingthorin avatar Jan 16 '24 15:01 kingthorin