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.

Sign up for GitHub

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

fix!: require V2 signatures #180

Merged
merged 2 commits into from
Sep 20, 2022
Merged

fix!: require V2 signatures #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
caa95eb
fix: require V2 signatures
lidel Sep 12, 2022
5e60f5d
docs: create output
lidel Sep 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# ipns <!-- omit in toc -->

[![ipfs.io](https://img.shields.io/badge/project-IPFS-blue.svg?style=flat-square)](http://ipfs.io)
[![IRC](https://img.shields.io/badge/freenode-%23ipfs-blue.svg?style=flat-square)](http://webchat.freenode.net/?channels=%23ipfs)
[![ipfs.tech](https://img.shields.io/badge/project-IPFS-blue.svg?style=flat-square)](https://ipfs.tech)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of making this change here, can you please PR these lines then running npx aegir check-project in the root of this repo will update the badges, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: ipfs/aegir#1073

[![Discord](https://img.shields.io/discord/806902334369824788?style=flat-square)](https://discord.gg/ipfs)
[![codecov](https://img.shields.io/codecov/c/github/ipfs/js-ipns.svg?style=flat-square)](https://codecov.io/gh/ipfs/js-ipns)
[![CI](https://img.shields.io/github/workflow/status/ipfs/js-ipns/test%20&%20maybe%20release/master?style=flat-square)](https://github.com/ipfs/js-ipns/actions/workflows/js-test-and-release.yml)
Expand Down Expand Up @@ -126,7 +125,7 @@ const validator = ipns.validator

Contains an object with `validate (marshalledData, key)` and `select (dataA, dataB)` functions.

The `validate` async function aims to verify if an IPNS record is valid. First the record is unmarshalled, then the public key is obtained and finally the record is validated (signature and validity are verified).
The `validate` async function aims to verify if an IPNS record is valid. First the record is unmarshalled, then the public key is obtained and finally the record is validated (`signatureV2` of CBOR `data` is verified).

The `select` function is responsible for deciding which ipns record is the best (newer) between two records. Both records are unmarshalled and their sequence numbers are compared. If the first record provided is the newer, the operation result will be `0`, otherwise the operation result will be `1`.

Expand All @@ -151,10 +150,12 @@ Returns a `Promise` that resolves to an object with the entry's properties eg:
```js
{
value: Uint8Array,
signature: Uint8Array,
signature: Uint8Array, // V1 (legacy, ignored)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

link to this PR and ipfs/js-ipfs#4207?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo unnecessary noise for readme – is someone wants to dig, they will be able to read specs: ipfs/specs#319

validityType: 0,
validity: Uint8Array,
sequence: 2
sequence: 2,
signatureV2: Uint8Array, // V2 signature of data field
data: Uint8Array // DAG-CBOR that was signed
}
```

Expand Down
6 changes: 3 additions & 3 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const _create = async (peerId: PeerId, value: Uint8Array, seq: number | bigint,
}

const privateKey = await unmarshalPrivateKey(peerId.privateKey)
const signatureV1 = await sign(privateKey, value, validityType, isoValidity)
const signatureV1 = await signLegacyV1(privateKey, value, validityType, isoValidity)
const data = createCborData(value, isoValidity, validityType, seq, ttl)
const sigData = ipnsEntryDataForV2Sig(data)
const signatureV2 = await privateKey.sign(sigData)
Expand Down Expand Up @@ -144,9 +144,9 @@ export { peerIdToRoutingKey } from './utils.js'
export { peerIdFromRoutingKey } from './utils.js'

/**
* Sign ipns record data
* Sign ipns record data using the legacy V1 signature scheme
*/
const sign = async (privateKey: PrivateKey, value: Uint8Array, validityType: IpnsEntry.ValidityType, validity: Uint8Array) => {
const signLegacyV1 = async (privateKey: PrivateKey, value: Uint8Array, validityType: IpnsEntry.ValidityType, validity: Uint8Array) => {
try {
const dataForSignature = ipnsEntryDataForV1Sig(value, validityType, validity)

Expand Down
5 changes: 2 additions & 3 deletions src/validator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import errCode from 'err-code'
import { toString as uint8ArrayToString } from 'uint8arrays/to-string'
import { equals as uint8ArrayEquals } from 'uint8arrays/equals'
import { IpnsEntry } from './pb/ipns.js'
import { parseRFC3339, extractPublicKey, ipnsEntryDataForV1Sig, ipnsEntryDataForV2Sig, unmarshal, peerIdFromRoutingKey, parseCborData } from './utils.js'
import { parseRFC3339, extractPublicKey, ipnsEntryDataForV2Sig, unmarshal, peerIdFromRoutingKey, parseCborData } from './utils.js'
import * as ERRORS from './errors.js'
import type { IPNSEntry } from './index.js'
import type { PublicKey } from '@libp2p/interface-keys'
Expand All @@ -27,8 +27,7 @@ export const validate = async (publicKey: PublicKey, entry: IPNSEntry) => {

validateCborDataMatchesPbData(entry)
} else {
signature = entry.signature ?? new Uint8Array(0)
dataForSignature = ipnsEntryDataForV1Sig(value, validityType, validity)
throw errCode(new Error('missing data or signatureV2'), ERRORS.ERR_SIGNATURE_VERIFICATION)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe split if ((entry.signatureV2 != null) && (entry.data != null)) { with an escape hatch thrown error for missing data, and one for signature, instead of combining them?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the more we combine errors, and the less detail we provide, the more problems we will run into like ipfs/interop#462 where it's much harder to track down issues than it needs to be.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep it simple and have single error: these fields are all-or-nothing.

}

// Validate Signature
Expand Down
24 changes: 21 additions & 3 deletions test/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,35 @@ describe('ipns', function () {
await ipnsValidator(peerIdToRoutingKey(peerId), marshal(entry))
})

it('should validate a v1 message', async () => {
it('should fail to validate a v1 (deprecated legacy) message', async () => {
const sequence = 0
const validity = 1000000

const entry = await ipns.create(peerId, cid, sequence, validity)

// extra fields added for v2 sigs
// remove the extra fields added for v2 sigs
delete entry.data
delete entry.signatureV2

await ipnsValidator(peerIdToRoutingKey(peerId), marshal(entry))
// confirm a v1 exists
expect(entry).to.have.property('signature')

await expect(ipnsValidator(peerIdToRoutingKey(peerId), marshal(entry))).to.eventually.be.rejected().with.property('code', ERRORS.ERR_SIGNATURE_VERIFICATION)
})

it('should fail to validate a v2 without v2 signature (ignore v1)', async () => {
const sequence = 0
const validity = 1000000

const entry = await ipns.create(peerId, cid, sequence, validity)

// remove v2 sig
delete entry.signatureV2

// confirm a v1 exists
expect(entry).to.have.property('signature')

await expect(ipnsValidator(peerIdToRoutingKey(peerId), marshal(entry))).to.eventually.be.rejected().with.property('code', ERRORS.ERR_SIGNATURE_VERIFICATION)
})

it('should fail to validate a bad record', async () => {
Expand Down