vmware / go-vcloud-director

Golang SDK for VMware Cloud Director
Other
80 stars 78 forks source link

PUT calls trigger cloudflare's WAF rules #416

Open artm opened 2 years ago

artm commented 2 years ago

To help us diagnose issues efficiently, please include:

[x] A short but descriptive title [x] A detailed description of the problem including relevant software versions and steps to reproduce

Our service provider uses Cloudflare WAF in front of the VCD. When creating the resources with terraform vcd provider, its PUT calls trigger multiple WAF rules (see below). The same calls from VCD web client don't.

We have compared the calls made by terraform and those made by the web client. The XML format differs slightly, the most obvious difference is dat terraform / govcd starts the request body with an XML preamble (<?xml version="1.0" encoding="UTF-8"?> / <?xml version="1.0" encoding="UTF-8" standalone="yes"?>) while the web client doesn't.

Reproducing the terraform call manually without the preamble doesn't trigger the WAF rules.

Reproducing the web client's call manually with the preamble triggers the WAF rules.

[x] A fix or workaround if you know it

Our current workaround is to have the service provider add exception rules to cloudflare config based on request method (PUT), path, user_agent, request's country of origin. But they are hesitant to making the exceptions too open and we have to negotiate each path prefix separately.

Could go-vcloud-director be configured not te send the XML preamble with the API calls, since it isn't required by the API?

The triggered Cloudflare WAF rules:

960032 · Method is not allowed by policy OWASP HTTP Policy Log
960010 · Request content type is not allowed by policy OWASP HTTP Policy Log
960024 · Meta-Character Anomaly Detection Alert - Repetative Non-Word Characters OWASP Generic Attacks Log
981318 · SQL Injection Attack: Common Injection Testing Detected OWASP SQL Injection Attacks Log
981245 · Detects basic SQL authentication bypass attempts 2/3 OWASP SQL Injection Attacks Log
981243 · Detects classic SQL injection probings 2/2 OWASP SQL Injection Attacks Log
973338 · XSS Filter - Category 3: Javascript URI Vector OWASP XSS Attacks Log
973301 · XSS Attack Detected OWASP XSS Attacks Log
973304 · XSS Attack Detected OWASP XSS Attacks Log
973335 · IE XSS Filters - Attack Detected OWASP XSS Attacks Log
973333 · IE XSS Filters - Attack Detected OWASP XSS Attacks Log
973332 · IE XSS Filters - Attack Detected OWASP XSS Attacks Log
dataclouder commented 2 years ago

The fix should be simple: we remove xml.Header from the requests, which results in modifying 6 lines of code

$ grep xml.Header *.go
access_control.go:  body := bytes.NewBufferString(xml.Header + string(marshaledXml))
api.go: // Only send data (and xml.Header) if the payload is actually provided to avoid sending empty body with XML header
api.go:     body := bytes.NewBufferString(xml.Header + string(marshaledXml))
edgegateway.go:     buffer := bytes.NewBufferString(xml.Header + string(output))
edgegateway.go:     buffer := bytes.NewBufferString(xml.Header + string(output))
orgvdcnetwork.go:               b := bytes.NewBufferString(xml.Header + string(output))

However, we need to make sure that the removal of xml.Header has no side effects, which requires some testing. The above change would remove the header from all requests, not only PUT

Didainius commented 2 years ago

Cross linking other WAF related issue we had some time ago - 661

vbauzys commented 2 years ago

Hi @artm, could you check if the current released changes are enough for your case?

artm commented 2 years ago

I'll test next week when back at work, thanks

On Tue, 11 Jan 2022, 14:26 vbauzysvmware, @.***> wrote:

Hi @artm https://github.com/artm, could you check if the current released changes are enough for your case?

— Reply to this email directly, view it on GitHub https://github.com/vmware/go-vcloud-director/issues/416#issuecomment-1009963368, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPVY3HRYL3YQWILZLVLEDUVQVZXANCNFSM5KASSYAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

artm commented 2 years ago

I can only test the issue using terraform provider, I upgraded it to the latest version (3.5.1) but it seems to still use the xml header in put requests:

2022/01/17 10:00:47 Request caller: govcd.(*VM).SetGuestCustomizationSection-->govcd.(*VM).SetGuestCustomizationSection-->govcd.executeRequestWithApiVersion-->govcd.(*Client).executeTaskRequest-->govcd.executeRequestCustomErr-->govcd.executeRequestCustomErr-->govcd.(*Client).newRequest
2022/01/17 10:00:47 PUT *****/api/vApp/vm-de2a5d4a-401f-40ef-825b-db895e3f1397/guestCustomizationSection
...
2022/01/17 10:00:47     User-Agent: [terraform-provider-vcd/v3.5.1 (linux/amd64; isProvider:false)]
2022/01/17 10:00:47     X-Vmware-Vcloud-Access-Token: [********]
2022/01/17 10:00:47     Accept: [application/*+xml;version=33.0]
2022/01/17 10:00:47 Request data: [2183]
<?xml version="1.0" encoding="UTF-8"?>

Should I open the ticket with the terraform provider as well?

vbauzys commented 2 years ago

don't need as construction of request happens in govcd