zio-http icon indicating copy to clipboard operation
zio-http copied to clipboard

HTTP spec conformance during CI

Open Saturn225 opened this issue 1 year ago • 17 comments

/claim #3083 fixes #3083

This PR integrates new HTTP conformance tests derived from the research paper "Who's Breaking the Rules? Studying Conformance to the HTTP Specifications and its Security Impact" by Jannis Rautenstrauch and Ben Stock. These tests now acts as a guardrails to ZIO -HTTP implementations adhere to the specifications and help identify potential security issues.

Conclusions

  1. The tests taken reference from http-conformance are categorised into 3 levels, Requirement, Recommendations and ABNF. The initial process is to add the conformance suite and I have added the Requirement and Recommendation level conformance tests which are critical to be tested to safeguard.

  2. I have ran http-conformance tool with simple zio-http server setup and observed analysis of tool with different categories Dangerous broken, Dangerous not broken, Not dangerous broken and Not Dangerous not broken. I have shifted towards first add tests for Dangerous ones and added them broken/not-broken then added not-dangerous ones.

Changes done:

Status Codes:

This specs verifies behaviour of the different Status Codes in Violations

  • 204 No Content which verifies no body is sent.
  • 205 Reset Content checks no body is sent.
  • 206 Partial Content checks the presence of Content-Range.
  • 206 Multipart Content checks Content-Range is excluded in multipart responses.
  • 206 Headers checks headers like ETag and Cache-Control are present.
  • 401 Unauthorized
    • checks the presence of WWW-Authenticate header.
    • checks 401 status when Authorization header is missing.
  • 405 Method Not Allowed checks the Allow header is present.
  • 407 Proxy Authentication Required verifies the Proxy-Authenticate header is present.
  • 304 Not Modified checks no body is returned for 304 Not Modified and verifies consistency with 200 OK and more....

Redirection (Location Header):

This tests added validates the presence of Location header in 300 Multiple Choices, 301 Moved Permanently, 302 Found, 303 See Other, 307 Temporary Redirect and 308 Permanent Redirect responses.

Headers and Metadata:

  • Range Header (206) checks Content-Range is present in 206 responses.
  • Content-Range (416) validates Content-Range in 416 Range Not Satisfiable.
  • Content-Length in CONNECT checks no Content-Length for 2XX CONNECT.
  • Transfer-Encoding in CONNECT checks no Transfer-Encoding for 2XX CONNECT.
  • CSP Header validates that only one Content-Security-Policy header is sent.

HTTP Methods:

  • HEAD Request (no body) verifies no body is sent in HEAD requests.
  • GET and HEAD Headers Consistency checks for matching headers in GET and HEAD.
  • POST Invalid Response Codes validates POST does not return 206, 304, or 416.
  • 501 Unknown HTTP Methods validates unknown methods return 501 Not Implemented.
  • 405 Blocked Methods validates non-allowed methods return 405 Method Not Allowed.

HTTP Versions:

  • Transfer-Encoding with HTTP/1.0 validates no Transfer-Encoding for HTTP/1.0.
  • Transfer-Encoding with HTTP/1.1 validates Transfer-Encoding for HTTP/1.1 or higher.
  • Whitespace in Start-Line validates rejection of requests with improper start-line formatting.
  • Bare CR in Headers checks compliance with CRLF formatting in headers.

Connection Management and Headers:

  • Close in Final Response validates Connection close is included when requested.
  • Bad Headers with CR, LF, or NUL validates headers with illegal characters are rejected.
  • Upgrade and Expect Headers validates 100 Continue before 101 Switching Protocols when both Upgrade and Expect headers are present.

Some of other added are 426 Upgrade Required and 101 Switching Protocols

Cache-Control Directives:

This spec added checks for unquoted values for max-age and s-maxage and quoted-string form for no-cache and private.

Content-Length and Transfer-Encoding:

This spec validates no Content-Length or Transfer-Encoding in 1xx and 204 responses and also to validate consistency of Content-Length between HEAD and GET as well as between 304 Not Modified and 200 OK.

Cookie Handling:

This spec guardrails no duplicate cookie attributes in Set-Cookie headers and no duplicate cookies with the same name and also the use of IMF-fixdate format for cookie expiration dates.

Miscellaneous:

Some of miscellaneous compliance tests added checks with headers such as Server, Content-Type, Accept-Patch and Date.

Possible Changes to Discuss:

Some tests related to Content-Security-Policy-Report-Only and Strict-Transport-Security (STS) are not yet supported in ZIO-HTTP, but I have raised tickets for their future inclusion #3171 and #3172 . Once these headers are supported, we can add more tests to validate their behavior. HTTP/2 is not yet supported in ZIO-HTTP. Once it is implemented, additional tests for HTTP/2 conformance will be necessary. I have mainly focused on to add the Requirement and Recommended tests which sets initial process to add conformance guardrail for zio-http. The ABNF checks could be added gradually into the suite

Other

I have fixed the all violating checks added as per the conformance suite added. Thanks @JannisBush for the detailed paper and the excellent tool!

Saturn225 avatar Sep 26 '24 07:09 Saturn225

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Sep 26 '24 07:09 CLAassistant

@Saturn225 HTTP/2 is out of scope. The failing test should be fixed first. I set this PR to draft for now. Once you fixed the issues open again. If you are stuck, you can contact me on discord

987Nabil avatar Sep 26 '24 21:09 987Nabil

The failing test is that cors header is adding OPTIONS method automatically in that spec. I will need to take care of this case in toHandler. I'll fix it

Saturn225 avatar Oct 08 '24 00:10 Saturn225

Sorry, I am busy these days. I come back to you when I have time.

987Nabil avatar Nov 08 '24 19:11 987Nabil

Thanks @987Nabil

Saturn225 avatar Nov 08 '24 19:11 Saturn225

@987Nabil @jdegoes Seems tricky why the entire workflows failing across the repo with following:

sbt: command not found

Any recent updates to the workflows?

Saturn225 avatar Dec 11 '24 14:12 Saturn225

@jdegoes @987Nabil Please take a look into this PR at your free time?

Saturn225 avatar Jan 06 '25 14:01 Saturn225

@jdegoes I wanted to follow up on this PR which has been open for 4 months. Could you please take a moment to review it? Your approval would help it move forward 🙏

Saturn225 avatar Jan 22 '25 17:01 Saturn225

@987Nabil Can you review this, please? 🙏

jdegoes avatar Jan 22 '25 21:01 jdegoes

Done @987Nabil

Saturn225 avatar Jan 23 '25 10:01 Saturn225

@987Nabil I have cleaned up the PR and marked the test as ignore for now

Saturn225 avatar Jan 23 '25 14:01 Saturn225

@kyri-petrou wants to take a look too

987Nabil avatar Feb 05 '25 15:02 987Nabil

@jdegoes @987Nabil @kyri-petrou Can you review this, please? 🙏

Saturn225 avatar Mar 30 '25 19:03 Saturn225

@kyri-petrou Can you review this, please? 🙏

Saturn225 avatar May 08 '25 05:05 Saturn225

Deploy Preview for zio-http ready!

Name Link
Latest commit 2ba8dcdff425d6b3014740f42d2db73acb3f3188
Latest deploy log https://app.netlify.com/projects/zio-http/deploys/68fc8f1db2503f00084d69ce
Deploy Preview https://deploy-preview-3169--zio-http.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar May 25 '25 10:05 netlify[bot]

@kyri-petrou @987Nabil could you please review/merge PR?

Saturn225 avatar May 26 '25 12:05 Saturn225