Adding XML and JSON Schema validation via validateSchema operator #1343
Conversation
Co-authored-by: Felipe Zipitría <3012076+fzipi@users.noreply.github.com>
… validation - Added full JSON Schema validation using kaptinlin/jsonschema - Added XML Schema validation using terminalstatic/go-xsd-validate - Implemented lazy initialization to improve performance - Added test cases
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1343 +/- ##
==========================================
- Coverage 84.01% 83.80% -0.22%
==========================================
Files 170 172 +2
Lines 9803 10034 +231
==========================================
+ Hits 8236 8409 +173
- Misses 1323 1380 +57
- Partials 244 245 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
FYI I'll take a look at the code coverage change, looks like I need to add a few more tests. I'm not sure what happened with the Tinygo test, I'll have to look into that one further. In the meantime, would you mind taking a look at the overall approach and let me know if you have any questions or concerns / things you would like changed? |
|
Thank you for your contribution!! |
| rawXml := buf.String() | ||
| if txVar := v.TX(); txVar != nil && v.RequestBody() != nil { | ||
| // Store the content type and raw body | ||
| txVar.Set("xml_request_body", []string{rawXml}) |
There was a problem hiding this comment.
is this how modsec handles this?
Otherwise, I think we should use the transaction context, as it becomes a future concern for data masking
|
|
||
| "github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes" | ||
| "github.com/kaptinlin/jsonschema" | ||
| xsdvalidate "github.com/terminalstatic/go-xsd-validate" |
There was a problem hiding this comment.
This library uses cgo, we can still use CGO, but CGO must always be a build flag, not part of the default package
cc @jcchavezs
Also, it doesn't look reliable we should look for alternatives
There was a problem hiding this comment.
I was looking at the same thing, for the build flag, thanks! I wasn't able to find a better package, but I would absolutely be open to suggestions. One of the considerations in using this package though was fast performance.
There was a problem hiding this comment.
I believe my comment didn't go through. I was saying since XML usage isn't crucial as we don't seem to have users using them I would just focus on JSON first.
There was a problem hiding this comment.
Thanks! I've removed the XML code - now everything is passing except a bit of code coverage- I should be able to add a little more testing tomorrow
Good callout. The way that libxml2 is initialized it does not look like it should resolve external entity references. However, I will try to add some tests to validate that specifically as well. |
Note - added some specific XXE validation testing as well. |
|
Thanks so much for the feedback so far! I hope to have some more information later today or tomorrow. |
…o v4.10.0 in testing/coreruleset/go.mod (corazawaf#1341) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Make sure that you've checked the boxes below before you submit PR:
Thanks for your contribution ❤️
Fixes Issue #1279
This implements the validateSchema operator as per the existing modsecurity documentation, but also adds on JSON support. Since it didn't fit in the current structure of Coraza I didn't include it here, but I also have additional documentation and a test server for validation at: https://github.com/cognitivegears/coraza_validate_schema_extras