From 83e0fbd3a51f2d2379635175c3adccc4fbe1c63d Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 11 Feb 2021 15:28:28 +0100 Subject: [PATCH 1/2] fix: decode dnslink but throw on encode This is a poc for enabling support for legacy non-cryptographic values while not allowing new ones to be encoded. --- src/profiles.js | 50 +++++++++++++++++++++++++++++++++++++++++++++---- test/test.js | 18 +++++++++++++++--- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/profiles.js b/src/profiles.js index 0d0e8b3..8249bb2 100644 --- a/src/profiles.js +++ b/src/profiles.js @@ -59,9 +59,25 @@ const encodes = { * @return {Buffer} */ ipns: (value) => { + const { multihash } = new CID(value) + + // Additional check for identifiers shorter + // than what inlined ED25519 pubkey would be + // https://github.com/ensdomains/ens-app/issues/849#issuecomment-777088950 + if (multihash.length < 38) { + const mh = multiH.decode(multihash) + // ED25519 pubkeys are inlined using identity hash function + // and we should not see anything shorter than that + if (mh.name === 'identity' && mh.length < 36) { + // One can read inlined string value via: + // console.log('ipns-ns id:', String(multiH.decode(new CID(value).multihash).digest)) + throw Error('ipns-ns allows only valid cryptographic libp2p-key identifiers, try using ED25519 pubkey instead') + } + } + // represent libp2p-key as a CID // https://github.com/libp2p/specs/blob/master/RFC/0001-text-peerid-cid.md - return new CID(1, 'libp2p-key', new CID(value).multihash).buffer + return new CID(1, 'libp2p-key', multihash).buffer }, /** * @param {string} value @@ -88,13 +104,39 @@ const decodes = { /** * @param {Buffer} value */ - cid: (value) => { + ipfs: (value) => { const cid = new CID(value).toV1(); return cid.toString(cid.codec === 'libp2p-key' ? 'base36' : 'base32') }, /** * @param {Buffer} value */ + ipns: (value) => { + const cid = new CID(value).toV1(); + + // Additional check for identifiers shorter + // than what inlined ED25519 pubkey would be + // https://github.com/ensdomains/ens-app/issues/849#issuecomment-777088950 + if (cid.multihash.length < 38) { + const mh = multiH.decode(cid.multihash) + // ED25519 pubkeys are inlined using identity hash function + // and we should not see anything shorter than that + if (mh.name === 'identity' && mh.length < 36) { + // Value is not a libp2p-key, return original string + console.warn('[ensdomains/content-hash] use of non-cryptographic identifiers in ipns-ns is deprecated and will be removed, migrate to ED25519 libp2p-key') + return String(multiH.decode(new CID(value).multihash).digest) + // TODO: start throwing an error (after some deprecation period) + // throw Error('ipns-ns allows only valid cryptographic libp2p-key identifiers, try using ED25519 pubkey instead') + // One can read inlined string value via: + // console.log('ipns-ns id:', String(multiH.decode(new CID(value).multihash).digest)) + } + } + + return cid.toString('base36') + }, + /** + * @param {Buffer} value + */ utf8: (value) => { return value.toString('utf8'); }, @@ -112,11 +154,11 @@ const profiles = { }, 'ipfs-ns': { encode: encodes.ipfs, - decode: decodes.cid, + decode: decodes.ipfs, }, 'ipns-ns': { encode: encodes.ipns, - decode: decodes.cid, + decode: decodes.ipns, }, 'default': { encode: encodes.utf8, diff --git a/test/test.js b/test/test.js index e5469d8..b6cfef5 100644 --- a/test/test.js +++ b/test/test.js @@ -7,7 +7,7 @@ const ipfs_CIDv0 = 'QmRAQB6YaCyidP37UdDnjFY5vQuiBrcqdyoW1CuDgwxkD4' const ipfs_CIDv1 = 'bafybeibj6lixxzqtsb45ysdjnupvqkufgdvzqbnvmhw2kf7cfkesy7r7d4' const ipfs_contentHash = 'e3010170122029f2d17be6139079dc48696d1f582a8530eb9805b561eda517e22a892c7e3f1f' const ipns_CIDv1 = 'k2k4r8kgnix5x0snul9112xdpqgiwc5xmvi8ja0szfhntep2d7qv8zz3' -const ipns_CIDv0_contentHash = 'e5010172122029f2d17be6139079dc48696d1f582a8530eb9805b561eda517e22a892c7e3f1f' +const ipns_peerID_B58_contentHash = 'e5010172122029f2d17be6139079dc48696d1f582a8530eb9805b561eda517e22a892c7e3f1f' const ipns_peerID_B58 = '12D3KooWG4NvqQVczTrWY1H2tvsJmbQf5bbA3xGYXC4FM3wWCfE4' const ipns_libp2pKey_CIDv1 = 'k51qzi5uqu5dihst24f3rp2ej4co9berxohfkxaenbq1wjty7nrd5e9xp4afx1' const ipns_ED25519_contentHash = 'e50101720024080112205cbd1cc86ac20d6640795809c2a185bb2504538a2de8076da5a6971b8acb4715' @@ -96,7 +96,7 @@ describe('content-hash', () => { it('should encode legacy PeerID (RSA B58)', () => { // RSA ones lookes like regular multihashes const actual = contentHash.encode('ipns-ns', ipfs_CIDv0); - actual.should.be.equal(ipns_CIDv0_contentHash); + actual.should.be.equal(ipns_peerID_B58_contentHash); }); it('should encode ED25519 PeerID (B58)', () => { // ED25519 are allowed to be encoded in Base58 for backward-interop @@ -109,18 +109,30 @@ describe('content-hash', () => { const actual = contentHash.encode('ipns-ns', ipns_libp2pKey_CIDv1); actual.should.be.equal(ipns_ED25519_contentHash); }); + it('should refuse to encode non-libp2p-key value inlined as identity multihash', () => { + expect(() => contentHash.encode('ipns-ns', '12uA8M8Ku8mHUumxHcu7uee')).to.throw('ipns-ns allows only valid cryptographic libp2p-key identifiers, try using ED25519 pubkey instead') + }); it('should getCodec', () => { const actual = contentHash.getCodec(ipns_ED25519_contentHash); actual.should.be.equal('ipns-ns'); }); it('should decode legacy PeerID to CIDv1 with libp2p-key codec', () => { - const actual = contentHash.decode(ipns_CIDv0_contentHash); + const actual = contentHash.decode(ipns_peerID_B58_contentHash); actual.should.be.equal(ipns_CIDv1); }); it('should decode ED25519 to CIDv1 with libp2p-key codec', () => { const actual = contentHash.decode(ipns_ED25519_contentHash); actual.should.be.equal(ipns_libp2pKey_CIDv1); }); + it('should decode deprecated DNSLink identifiers', () => { + // DNSLink is fine to be used before ENS resolve occurs, but should be avoided after + // Context: https://github.com/ensdomains/ens-app/issues/849#issuecomment-777088950 + // For now, we allow decoding of legacy values: + const deprecated_dnslink_contentHash = 'e5010170000f6170702e756e69737761702e6f7267' + const deprecated_dnslink_value = 'app.uniswap.org' + const actual = contentHash.decode(deprecated_dnslink_contentHash) + actual.should.be.equal(deprecated_dnslink_value) + }); }); describe('onion', () => { it('should encode', () => { From adff4af784ad89b7cad990dc00b66e5423b5007b Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Mon, 15 Feb 2021 20:28:37 +0100 Subject: [PATCH 2/2] refactor: isCryptographicIPNS(cid) check Replaces duplicated code with a single reusable check --- src/profiles.js | 63 ++++++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/src/profiles.js b/src/profiles.js index 8249bb2..de4f7e5 100644 --- a/src/profiles.js +++ b/src/profiles.js @@ -33,6 +33,33 @@ const hexStringToBuffer = (hex) => { return multiH.fromHexString(res); } +/** + * Validates IPNS identifier to safeguard against insecure names. + * @param {CID} name ised in ipns-ns + * @return {bool} + */ +const isCryptographicIPNS = (cid) => { + try { + const { multihash } = cid + // Additional check for identifiers shorter + // than what inlined ED25519 pubkey would be + // https://github.com/ensdomains/ens-app/issues/849#issuecomment-777088950 + if (multihash.length < 38) { + const mh = multiH.decode(multihash) + // ED25519 pubkeys are inlined using identity hash function + // and we should not see anything shorter than that + if (mh.name === 'identity' && mh.length < 36) { + // One can read inlined string value via: + // console.log('ipns-ns id:', String(multiH.decode(new CID(value).multihash).digest)) + return false + } + } + // ok, CID looks fine + return true + } catch (_) { return false } + return false +} + /** * list of known encoding, * encoding should be a function that takes a `string` input, @@ -59,25 +86,13 @@ const encodes = { * @return {Buffer} */ ipns: (value) => { - const { multihash } = new CID(value) - - // Additional check for identifiers shorter - // than what inlined ED25519 pubkey would be - // https://github.com/ensdomains/ens-app/issues/849#issuecomment-777088950 - if (multihash.length < 38) { - const mh = multiH.decode(multihash) - // ED25519 pubkeys are inlined using identity hash function - // and we should not see anything shorter than that - if (mh.name === 'identity' && mh.length < 36) { - // One can read inlined string value via: - // console.log('ipns-ns id:', String(multiH.decode(new CID(value).multihash).digest)) + const cid = new CID(value) + if (!isCryptographicIPNS(cid)) { throw Error('ipns-ns allows only valid cryptographic libp2p-key identifiers, try using ED25519 pubkey instead') - } } - - // represent libp2p-key as a CID + // Represent IPNS name as a CID with libp2p-key codec // https://github.com/libp2p/specs/blob/master/RFC/0001-text-peerid-cid.md - return new CID(1, 'libp2p-key', multihash).buffer + return new CID(1, 'libp2p-key', cid.multihash).buffer }, /** * @param {string} value @@ -112,26 +127,14 @@ const decodes = { * @param {Buffer} value */ ipns: (value) => { - const cid = new CID(value).toV1(); - - // Additional check for identifiers shorter - // than what inlined ED25519 pubkey would be - // https://github.com/ensdomains/ens-app/issues/849#issuecomment-777088950 - if (cid.multihash.length < 38) { - const mh = multiH.decode(cid.multihash) - // ED25519 pubkeys are inlined using identity hash function - // and we should not see anything shorter than that - if (mh.name === 'identity' && mh.length < 36) { + const cid = new CID(value).toV1() + if (!isCryptographicIPNS(cid)) { // Value is not a libp2p-key, return original string console.warn('[ensdomains/content-hash] use of non-cryptographic identifiers in ipns-ns is deprecated and will be removed, migrate to ED25519 libp2p-key') return String(multiH.decode(new CID(value).multihash).digest) // TODO: start throwing an error (after some deprecation period) // throw Error('ipns-ns allows only valid cryptographic libp2p-key identifiers, try using ED25519 pubkey instead') - // One can read inlined string value via: - // console.log('ipns-ns id:', String(multiH.decode(new CID(value).multihash).digest)) - } } - return cid.toString('base36') }, /**