feat: handle different digests in manifest by upils · Pull Request #322 · canonical/chisel

upils · 2026-09-02T10:19:34Z

Have you signed the CLA?

Ubuntu archives publish multiple checksums per package (SHA256 and SHA512 today, SHA512-only on 26.10+), but the manifest recorded only a single SHA256 digest, which is wrong. Chisel now records digests published by the archive (and supported by Chisel) for each package in the manifest, while the strongest one continues to be used for fetch verification and content-addressable caching. The manifest can effectively contain an new sha512 field alongside sha256, so manifests remain readable and byte-compatible across old and new readers.

API changes: manifest.Package gains Digests map[string]string and a NewPackage constructor that validates digest kinds and derives the legacy Digest field (sha256 when present, sha512 otherwise), which is kept for source compatibility with existing consumers. manifestutil.PackageInfo replaces PkgDigest()/PkgDigestKind() with PkgDigests(). This requires at least a minor release.

Also of note: fetching a package whose section advertises no supported digest is now an error rather than a silent unverified download.

Fixes #305

upils · 2026-09-02T11:42:52Z

 	"io"

 	"github.com/canonical/chisel/internal/archive"
+	"github.com/canonical/chisel/internal/cache"


[Note to reviewer]: Importing cache in more and more packages only to get access to the list of supported digestkinds looks increasingly wrong. Conceptually it does not make a lot of sense that archive or manifesutil depends on cache. I am tempted to extract the digestkind-related bits out of cache to a dedicated package. If we decide to proceed, I will do that in a follow-up as I don't want to pollute this PR with a refactor.

lczyk

ok, discussed this to exhaustion in a separate thread so i will just drop a digest (🥁🥁🐍) of that conversation here:

fixed 26.10
changes the manifest for older releases to sha512(!), even 20.04 because chisel resolves from -updates.
chisel is release-agnostic so any special rules for e.g. 26.04 or earlier are off-the-table
we could maintain backwards compatibility by choosing to always record sha256 preferentially over sha512 but then:
- we're weakening the manifest
- we're decoupling validation from the manifest, since validation would stay 512 ( or we'd flip validation too to prefer weaker sha. no. )
we could take this PR and bump the schema version of the manifest 1.0 -> 1.1

atm it feels to me like the last option is the best, since all others introduce in-perpetuum compromises which go in hacky/insecure directions.

lczyk · 2026-09-03T09:27:11Z

also, leaving a note here so we don't forget, this was not covered in #306 and, possibly, should have been. we might need to look into test coverage for chisel.

lczyk

this version is almost ok imo. it does "kick the can down the road" regarding the breaking change -- at some point we'll have to flip the preference to recording the strongest supported digest, but we can do that in 1.6.0 rather than now and in a rush.

my one issue is that it's a regression from 1.5.0 in what we verify the .debs against -- the per-package digest inside Packages.gz, not the index itself, which this PR doesn't touch. 1.5.0 checks those with sha512, and ace8a36 (this PR's current HEAD) flips them back to sha256. all releases until 26.04 have only sha256 in InRelease, but their Packages.gz publish sha256 and sha512 (curl -fsSL http://archive.ubuntu.com/ubuntu/dists/noble-updates/main/binary-amd64/Packages.gz | gzcat | grep SHA512), so in 1.5.0 they are verified with sha512.

seemingly, if we wanted to address this, we'd need to decouple the verification (sha512) form what's recorded in the manifest (sha256) which completely defeats the purpose of the manifest. no. here is a proposed solution though: verify both shas. strongest-first as the primary, the way we want it, then, if sha256 is also published, verify that too since that's the one we will be recording. abit of a perf hit, but i think it will give us the best result we could get with the constraints we have:

26.10 works -- sha512 only, nothing to double-check
manifests unchanged on 20.04..26.04, no schema bump
what we record is what we checked
we keep the strongest published digest for verification, so no regression from 1.5.0

lczyk

after a separate discussion, dropping to sha256 validation for now is also a good way forward. 👍

upils · 2026-09-23T09:28:04Z

+	SHA256  string `json:"sha256,omitempty"`
+	SHA512  string `json:"sha512,omitempty"`
+	SHA384  string `json:"sha384,omitempty"`


[Note to reviewer]: The set of supported digest kinds in repeated in several places in this package. I tried different approaches to avoid this and improve maintainability but none was notably better than the current approach here.

upils · 2026-09-23T09:30:40Z

-	for _, f := range order {
-		if d, _, ok := control.ParsePathInfo(release.Get(f.name), path); ok {
-			return d, f
+func findDigest(release control.Section, path string) (digest string, field digestField, ok bool) {


[Note to reviewer]: This change in signature (returning a bool) addresses a comment of the previous PR that was missed. Same thing for packageDigests below.

lczyk · 2026-09-24T20:55:01Z

+			Name:    "package1",
+			Version: "v1",
+			Arch:    "a1",
+			Digests: map[cache.DigestKind]string{cache.SHA384: "s384"},


note that cache.SHA384 caches with sha-3 (sha3.New384(), added in #298). there is also sha-2-384 and that could be a source of confusion. maybe the manifest key should be called SHA3_384 instead?

great loc to comment on this, ik 😅

lczyk · 2026-09-24T21:12:28Z

note that copa has chisel manifest support (project-copacetic/copacetic#1667) and atm it will hard-fail for rows w/out sha256. we should let them know and maybe even do a followup pr. they're using the canonical/chisel-manifest mirror so we need to update that too ofc (canonical/chisel-manifest#3).

lczyk · 2026-09-24T21:14:59Z


-var digestKinds = []DigestKind{SHA256, SHA384, SHA512}
+// digestKinds sorted in decreasing order of strength.
+var digestKinds = []DigestKind{SHA384, SHA512, SHA256}


Suggested change

var digestKinds = []DigestKind{SHA384, SHA512, SHA256}

var digestKinds = []DigestKind{SHA512, SHA384, SHA256}

lczyk

im not sold on the custom martial and unmartiallers.

also, since we're leaving Digest for compat, how about we just make it always sha256 ( yes, sometimes compute it ). then its 100% compatibility, and consumer like copa dont need patching

lczyk · 2026-09-24T21:17:57Z

@@ -12,13 +13,124 @@ import (
 const Schema = "1.0"

 type Package struct {


id love to avoid the custom marshall and unmarshall. what about something like:

type Package struct { Kind string `json:"kind"` Name string `json:"name,omitempty"` Version string `json:"version,omitempty"` } func (p Package) Digests() map[string]string

??

lczyk · 2026-09-24T21:21:09Z

+	return o, nil
+}
+
+func (p *Package) MarshalJSON() ([]byte, error) {


ince Package no longer has json tags, json.Marshal(pkg) on a value (not a pointer) skips this method and outputs {"Kind":"package","Name":...}.

Suggested change

func (p *Package) MarshalJSON() ([]byte, error) {

func (p Package) MarshalJSON() ([]byte, error) {

lczyk · 2026-09-24T21:22:26Z

+		Version: p.Version,
+		Arch:    p.Arch,
+	}
+	for kind, digest := range p.Digests {


Digest is never read here, so &manifest.Package{Digest: x} marshals with no digest.

lczyk · 2026-09-24T21:23:10Z

+		case "sha384":
+			pj.SHA384 = digest
+		default:
+			return nil, fmt.Errorf("cannot marshal package %q: unsupported digest kind %q", p.Name, kind)


marshalling a Package couldn't fail before; with a free-form map it now can. :/

lczyk · 2026-09-24T21:24:07Z

+			digests[kind] = digest
+		}
+	}
+	pkg, err := NewPackage(&PackageOptions{


going through NewPackage here drops the decoded kind (NewPackage hardcodes "package"), and the error below can't happen. fill the struct directly instead?

lczyk · 2026-09-24T21:25:22Z

+	Digests map[string]string
+}
+
+func NewPackage(opts *PackageOptions) (*Package, error) {


NewPackage(nil) panics. do we need NewPackage / PackageOptions as public api anyway?

lczyk · 2026-09-24T21:27:56Z

+	}, nil
+}
+
+func getValidOptions(options *PackageOptions) (*PackageOptions, error) {


Suggested change

func getValidOptions(options *PackageOptions) (*PackageOptions, error) {

func validateDigests(digests map[string]string) error

would be a bit closer to what the func is doing atm no? then ofc

if err := validateDigests(opts.Digests); err != nil { ... }

feat: handle different digests in manifest

f1d045c

upils marked this pull request as ready for review September 2, 2026 11:23

docs: refine comments

a3e7036

upils commented Sep 2, 2026

View reviewed changes

upils added 4 commits September 2, 2026 14:11

style: cleaning

ef98829

fix: fail loudly on invalid marshalling input

2013aab

test: improve TestManifestRead

1f8982e

docs: cleaning

e013b76

lczyk suggested changes Sep 2, 2026

View reviewed changes

lczyk mentioned this pull request Sep 4, 2026

bug(26.10): chisel cut failing for 26.10 #305

Open

upils added 2 commits September 4, 2026 12:09

fix: use sha256 for packages by default

ace8a36

docs: refine comments

1e5c4d8

lczyk suggested changes Sep 5, 2026

View reviewed changes

lczyk approved these changes Sep 7, 2026

View reviewed changes

lczyk self-requested a review September 7, 2026 15:17

lczyk approved these changes Sep 8, 2026

View reviewed changes

upils added 11 commits September 16, 2026 16:19

fet: record multiple hashes in the manifest

cb3f498

fix: order by strength

5466124

refactor: simplify getting strongest pkg digest kind

7ab32ed

tests: cleaning

ca4f18d

docs: refine comments

104f089

tests: rename Hashes field to fit usage

8009bb0

docs: remove useless comment

d0d2b9c

style: improve naming and docs

02919e5

fix: make sure reported digest values are not empty

ea1734a

style: linting

f7011a8

fix: revert unrelated change in behavior

6c03948

lczyk self-requested a review September 17, 2026 21:54

fix: keep Digest field to avoid breaking change

e53e543

upils added 3 commits September 22, 2026 17:23

feat: add cleaner package creation

a73fbdc

refactor: rework NewPackage

811c992

style: clean docs and tests

42eed4b

upils commented Sep 23, 2026

View reviewed changes

docs: add deprecation notice for Digest field

2e2c407

upils added the Priority Look at me first label Sep 24, 2026

lczyk reviewed Sep 24, 2026

View reviewed changes

lczyk suggested changes Sep 24, 2026

View reviewed changes

	var digestKinds = []DigestKind{SHA384, SHA512, SHA256}
	var digestKinds = []DigestKind{SHA512, SHA384, SHA256}

		@@ -12,13 +13,124 @@ import (
		const Schema = "1.0"

		type Package struct {

	func (p *Package) MarshalJSON() ([]byte, error) {
	func (p Package) MarshalJSON() ([]byte, error) {

	func getValidOptions(options PackageOptions) (PackageOptions, error) {
	func validateDigests(digests map[string]string) error

Conversation

upils commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

upils Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lczyk left a comment

Choose a reason for hiding this comment

Uh oh!

lczyk commented Sep 3, 2026

Uh oh!

lczyk left a comment

Choose a reason for hiding this comment

Uh oh!

lczyk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lczyk commented Sep 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lczyk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

upils commented Sep 2, 2026 •

edited

Loading

upils Sep 2, 2026 •

edited

Loading