-
Notifications
You must be signed in to change notification settings - Fork 237
IPIP-512: Limit Identity CID Size to 128 Bytes in UnixFS Contexts #512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ec58da9
to
f494ecd
Compare
🚀 Build Preview on IPFS ready
|
f494ecd
to
53b3198
Compare
documents the 128-byte limit for identity cids in unixfs contexts, with rationale from community discussions and test fixtures
53b3198
to
59dc0fa
Compare
* fix: enforce identity CID size limits - validate --inline-limit against verifcid.MaxDigestSize - add error when --hash=identity exceeds size limit - add tests for identity CID overflow scenarios - update help text to show maximum inline limit This prevents creation of unbounded identity CIDs by enforcing the 128-byte limit defined in ipfs/boxo#1018 Fixes #6011 IPIP: ipfs/specs#512
- Most users are unaffected as identity CIDs require explicit opt-in | ||
|
||
Implementations upgrading to support this IPIP will need to: | ||
1. Add validation to reject oversized identity CIDs when reading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if users do have identity cids, what is the recommendation for conversion to non-identity-cids if we block on read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These details are left up to implementation, but as an example:
- Kubo will error if you try to onboard data over limit with explicit
--hash=identity
- In more relaxed context where Kubo does things like autosharding HAMTS, we do similar autoswitch for identity. For eexample, MFS code in fix(verifcid): enforce size limit for identity CIDs boxo#1018 automatically switches from identity to raw or dag-pb once the limit is reached.
- change status from proposal to ratified - add GO: boxo v0.35.0 (kubo v0.38.1) implementation reference - add JS: helia unixfs v6.0.1 (helia v6.0.1) implementation reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you all for feedback. Seems the consensus is this is a good idea / security feature.
This got ratified – this limit has been implemented in:
- Boxo v0.35.0 (shipped in Kubo v0.38.1)
- Helia UnixFS v6.0.1 (shipped in Helia v6.0.1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I arrive late to the party but the wording on a few sentences is not great.
I skimmed through this PR and thought the CID limit would be 128bytes thus the payload limit is 128 - CID overhead.
I've noted the places where in a vacuum the sentence is incorrect.
Also calling it digest limit is weird because it's not the digest of anything, maybe payload limit
would be better ?
Altho it wouldn't be consistent with other multihashes;
Altho altho inline CIDs are by design inconsistent with other multihashes.
But that looks out of scope.
addressing feedback from #512 (review)
This IPIP proposes documenting the 128-byte limit for identity cids in unixfs contexts, with rationale from community discussions and test fixtures.
References