Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1922565
Summary: | EFI HTTP boot fails if the HTTP headers are lower case | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Juan Orti <j.orti.alcaine> |
Component: | shim | Assignee: | Peter Jones <pjones> |
Status: | NEW --- | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | 33 | CC: | berrange, crobinso, jortialc, kraxel, lersek, mjg59, pbonzini, philmd, pjones, virt-maint |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | x86_64 | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | Type: | Bug | |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Juan Orti
2021-01-30 07:30:02 UTC
(Thanks for the CC, Dan.) Juan, I believe there must be more to this than just a "content-length" / "Content-Length" case difference. The "Content-Length" header is defined as follows, in edk2's "MdePkg/Include/IndustryStandard/Http11.h": #define HTTP_HEADER_CONTENT_LENGTH "Content-Length" By grepping for HTTP_HEADER_CONTENT_LENGTH, we find several uses; all uses are arguments to the HttpFindHeader() function. Quoting (part of) that function, from "NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c": for (Index = 0; Index < HeaderCount; Index++){ // // Field names are case-insensitive (RFC 2616). // if (AsciiStriCmp (Headers[Index].FieldName, FieldName) == 0) { return &Headers[Index]; } } return NULL; So the header lookup already happens case-insensitively. I don't know if the edk2 HTTP implementation intends to support HAproxy (or any similar reverse proxy setup). I suggest filing an upstream bug report at <MdePkg/Include/IndustryStandard/Http11.h>, and CC'ing the NetworkPkg maintainer (Maciej Rabeda <maciej.rabeda.com>). FWIW, I have never even tested the HTTP server of dnsmasq (even without HAproxy). The RHEL[78] manuals describe httpd (Apache); I've tested those (not with HAproxy though). This could be totally uncharted territory (hence my suggestion to file an upstream bug). Technically, I could of course file the upstream bug myself, but (a) I have no reproducer, (b) more details / configs about the environment will almost certainly be requested by the maintainer; it's best if those come directly from the reporter. (In reply to Laszlo Ersek from comment #1) > I suggest filing an upstream bug report at > <MdePkg/Include/IndustryStandard/Http11.h>, and CC'ing the NetworkPkg > maintainer (Maciej Rabeda <maciej.rabeda.com>). Clipboard user malfunction; sorry about that. The upstream bug tracker URL is <https://bugzilla.tianocore.org/>. (In reply to Juan Orti from comment #0) > Description of problem: > After adding a reverse proxy (HAproxy) to my EFI HTTP boot setup, my EFI VMs > no longer boot with the error: > > ~~~ > >>Start HTTP Boot over IPv4.... > Station IP address is 192.168.5.141 > > URI: http://bootserver.lan/grub/shim.efi > File Size: 1210776 Bytes > Downloading...100%Failed to get Content-Lenght > Invalid image > Failed to read header: Unsupported > Failed to load image: Unsupported > start_imagee() returned Unsupported > ~~~ Looking at these messages a second time, the "Downloading....100%" part makes me think that ED2 has succesfully downloaded the shim.efi file, and the error messages are coming from the next phase Indeed if i look at the shim source https://github.com/rhboot/shim/ then we can find: Invalid image Failed to read header: Unsupported Failed to load image: Unsupported start_imagee() returned Unsupported are all messages in the shim.c source file. The one I can't find is the "Failed to get Content-Lenght" message - possibly that comes from UEFI services that shim calls into ? (In reply to Laszlo Ersek from comment #1) > Juan, I believe there must be more to this than just a "content-length" / > "Content-Length" case difference. Hi Laszlo, Well, I can reproduce it quite easily with the "content-length" header. Here are all the HTTP headers: This works: $ curl -4 -v -o /dev/null http://bootserver.lan/grub/shim.efi * Connected to bootserver.lan (192.168.5.1) port 80 (#0) > GET /grub/shim.efi HTTP/1.1 > Host: bootserver.lan > User-Agent: curl/7.71.1 > Accept: */* > * Mark bundle as not supporting multiuse < HTTP/1.1 200 OK < server: nginx/1.19.6 < date: Tue, 02 Feb 2021 11:00:10 GMT < content-type: application/efi < Content-Length: 1210776 < last-modified: Wed, 27 Jan 2021 20:50:22 GMT < etag: "6011d20e-127998" < accept-ranges: bytes < When I remove the mentioned workaround in HAproxy, content-length is in lower case and the VM no longer boots with the error of the bug descritpion: $ curl -4 -v -o /dev/null http://bootserver.lan/grub/shim.efi * Connected to bootserver.lan (192.168.5.1) port 80 (#0) > GET /grub/shim.efi HTTP/1.1 > Host: bootserver.lan > User-Agent: curl/7.71.1 > Accept: */* > * Mark bundle as not supporting multiuse < HTTP/1.1 200 OK < server: nginx/1.19.6 < date: Tue, 02 Feb 2021 11:01:55 GMT < content-type: application/efi < content-length: 1210776 < last-modified: Wed, 27 Jan 2021 20:50:22 GMT < etag: "6011d20e-127998" < accept-ranges: bytes < > I don't know if the edk2 HTTP implementation intends to support HAproxy (or > any similar reverse proxy setup). I suggest filing an upstream bug report at > <MdePkg/Include/IndustryStandard/Http11.h>, and CC'ing the NetworkPkg > maintainer (Maciej Rabeda <maciej.rabeda.com>). > > FWIW, I have never even tested the HTTP server of dnsmasq (even without > HAproxy). The RHEL[78] manuals describe httpd (Apache); I've tested those > (not with HAproxy though). This could be totally uncharted territory (hence > my suggestion to file an upstream bug). My HTTP server is nginx with HAproxy in front as reverse proxy. The client has no knowledge if there's a reverse proxy, so I think it shouldn't matter. > > Technically, I could of course file the upstream bug myself, but (a) I have > no reproducer, (b) more details / configs about the environment will almost > certainly be requested by the maintainer; it's best if those come directly > from the reporter. Sure, I'll open a bug upstream. Thanks. (In reply to Daniel Berrangé from comment #3) > (In reply to Juan Orti from comment #0) > > Description of problem: > > After adding a reverse proxy (HAproxy) to my EFI HTTP boot setup, my EFI VMs > > no longer boot with the error: > > > > ~~~ > > >>Start HTTP Boot over IPv4.... > > Station IP address is 192.168.5.141 > > > > URI: http://bootserver.lan/grub/shim.efi > > File Size: 1210776 Bytes > > Downloading...100%Failed to get Content-Lenght > > Invalid image > > Failed to read header: Unsupported > > Failed to load image: Unsupported > > start_imagee() returned Unsupported > > ~~~ > > Looking at these messages a second time, the "Downloading....100%" part > makes me think that ED2 has succesfully downloaded the shim.efi file, and > the error messages are coming from the next phase Oops, thanks for catching this; I missed it. > Indeed if i look at the shim source https://github.com/rhboot/shim/ then we > can find: > > Invalid image > Failed to read header: Unsupported > Failed to load image: Unsupported > start_imagee() returned Unsupported > > are all messages in the shim.c source file. > > The one I can't find is the "Failed to get Content-Lenght" message - > possibly that comes from UEFI services that shim calls into ? Both the string in question and the underlying bug are in shim; from commit 3d79bcb2651b ("Add the optional HTTPBoot support", 2016-09-06). The headers are scanned with the "strcmpa()" function [src/httpboot.c]: /* Check the length of the file */ for (i = 0; i < rx_message.HeaderCount; i++) { if (!strcmpa(rx_message.Headers[i].FieldName, (CHAR8 *)"Content-Length")) { *buf_size = ascii_to_int(rx_message.Headers[i].FieldValue); } } if (*buf_size == 0) { perror(L"Failed to get Content-Lenght\n"); goto error; } where "strcmpa()" is a gnu-efi function [lib/str.c]: UINTN strcmpa ( IN CONST CHAR8 *s1, IN CONST CHAR8 *s2 ) // compare strings { while (*s1) { if (*s1 != *s2) { break; } s1 += 1; s2 += 1; } return *s1 - *s2; } So strcmpa() is case-sensitive, and therefore it is not suitable for the header lookup in shim. This BZ should be reassigned to the "shim" component. |