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

2.0.13 micro-2.0.13-linux-arm.tar.gz may have been tampered with #3322

Open
Eeems opened this issue May 29, 2024 · 12 comments
Open

2.0.13 micro-2.0.13-linux-arm.tar.gz may have been tampered with #3322

Eeems opened this issue May 29, 2024 · 12 comments

Comments

@Eeems
Copy link

Eeems commented May 29, 2024

I help maintain a repository of software for the remarkable tablet. One of the packages in this repository is micro. We validate source files against a sh256sum to make sure they have not been modified or tampered with. As part of some work I've been doing on our build system, I've discovered that micro-2.0.13-linux-arm.tar.gz no longer matches the sha256sum we captured on 2023-12-08. The last time we built the micro package and know for sure that the file still matched the sha256sum was 2023-12-26.

Upon inspecting the release artifacts as reported by github, it appears that the release was created at 2023-10-21T22:38:29Z and that this artifact was replaced at 2023-10-22T20:54:07Z. Both of these timestamps are before the work I did on packaging micro 2.0.13 in toltec.

You can find the URL and sha256sum used by our build system here: https://github.com/toltec-dev/toltec/blob/b19ecf36f92d6cf2b8e900bcc2896314e51bb816/package/micro/package#L14-L15
The new sha256sum that the file appears to have is adb9cf644354a5c85819db40e1a427f0f4951b172597bbcd3ef94ecc4a8c4b75.

I would appreciate some help confirming that this file has not been tampered with, and if it has, getting it replaced with a fresh-unmodified version that we can trust.

@Andriamanitra
Copy link
Contributor

I believe the change in checksum is related to https://github.com/orgs/community/discussions/45830

@Eeems
Copy link
Author

Eeems commented May 29, 2024

I believe the change in checksum is related to https://github.com/orgs/community/discussions/45830

The file downloaded is a release artifact, not the automated .tar.gz source archive that github provides. The timeline for the linked discussion also is from January 2023 with the latest comment being on May 22 2023, which is months before I started updating the micro package in toltec. According to the accepted answer, the change was also reverted.

@dmaluka
Copy link
Collaborator

dmaluka commented May 29, 2024

Do you happen to have the old version of this file (with cbbed4e69567871462464049646dc11fdad8b8c75fde5d75856068c2cfbd2d38 sha256sum)?

@dmaluka
Copy link
Collaborator

dmaluka commented May 29, 2024

Github shows that this file was uploaded on Oct 22, 2023: https://github.com/zyedidia/micro/releases/tag/v2.0.13

If the file was replaced later and it wasn't a Github issue, Github would probably show a later date?

@Eeems
Copy link
Author

Eeems commented May 29, 2024

Do you happen to have the old version of this file (with cbbed4e69567871462464049646dc11fdad8b8c75fde5d75856068c2cfbd2d38 sha256sum)?

I do not.

Github shows that this file was uploaded on Oct 22, 2023: https://github.com/zyedidia/micro/releases/tag/v2.0.13

If the file was replaced later and it wasn't a Github issue, Github would probably show a later date?

I would expect Github to show a later date. Since it's not, and the sha256sum has changed, I find this concerning.

As this is a potential security issue, I would highly recommend reviewing the contents of the file and seeing if they match what you expect. If you have reproducible builds, it would be easy to just compare against another build. If your builds are not reproducible, it would probably be fine to just replace the artifact with a new, known-good, build. Then you can inspect the contents closer to see what could have changed without concern that users might be downloading and running something malicious.

If you can confirm that the release artifact has been tampered with, it would likely mean that you'll need to report this to Github to investigate further.

@dmaluka
Copy link
Collaborator

dmaluka commented May 30, 2024

There is also a possibility that your build system calculated the checksum incorrectly.

Which is why it would be helpful to have the original file. Otherwise there is really no evidence.

@zyedidia Do you know if it is possible to recreate exactly the same build as https://github.com/zyedidia/micro/releases/tag/v2.0.13 ? What exact version of Go was it compiled with?

@Eeems
Copy link
Author

Eeems commented May 30, 2024

There is also a possibility that your build system calculated the checksum incorrectly.

Since I get the sha256sum by hand and feed it into the build system, this would not be the case. That would also not explain why it worked before, but not now if the source file has not changed.

You can review the code if you'd like: https://github.com/toltec-dev/toltec/blob/b19ecf36f92d6cf2b8e900bcc2896314e51bb816/scripts/toltec/util.py#L46-L56

This code has not changed since 2021-07-21 and uses python's hashlib to generate the sha256sum. When generating the hash by hand, I use sha256sum provided by coreutils.

Which is why it would be helpful to have the original file. Otherwise there is really no evidence.

I wouldn't say we have no evidence. The evidence we do have, unfortunately, just takes more digging.

I know that when I pulled down the file to manually get the sha256sum, it was the value we put into the package recipe in toltec. This was then validated as part of the build multiple times on the PR. Once when merged into our testing branch. Three times as part of the process of prepping to release to our stable branch, and then one more time as part of the release to stable.

Every single time it's built and validated, it is pulled fresh from github. No caching is done on our end of source files.

When pulling down the file now for either our build process to validate, or to manually get the sha256sum on one of my machines, the file has a different hash.

As a sanity check, I will trigger a rebuild of the package using the same build system as we had at the time. Due to github's runners changing, there might be a difference. This part of the process happens on the runner, and not inside a versioned container, which is where the build itself happens. You can find the in-progress build here: toltec-dev/toltec#869

@niten94
Copy link
Contributor

niten94 commented May 30, 2024

There was this issue related with the ARM version of micro v2.0.13 but there is a comment that @zyedidia posted around the time when the file was replaced: #2986 (comment)

The file was replaced at 2023-10-22 so I do not know why the SHA256 sum was matched at 2023-12-26.

@Eeems
Copy link
Author

Eeems commented May 30, 2024

I'm currently struggling with getting the sanity check of the build process to work, as changes to the github runners seem to have broken things. Trying to fix any of it may invalidate the testing I'm doing as well.

There was this issue related with the ARM version of micro v2.0.13 but there is a comment that @zyedidia posted around the time when the file was replaced: #2986 (comment)

The file was replaced at 2023-10-22 so I do not know why the SHA256 sum was matched at 2023-12-26.

It's good to know that the dates are as we expect, and that there was a reason it was updated.

I have a vague recollection of seeing that the artifact date was newer than the rest of the ones on the release, and then finding that issue explaining why it was at the time, so I don't necessarily think it's related to why the hash is different.

@Eeems
Copy link
Author

Eeems commented May 30, 2024

Well, I have figured out why the build probably worked despite the sha256sum not matching. Here is the code that raises the error if it's not valid:
https://github.com/toltec-dev/toltec/blob/b19ecf36f92d6cf2b8e900bcc2896314e51bb816/scripts/toltec/builder.py#L222

            if source.checksum not in ("SKIP", source.checksum):
                raise BuildError(
                    f"Invalid checksum for source file {source.url}:\n"
                    f"  expected {source.checksum}\n"
                    f"  actual   {file_sha}"
                )

This will never fail, as source.checksum will always be the same as itself. I know that I've had errors raised due to the hash not matching what is expected. Which is very confusing, as I now have no idea how that would ever actually happen.

Why we are encountering this now is because I'm working on replacing part of our build process with a tool that has been moved out of the tree into its own repository. This repo doesn't have this bug: https://github.com/toltec-dev/build/blob/a4952e3103a89d880fc4da51cbb737a10ee127cb/toltec/builder.py#L246-L251

How I got a different hash when I manually pulled it to update the recipe is still not clear to me.

I personally am now comfortable closing this issue as invalid at this point, as the evidence I thought I had is now invalid. I'll leave that up to the team here how they want to handle it, though. It might be worth adding some mechanism to add hashing your release artifacts in a way that you can verify that they haven't been modified, especially since the installation script downloads them without validating that it hasn't been modified by a man-in-the-middle, or another supply chain attack.

@dmaluka
Copy link
Collaborator

dmaluka commented May 30, 2024

I'll leave that up to the team here how they want to handle it, though. It might be worth adding some mechanism to add hashing your release artifacts in a way that you can verify that they haven't been modified, especially since the installation script downloads them without validating that it hasn't been modified by a man-in-the-middle, or another supply chain attack.

Yeah, maybe we should consider uploading tarball checksums, along with the tarballs themselves. I don't know how strong a security measure it would be (ultimately we still have to trust Github), but without remembering checksums somewhere we are not able to perform even basic integrity checks, regardless of security.

@zyedidia what do you think? (You are still the only one with the needed access rights to implement such things.)

@JoeKar
Copy link
Collaborator

JoeKar commented Jun 12, 2024

Yeah, maybe we should consider uploading tarball checksums, along with the tarballs themselves.

Will be considered within #3334 (changes still local so far) as proof of concept. In case the new approach works (incl. publish) then it will be done for releases too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants