Practical limits for recursive references

[jzelinskie]

The implications that an OCI Index could reference other Indexes is scary to a registry author because they need to follow these relationships in order to effectively garbage collect blobs.

At KubeCon EU some registry stakeholders discussed potentially adding a limit to the depth in which a recursive relationship should be.

I'm not sure if this is best specified in the image-spec or distribution-spec.

[cyphar]

I would argue distribution-spec would be the best place to do this (and #781) -- since it's not really an issue outside of registries. With "OCIv2" the depth of references will increase (depending on whether we do a full directory-based Merkle tree or some more linear structure it might increase by a little or a lot).

But I might be okay with having some statement that "if you encounter an image with more than 20 layers of references, implementations MAY reject the image as being invalid". But even that makes me a little nervous about forwards-compatibility.

[glyn]

The problem with picking a specific limit is that, if long sequences of references have a legitimate usecase, then someone will eventually hit a situation where the limit becomes an unnatural restriction. Why not simply prohibit cyclic references? Since most images will have fairly shallow reference trees, it should be straightforward for registry code to check for cyclic references.

[josephschorr]

@glyn Because it can still be used by bad actors as an attack, even without cyclic references; just generate a tree of depth 1 million.

[jonjohnsonjr]

Since most images will have fairly shallow reference trees, it should be straightforward for registry code to check for cyclic references.

Cyclic references should be effectively impossible since the index is content-addressable (the A in Merkle DAG is "acyclic"). I guess this could happen if a registry isn't checking digests but is doing reference counting, but I'd say that's a pretty egregious bug in the registry :)

The implications that an OCI Index could reference other Indexes is scary to a registry author because they need to follow these relationships in order to effectively garbage collect blobs.

I don't think this is actually that scary. If you're doing reference counting, you only need to consider one level of references for a given HTTP request. The client would be responsible for actually traversing the dependency graph to delete each index in the appropriate order, so most of the work would actually fall on the client. Of course this depends on the registry implementation, but this wouldn't seem like a problem for most straightforward approaches (happy to be enlightened if I'm missing something, though I suspect most issues will be for historical reasons 😄 ).

I'd be more worried about https://github.com/opencontainers/image-spec/issues/781 but I'd suggest registries could just return an HTTP 413 if the incoming manifest/index is too burdensome to store.

I would argue distribution-spec would be the best place to do this

Agreed, this seems like something most clients would have to consider when interacting with a registry, so we should add some notes to the registry spec.

[glyn]

@glyn Because it can still be used by bad actors as an attack, even without cyclic references; just generate a tree of depth 1 million.

That sounds like a very expensive attack, but point taken @josephschorr.

Since most images will have fairly shallow reference trees, it should be straightforward for registry code to check for cyclic references.

Cyclic references should be effectively impossible since the index is content-addressable (the A in Merkle DAG is "acyclic").

Doh! If you, @jonjohnsonjr, keep telling me this, I'll eventually remember it. Apologies.

[SteveLasker]

Is this something the spec should limit, or something each registry would specify? We each have layer count and size constraints. We have different repo counts. Some of these are cost management, some are just reasonable expectations to set with customers. Is this any different?

As we try to close out old issues, just wondering if we really need this one as active?

[josephschorr]

@SteveLasker Personally, I'd feel more comfortable if reasonable limits were defined in the specification, as that would allow tools to check and verify before pushing occurred. It also has the added bonus of consistency across implementations, which means any manifest pushed to one registry should work with another, assuming they meet the specification.

[SteveLas]

What do folks believe are reasonable sizes? As a cohort, I was initially concerned with the 256 limit to path size on a repo as ACR supports nested namespaces. I then tried it out, to see what <256 really looked like: docker pull demo42.azurecr.io/repo01/repo02/repo03/repo04/repo05/repo06/repo07/repo08/repo09/repo10/repo11/repo12/repo13/repo14/repo15/repo16/repo17/repo18/repo19/repo20/repo21/repo22/repo23/repo2/repo24/repo25/repo26/repo27/repo28/repo29/repo30/repo31/hello-world:aab1

Yeah, I now think 256 is completely within reasonable limits. :)

So, what do we think are reasonable limits for, to limit DOS attacks, or just runaway code?

• manifest: layer count
• index: manifest count
• layer size
• config size

And, while I agree with Joey it would be nice to have consistency, if we can't agree, is there a problem with having registries implement limits? I'm just looking to hopefully focus our mindpower on a shorter list of active issues.

[vrothberg]

So, what do we think are reasonable limits for, to limit DOS attacks, or just runaway code?

The containers/image library is mostly using a limits of 4 MB some of which may apply to the distribution spec (e.g., token and error-response sizes). Until now we have not experienced any issues with these limits.

Setting these limits was motivated by fixing CVE-2020-1702.