go-vcloud-director icon indicating copy to clipboard operation
go-vcloud-director copied to clipboard

PUT calls trigger cloudflare's WAF rules

Open artm opened this issue 4 years ago • 6 comments

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

artm avatar Dec 14 '21 11:12 artm

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

dataclouder avatar Dec 15 '21 13:12 dataclouder

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

Didainius avatar Dec 15 '21 14:12 Didainius

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

vbauzys avatar Jan 11 '22 13:01 vbauzys

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 avatar Jan 11 '22 13:01 artm

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?

artm avatar Jan 17 '22 09:01 artm

don't need as construction of request happens in govcd

vbauzys avatar Jan 17 '22 10:01 vbauzys