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.

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

[regression] Sending large header over http2 fails with a send error #11138

Closed
emilio opened this issue May 18, 2023 · 9 comments
Closed

[regression] Sending large header over http2 fails with a send error #11138

emilio opened this issue May 18, 2023 · 9 comments
Assignees
@icing
Labels
HTTP/2 regression

Comments

@emilio
Copy link
Contributor

emilio commented May 18, 2023

I did this

fc2f1e5 introduced a hardcoded limit in max_line_length for http2 of 4kb, which is hit for requests with long headers.

Sending a request as the one in glandium/git-cinnabar#314 fails now where it didn't fail before updating curl.

I expected the following

The request is sent successfully.

curl/libcurl version

curl 8.1.0 (x86_64-pc-linux-gnu) libcurl/8.1.0 OpenSSL/3.0.8 zlib/1.2.13 brotli/1.0.9 zstd/1.5.5 libidn2/2.3.4 libpsl/0.21.2 (+libidn2/2.3.4) libssh2/1.10.0 nghttp2/1.53.0
Release-Date: 2023-05-17
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL threadsafe TLS-SRP UnixSockets zstd

operating system

Linux crimson 6.3.2-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 11 May 2023 16:40:42 +0000 x86_64 GNU/Linux
@emilio emilio mentioned this issue May 18, 2023
Closed
@emilio
Copy link
Contributor Author

emilio commented May 18, 2023

Downgrading curl to:

curl 7.88.1-DEV (x86_64-pc-linux-gnu) libcurl/7.88.1-DEV OpenSSL/3.0.8 zlib/1.2.13 brotli/1.0.9 zstd/1.5.5 libidn2/2.3.4 libpsl/0.21.2 (+libidn2/2.3.4) libssh2/1.10.0 nghttp2/1.53.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL threadsafe TLS-SRP UnixSockets zstd

Fixes my issue. Since I'll close the git-cinnabar issue, I'll post the whole request body here:

(rr) p (char*)buf
$60 = 0x7f3a4014b650 "GET /mozilla-unified?cmd=known HTTP/2\r\nHost: hg.mozilla.org\r\nUser-Agent: mercurial/proto-1.0\r\nAccept-Encoding: deflate, gzip, br, zstd\r\nAccept: application/mercurial-0.1\r\nX-HgArg-1: nodes=16ff8fc5362f32c20defdb573b43ac07934bc965+a608ba89e49dd149c588d4ce30b411e1ca1d3588+d8d2a10a8572879d96384bcdbf90c7a5a9ad76b3+62bd36d202ed1c78d0964c50833ec40a04635fce+bc5188a32f31d72f3219cc2a73a729a2e1d00766+4307d141e63a6845295b829718e3eace79c88d0d+19655f2f9f78de6a502564c25aa55c7f35d70ad4+c64d97a1ebd769831a69410b1064749bf7630a64+ddf00cef28a3c514756f22046f61b1e31c9bb079+a68ae831712eff49af34d05379f01afe93ed1829+25093037bf154fcf729972d04b5bd7252fef4fd6+777e60ca88535403b547fcb3271f4baf63c17af2+4a108e94d3e2ad432d6b3c146dc82eeab4ae0af1+49d526f64476d90d6efeb603fa3a5a088a36693f+1603dbee8b8fef97a571c91608865213a4409292+ec8ecfb028db5af1ad623ecc4e55793f89b2ffed+6397ea663a90d7f5cf126c14f91ed300b75dd431+878e08c9206e2e387f3675e820fbb2b13bc5abe4+84cf31ae13721d90037bc165114ec284ceba86c0+7c7d7a61626862a73571eda6abfe602097e862d0+c1316a97b8db5fc648e49f13be8641f1e0c0f061+b1783e58b3486d002c1a248a73d503deff2a0b3d+492e61ecac3271b126dda3c224987fbcaa7e5252+8202582a9a66c9efe18560ec5dc6f341d9d0ef7e+bfe79898a2759778423a095b171886864360b363+277ac59f231b63ef16a969f789bc48d520b3e922+dcd0b16e1c3a025ee62e4ad9cd9178d98daaeaf6+083b1a8696f49707937de1c79f168081175a59cb+6f66ec6d601552f608acf38819a308487c03f83d+f4e126f283033d49bae7c6c3b55a85c9ad207a9d+6313147f43256bdce8d3d88347cc6204add79bab+6649cb78882746f57822dba0784f451fd9b4a237+e5d1bdafb7dd2cfb064bceb54c3fce3264a223ae+91652fc1498abb09b8452dc82d8fe33095d209d5+51950323a5e74a3cbc5433da519cf6529ff00e79+08b661f9b8062bb936ab8579e2c2336c67e55618+55647dd6d327e27ce07f7fcce068b9c25677d947+f3e9dee9154b516961688fecc65624781f128da6+2920781cb5978ce9c9077fae65e33f4ed27446a8+689644dc29b307545350a7a9f2f92ecf9da828a7+fa49429821ddf9ab1ea0abf01b8fed8c5b8cf876+b1fbb120929039103ff3fc794b82f8d22e0aa9d2+756dd7fd01d25ed1f54fe732fa6d005ac734db89+1d882fe7a3ad799f07620020ee3421fecacdfdd3+051ba49bcc965e9aa98d9e2c591bd0788f7ab4fa+89799030c274bbd813914298d03f3437b3e353f6+8b24cbe32580d0ae32c8b0986b6f2381bbd59d5a+05b400a43f8a9b4dc93520feef0908188935e3f0+2eebb400bad1d889f1a3f0b96968869a851c69b6+7238115ace0f85931db7b3dd7d84dbc00eaaa7fd+5b8d591f2080e7e4783de822a105c3cb48ed9387+b1f9f3d332d7ad2c2ec0b2780a7d001b66f02066+b035862137ef66d155de7c63377dc7498d8ebeb8+6389b4d6d6157be5cb2aa79ca21ecf141520604a+78ac2da529f8d453113dd01c56f9a263f52e09d9+f859d6b6a6bd4ea03464c5fa730da2db64ae2c54+7249a48e560fa1afc74dfb6a1a8a0603deb619f3+de2c09ba8dbe5080930a0c66eeae92c85e4c3a15+d7367dac25aa2a65367f5e184569aa599e31df79+d557444966f372e23b8009f3cd41c97f865a6c79+3fbac44e0010a7db0021a391bcb2d95525a94f31+7a54692e579d3366d23773a313c07c638cb7fe6a+efb40003fcd056cc11a73ee051ae28a19c73cc85+191e407d938b9c3abb8248a9813832fab238a529+80c5fc972b2f37f22b8d7983ab13f530896f6b95+6485695c62ad3061516c33d5c07f906dff8d1ab4+af1d91bf1a80e5f330ea307419bbee22a9eb33dd+ccbc709d1f73f0613cb9f9cb635abbe678a531a5+fe6b6c9b46478af2844bfb5d1bf2ac98f8dc320f+5932661275c48aeda2b6357735d67301e33c33d2+13f1c5d32ca9ed292da7f3c0f1ca0a5e37bcf719+1149e6b05418a17d289e4793d6a9731baccef6d6+8e62583019790339148ab06653e69f6f5d63e2d6+ed5ef65679d3b7c6069620a94dfbf466e9ffa9e2+3782a83158c83b334d702e64080d51916d9f8d59+c4852a3bb4e1dc8e33b4737ba6f9acdbb224198d+8250282e5e2c6c14dc5480af036d802ac4cd5cdb+d3c8be48f332cb92f85fbd31411e77910f69a91e+b211cae3fc82de2cdc9a429a04b6c0af23c32968+82f10970d26d10406ca467ae395f13dcd8c2b4f2+b03f472e53eac6b29f30e8e62135223cc128350b+a30c398c742481081dd50a83fe1899494a12be4c+f3b1bd79676680b147f095de29b5d496aa61a2f4+214e30534e5e8eb93c6844307fbeb57383a8adc0+7d34e292dda169e32e5796f2f30d9093cdacd046+3897ecb10cb90791be5549eb9b5b55d544aad6e2+6b851adb82e975f76dab60adc8c9379c98c578f1+bda37490061993d7cc0762728b3f2ab957d43e6a+3a6773129ff8ace8876e99542ef75dec086cee5f+380eb86358625267deee8404e4039d74e81962da+5e24ad5d9af8e4a33c44f24e3391290bd41ccc5c+0724a8d56f44484f6e07cfdc5d6b88f1a0e84669+54cdf4ab906168b5bfa43c14ceea67686e168658+467564547cc72d78306897af0685a8a1a718822f+5da351f7e0105f06818a1f9b0b845440fc3166c2+dcc6d7a0dc0056f23d4f0564b708513836961f1a+5350524bb6543084fbba8604ce050794d0bc70ae+f21620428eb73c49891ba403bdb4ed799dc32c21+f07a340953ea75e0236f2fdc53504eed91057d34+e172a4684a8d6f61a58277c3f3e7498bbfc6b08d\r\n\r\n"

The cURL stack is:

#0  h2_submit (err=0x7f3a4576b654, len=4291, buf=0x7f3a4014b650, data=0x7f3a40000d30, cf=0x7f3a40006360, pstream=<synthetic pointer>) at /usr/src/debug/curl/curl/lib/http2.c:1863
#1  cf_h2_send (cf=0x7f3a40006360, data=0x7f3a40000d30, buf=0x7f3a4014b650, len=4291, err=0x7f3a4576b654) at /usr/src/debug/curl/curl/lib/http2.c:2070
#2  0x00007f3a463bb65b in Curl_write (data=<optimized out>, sockfd=<optimized out>, mem=<optimized out>, len=<optimized out>, written=0x7f3a4576b6a0) at /usr/src/debug/curl/curl/lib/sendf.c:175
#3  0x00007f3a4639e91e in Curl_buffer_send (in=0x7f3a4576b770, data=0x7f3a40000d30, http=0x7f3a4013ec10, bytes_written=0x7f3a40002150, included_body_bytes=0, socketindex=<optimized out>) at /usr/src/debug/curl/curl/lib/http.c:1377
#4  0x00007f3a463ac509 in Curl_http_bodysend (httpreq=HTTPREQ_GET, r=0x7f3a4576b770, conn=0x7f3a401357d0, data=0x7f3a40000d30) at /usr/src/debug/curl/curl/lib/http.c:2783
#5  Curl_http (data=0x7f3a40000d30, done=<optimized out>) at /usr/src/debug/curl/curl/lib/http.c:3352
#6  0x00007f3a463c0108 in multi_do (done=0x7f3a4576b8a9, data=<optimized out>) at /usr/src/debug/curl/curl/lib/multi.c:1597
#7  multi_runsingle (multi=multi@entry=0x556070b2c480, nowp=nowp@entry=0x7f3a4576b960, data=data@entry=0x7f3a40000d30) at /usr/src/debug/curl/curl/lib/multi.c:2201
#8  0x00007f3a463c27d5 in curl_multi_perform (multi=0x556070b2c480, running_handles=0x7f3a4576ba60) at /usr/src/debug/curl/curl/lib/multi.c:2745

@bagder bagder assigned bagder and icing and unassigned bagder May 18, 2023
@bagder
Copy link
Member

bagder commented May 18, 2023

We should be able to increase that (double?) at fairly low risk I think...

@bagder
Copy link
Member

bagder commented May 18, 2023

Seems like a candidate for the 8.1.1 release!

@emilio
Copy link
Contributor Author

emilio commented May 18, 2023

(Just curious, why does there need to be a limit to begin with?)

emilio added a commit to emilio/curl that referenced this issue May 18, 2023
@emilio
http2: Double http request parser max line length.
8f36f14 
This works around curl#11138, by doubling the limit, and should be a
relatively safe fix.

Ideally the buffer would grow as needed and there would be no need for a
limit? But that might be follow-up material.

Ref: curl#11138
@emilio emilio mentioned this issue May 18, 2023
Closed
@emilio
Copy link
Contributor Author

emilio commented May 18, 2023 •
edited

I sent #11139, doubling the buffer size as suggested, though I think ideally there should be no limit? I don't have much context on that code tho.

@icing
Copy link
Contributor

icing commented May 18, 2023

Thanks for making that PR. I agree that 8K is the better limit. not sure what I was thinking at the time. Most servers have such limits as well, for example Apache has 8K by default (https://httpd.apache.org/docs/2.4/mod/core.html#limitrequestfieldsize)

When processing HTTP/1.1, having maximum field/line length is sensible since otherwise someone injecting requests can cause unlimited memory growth. This is one way to DoS http processing.

@icing
Copy link
Contributor

icing commented May 18, 2023 •
edited

@bagder: is there any length enforcement on headers? Thinking about cookies especially. Maybe 8K is even not enough? What do we want to process?

Hmm, reading the cookie discussion on the mailing list, 8K for one line, e.g. one header field, seems to be a good limit.

@bagder
Copy link
Member

bagder commented May 18, 2023

Yeah, I think we use 1MB total max for a h1 request (DYN_HTTP_REQUEST), but each header field limited to 8K seems sensible. I don't think we have a set limit for individual h1 header fields.

@glandium
Copy link

FWIW, git-cinnabar decides the max length of the headers it sends based on what the mercurial server reports as its httpheader value. https://hg.mozilla.org/mozilla-central?cmd=capabilities says httpheader=6144. So yeah, 4k is definitely smaller than that. Mercurial's default for that value is 1024, so 6144 is definitely something that was set specifically on the Mozilla server... (digging...) it was set here: https://hg.mozilla.org/hgcustom/version-control-tools/rev/d34c25360eb14189a599540f7f36c1ae2159ad5f , based on the HTTP server having a 8k limit.

@inglor inglor mentioned this issue May 18, 2023
Closed
@bagder bagder closed this as completed in 77c9a98 May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@icing icing

Labels
HTTP/2 regression
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@icing @bagder @glandium @emilio