Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use fedora 33, set large --max-http-header-size #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Use fedora 33, set large --max-http-header-size #39

wants to merge 2 commits into from

Conversation

cben
Copy link
Contributor

@cben cben commented May 18, 2021

Closes #38.

  • raised limit so unlikely to happen.
  • if happens, returns clearer "431 Request Header Fields Too Large" instead of "400 Bad Request".
  • if happens, log still doesn't show anything. I think that's OK given the clear http code.

cben added 2 commits May 18, 2021 14:13
@cben
Use fedora 33 with new NodeJS 14.16
c7f31b3 
Fedora 29 has nodejs 10.16.3 which is EOL.
Fedora 33 (and 34) currently has 14.16.1.

The newer NodeJS improves RedHatInsights#38
by returning 431 Request Header Fields Too Large
instead of 400 Bad Request that NodeJS 10 returned.
@cben
Set --max-http-header-size=65536
e18f938 
64KB is larger than most server limits, but as a proxy,
it's not insights-proxy's job to set limits, let backend do it.
@cben
Copy link
Contributor Author

cben commented May 18, 2021 •
edited

New image size is 403MB, down from 616MB.
Pushed image to quay.io/redhat-sd-devel/insights-proxy:pull-39

@elad661
Copy link

elad661 commented May 19, 2021

Nice! LGTM

@elad661
Copy link

elad661 commented May 19, 2021

Tested locally, I'm getting

[INSIGHTS] An error occurred while resolving an ESI tag for the prod.foo.redhat.com host
[INSIGHTS] [While requesting GET|https://prod.foo.redhat.com:1337/apps/chrome/snippets/head.html|Accept:text/html, application/xhtml+xml, application/xml]: Error: [While requesting GET|https://prod.foo.redhat.com:1337/apps/chrome/snippets/head.html|Accept:text/html, application/xhtml+xml, application/xml]: Parse Error: Content-Length can't be present with Transfer-Encoding
[INSIGHTS]     at TLSSocket.socketOnData (_http_client.js:509:22)
[INSIGHTS]     at TLSSocket.emit (events.js:315:20)
[INSIGHTS]     at addChunk (internal/streams/readable.js:309:12)
[INSIGHTS]     at readableAddChunk (internal/streams/readable.js:284:9)
[INSIGHTS]     at TLSSocket.Readable.push (internal/streams/readable.js:223:10)
[INSIGHTS]     at TLSWrap.onStreamRead (internal/stream_base_commons.js:188:23) {
[INSIGHTS]   bytesParsed: 571,
[INSIGHTS]   code: 'HPE_UNEXPECTED_CONTENT_LENGTH',
[INSIGHTS]   reason: "Content-Length can't be present with Transfer-Encoding",
[INSIGHTS]   rawPacket: <Buffer 48 54 54 50 2f 31 2e 31 20 32 30 30 20 4f 4b 0d 0a 41 63 63 65 73 73 2d 43 6f 6e 74 72 6f 6c 2d 41 6c 6c 6f 77 2d 4f 72 69 67 69 6e 3a 20 2a 0d 0a 41 ... 1156 more bytes>,
[INSIGHTS]   request: {
[INSIGHTS]     url: 'https://prod.foo.redhat.com:1337/apps/chrome/snippets/head.html',
[INSIGHTS]     headers: { accept: 'text/html, application/xhtml+xml, application/xml' },
[INSIGHTS]     gzip: true,
[INSIGHTS]     method: 'GET'
[INSIGHTS]   }
[INSIGHTS] }

Parse Error: Content-Length can't be present with Transfer-Encoding

@elad661
Copy link

elad661 commented May 20, 2021

Looks like this is a problem in the Akamai configuration. The response for the ESI snippets contains both transfer-encoding: chunked and content-length: 967.

Newer NodeJS refuses to parse this response as having both these headers together could allow for an HTTP request smuggling attack.

@karelhala is there any way this can be fixed in the Akamai side? otherwise we'll have to keep insights-proxy using an old version of NodeJS forever

curl output (click to expand) 
curl -vk -H "accept: 'text/html'" https://prod.foo.redhat.com:1337/apps/chrome/snippets/body.html 
*   Trying 127.0.0.1:1337...
* Connected to prod.foo.redhat.com (127.0.0.1) port 1337 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: /etc/pki/tls/certs/ca-bundle.crt
*  CApath: none
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384
* ALPN, server accepted to use http/1.1
* Server certificate:
*  subject: C=US; ST=North Carolina; L=Raleigh; O=Red Hat Inc.; CN=qa.cloud.redhat.com
*  start date: Aug 30 19:28:40 2019 GMT
*  expire date: Aug 29 19:28:40 2021 GMT
*  issuer: O=Red Hat; OU=prod; CN=Certificate Authority
*  SSL certificate verify ok.
> GET /apps/chrome/snippets/body.html HTTP/1.1
> Host: prod.foo.redhat.com:1337
> User-Agent: curl/7.76.1
> accept: 'text/html'
> 
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Methods: GET, POST, OPTIONS, PUT, PATCH, DELETE
< Access-Control-Allow-Headers: X-Requested-With,content-type
< Access-Control-Allow-Credentials: true
< server: AkamaiNetStorage
< content-type: text/html
< etag: "73bfd4948b393352ac0e95611e6d03a5:1619630947.460165"
< x-frame-options: SAMEORIGIN
< x-content-type-options: nosniff
< x-fms-debug-esi: 1
< vary: Accept-Encoding
< cache-control: max-age=147
< date: Thu, 20 May 2021 08:37:45 GMT
< connection: close
< transfer-encoding: chunked
< content-length: 967
< 
* Closing connection 0
* TLSv1.3 (OUT), TLS alert, close notify (256):
<div class="pf-c-page pf-m-redhat-font" id="page"><div class="pf-c-page__drawer"><main class="pf-c-page__main pf-l-page__main" id="temp" role="temp" widget-type="InsightsTempPage" data-ouia-safe="false"><section class="pf-m-light pf-c-page-header pf-l-page-header pf-c-page__main-section pf-l-page__main-section pf-m-light" widget-type="InsightsPageHeader"><div class="pf-c-content"><h1 class="pf-c-title pf-m-2xl ins-l-page__header--loading" widget-type="InsightsPageHeaderTitle"><div class="ins-c-skeleton ins-c-skeleton__sm">&nbsp;</div></h1></div></section><section class="pf-c-page__main-section pf-l-page__main-section pf-l-page__main-section--loading" page-type="inventory"><div class="ins-c-spinner ins-m-center" role="status"><span class="pf-u-screen-reader">Loading...</span></div></section></main></div></div><script src="./apps/chrome/js/chrome-root.cb745b49f62189b30e4c.js"></script><main id="root" role="main" hidden="true" style="display: none"></main>

@cben
Copy link
Contributor Author

cben commented May 23, 2021

Q: why is it fetching the snippet back from the proxy — https://prod.foo.redhat.com:1337/apps/chrome/snippets/head.html ?
I'd expect it to fetch it directly from Akamai at https://cloud.redhat.com/apps/chrome/snippets/head.html — which has content-length: 623 but no transfer-encoding.

$ curl -v --insecure --header 'Accept:text/html, application/xhtml+xml, application/xml' https://cloud.redhat.com/apps/chrome/snippets/head.html
*   Trying 2a02:14a:102:38a::d7c:443...
* Connected to cloud.redhat.com (2a02:14a:102:38a::d7c) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/pki/tls/certs/ca-bundle.crt
  CApath: none
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384
* ALPN, server accepted to use h2
* Server certificate:
*  subject: C=US; ST=North Carolina; L=Raleigh; O=Red Hat, Inc.; CN=openshift.redhat.com
*  start date: Oct  5 00:00:00 2020 GMT
*  expire date: Jun 16 12:00:00 2021 GMT
*  issuer: C=US; O=DigiCert Inc; OU=www.digicert.com; CN=GeoTrust RSA CA 2018
*  SSL certificate verify ok.
* Using HTTP2, server supports multi-use
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x55e136ce2370)
> GET /apps/chrome/snippets/head.html HTTP/2
> Host: cloud.redhat.com
> user-agent: curl/7.71.1
> accept:text/html, application/xhtml+xml, application/xml
> 
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Connection state changed (MAX_CONCURRENT_STREAMS == 100)!
< HTTP/2 200 
< server: AkamaiNetStorage
< content-length: 623
< content-type: text/html
< x-frame-options: SAMEORIGIN
< x-content-type-options: nosniff
< x-fms-debug-esi: 1
< etag: "5b5d59be2fed6ae1f1d2621238595da5:1619540476.787924"
< cache-control: max-age=579
< date: Sun, 23 May 2021 15:16:18 GMT
< 
* Connection #0 to host cloud.redhat.com left intact
<meta http-equiv="x-ua-compatible" content="ie=edge"/><meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no"/><base href="/"/><link rel="icon" type="image/png" href="https://access.redhat.com/webassets/avalon/g/favicon.ico"/><link rel="stylesheet" href="./apps/chrome/chrome.0d00ed5011a5960e863520db902007c3.css"/><script src="https://browser.sentry-cdn.com/5.15.0/bundle.min.js" crossorigin="anonymous"></script><script type="text/javascript"> window.insights = {chrome: { isChrome2: true }} </script><script id="dpal" src="https://www.redhat.com/ma/dpal.js" type="text/javascript"></script>

@karelhala
Copy link
Contributor

All the requests are going trough the proxy even though they are then fetched from cloud.redhat.com. I'll take a look what could be done with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Silent 400 error when headers exceed 8KiB
3 participants
@cben @elad661 @karelhala