HomeACcessoryKid / life-cycle-manager

Initial install, WiFi settings and over the air firmware upgrades for any esp-open-rtos repository on GitHub
Apache License 2.0
60 stars 11 forks source link

Broken HTTP header and Finally answer from GitHub #23

Closed HomeACcessoryKid closed 3 years ago

HomeACcessoryKid commented 4 years ago

The first and a long time the only standardised message from GitHub 'support staff'

Malcolm (GitHub Developer Support) Feb 29, 5:16 PM UTC

Hello HacK,

Thank you for writing in to GitHub! We appreciate the feedback, I will go ahead and pass this information over to the right teams. Please write back if you have any additional questions.

All the best, GitHub Staff


HacK Feb 29, 3:02 PM UTC

Since today GitHub has changed their HTTP headers and has chosen to use lowercase instead of the normal capital first letter. This results in the header "Location: "(RFC2616:14.30) to have become "location: " MANY amateur coders have no code in place for case-insensitive parsing because they are not aware! While I have now found out that the RFC allows for the capitalization to be insensitive, the reasonable thing to do for GitHub is to stick to the text as presented in the RFC and use "Location: "

In my case there are over 20.000 devices out there in the world that depend on my code to update via OTA HomeACcessoryKid/life-cycle-manager and they cannot anymore. Of course the self-update of OTA will become smarter, but for the sake of helping the community I really really hope that GitHub will fix this back to the old RFC suggested format. It doesn't hurt them and helps amateurs around the world.

Thanks, HacK

PS. this has also been posted to the forum for API but I feel there it is a bit out of place... https://github.community/t5/GitHub-API-Development-and/GitHub-changed-the-capitalisation-of-the-HTTP-headers-and-OTA/m-p/48247 PPS. I hope this doesn't happen to AWS as well, but for now it is OK PPPS. There are still 'normal' headers in the reply, just not the top ones anymore HTTP/1.1 302 Found date: Sat, 29 Feb 2020 14:47:10 GMT content-type: text/html; charset=utf-8 server: GitHub.com status: 302 Found vary: X-PJAX, Accept-Encoding, Accept, X-Requested-With location: https://github.com/HomeACcessoryKid/life-cycle-manager/releases/tag/1.0.0 cache-control: no-cache Age: 0 Set-Cookie: Set-Cookie: logged_in=no; Path=/; Domain=github.com; Expires=Mon, 01 Mar 2021 14:47:10 GMT; HttpOnly; Secure Content-Length: 139 X-GitHub-Request-Id: DF95:2F739:3CBF7FE:57D6ABE:5E5A796E

HomeACcessoryKid commented 4 years ago

LCM 2.0.1 is now the latest release. The readme has instructions to upgrade using solder iron. No news from GitHub

danielhelmstedt commented 4 years ago

Is there any way around this using a transparent proxy (Charles) to rewrite the headers?

HomeACcessoryKid commented 4 years ago

Thanks to @tonysprenk we finally got a response from GitHub. And I learned something new, that with HTTP/2 the headers are expected to be lowercase always. This means that GitHub is making their infrastructure ready for the future and I agree it makes sense for them.

So, I accept our loss and that it was my lack of knowing that a HTTP header has to be case insensitive that caused all this.

Case closed, but I will leave the issue open for people to find it easily

HomeACcessoryKid commented 3 years ago

NEW OUTAGE: (and an unexpected fix!)
About one year after this issue happened, GitHub has changed something again. They are not hosted on AWS anymore and have a new hostname to serve the assets. The issue is that they used a certificate that authenticates another hostname. The issue has been sent to GitHub Support and waiting for a response

BUT , the switchover actually brought the fix for the issue we started with, one year ago. The header: location is once more with a capital 'L' Location: https://github-releases.githubusercontent.com/...

So, if you still have a frozen device out there, once the certificate has been fixed, I predict it will come back to life...

Stay Tuned

peros550 commented 3 years ago

Awesome!!!!

Kienz commented 3 years ago

Should 2.1.0 be released official that it does work? Actually it tries to download the version 2.0.2 and it doesn't work.

GET /HomeACcessoryKid/life-cycle-manager/releases/download/2.0.2/otaboot.bin.sig HTTP/1.1
Host: github.com

--- ota_connect LocalPort=e1a6 DNS IP:140.82.121.3 local..OK remote..OK SSL..OK set_fd to github.com port 443..failed, return [-0x1]
wolfSSL_send error = -322

--- ota_boot...1
--- running ota-main software
--- ota_compare 2.0.2 with 1.0.0=1
--- ota_get_hash
--- ota_get_file_ex
GET /HomeACcessoryKid/life-cycle-manager/releases/download/2.0.2/otaboot.bin.sig HTTP/1.1
Host: github.com

--- ota_connect LocalPort=e1a6 DNS IP:140.82.121.3 local..OK remote..OK SSL..OK set_fd to github.com port 443..OK
sent OK
HTTP returns 302
--- ota_connect LocalPort=e1a7 DNS IP:185.199.108.154 local..OK remote..OK
sent OK
HTTP returns 302
--- ota_connect LocalPort=e1a7 DNS IP:185.199.108.154 local..OK remote..OK SSL..OK set_fd to github-releases.githubusercontent.com port 443..OK SSL..OK set_fd to github-releases.githubusercontent.com port 443..failed, return [-0x1]
wolfSSL_send error = -322

--- ota_get_file
--- ota_get_file_ex
GET /HomeACcessoryKid/life-cycle-manager/releases/download/2.0.2/otaboot.bin HTTP/1.1
Host: github.com

--- ota_connect LocalPort=e1a8 DNS IP:140.82.121.3 local..OK remote..failed, return [-0x1]
wolfSSL_send error = -322
HomeACcessoryKid commented 3 years ago

error -322 is the name mismatch. We depend on GitHub to fix their mistake... Once 2.1.0 which is a pre-release is tested AND works fine, it can be promoted to the latest release. BUT, it is likely that it will not be needed, and then I will not publish it, but remove it. MAYBE I create a 2.1.1 if there is an unforeseen issue left...

dbussink commented 3 years ago

About one year after this issue happened, GitHub has changed something again. They are not hosted on AWS anymore and have a new hostname to serve the assets. The issue is that they used a certificate that authenticates another hostname. The issue has been sent to GitHub Support and waiting for a response

We have moved asset downloads to our CDN which as greatly improved download speeds for a lot of users (sometimes up to 10x faster release downloads). This is the reason for the move to a different domain.

One thing here though is that our CDN setup requires SNI. SNI is something that has been supported by the clients we commonly see that download releases for a very long time. It looks like SNI support is not enabled in the WolfSSL build here though and not used when downloading an asset. These days SNI is really a requirement, there's a lot of the internet that doesn't work without it. We also have a number of other GitHub domains where we already require it with no issues so far until this one was reported to us.

There's a few things we can offer here from our side. First of all, I've opened a pull request with what I think is the code needed to add SNI support to this problem in https://github.com/HomeACcessoryKid/life-cycle-manager/pull/28.

Second, as part of this rollout what we can do is opt-out this repository temporarily from the new CDN delivery, so that the new release can be distributed with SNI support for the downloads. It means that the files will be served through the legacy S3 bucket that was used before.

I see that with https://github.com/HomeACcessoryKid/life-cycle-manager/commit/7dac5384396c628118211f65e545db2781157b11 you're trying to track the CA(s) that we use? I would recommend not depending on the specific CAs for better future resilience and to use a more generic list of root CAs like the ones that Mozilla provides. The current approach looks a bit like emulating https://developer.mozilla.org/en-US/docs/Web/HTTP/Public_Key_Pinning, which is something that was deprecated / removed from browsers due to various issues and the brittleness of it.

What is the kind of minimum time frame here needed for this opt-out? Would a week suffice? Or would you need significantly more time? If you let me know when to enable the temporary opt-out, I can do that on our side and release assets for this repository would be served through S3 again.

HomeACcessoryKid commented 3 years ago

First of all, thanks for reaching out via this channel, much appreciated.

While in general all the advice you give is good, the challenge is running this in a microcontroller with limited ram. It took me an effort to squeeze the tls1.2 support in and e.g. had to use Content-Range to not collect more than 4k at a time, else I ran out of RAM. Loading a big list of CA's is not going to fit, I think.

Supporting SNI is something I would be open to, if it compiles and still works which I assume it will.

The offer to temporary exclude us from the SNI requirement is good but be aware of the nature of how heavy it is to perform an update on dozens of devices, as well as the community might not be aware of this immediately. A week seems way too short. I would sooner say two month or six months.

I wonder however if the subjectAltName cannot be extended on the no-SNI certificate? That would be a lot less trouble and probably can stay there for a year or forever. I do promise to apply SNI support (if it works) so that we tackle the issue at the root.

Thanks, HacK

HomeACcessoryKid commented 3 years ago

One more thing ;-) Using you new infrastructure would make some people very happy because it would unblock their devices that depended on the Uppercase L in the Location: header So, falling back to S3, as reasonable as it seems is the lesser option.

dbussink commented 3 years ago

While in general all the advice you give is good, the challenge is running this in a microcontroller with limited ram. It took me an effort to squeeze the tls1.2 support in and e.g. had to use Content-Range to not collect more than 4k at a time, else I ran out of RAM. Loading a big list of CA's is not going to fit, I think.

Ah yeah, that's not a lot of headroom indeed. Looking at the certificate bundle size on a recent Debian system I have it looks like 200k, but that might then already be too big? I don't really have a good answer here then, except for that it might happen at some point that the certificate provider is switched which means there might be an issue then.

I wonder however if the subjectAltName cannot be extended on the no-SNI certificate? That would be a lot less trouble and probably can stay there for a year or forever.

This isn't trivial for our setup to change, so I'm afraid this isn't going to happen really. The answer is that SNI should be added and also why I opened https://github.com/HomeACcessoryKid/life-cycle-manager/pull/28 so that it's robust for the future as well.

So, falling back to S3, as reasonable as it seems is the lesser option.

The header issue is entirely separate from this. It would still see a capital L also with the opt-out to temporarily fall back to S3.

A week seems way too short. I would sooner say two month or six months.

How far would you get with a month? Is there a general way for you to communicate with users or does it depend on if folks check things like this repository? I guess there is no auto update that checks if something is available at some interval or is there?

HomeACcessoryKid commented 3 years ago

While in general all the advice you give is good, the challenge is running this in a microcontroller with limited ram. It took me an effort to squeeze the tls1.2 support in and e.g. had to use Content-Range to not collect more than 4k at a time, else I ran out of RAM. Loading a big list of CA's is not going to fit, I think.

Ah yeah, that's not a lot of headroom indeed. Looking at the certificate bundle size on a recent Debian system I have it looks like 200k, but that might then already be too big? I don't really have a good answer here then, except for that it might happen at some point that the certificate provider is switched which means there might be an issue then.

HAHAHA When I start with HelloWorld, i have 32k RAM available ;-) I now spend 3k on certificates...

I wonder however if the subjectAltName cannot be extended on the no-SNI certificate? That would be a lot less trouble and probably can stay there for a year or forever.

This isn't trivial for our setup to change, so I'm afraid this isn't going to happen really. The answer is that SNI should be added and also why I opened #28 so that it's robust for the future as well.

That settles it then for falling back to S3. I just compiled and did a quick test with SNI. It works fine. And your server is indeed A LOT faster!

So, falling back to S3, as reasonable as it seems is the lesser option.

The header issue is entirely separate from this. It would still see a capital L also with the opt-out to temporarily fall back to S3.

We'll see about that, but it will be what it is...

A week seems way too short. I would sooner say two month or six months.

How far would you get with a month? Is there a general way for you to communicate with users or does it depend on if folks check things like this repository? I guess there is no auto update that checks if something is available at some interval or is there?

No, it is grapevine, and one way communication. The few people subscribed to this thread will know if they bother to read their mail, but others might hear indirectly or walk in here in time. I think 2 months is a reasonable minimum.

dbussink commented 3 years ago

I just compiled and did a quick test with SNI. It works fine. And your server is indeed A LOT faster!

Good to know! And yeah, the goal here has been explicitly for us to improve delivery a lot for our customers, especially those further away from the US.

We'll see about that, but it will be what it is...

Opted this repository into the legacy S3 download behavior and this is what it looks like (I cut some noise from the log):

curl -O -L -v --http1.1 https://github.com/HomeACcessoryKid/life-cycle-manager/releases/download/2.1.0/otaboot.bin
*   Trying 140.82.121.3...
* TCP_NODELAY set
* Expire in 200 ms for 4 (transfer 0x558364df6f90)
* Connected to github.com (140.82.121.3) port 443 (#0)
* ALPN, offering http/1.1
* SSL connection using TLSv1.3 / TLS_AES_128_GCM_SHA256
* ALPN, server accepted to use http/1.1
* Server certificate:
*  subject: C=US; ST=California; L=San Francisco; O=GitHub, Inc.; CN=github.com
*  start date: May  5 00:00:00 2020 GMT
*  expire date: May 10 12:00:00 2022 GMT
*  subjectAltName: host "github.com" matched cert's "github.com"
*  issuer: C=US; O=DigiCert Inc; OU=www.digicert.com; CN=DigiCert SHA2 High Assurance Server CA
*  SSL certificate verify ok.
} [5 bytes data]
> GET /HomeACcessoryKid/life-cycle-manager/releases/download/2.1.0/otaboot.bin HTTP/1.1
> Host: github.com
> User-Agent: curl/7.64.0
> Accept: */*
> 
< HTTP/1.1 302 Found
< Date: Wed, 03 Feb 2021 09:55:45 GMT
< Content-Type: text/html; charset=utf-8
< Server: GitHub.com
< Status: 302 Found
< Vary: X-PJAX, Accept-Encoding, Accept, X-Requested-With, Accept-Encoding
< Location: https://github-production-release-asset-2e65be.s3.amazonaws.com/127610504/392b6c00-6307-11eb-91ec-6c638e3038db?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=<snip>
< Cache-Control: no-cache
< Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
< X-Frame-Options: deny
< X-Content-Type-Options: nosniff
< X-XSS-Protection: 1; mode=block
< Referrer-Policy: no-referrer-when-downgrade
< Expect-CT: max-age=2592000, report-uri="https://api.github.com/_private/browser/errors"
{ [5 bytes data]
< Content-Security-Policy: default-src 'none'; base-uri 'self'; block-all-mixed-content; connect-src 'self' uploads.github.com www.githubstatus.com collector.githubapp.com api.github.com github-cloud.s3.amazonaws.com github-production-repository-file-5c1aeb.s3.amazonaws.com github-production-upload-manifest-file-7fdce7.s3.amazonaws.com github-production-user-asset-6210df.s3.amazonaws.com cdn.optimizely.com logx.optimizely.com/v1/events wss://alive.github.com online.visualstudio.com/api/v1/locations; font-src github.githubassets.com; form-action 'self' github.com gist.github.com; frame-ancestors 'none'; frame-src render.githubusercontent.com; img-src 'self' data: github.githubassets.com identicons.github.com collector.githubapp.com github-cloud.s3.amazonaws.com *.githubusercontent.com; manifest-src 'self'; media-src 'none'; script-src github.githubassets.com; style-src 'unsafe-inline' github.githubassets.com; worker-src github.com/socket-worker-5029ae85.js gist.github.com/socket-worker-5029ae85.js
{ [5 bytes data]
< Set-Cookie: logged_in=no; Path=/; Domain=github.com; Expires=Thu, 03 Feb 2022 09:57:30 GMT; HttpOnly; Secure; SameSite=Lax
< Content-Length: 637
< X-GitHub-Request-Id: CDDA:2F4F:73991B:925185:601A738A
< 
* Ignoring the response-body
{ [637 bytes data]
100   637  100   637    0     0   5096      0 --:--:-- --:--:-- --:--:--  5137
* Connection #0 to host github.com left intact
* Issue another request to this URL: 'https://github-production-release-asset-2e65be.s3.amazonaws.com/127610504/392b6c00-6307-11eb-91ec-6c638e3038db?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=<snip>'
*   Trying 52.217.13.204...
* TCP_NODELAY set
* Expire in 200 ms for 4 (transfer 0x558364df6f90)
* Connected to github-production-release-asset-2e65be.s3.amazonaws.com (52.217.13.204) port 443 (#1)
* ALPN, offering http/1.1
* SSL connection using TLSv1.2 / ECDHE-RSA-AES128-GCM-SHA256
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: C=US; ST=Washington; L=Seattle; O=Amazon.com, Inc.; CN=*.s3.amazonaws.com
*  start date: Nov  9 00:00:00 2019 GMT
*  expire date: Mar 12 12:00:00 2021 GMT
*  subjectAltName: host "github-production-release-asset-2e65be.s3.amazonaws.com" matched cert's "*.s3.amazonaws.com"
*  issuer: C=US; O=DigiCert Inc; OU=www.digicert.com; CN=DigiCert Baltimore CA-2 G2
*  SSL certificate verify ok.
} [5 bytes data]
> GET /127610504/392b6c00-6307-11eb-91ec-6c638e3038db?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=<snip> HTTP/1.1
> Host: github-production-release-asset-2e65be.s3.amazonaws.com
> User-Agent: curl/7.64.0
> Accept: */*
> 
{ [5 bytes data]
< HTTP/1.1 200 OK
< Date: Wed, 03 Feb 2021 09:57:32 GMT
< Last-Modified: Sat, 30 Jan 2021 13:27:05 GMT
< ETag: "8e127047f8d5e97d68bf7c9c1af5ad62"
< Content-Disposition: attachment; filename=otaboot.bin
< Accept-Ranges: bytes
< Content-Type: application/octet-stream
< Content-Length: 436688
< Server: AmazonS3
< 
{ [5 bytes data]
100  426k  100  426k    0     0   218k      0  0:00:01  0:00:01 --:--:--  318k
* Connection #1 to host github-production-release-asset-2e65be.s3.amazonaws.com left intact

As you can see here, there's also the Location header being used with HTTP/1.1 (testing with a recent curl will default to HTTP/2 which always enforces lowercase headers as part of the spec).

I think 2 months is a reasonable minimum.

Alright, let's keep it at 2 months and I'll slate the cleanup for the legacy parts on our side for early April then.

HomeACcessoryKid commented 3 years ago

Thanks a lot, this is great news! I can confirm that both issues are now solved - at least till April - using the original AWS S3 server. We will work to spread the news to as many as possible.

--- ota_set_verify...ON
certs size: 2628
TIME: Wed Feb  3 10:11:42 2021
--- ota_get_hash
--- ota_get_file_ex
GET /HomeACcessoryKid/life-cycle-manager/releases/download/2.0.2/certs.sector.sig HTTP/1.1
Host: github.com

--- ota_connect LocalPort=f7f5 DNS IP:140.82.121.4 local..OK remote..OK SSL..OK set_fd to github.com port 443..OK
sent OK
HTTP/1.1 302 Found
Date: Wed, 03 Feb 2021 10:11:38 GMT
Content-Type: text/html; charset=utf-8
Server: GitHub.com
Status: 302 Found
Vary: X-PJAX, Accept-Encoding, Accept, X-Requested-With, Accept-Encoding
Location: https://github-production-release-asset-2e65be.s3.amazonaws.com/127610504/a56e2200-8170-11ea-9a92-b40d9388168c?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20210203%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20210203T101138Z&X-Amz-Expires=300&X-Amz-Signature=4fcfc60e0b2835442c29d4927b5016b85c7c2a1a8a49e8eb13b7a9d83c8f8ce8&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=127610504&response-content-disposition=attachment%3B%20filename%3Dcerts.sector.sig&response-content-type=application%2Foctet-stream
HTTP returns 302
--- ota_connect LocalPort=f7f6 DNS IP:52.216.129.147 local..OK remote..OK SSL..OK set_fd to github-production-release-asset-2e65be.s3.amazonaws.com port 443..OK
send request......OK
8c1d63...c6ef10  so far collected 155 bytes
--- ota_boot...1
--- running ota-main software
RavenSystem commented 3 years ago

@dbussink many thanks for your collaboration and help!! Thousands of users use this firmware to install and update custom IoT devices, and many of them have old devices that will can be update now.

@HomeACcessoryKid many thanks to you too for update firmware and keep it up to date with GitHub' systems.

dbussink commented 3 years ago

@HomeACcessoryKid I'm not entirely sure if this will affect things or not, but AWS is also moving certificate provider, away from Digicert to their own: https://forums.aws.amazon.com/ann.jspa?annID=7541. That is starting March 23, 2021. If the new root is not part of the images yet, I'd imagine that would also break the pointing still at S3 workaround a bit earlier than expected?

HomeACcessoryKid commented 3 years ago

Thanks Dirkjan for making us aware of this.

Actually, LCM is able to handle this situation. If you look at the flow diagram in the readme, top-center, you see that we can download new CA certs if needed. So in that respect, the public-key-pinning is a flexible type.

It looks like they will have 4 layers of certificates which might break my device merely by the amount of memory to load the intermediates in RAM...

 0 s:/CN=*.s3.eu-west-3.amazonaws.com
   i:/C=US/O=Amazon/OU=Server CA 1B/CN=Amazon
 1 s:/C=US/O=Amazon/OU=Server CA 1B/CN=Amazon
   i:/C=US/O=Amazon/CN=Amazon Root CA 1
 2 s:/C=US/O=Amazon/CN=Amazon Root CA 1
   i:/C=US/ST=Arizona/L=Scottsdale/O=Starfield Technologies, Inc./CN=Starfield Services Root Certificate Authority - G2
 3 s:/C=US/ST=Arizona/L=Scottsdale/O=Starfield Technologies, Inc./CN=Starfield Services Root Certificate Authority - G2
   i:/C=US/O=Starfield Technologies, Inc./OU=Starfield Class 2 Certification Authority

But I will load the certificate /C=US/O=Amazon/CN=Amazon Root CA 1 which is a proper CA by itself and that will solve that issue.

Considering I was planning a GUI update version anyhow, I will add this certificate and we should survive this seamlessly. And after that we will enjoy the high speed GitHub servers ;-)

HomeACcessoryKid commented 3 years ago

One setback, this certificate is 1648 bytes which is 300 bytes bigger than the space left in the certs.sector file

This means that I can only update it after amazon switched and the old certificate is useless anyway

Maybe that is too much trouble, but if people still want me to do this after March 23th, then please request it here and I will prepare it for that extra week.

What do you say?

RavenSystem commented 3 years ago

Hi @dbussink,

Please, would be possible to remove new Link: header introduced today or move Location: header at top in my repositories? Now there is an issue with this because headers use too much length and my IoT firmware was not ready to manage that. I am ready to release a fixed version, but now thousand of IoT devices that use my firmware are stuck.

I hope it will be possible. Many thanks.

Repositories: RavenSystem/haa RavenSystem/haabeta RavenSystem/haa_ota RavenSystem/ravencore

HTTP/1.1 302 Found
Date: Tue, 16 Feb 2021 22:14:48 GMT
Content-Type: text/html; charset=utf-8
Server: GitHub.com
Link: <https://github.githubassets.com/assets/chunk-frameworks-aaac5e7e.js>; integrity=sha512-qqxefjEZzhP8x0ed+F3sDEdSogyDs+0ThK6EYXtqye9PJ26t5C996Q9EuRBr+296TAbc1rimsOSeZjZXJac1cA==; rel=preload; crossorigin=anonymous; as=script,<https://github.githubassets.com/assets/chunk-vendor-6594a208.js>; integrity=sha512-ZZSiCHJ85gkTIyggKfuBXsl3a7e+GPhw6iUguJ6eki2dXkGnOFBpQlUUG+gcTSJzgE8ii1cYyM7aLYXZeSeDbA==; rel=preload; crossorigin=anonymous; as=script,<https://github.githubassets.com/assets/behaviors-bf28db4d.js>; integrity=sha512-vyjbTdEGTiemoiT41KyVSh3+NxJv285wa1qlccyGrTsQsAAvhBSUrT0t1LZbJVEfccSTx+zLAyTekiljOX2wKA==; rel=preload; crossorigin=anonymous; as=script,<https://github.githubassets.com/assets/environment-f0adafbf.js>; integrity=sha512-8K2vvwbW+6H27Nad5ydg8PA2/aMD/LKq+EiK9s0U0hhVZxCI2tWBsYk9beAtisRw2j+Or5k2/F+6dk02nmj/PA==; rel=preload; crossorigin=anonymous; as=script
Vary: X-PJAX, Accept-Encoding, Accept, X-Requested-With, Accept-Encoding
Location: https://github-releases.githubusercontent.com/166993980/0d6ebb80-63c5-11eb-9470-ca104cafc50c?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20210216%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20210216T221448Z&X-Amz-Expires=300&X-Amz-Signature=981fe65553fb61d6e7ebc3bdc3facd9a38d9b95d5ba490f93f7cb1bb94395883&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=166993980&response-content-disposition=attachment%3B%20filename%3Dotamain.bin&response-content-type=application%2Foctet-stream
Cache-Control: no-cache
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
X-Frame-Options: deny
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
Referrer-Policy: no-referrer-when-downgrade
Expect-CT: max-age=2592000, report-uri="https://api.github.com/_private/browser/errors"
Content-Security-Policy: default-src 'none'; base-uri 'self'; block-all-mixed-content; connect-src 'self' uploads.github.com www.githubstatus.com collector.githubapp.com api.github.com github-cloud.s3.amazonaws.com github-production-repository-file-5c1aeb.s3.amazonaws.com github-production-upload-manifest-file-7fdce7.s3.amazonaws.com github-production-user-asset-6210df.s3.amazonaws.com cdn.optimizely.com logx.optimizely.com/v1/events wss://alive.github.com online.visualstudio.com/api/v1/locations; font-src github.githubassets.com; form-action 'self' github.com gist.github.com; frame-ancestors 'none'; frame-src render.githubusercontent.com; img-src 'self' data: github.githubassets.com identicons.github.com collector.githubapp.com github-cloud.s3.amazonaws.com user-images.githubusercontent.com/ *.githubuser
HomeACcessoryKid commented 3 years ago

@dbussink, this also broke the latest version of LCM ! I think that using AWS is not going to make a difference, since this is caused by the server adding the Link: header line which is 886 bytes just by itself. We can only escape from this situation if the server places the Link: header AFTER the Location: header line.

Could you comment on this? Can this be corrected by Github server team?

Thanks in advance!!!

dbussink commented 3 years ago

I think that using AWS is not going to make a difference

@HomeACcessoryKid is right here, this has nothing to do with the new releases endpoint, but with a separate change.

It looks like we're able to change this on our side, so this should be updated in the next few hours. I'll let you all know here once it's out. I do would like to call out that we can't guarantee that headers will be in any specific order or that we send things within a certain buffer size forever into the future. These pages are mainly targeted at browsers, so things like CSP headers in this case the Link headers will be something that can be added to / changed over time.

This means that I do strongly encourage that any client is robust to larger headers to adopt a method where it's possible to walk over the headers and read things more incrementally instead of having to read it all into one buffer at once. This also ensures that things here are future proof.

Another option would be to look at the API instead as the URL to point to. That will still redirect to the actual download file, but with the API we are generally more conservative with changes. https://docs.github.com/en/rest/reference/repos#get-a-release-asset would be the endpoint to get a release asset. I can't guarantee though for the API either that we'll never add new headers or that you'll never run into an issue like this there either.

HomeACcessoryKid commented 3 years ago

Thanks Dirkjan,

You are right about the strategy being not the best one and that using the api one would seem to be better We actually explored that route and it is even more broken with respect to small devices and memory limits. This is why we stayed with the current approach. I did suggest an api-light approach but there was never an answer from the GitHub side anymore. I suggest I share my phone-number in a private channel and we brainstorm a bit about this?

I wonder if I can collect headers as well in a chunked mode or maybe I need to revise the entire header processing altogether. That will take a bit of time since it is more fundamental than what I fixed early February.

Anyway, thanks once more for your speedy response and offer to (try to) fix this

dbussink commented 3 years ago

It looks like we're able to change this on our side, so this should be updated in the next few hours.

This change has rolled out so the Link headers are now no longer there.

I wonder if I can collect headers as well in a chunked mode or maybe I need to revise the entire header processing altogether. That will take a bit of time since it is more fundamental than what I fixed early February.

Yeah, a strategy like this would make sure that things are more robust into the future. Headers are always guaranteed to be line by line plus even though there's no limit to a single header line in the HTTP spec, in practice there are limits that you can rely on. Most browsers & HTTP servers limit things in the 4k - 8k range, but for Location redirects I suspect it will be robust also with a limit of up to say 1k (today it looks like the link is 531 bytes for the direct S3 link).

We actually explored that route and it is even more broken with respect to small devices and memory limits.

What are these specific issues? Is it mostly that it needs multiple API requests to achieve the same goals? Or is there something else? In general, API request are "simpler" and we do know that they also get used for some other embedded devices successfully.

I did suggest an api-light approach but there was never an answer from the GitHub side anymore. I suggest I share my phone-number in a private channel and we brainstorm a bit about this?

Adding new APIs always does come with a building and maintenance cost for us that isn't something that is always trivial. I can't really commit to anything here myself anyway, so I doubt it's super useful to dive much deeper into this at this point.

HomeACcessoryKid commented 3 years ago

Confirmed to have solved the issue! Many thanks!

About the API, it is based on multi layered output. If you request one layer, you get the content of all the layers inside of it included. This blows the payload beyond parsing sizes. I'll spend some time soon expanding that, but for now, THANKS!

RavenSystem commented 3 years ago

@dbussink I can confirm that OTA is working fine again. Many thanks!!

I will update HAA Installer system to download up to 8KB of data before searching for Location: header, because, as @HomeACcessoryKid says, GitHub API requires a lot of memory resources that are not available in our platforms.

dbussink commented 3 years ago

I don't really have a good answer here then, except for that it might happen at some point that the certificate provider is switched which means there might be an issue then.

I feel like I keep have to bring bad news here, but this looks to be happening sooner rather than later. We're using Digicert as our main certificate provider (which you already have a root stored for), but Digicert has relatively recently in November 2020 introduced new intermediates that they are using to sign new certificate requests.

New intermediates don't have to be a problem, but they are also using a new root for these intermediates. See also https://www.digicert.com/kb/digicert-root-certificates.htm under "November 2020 Intermediate Certificates". You'll see that those are all signed with "DigiCert Global Root CA" and not anymore with "DigiCert High Assurance EV Root CA" which is the one I think you have stored in this repository.

The current github.com certificate expires in August, but we're also looking at rolling out optimizations soon that will mean a new certificate on our end which then will be signed by the new root.

Hopefully you're either able to somehow update this out of band, or issue a new update soon then to add this new root before things will break for users here.

HomeACcessoryKid commented 3 years ago

Thanks Dirkjan. This is something the software is designed to handle without issues and even after the act. We appreciate the heads up, so we can align the new certificate push with the moment it becomes relevant. Not at home this weekend, so I will look at the details later, and most likely will combine it with the AWS update of 23 March

dbussink commented 3 years ago

This is something the software is designed to handle without issues and even after the act.

Alright, we're looking on our side then to roll out that change in the upcoming week, but it sounds like you all can handle it then.

HomeACcessoryKid commented 3 years ago

@dbussink Could you specify a bit more narrow the timing, and how does it relate to the AWS change on 23/3 and how does it relate to the LCM exception status on the old GitHub server? Will the old GH server ALSO use the new DigiCert root? Because if not, I would need to load 3 certs and that cannot fit in 4096 bytes. And till when do we intend to keep the exception in place? Trying to plot the path forward in more detail and wary of the pitfalls that could be out there.

dbussink commented 3 years ago

@dbussink Could you specify a bit more narrow the timing, and how does it relate to the AWS change on 23/3 and how does it relate to the LCM exception status on the old GitHub server?

We have shipped the certificate change last week, so both github.com & api.github.com have a different certificate root now, but only if the client supports ECDSA. I did look at the WolfSSL options in this repository which seems to imply that that support is enabled, but I'm not 100% sure.

This change is not related to the AWS change on March 23 and not related to the exception of routing things to the new CDN URL.

Will the old GH server ALSO use the new DigiCert root?

What do you mean by "old GH server"? Are you referencing here to the old S3 URL? That is independent from the change we have made and is all on the AWS side out of our control.

HomeACcessoryKid commented 3 years ago

The switch over to the final (for now, 2021Q1) situation happened somewhere around 31 march. I am closing this issue since it is all history now. The certificate change referred to above did not impact version 2.1.2, not sure why, but it is true. 2.1.2 is the relevant version for now.