Question: unpack: reader size mismatch discarding trailing archive bits

[dtrudg]

This is a question rather than an issue report, as I cannot reproduce with any other image, and our reporter cannot provide the image they have which causes this behavior. Please feel free to close for lack of information!

We have had a report of an umoci unpack failing on an image (which unfortunately the reporter cannot provide).

apptainer/singularity#5799

Umoci reports:

discard trailing archive bits: expected 31101 bytes (only 31100 bytes in stream): verified reader size mismatch

Looking at the code, umoci is discarding padding etc. here as expected but I'm not clear why the stream could end up 1 byte shorter than expected.

Just wondering whether this has ever been seen before?

[cyphar]

I've not seen this before in the wild. How was the original image generated? If this error is not some other bug in umoci, it indicates that the descriptor pointing to a layer said it was 31101 bytes long but when the data was read it only contained 31100 bytes. If another tool was used to generate the images, that might be a bug in that other tool.

This kind of error is similar to a hash mismatch, which is why it's a fatal error. (The reason the error comes up in the discard trailing archive bits code is because that's the last thing to read data from the verified reader -- the verified reader completes its verification on Close.) I would also suggest that they check that the blobs in the image actually match their hashes (sadly umoci doesn't have a command for this -- I really should add one, let me open an issue for that):

% find $path_to_image -type f -print0 | xargs -0 sha256sum

They should see that all the files in blobs/sha256 match their filename, like this:

% find nextcloud -type f -print0 | xargs -0 sha256sum
a076a628af6f7dcabc536bee373c0d9b48d9f0516788e64080c4e841746e6ce6  nextcloud/blobs/sha256/a076a628af6f7dcabc536bee373c0d9b48d9f0516788e64080c4e841746e6ce6
02bab8795938bb41373cc51cd3ccce577cf99b3ddd42d4b109cecc95bc8e1d51  nextcloud/blobs/sha256/02bab8795938bb41373cc51cd3ccce577cf99b3ddd42d4b109cecc95bc8e1d51
657d9d2c68b9212bb09c223e67cd72afe8607a1ee7898df13a2f91caf4e5e28d  nextcloud/blobs/sha256/657d9d2c68b9212bb09c223e67cd72afe8607a1ee7898df13a2f91caf4e5e28d
f47b5ee58e916d4e3dba85b5821b5c1c348f93da7eaf6d243d5dcdc4a388df32  nextcloud/blobs/sha256/f47b5ee58e916d4e3dba85b5821b5c1c348f93da7eaf6d243d5dcdc4a388df32
2b62153f094cc087cefcbc3201f0a234daad76d7ebba66568e128ef5e3d36dcc  nextcloud/blobs/sha256/2b62153f094cc087cefcbc3201f0a234daad76d7ebba66568e128ef5e3d36dcc
60b09083723b248aebacd8890c19cc2f3c5e6c10b702921bdae154cd120fd37d  nextcloud/blobs/sha256/60b09083723b248aebacd8890c19cc2f3c5e6c10b702921bdae154cd120fd37d
1701d4d0a47852eaac1ece27a35b9e61d80cb43ecd37ce412933e0344e0a80d3  nextcloud/blobs/sha256/1701d4d0a47852eaac1ece27a35b9e61d80cb43ecd37ce412933e0344e0a80d3
b058a575d643cdc86975f56d6b265138841b804be61aedc8c8647b9082f78e8d  nextcloud/blobs/sha256/b058a575d643cdc86975f56d6b265138841b804be61aedc8c8647b9082f78e8d
1ad503736966408fb75a2407c72e75dcbcb99f3210db3fda1b22cda7d49d59b4  nextcloud/blobs/sha256/1ad503736966408fb75a2407c72e75dcbcb99f3210db3fda1b22cda7d49d59b4
ae67689a49623cdef2a10d0ec0b8b6c10316fbaf24f8872cd9df02e897d9da0d  nextcloud/blobs/sha256/ae67689a49623cdef2a10d0ec0b8b6c10316fbaf24f8872cd9df02e897d9da0d
730b1f7e463f733300b62f2e4058fc7f656e2febc8a40a07166e0b94f234145d  nextcloud/blobs/sha256/730b1f7e463f733300b62f2e4058fc7f656e2febc8a40a07166e0b94f234145d
678348961241459501538ddc40a9eaa56775cac605e6f59ea40fdb1724afd2be  nextcloud/blobs/sha256/678348961241459501538ddc40a9eaa56775cac605e6f59ea40fdb1724afd2be
e14e469c4fd41311cce15a347e3eae246772b61e50ba62622beb3b9343545ec4  nextcloud/blobs/sha256/e14e469c4fd41311cce15a347e3eae246772b61e50ba62622beb3b9343545ec4
070885fb2d4627b5b6db3e48473023eef7cb2bc6210e5cc94f6d420fa60405d1  nextcloud/blobs/sha256/070885fb2d4627b5b6db3e48473023eef7cb2bc6210e5cc94f6d420fa60405d1
1bcc201211f7c07ff3bbdef6565d4773bf1e81c9d935fb9d0b5e61392e38684e  nextcloud/blobs/sha256/1bcc201211f7c07ff3bbdef6565d4773bf1e81c9d935fb9d0b5e61392e38684e
011d54b86ea6e74961b021b33e72e3fef01c06e244de73421b68867d3663231e  nextcloud/blobs/sha256/011d54b86ea6e74961b021b33e72e3fef01c06e244de73421b68867d3663231e
fd3f64eaf097b4001dadc9f0baf9d73acb5e62086cb4f39b27ff7ed1cc31b011  nextcloud/blobs/sha256/fd3f64eaf097b4001dadc9f0baf9d73acb5e62086cb4f39b27ff7ed1cc31b011
ad0e170be1e1c5d13f37fd84f7980c4857bdf58de865930902a2f3ccdb0306c2  nextcloud/blobs/sha256/ad0e170be1e1c5d13f37fd84f7980c4857bdf58de865930902a2f3ccdb0306c2
b4f30a076de6f22c00ed10abf102b3cb2b9d3823307c89e81aa722fb89326a8f  nextcloud/blobs/sha256/b4f30a076de6f22c00ed10abf102b3cb2b9d3823307c89e81aa722fb89326a8f
785619dd31e8ce23c1d1b4fc42a513109bc78f8d5bb2daa85a2c2b5d251a755b  nextcloud/blobs/sha256/785619dd31e8ce23c1d1b4fc42a513109bc78f8d5bb2daa85a2c2b5d251a755b
515c07cf22f38d9f100107ce73b751a1d20d7ccec304e9d6f6692a895adde061  nextcloud/blobs/sha256/515c07cf22f38d9f100107ce73b751a1d20d7ccec304e9d6f6692a895adde061
4b835266805921ac5ad6ccff8195e20bcb2301ebf72f76bd2eb83433b1efdb1a  nextcloud/blobs/sha256/4b835266805921ac5ad6ccff8195e20bcb2301ebf72f76bd2eb83433b1efdb1a
0cdedc0869d60bb59e3dde736f996025e70318180587e049cfa22f80d67aa8e5  nextcloud/oci-layout
73245291c9c2a75668bd89fe43ca20a42f8c564f0b36a5a5811e11c1eebc6f2b  nextcloud/index.json

[dtrudg]

Thanks for the input! I'll try and collect some more information from the user with the issue,

[cyphar]

It is possible that the image is valid and it contains an extra byte at the end of the stream which umoci can't read (maybe someone added a NULL byte at the end of the GZIP stream?). If that is the issue we can work around it by reading any extra bytes after we've drained the gzip reader.

[dtrudg]

Hi @cyphar - thanks for your input on this. Unfortunately the reporter hasn't provided any more information at this point. I'll ping them again.

[cyphar]

I just tried it and it looks like gzip ignores trailing bytes so we can safely discard them. I think this patch might solve the issue:

diff --git a/oci/layer/unpack.go b/oci/layer/unpack.go
index 453b3a6a9b1f..6f129fcfa26b 100644
--- a/oci/layer/unpack.go
+++ b/oci/layer/unpack.go
@@ -273,9 +273,15 @@ func UnpackRootfs(ctx context.Context, engine cas.Engine, rootfsPath string, man
 		// in the later diff_id check failing because the digester didn't get
 		// the whole uncompressed stream). Just blindly consume anything left
 		// in the layer.
-		if _, err = io.Copy(ioutil.Discard, layer); err != nil {
+		if _, err := io.Copy(ioutil.Discard, layer); err != nil {
 			return errors.Wrap(err, "discard trailing archive bits")
 		}
+		// Same goes for compressed layers -- it seems like some gzip
+		// implementations add trailing NUL bytes, which Go doesn't slurp up.
+		// Just eat up the rest of the remaining bytes and discard them.
+		if _, err := io.Copy(ioutil.Discard, layerRaw); err != nil {
+			return errors.Wrap(err, "discard trailing raw bits")
+		}
 		if err := layerData.Close(); err != nil {
 			return errors.Wrap(err, "close layer data")
 		}
diff --git a/pkg/hardening/verified_reader.go b/pkg/hardening/verified_reader.go
index 0063aea328f2..20b03dd6e983 100644
--- a/pkg/hardening/verified_reader.go
+++ b/pkg/hardening/verified_reader.go
@@ -160,6 +160,12 @@ func (v *VerifiedReadCloser) Read(p []byte) (n int, err error) {
 // Close is a wrapper around VerifiedReadCloser.Reader, but with a digest check
 // which will return an error if the underlying Close() didn't.
 func (v *VerifiedReadCloser) Close() error {
+	// Consume any remaining bytes to make sure that we've actually read to the
+	// end of the stream. VerifiedReadCloser.Read will not read past
+	// ExpectedSize+1, so we don't need to add a limit here.
+	if _, err := io.Copy(ioutil.Discard, v); err != nil {
+		return errors.Wrap(err, "consume remaining unverified stream")
+	}
 	// Piped to underlying close.
 	err := v.Reader.Close()
 	if err != nil {

[cyphar]

I might also want to add some kind of debugging information to indicate that there were trailing bytes, since this indicates a bug in the image generator being used.

[cyphar]

I think #360 should fix this issue.

[dtrudg]

Thank you for your work on this. I'll try to get the affected user on our side to try it out with their image.

[cyphar]

Ah it turns out that patch is broken because of a bug in https://github.com/klauspost/pgzip. I will try to rustle up a slightly different patch we can merge today while we wait for #360 to be mergeable.

[cyphar]

@dtrudg I will release a new version of umoci tomorrow -- is it possible to get the user to try with the updated version at some point?

[dtrudg]

Thanks @cyphar - the user hasn't responded to my previous question, unfortunately, but I'll create a PR today on our end with the current umoci master and ask if they can test.

Thanks again.

[dtrudg]

@cyphar - many thanks again for this. We released with umoci 0.4.7 today, so we should find out in due course if it's a fix. Apologies I haven't been able to get confirmation from the user.

[cyphar]

No worries, let me know if you get any response. :D