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

Reproducible builds and strict dependency verification #30685

Closed
pboguslawski opened this issue Apr 24, 2024 · 13 comments
Closed

Reproducible builds and strict dependency verification #30685

pboguslawski opened this issue Apr 24, 2024 · 13 comments
Labels

Comments

@pboguslawski
Copy link
Contributor

Description

Gitea build process was found not reproducible. Please force strict verification against committed checksums for every dependency to avoid surprises and to better protect against supply chain attacks.

npi ci should probably be used instead npm install and break install process if any checksum mismatch occurs. Same for go modules (if works different now).

Related: #29326 (comment)

Gitea Version

1.21

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Compiled from sources.

Database

MySQL/MariaDB

@silverwind
Copy link
Member

silverwind commented Apr 24, 2024

As far as I'm aware npm i --no-save is equivalent to npm ci and will install exactly what is in the lockfile.

I've avoided npm ci in the past because of past bugs related to native modules, but I think we could give it another try, assuming it may have fixed those bugs by now.

@silverwind
Copy link
Member

silverwind commented Apr 24, 2024

One problem of npm ci is that it does a full reinstall every time, as opposed to npm i which is much faster after the first invocation:

$ npm i --no-save

up to date in 362ms
$ npm ci

added 1015 packages in 6s

@pboguslawski
Copy link
Contributor Author

Consider following npm manual and use only commands that are officially designed to guarantee reproducibility (is npm i --no-lock official npm ci synonym?).

If performance tricks are required that may cause reproducibility problems - should be probably separated from main building path, which should be 100% reproducible and verified (security).

@silverwind
Copy link
Member

At least for development, I prefer the fast variant, but we could likely make the build step trigger a make target that runs npm ci.

@silverwind
Copy link
Member

Thought I do think I stand correct that npm ci has many unresolved bugs, like npm/cli#4942 for example.

@silverwind
Copy link
Member

#30688

@silverwind
Copy link
Member

I'll close this. As far as I'm aware npm install --no-save is completely reproducible already. #30689 will fix the issue with running generate-image before and for main branch I'm thinking of creating a separate lockfile just for this target.

@pboguslawski
Copy link
Contributor Author

pboguslawski commented Apr 25, 2024

It seems that npm install --no-save with package name specified is not reproducible.

Let's simulate package version we're depending on was republished by attacker and now includes very bad stuff we definitely don't want to see on our servers:

$ npm --version
9.8.1

$ git clone https://github.com/go-gitea/gitea

$ cd gitea

$ git checkout 938b59199480d1b3730388982eb87d592e976849

$ sed -i 's|sha512-3tlv/dIP7FWvj3BsbHrGLJ6l/oKh1O3TcgBqMn+yyCagOxc23fyzDS6HypQbgxWbkpDnf52p1LuR4eWDQ/K9WQ==|sha512-jeC1axXpnb0/2nn/Y1LPuLdgXBLH7aDcHu4KEKfqw3CUhX7ZpfBSlPKyqXE6btIgEzfWtrX3/tyBCaCvXvMkOw==|g' package-lock.json

$ git diff
diff --git a/package-lock.json b/package-lock.json
index 88a0ca3b0..ab964c4bb 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -2949,7 +2949,7 @@
     "node_modules/colorette": {
       "version": "2.0.19",
       "resolved": "https://registry.npmjs.org/colorette/-/colorette-2.0.19.tgz",
-      "integrity": "sha512-3tlv/dIP7FWvj3BsbHrGLJ6l/oKh1O3TcgBqMn+yyCagOxc23fyzDS6HypQbgxWbkpDnf52p1LuR4eWDQ/K9WQ=="
+      "integrity": "sha512-jeC1axXpnb0/2nn/Y1LPuLdgXBLH7aDcHu4KEKfqw3CUhX7ZpfBSlPKyqXE6btIgEzfWtrX3/tyBCaCvXvMkOw=="
     },
     "node_modules/combined-stream": {
       "version": "1.0.8",

npm install --no-save with package name does not respect locked dependency from package-lock.json (no checksum err, silent version update):

$ npm cache clean --force; rm -rf node_modules

$ npm install --no-save colorette

$ npm list colorette
gitea@ /tmp/gitea
└─┬ [email protected]
  └── [email protected]

npm install --no-save without package name respects locked dependency and fails:

$ npm cache clean --force; rm -rf node_modules

$ npm install
[...]
npm ERR! code EINTEGRITY
npm ERR! sha512-jeC1axXpnb0/2nn/Y1LPuLdgXBLH7aDcHu4KEKfqw3CUhX7ZpfBSlPKyqXE6btIgEzfWtrX3/tyBCaCvXvMkOw== integrity checksum failed when using sha512: wanted sha512-jeC1axXpnb0/2nn/Y1LPuLdgXBLH7aDcHu4KEKfqw3CUhX7ZpfBSlPKyqXE6btIgEzfWtrX3/tyBCaCvXvMkOw== but got sha512-3tlv/dIP7FWvj3BsbHrGLJ6l/oKh1O3TcgBqMn+yyCagOxc23fyzDS6HypQbgxWbkpDnf52p1LuR4eWDQ/K9WQ==. (5062 bytes)
[...]

So #30688 seems better than #30689. And npm ci if possible would probably be the best solution to avoid similar surprises in the future when building for production.

@silverwind
Copy link
Member

silverwind commented Apr 25, 2024

You are right that the dependencies of generate-images are not reproducible. They' can't be because they are not in a lockfile.

I see generate-images currently like a "no warranties" kind of thing, not something we support to fully work. Better would be some kind of other SVG to PNG/SVG processing tool.

npm i --no-save with a lockfile and no arguments is reproducible though, at least since npm v7 IIRC.

That said, I have a branch to split into another package.json at silverwind@85090d3, but it's not in a state where I'm completely happy with it. I think I will change it to track only this one script's dependencies instead.

@pboguslawski
Copy link
Contributor Author

Why not solution proposed in #30688 to avoid messing with separate package.json & package-lock.json?

@silverwind
Copy link
Member

silverwind commented Apr 25, 2024

Because canvas module is native so it falls back to C++ compilation if there is no prebuilt binary for the os/arch and if that fails, the user would see unsightly errors in the build log that are not actually related to the application. Do note that we support OS like BSD derivates where likely compilation is the only option for canvas.

@pboguslawski
Copy link
Contributor Author

pboguslawski commented Apr 25, 2024

Consider dropping binary images & whole generate-images stuff and switch to simple one SVG logo (with option to have separate SVG for favicon). It's 2024 and serious browsers should handle it. Simple SVG overwriting will be enough to change logo. Or maybe config parameter to point to SVG files with custom logo/favicon.

@silverwind
Copy link
Member

Yes, that's the long-term plan, but IIRC things like IOS homescreen icons still only support raster images, but it's not so important to not consider dropping it.

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants