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

fix: seed and private key validation and return type in registrars #1324

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 11 additions & 1 deletion packages/askar/src/wallet/AskarWallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import type {
import type { Session } from '@hyperledger/aries-askar-shared'

import {
isValidSeed,
isValidPrivateKey,
JsonTransformer,
RecordNotFoundError,
RecordDuplicateError,
Expand Down Expand Up @@ -344,7 +346,15 @@ export class AskarWallet implements Wallet {
public async createKey({ seed, privateKey, keyType }: WalletCreateKeyOptions): Promise<Key> {
try {
if (seed && privateKey) {
throw new AriesFrameworkError('Only one of seed and privateKey can be set')
throw new WalletError('Only one of seed and privateKey can be set')
}

if (seed && !isValidSeed(seed, keyType)) {
throw new WalletError('Invalid seed provided')
}

if (privateKey && !isValidPrivateKey(privateKey, keyType)) {
throw new WalletError('Invalid private key provided')
}

if (keyTypeSupportedByAskar(keyType)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/askar/src/wallet/__tests__/AskarWallet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const walletConfig: WalletConfig = {
describe('AskarWallet basic operations', () => {
let askarWallet: AskarWallet

const seed = TypedArrayEncoder.fromString('sample-seed')
const seed = TypedArrayEncoder.fromString('sample-seed-min-of-32-bytes-long')
const privateKey = TypedArrayEncoder.fromString('2103de41b4ae37e8e28586d84a342b67')
const message = TypedArrayEncoder.fromString('sample-message')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const walletConfig: WalletConfig = {

describeSkipNode17And18('BBS Signing Provider', () => {
let wallet: Wallet
const seed = TypedArrayEncoder.fromString('sample-seed')
const seed = TypedArrayEncoder.fromString('sample-seed-min-of-32-bytes-long')
const message = TypedArrayEncoder.fromString('sample-message')

beforeEach(async () => {
Expand All @@ -41,7 +41,7 @@ describeSkipNode17And18('BBS Signing Provider', () => {
test('Create bls12381g2 keypair', async () => {
await expect(wallet.createKey({ seed, keyType: KeyType.Bls12381g2 })).resolves.toMatchObject({
publicKeyBase58:
't54oLBmhhRcDLUyWTvfYRWw8VRXRy1p43pVm62hrpShrYPuHe9WNAgS33DPfeTK6xK7iPrtJDwCHZjYgbFYDVTJHxXex9xt2XEGF8D356jBT1HtqNeucv3YsPLfTWcLcpFA',
'25TvGExLTWRTgn9h2wZuohrQmmLafXiacY4dhv66wcbY8pLbuNTBRMTgWVcPKh2wsEyrRPmnhLdc4C7LEcJ2seoxzBkoydJEdQD8aqg5dw8wesBTS9Twg8EjuFG1WPRAiERd',
keyType: KeyType.Bls12381g2,
})
})
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export { Jwk } from './JwkTypes'
export { JwsService } from './JwsService'

export * from './jwtUtils'
export * from './keyUtils'

export { KeyType } from './KeyType'
export { Key } from './Key'
Expand Down
27 changes: 27 additions & 0 deletions packages/core/src/crypto/keyUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Buffer } from '../utils'

import { KeyType } from './KeyType'

export function isValidSeed(seed: Buffer, keyType: KeyType): boolean {
const minimumSeedLength: Record<KeyType, number> = {
[KeyType.Ed25519]: 32,
[KeyType.X25519]: 32,
[KeyType.Bls12381g1]: 32,
[KeyType.Bls12381g2]: 32,
[KeyType.Bls12381g1g2]: 32,
}

return Buffer.isBuffer(seed) && seed.length >= minimumSeedLength[keyType]
}

export function isValidPrivateKey(privateKey: Buffer, keyType: KeyType): boolean {
const privateKeyLength: Record<KeyType, number> = {
[KeyType.Ed25519]: 32,
[KeyType.X25519]: 32,
[KeyType.Bls12381g1]: 32,
[KeyType.Bls12381g2]: 32,
[KeyType.Bls12381g1g2]: 32,
}

return Buffer.isBuffer(privateKey) && privateKey.length === privateKeyLength[keyType]
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,22 @@ describe('dids', () => {
],
id: 'did:key:z6MkpGR4gs4Rc3Zph4vj8wRnjnAxgAPSxcR8MAVKutWspQzc',
},
secret: { privateKey: '96213c3d7fc8d4d6754c7a0fd969598e' },
secret: { privateKey: TypedArrayEncoder.fromString('96213c3d7fc8d4d6754c7a0fd969598e') },
},
})
})

it('should create a did:peer did', async () => {
const privateKey = TypedArrayEncoder.fromString('e008ef10b7c163114b3857542b3736eb')

const did = await agent.dids.create<PeerDidNumAlgo0CreateOptions>({
method: 'peer',
options: {
keyType: KeyType.Ed25519,
numAlgo: PeerDidNumAlgo.InceptionKeyWithoutDoc,
},
secret: {
privateKey: TypedArrayEncoder.fromString('e008ef10b7c163114b3857542b3736eb'),
privateKey,
},
})

Expand Down Expand Up @@ -153,7 +155,7 @@ describe('dids', () => {
],
id: 'did:peer:0z6Mkuo91yRhTWDrFkdNBcLXAbvtUiq2J9E4QQcfYZt4hevkh',
},
secret: { privateKey: 'e008ef10b7c163114b3857542b3736eb' },
secret: { privateKey },
},
})
})
Expand Down
26 changes: 2 additions & 24 deletions packages/core/src/modules/dids/methods/key/KeyDidRegistrar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,6 @@ export class KeyDidRegistrar implements DidRegistrar {
}
}

if (seed && (typeof seed !== 'object' || seed.length !== 32)) {
return {
didDocumentMetadata: {},
didRegistrationMetadata: {},
didState: {
state: 'failed',
reason: 'Invalid seed provided',
},
}
}

if (privateKey && (typeof privateKey !== 'object' || privateKey.length !== 32)) {
return {
didDocumentMetadata: {},
didRegistrationMetadata: {},
didState: {
state: 'failed',
reason: 'Invalid private key provided',
},
}
}

try {
const key = await agentContext.wallet.createKey({
keyType,
Expand Down Expand Up @@ -81,8 +59,8 @@ export class KeyDidRegistrar implements DidRegistrar {
// we can only return it if the seed was passed in by the user. Once
// we have a secure method for generating seeds we should use the same
// approach
seed: options.secret?.seed?.toString(),
privateKey: options.secret?.privateKey?.toString(),
seed: options.secret?.seed,
privateKey: options.secret?.privateKey,
},
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { KeyType } from '../../../../../crypto'
import { Key } from '../../../../../crypto/Key'
import { TypedArrayEncoder } from '../../../../../utils'
import { JsonTransformer } from '../../../../../utils/JsonTransformer'
import { WalletError } from '../../../../../wallet/error'
import { DidDocumentRole } from '../../../domain/DidDocumentRole'
import { DidRepository } from '../../../repository/DidRepository'
import { KeyDidRegistrar } from '../KeyDidRegistrar'
Expand Down Expand Up @@ -53,7 +54,7 @@ describe('DidRegistrar', () => {
did: 'did:key:z6MksLeew51QS6Ca6tVKM56LQNbxCNVcLHv4xXj4jMkAhPWU',
didDocument: didKeyz6MksLeFixture,
secret: {
privateKey: '96213c3d7fc8d4d6754c712fd969598e',
privateKey,
},
},
})
Expand All @@ -78,10 +79,10 @@ describe('DidRegistrar', () => {
})
})

it('should return an error state if an invalid private key is provided', async () => {
it('should return an error state if a key creation error is thrown', async () => {
mockFunction(walletMock.createKey).mockRejectedValueOnce(new WalletError('Invalid private key provided'))
const result = await keyDidRegistrar.create(agentContext, {
method: 'key',

options: {
keyType: KeyType.Ed25519,
},
Expand All @@ -95,7 +96,7 @@ describe('DidRegistrar', () => {
didRegistrationMetadata: {},
didState: {
state: 'failed',
reason: 'Invalid private key provided',
reason: expect.stringContaining('Invalid private key provided'),
},
})
})
Expand Down
26 changes: 2 additions & 24 deletions packages/core/src/modules/dids/methods/peer/PeerDidRegistrar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,28 +41,6 @@ export class PeerDidRegistrar implements DidRegistrar {
}
}

if (seed && (typeof seed !== 'object' || seed.length !== 32)) {
return {
didDocumentMetadata: {},
didRegistrationMetadata: {},
didState: {
state: 'failed',
reason: 'Invalid seed provided',
},
}
}

if (privateKey && (typeof privateKey !== 'object' || privateKey.length !== 32)) {
return {
didDocumentMetadata: {},
didRegistrationMetadata: {},
didState: {
state: 'failed',
reason: 'Invalid private key provided',
},
}
}

const key = await agentContext.wallet.createKey({
keyType,
seed,
Expand Down Expand Up @@ -119,8 +97,8 @@ export class PeerDidRegistrar implements DidRegistrar {
// we can only return it if the seed was passed in by the user. Once
// we have a secure method for generating seeds we should use the same
// approach
seed: options.secret?.seed?.toString(),
privateKey: options.secret?.privateKey?.toString(),
seed: options.secret?.seed,
privateKey: options.secret?.privateKey,
},
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { KeyType } from '../../../../../crypto'
import { Key } from '../../../../../crypto/Key'
import { TypedArrayEncoder } from '../../../../../utils'
import { JsonTransformer } from '../../../../../utils/JsonTransformer'
import { WalletError } from '../../../../../wallet/error'
import { DidCommV1Service, DidDocumentBuilder } from '../../../domain'
import { DidDocumentRole } from '../../../domain/DidDocumentRole'
import { getEd25519VerificationMethod } from '../../../domain/key-type/ed25519'
Expand Down Expand Up @@ -54,7 +55,7 @@ describe('DidRegistrar', () => {
did: 'did:peer:0z6MksLeew51QS6Ca6tVKM56LQNbxCNVcLHv4xXj4jMkAhPWU',
didDocument: didPeer0z6MksLeFixture,
secret: {
privateKey: '96213c3d7fc8d4d6754c712fd969598e',
privateKey,
},
},
})
Expand All @@ -79,7 +80,9 @@ describe('DidRegistrar', () => {
})
})

it('should return an error state if an invalid private key is provided', async () => {
it('should return an error state if a key creation error is thrown', async () => {
mockFunction(walletMock.createKey).mockRejectedValueOnce(new WalletError('Invalid private key provided'))

const result = await peerDidRegistrar.create(agentContext, {
method: 'peer',
options: {
Expand All @@ -96,7 +99,7 @@ describe('DidRegistrar', () => {
didRegistrationMetadata: {},
didState: {
state: 'failed',
reason: 'Invalid private key provided',
reason: expect.stringContaining('Invalid private key provided'),
},
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/indy-sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@
"@aries-framework/anoncreds": "0.3.3",
"@aries-framework/core": "0.3.3",
"@types/indy-sdk": "1.16.26",
"@stablelib/ed25519": "^1.0.3",
"class-transformer": "^0.5.1",
"class-validator": "^0.14.0",
"rxjs": "^7.2.0",
"tsyringe": "^4.7.0"
},
"devDependencies": {
"@stablelib/ed25519": "^1.0.3",
"rimraf": "^4.0.7",
"typescript": "~4.9.4"
}
Expand Down
6 changes: 3 additions & 3 deletions packages/indy-sdk/src/dids/IndySdkSovDidRegistrar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {
} from '@aries-framework/core'
import type { NymRole } from 'indy-sdk'

import { DidDocumentRole, DidRecord, DidRepository } from '@aries-framework/core'
import { KeyType, isValidPrivateKey, DidDocumentRole, DidRecord, DidRepository } from '@aries-framework/core'

import { IndySdkError } from '../error'
import { isIndyError } from '../error/indyError'
Expand All @@ -34,7 +34,7 @@ export class IndySdkSovDidRegistrar implements DidRegistrar {
const { alias, role, submitterDid, indyNamespace } = options.options
const privateKey = options.secret?.privateKey

if (privateKey && (typeof privateKey !== 'object' || privateKey.length !== 32)) {
if (privateKey && !isValidPrivateKey(privateKey, KeyType.Ed25519)) {
return {
didDocumentMetadata: {},
didRegistrationMetadata: {},
Expand Down Expand Up @@ -120,7 +120,7 @@ export class IndySdkSovDidRegistrar implements DidRegistrar {
// we can only return it if the seed was passed in by the user. Once
// we have a secure method for generating seeds we should use the same
// approach
privateKey: options.secret?.privateKey?.toString(),
privateKey: options.secret?.privateKey,
},
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe('IndySdkSovDidRegistrar', () => {
})

it('should correctly create a did:sov document without services', async () => {
const privateKey = '96213c3d7fc8d4d6754c712fd969598e'
const privateKey = TypedArrayEncoder.fromString('96213c3d7fc8d4d6754c712fd969598e')

const registerPublicDidSpy = jest.spyOn(indySdkSovDidRegistrar, 'registerPublicDid')
registerPublicDidSpy.mockImplementationOnce(() => Promise.resolve('R1xKJw17sUoXhejEpugMYJ'))
Expand All @@ -144,7 +144,7 @@ describe('IndySdkSovDidRegistrar', () => {
role: 'STEWARD',
},
secret: {
privateKey: TypedArrayEncoder.fromString(privateKey),
privateKey,
},
})

Expand Down Expand Up @@ -206,7 +206,7 @@ describe('IndySdkSovDidRegistrar', () => {
})

it('should correctly create a did:sov document with services', async () => {
const privateKey = '96213c3d7fc8d4d6754c712fd969598e'
const privateKey = TypedArrayEncoder.fromString('96213c3d7fc8d4d6754c712fd969598e')

const registerPublicDidSpy = jest.spyOn(indySdkSovDidRegistrar, 'registerPublicDid')
registerPublicDidSpy.mockImplementationOnce(() => Promise.resolve('R1xKJw17sUoXhejEpugMYJ'))
Expand All @@ -227,7 +227,7 @@ describe('IndySdkSovDidRegistrar', () => {
},
},
secret: {
privateKey: TypedArrayEncoder.fromString(privateKey),
privateKey,
},
})

Expand Down
12 changes: 11 additions & 1 deletion packages/indy-sdk/src/wallet/IndySdkWallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import {
Key,
SigningProviderRegistry,
TypedArrayEncoder,
isValidSeed,
isValidPrivateKey,
} from '@aries-framework/core'
import { inject, injectable } from 'tsyringe'

Expand Down Expand Up @@ -415,7 +417,15 @@ export class IndySdkWallet implements Wallet {
public async createKey({ seed, privateKey, keyType }: WalletCreateKeyOptions): Promise<Key> {
try {
if (seed && privateKey) {
throw new AriesFrameworkError('Only one of seed and privateKey can be set')
throw new WalletError('Only one of seed and privateKey can be set')
}

if (seed && !isValidSeed(seed, keyType)) {
throw new WalletError('Invalid seed provided')
}

if (privateKey && !isValidPrivateKey(privateKey, keyType)) {
throw new WalletError('Invalid private key provided')
}

// Ed25519 is supported natively in Indy wallet
Expand Down
2 changes: 1 addition & 1 deletion packages/indy-sdk/tests/sov-did-registrar.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe('dids', () => {
id: `did:sov:${indyDid}`,
},
secret: {
privateKey: privateKey.toString(),
privateKey,
},
},
})
Expand Down
Loading