[Additional Layer Store] Use TOCDigest as ID of each layer#1673
[Additional Layer Store] Use TOCDigest as ID of each layer#1673AkihiroSuda merged 2 commits intocontainerd:mainfrom
Conversation
f345be0 to
5b9b272
Compare
mtrmac
left a comment
There was a problem hiding this comment.
(I’m afraid I know nothing about this codebase; I could be completely off-base.)
| if l.Digest.String() != target.Digest.String() { | ||
| continue // This is not the target layer; nop | ||
| r.layerDigestToTOCDigestMu.Lock() | ||
| layerTOCDigest, ok := r.layerDigestToTOCDigest[refspec.String()][l.Digest.String()] |
There was a problem hiding this comment.
What happens when the manifest contains multiple layers with the same compressed digest but different TOC digests?
Note that such a situation can “legitimately” happen, where all digests validate, if an attacker decides to create such a multi-TOC layer; and the TOCs can differ.
Whether the compressedDigest is validated or not, c/image image signing relies the ALS backed enforcing exactly the TOC digest provided over the FUSE API. Looking at how resolveLayer uses the layerDigestToTOCDigest map, it seems to me that the data flow goes (TOC digest -> compressed digest (?) -> possibly-different TOC digest) and potentially uses the wrong TOC digest for some operations.
Here, getCachedLayer is called with the layerTOCDigest and not the original tocDigest. Luckily, it is followed by a Info().TOCDigest == tocDigest check, but still, the opportunity to replace the TOC digest worries me.
In principle, ((repo, compressed digest) <-> TOC digest) is an N to M mapping. It’s reasonable to assume that the same compressed digest implies the same TOC digest, except when under attack; the same TOC digest can legitimately map to different compressed digest values (in the same repo, or in different repos).
There was a problem hiding this comment.
Removed layerDigestToTOCDigest.
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
81beb1b to
8d185bf
Compare
b5fdd8c to
f45bff0
Compare
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
f45bff0 to
a1573c2
Compare
Needs: containers/storage#1924 and containers/image#2416
This commit changes stargz-store to use TOCDigest as the layer specifier.
For example, the extracted view of a layer diff contents is provided in the following path:
When an access happens to a layer, stargz store searches the layer that has the TOC matching to the specified TOCDigest.
If it fails to get the layer, stargz-store returns EIO.