Skip to content

Verify FILE_CKSUM (sha256) in pkg_summary(5) on download.#141

Open
riastradh wants to merge 2 commits intoNetBSDfr:masterfrom
riastradh:riastradh-20260105-hashfile
Open

Verify FILE_CKSUM (sha256) in pkg_summary(5) on download.#141
riastradh wants to merge 2 commits intoNetBSDfr:masterfrom
riastradh:riastradh-20260105-hashfile

Conversation

@riastradh
Copy link
Copy Markdown

@riastradh riastradh commented Jan 6, 2026

This is important for detecting version rollback attacks -- verifying a signature on the package itself doesn't help, because the old one also has a valid signature.

Relies on patch proposed for pkg_install to make pkg_info -X generate the FILE_CKSUM lines: https://mail-index.NetBSD.org/tech-pkg/2026/01/06/msg031853.html

(Based on #140 to make schema changes easier and more reliable.)

Rather than having to craft a new CHECK_DB_LATEST that fails with all
old schema versions and works with the new schema version every time
we make a schema change, we can just change the user_version number.
This is important for detecting version rollback attacks -- verifying
a signature on the package itself doesn't help, because the old one
also has a valid signature.
@riastradh riastradh force-pushed the riastradh-20260105-hashfile branch from 263c235 to 204174b Compare January 17, 2026 21:40
@jperkin
Copy link
Copy Markdown
Contributor

jperkin commented Jan 28, 2026

I have a couple of issues with this.

Firstly it breaks builds outside of pkgsrc:

$ make
/Library/Developer/CommandLineTools/usr/bin/make  all-am
  CC       pkgin-actions.o
actions.c:45:10: fatal error: 'sha2.h' file not found
   45 | #include <sha2.h>
      |          ^~~~~~~~
1 error generated.
make[1]: *** [pkgin-actions.o] Error 1
make: *** [all] Error 2

I've put a reasonable amount of effort into making pkgin build separately from pkgsrc, and would like to retain that. There are some options for pulling in a compat sha2 if the system doesn't provide one but they either use a different API or end up pulling in quite a lot more code. I'd need to investigate this further.

Secondly I'm not keen on sha256 being hardcoded in a bunch of places, and have an aversion to spaces in column names. Given the pkg_summary format allows for arbitrary hashes, I think I'd prefer a separate table where these are stored along with an associated hash per entry for extensibility.

I'm happy to work on this, but just wanted to post something to say that I am looking at this, as I need to cut a new pkgin release now for the pkgtools upgrade issue and this will have to be deferred until later. I have added initial FILE_CKSUM to pkgsrc-rs so have a test-bed for this, and am looking to extend that to support different hash types.

@riastradh
Copy link
Copy Markdown
Author

I've put a reasonable amount of effort into making pkgin build separately from pkgsrc, and would like to retain that. There are some options for pulling in a compat sha2 if the system doesn't provide one but they either use a different API or end up pulling in quite a lot more code.

Sorry -- I didn't test it, but I did add a libnbcompat fallback, and I'm a little surprised that didn't work because there are already libnbcompat fallbacks for other things in pkgin. Will investigate when I have a chance.

If nothing else, we can just pull in a portable C SHA-256 from NetBSD libc. Not perfect but enough to get it working and able to be improved later, costing around only 12 KiB of text (probably half that if we strip out SHA-512 and SHA-384 too).

Secondly I'm not keen on sha256 being hardcoded in a bunch of places, and have an aversion to spaces in column names. Given the pkg_summary format allows for arbitrary hashes, I think I'd prefer a separate table where these are stored along with an associated hash per entry for extensibility.

I appreciate the concern for hash agility, but does the perfect need to be the enemy of the good here and hold up a security measure?

Here's the considerations that went into the choices I made:

  1. The database in question is a cache. We don't need to worry too much about the names of columns used internally by pkgin because we can just blow the database away and recreate it if need be (especially if Use an application id and user version number for schema. #140 is merged). The pkg_summary file format is not a cache, but it already has the necessary agility and has for decades.
  2. Using a space in the column name avoided the possibility of accidentally colliding with any existing pkg_summary keys. (But I would be fine with changing it to FILE_CKSUM_sha256 or something if that makes it easier.)
  3. As a practical matter, SHA-256 is adequate for security (length extension attacks are irrelevant here), and provides the best path for hardware acceleration to speed up verification later.
  4. Adding another table would have made a much larger patch to add and apply queries for managing the table; this fit more easily into the existing logic.
  5. This patch is much simpler than changes that would introduce a hash abstraction, implement the hash abstraction for several hashes, apply the hash abstraction to whatever set of hashes are actually used in the summary file, test it all, and so on. (Do you test every hash, or create a user configuration knob for which hashes to test, or what?)

That said, the patch at the very least already identifies all the places where you will need to substitute "hash" for "sha256" in order to do (5) if desired later, making it easy to do that change if you want.

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

Successfully merging this pull request may close these issues.

2 participants