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

ipfs add with exotic hash functions panics (the daemon and the client) #9297

Closed
3 tasks done
Tracked by #9237
mrd0ll4r opened this issue Sep 22, 2022 · 5 comments · Fixed by #9303
Closed
3 tasks done
Tracked by #9237

ipfs add with exotic hash functions panics (the daemon and the client) #9297

mrd0ll4r opened this issue Sep 22, 2022 · 5 comments · Fixed by #9303
Assignees
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization

Comments

@mrd0ll4r
Copy link
Contributor

Checklist

Installation method

ipfs-update or dist.ipfs.tech

Version

Kubo version: 0.15.0
Repo version: 12
System version: amd64/linux
Golang version: go1.18.5

Config

default

Description

Hello! :)

I'm experimenting a bit with more exotic hash functions and ipfs add.
For that, I'm adding some ~5MiB binary via ipfs add --only-hash --cid-version 1 --hash <hash function> <file>

It's not clear from the documentation which functions are supported, but a quick skim through the source led to go-multihash/Names, and indeed these are accepted by the client (and daemon, I guess).

This all works for normal hash functions (SHA2, SHA3, ...), but behaves weirdly for more exotic ones.

  • poseidon-bls12_381-a2-fc1, x11, sha2-256-trunc254-padded, with a running daemon causes Error: write tcp 127.0.0.1:45460->127.0.0.1:5001: write: connection reset by peer on the CLI, and the daemon panics:
Daemon is ready
panic: unknown multihash code 46081 (0xb401): no such hash registered

goroutine 144031 [running]:
github.com/ipfs/go-merkledag.(*ProtoNode).RawData(...)
        github.com/ipfs/[email protected]/node.go:229
github.com/ipfs/go-merkledag.(*ProtoNode).Cid(0xc0113c7eb8)
        github.com/ipfs/[email protected]/node.go:333 +0x245
github.com/ipfs/go-merkledag.(*ProtoNode).String(0xc002ad7b64?)
        github.com/ipfs/[email protected]/node.go:346 +0x19
github.com/ipfs/go-mfs.NewRoot({0x2a15400, 0xc00d34f920}, {0x2a1b430, 0xc00cf02270}, 0xc0113c7eb8, 0xc00d2c9960?)
        github.com/ipfs/[email protected]/root.go:121 +0x20b
github.com/ipfs/kubo/core/coreapi.(*UnixfsAPI).Add(0xc00a48edc0, {0x2a15358?, 0xc00a658640?}, {0x2a0a120, 0xc00ad21640}, {0xc00a319a40, 0xc, 0x14})
        github.com/ipfs/[email protected]/core/coreapi/unixfs.go:181 +0x19fd
github.com/ipfs/kubo/core/commands.glob..func6.1()
        github.com/ipfs/[email protected]/core/commands/add.go:249 +0x110
created by github.com/ipfs/kubo/core/commands.glob..func6
        github.com/ipfs/[email protected]/core/commands/add.go:246 +0xf1b

Trying again then panics the client:

panic: unknown multihash code 46081 (0xb401): no such hash registered

goroutine 45 [running]:
github.com/ipfs/go-merkledag.(*ProtoNode).RawData(...)
        github.com/ipfs/[email protected]/node.go:229
github.com/ipfs/go-merkledag.(*ProtoNode).Cid(0xc000a87eb8)
        github.com/ipfs/[email protected]/node.go:333 +0x245
github.com/ipfs/go-merkledag.(*ProtoNode).String(0xc001c497b4?)
        github.com/ipfs/[email protected]/node.go:346 +0x19
github.com/ipfs/go-mfs.NewRoot({0x2a15400, 0xc0002b7650}, {0x2a1b430, 0xc000e2a540}, 0xc000a87eb8, 0xc000e201e0?)
        github.com/ipfs/[email protected]/root.go:121 +0x20b
github.com/ipfs/kubo/core/coreapi.(*UnixfsAPI).Add(0xc000e3c420, {0x2a15358?, 0xc000257a40?}, {0x2a0a120, 0xc000257a00}, {0xc00099a820, 0xc, 0x14})
        github.com/ipfs/[email protected]/core/coreapi/unixfs.go:181 +0x19fd
github.com/ipfs/kubo/core/commands.glob..func6.1()
        github.com/ipfs/[email protected]/core/commands/add.go:249 +0x110
created by github.com/ipfs/kubo/core/commands.glob..func6
        github.com/ipfs/[email protected]/core/commands/add.go:246 +0xf1b
  • murmur3-x64-64 and md5 return Error: potentially insecure hash functions not allowed, which is surprising seeing how SHA1 is accepted, maybe. This is mostly fine, but it looks like I'm streaming some of the data to the daemon, it's starting to chunk, and then decides "no, I won't give you the CID".

Expected Behavior

  • Some documentation as to what functions are accepted by ipfs add
  • Not panicking the daemon
@mrd0ll4r mrd0ll4r added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Sep 22, 2022
@Jorropo
Copy link
Contributor

Jorropo commented Sep 22, 2022

The rational about sha1 is explained here: #8650 (comment)

The panic is indeed not nice, I'll fix that.

FWIW The list of allowed hash functions is here:
https:/ipfs/go-verifcid/blob/d870bba8ca7e7464ef2af88a1916fe13be169145/validate.go#L16-L51

This could be documented better, this is less important to fix tho.

@mrd0ll4r
Copy link
Contributor Author

mrd0ll4r commented Sep 22, 2022

Nice, thanks :)

@Jorropo
Copy link
Contributor

Jorropo commented Sep 22, 2022

Also about sha1 being broken.

It is still belived to by cryptographically impossible to find a collision for a given a random precise target hash.

What has been proven is collisions when the attacker control both files.

In other words, if I add some files with sha1 you cannot find a collision that will let you replace my files with yours.

However you can find two files (that you control) which have the same hash.

This is an issue if you start trusting other peoples files.

So it's not that bad (you shouldn't use it tho).

@mrd0ll4r
Copy link
Contributor Author

I mostly agree with your stance on SHA1, I was just surprised by the behavior of the client :)

Edit: While I have you here -- I think some of those functions have variable output sizes, like blake3 (and all the Keccaks and SHA3?). If that's true, are there plans to specify this via ipfs add in the future?

Edit2: reorganizing comments...

@Jorropo
Copy link
Contributor

Jorropo commented Sep 22, 2022

While I have you here -- I think some of those functions have variable output sizes, like blake3 (and all the Keccaks and SHA3?). If that's true, are there plans to specify this via ipfs add in the future?

Blake3 support variable sized digest but only for the verification (up to 128 bytes because I had no clue what limit I should have put in).
Adding the option to ipfs add include lots of refactoring work we are not intrested in doing right now, if you want to see the option, please send us a PR. 🙂 #8650 (comment)

SHA3 and Keccaks were added at a time where we didn't supported variable sized digests in go-multihash, now that we implemented variable digests for blake3 it should be easy to update sha3 and keccak to use the same plumbing that blake3 use and have variable sized verification for them too.

You can lookup how blake3 verification is implemented if you want to submit a PR to have variable sha3 and keccak:
multiformats/go-multihash#161

@BigLep BigLep mentioned this issue Sep 22, 2022
Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Sep 26, 2022
Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Sep 26, 2022
Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Sep 26, 2022
Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Dec 5, 2022
… hashing function""

I was too much trigger happy and caught ipfs#9297's regression test in the workaround revert.
Jorropo added a commit that referenced this issue Dec 7, 2022
… hashing function""

I was too much trigger happy and caught #9297's regression test in the workaround revert.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants