Skip to content
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

HTTP fetch of fs-migrations should use CAR #9159

Closed
lidel opened this issue Aug 2, 2022 · 5 comments · Fixed by #10324
Closed

HTTP fetch of fs-migrations should use CAR #9159

lidel opened this issue Aug 2, 2022 · 5 comments · Fixed by #10324
Assignees
Labels
effort/days Estimated to take multiple days, but less than a week exp/expert Having worked on the specific codebase is important kind/bug A bug in existing code (including security flaws) kind/maintenance Work required to avoid breaking changes or harm to project's status quo P1 High: Likely tackled by core team if no one steps up topic/security Topic security

Comments

@lidel
Copy link
Member

lidel commented Aug 2, 2022

Version: 0.14.x

Fetching migration data from IPFS was added in #8064, but we did not have #8758 at the time and HTTP fetch is still delegating trust to the gateway.

I consider that a bug: migrations should be fetched in trustless mode as a CAR by requesting them as ?format=car and verifying every block before applying the migration.

This allows us to use third-party gateways af fallback and/or in scenarios where ipfs.io is blocked by ISP etc.

@lidel lidel added kind/bug A bug in existing code (including security flaws) topic/security Topic security exp/expert Having worked on the specific codebase is important P1 High: Likely tackled by core team if no one steps up kind/maintenance Work required to avoid breaking changes or harm to project's status quo effort/days Estimated to take multiple days, but less than a week labels Aug 2, 2022
@BigLep
Copy link
Contributor

BigLep commented Sep 30, 2022

@Jorropo will tackle this after some related light-client work he is doing.

@BigLep
Copy link
Contributor

BigLep commented Nov 17, 2022

@lidel : is this a duplicate of #8851 ?

I see there is also this PR: #9250

@lidel
Copy link
Member Author

lidel commented May 16, 2023

I've closed #8851 as a duplicate (this one has more context).

@hacdias hacdias self-assigned this May 16, 2023
@hacdias
Copy link
Member

hacdias commented May 17, 2023

I'm marking this as blocked by the IPIP-402. Once we have that, our life will be easier. Also, I want to add that we need IPNS resolution to solve this too. Copy of my comment:

I just took a second look at this and indeed it'll be much easier once we have widespread IPIP-0402. There are two main problems here:

  1. We need to be able to resolve distPath. If it is a /ipfs path, we're good to go. If it's a /ipns path we have to:
    a. Check if it's regular IPNS. If so, we can fetch record from gateway via ?format=ipns-record, validate and get the /ipfs path.
    b. If it's DNSLink (most likely), setup a NameSys resolver and resolve it via DNS.
  2. With the distPath being an /ipfs path, we can now request a CAR for {distPath}{filePath}?dag-scope=entity and verify if the CAR and path resolves correctly. Use that data for the migration file.

I was hoping we could do something now, but it'd just be too complicated and we're already working on IPIP-402 so I would mark this as blocked for now.

@lidel
Copy link
Member Author

lidel commented Aug 3, 2023

IPIP-402 shipped. Once ipfs.io upgrades to Kubo 0.21 or later, we will be able to fetch /ipfs/cid/path/file as entity+path and verify it from any gateway.

We should make it work with all gateways, so if anything is missing in CAR response, fetch it block-by-block (we have prior art in bifrost-gateway).

I don't think we should block on IPNS record resolution.
We do not use it for migrations, and for that we have hardcoded CID, we only care about /ipfs/ paths:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/days Estimated to take multiple days, but less than a week exp/expert Having worked on the specific codebase is important kind/bug A bug in existing code (including security flaws) kind/maintenance Work required to avoid breaking changes or harm to project's status quo P1 High: Likely tackled by core team if no one steps up topic/security Topic security
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants