Skip to content

fix: json ld fixes and aca-py interop fixes #1865

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

fix: json ld fixes and aca-py interop fixes #1865

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
eee749f
feat: squashing commits
KolbyRKunz May 14, 2024
34f1f3e
feat: merged 0.5.3
KolbyRKunz May 14, 2024
2c9e53b
feat: merging
KolbyRKunz May 14, 2024
29f7d73
Merge branch 'main' into feat/JsonLdFixes5.3
KolbyRKunz May 14, 2024
9eeb5c8
feat: Updating warnings in didExchange, updated error messages in key…
KolbyRKunz May 15, 2024
187cf4e
feat: updated to 0.5.2 and adding additional changes
KolbyRKunz May 23, 2024
b641f1a
feat: small changes
KolbyRKunz May 23, 2024
5a15fb4
feat: constructing unqualified schema correctly
KolbyRKunz May 24, 2024
ee6403c
feat: fixed unqualified schema id
KolbyRKunz May 24, 2024
cd1befd
feat: adding try catch back in
KolbyRKunz May 28, 2024
53d496b
feat: getting schema did from metadata
KolbyRKunz May 28, 2024
888909d
feat: fixed import in vdr for react native
KolbyRKunz Jun 14, 2024
61ab47b
chore: merging main
KolbyRKunz Jun 14, 2024
41d4180
feat: added additional check to multikey and removed multikey resolut…
KolbyRKunz Jun 14, 2024
4493bb9
fix: removed extra type
KolbyRKunz Jun 14, 2024
79bd5ce
formatting
KolbyRKunz Jun 14, 2024
aedc9b3
feat: require a didRotate or didDoc attachment so we can validate sig…
KolbyRKunz Jun 17, 2024
d9940a2
feat: removing redundent changes
KolbyRKunz Jun 17, 2024
2a10e10
feat: removing did:peer:2 changes
KolbyRKunz Jun 17, 2024
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
13 changes: 11 additions & 2 deletions packages/anoncreds/src/utils/w3cAnonCredsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
isUnqualifiedRevocationRegistryId,
isIndyDid,
getUnQualifiedDidIndyDid,
isUnqualifiedIndyDid,
} from './indyIdentifiers'
import { W3cAnonCredsCredentialMetadataKey } from './metadata'

Expand Down Expand Up @@ -166,6 +167,14 @@ export function getStoreCredentialOptions(
return storeCredentialOptions
}

// The issuer of the schema does not always match the issuer of the credential definition thus the unqualified schema id needs to be derived from both values
function getUnqualifiedSchemaId(schemaIssuerId: string, schemaId: string) {
const schemaDid = schemaIssuerId.split(':')[3]
const split = getUnQualifiedDidIndyDid(schemaId).split(':')
split[0] = schemaDid
return split.join(':')
}

export function getW3cRecordAnonCredsTags(options: {
credentialSubject: W3cCredentialSubject
issuerId: string
Expand Down Expand Up @@ -199,10 +208,10 @@ export function getW3cRecordAnonCredsTags(options: {
anonCredsMethodName: methodName,
anonCredsRevocationRegistryId: revocationRegistryId,
anonCredsCredentialRevocationId: credentialRevocationId,
...(isIndyDid(issuerId) && {
...((isIndyDid(issuerId) || isUnqualifiedIndyDid(issuerId)) && {
anonCredsUnqualifiedIssuerId: getUnQualifiedDidIndyDid(issuerId),
anonCredsUnqualifiedCredentialDefinitionId: getUnQualifiedDidIndyDid(credentialDefinitionId),
anonCredsUnqualifiedSchemaId: getUnQualifiedDidIndyDid(schemaId),
anonCredsUnqualifiedSchemaId: getUnqualifiedSchemaId(schema.issuerId, schemaId),
anonCredsUnqualifiedSchemaIssuerId: getUnQualifiedDidIndyDid(schema.issuerId),
anonCredsUnqualifiedRevocationRegistryId: revocationRegistryId
? getUnQualifiedDidIndyDid(revocationRegistryId)
Expand Down
88 changes: 43 additions & 45 deletions packages/core/src/modules/connections/DidExchangeProtocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,58 +523,56 @@ export class DidExchangeProtocol {
if (message instanceof DidExchangeResponseMessage) {
const didRotateAttachment = message.didRotate

if (!didRotateAttachment) {
throw new DidExchangeProblemReportError('DID Rotate attachment is missing.', {
problemCode: DidExchangeProblemReportReason.ResponseNotAccepted,
})
}

const jws = didRotateAttachment.data.jws
if (didRotateAttachment) {
const jws = didRotateAttachment.data.jws

if (!jws) {
throw new DidExchangeProblemReportError('DID Rotate signature is missing.', {
problemCode: DidExchangeProblemReportReason.ResponseNotAccepted,
})
}
if (!jws) {
throw new DidExchangeProblemReportError('DID Rotate signature is missing.', {
problemCode: DidExchangeProblemReportReason.ResponseNotAccepted,
})
}

if (!didRotateAttachment.data.base64) {
throw new CredoError('DID Rotate attachment is missing base64 property for signed did.')
}
if (!didRotateAttachment.data.base64) {
throw new CredoError('DID Rotate attachment is missing base64 property for signed did.')
}

// JWS payload must be base64url encoded
const base64UrlPayload = base64ToBase64URL(didRotateAttachment.data.base64)
const signedDid = TypedArrayEncoder.fromBase64(base64UrlPayload).toString()
// JWS payload must be base64url encoded
const base64UrlPayload = base64ToBase64URL(didRotateAttachment.data.base64)
const signedDid = TypedArrayEncoder.fromBase64(base64UrlPayload).toString()

if (signedDid !== message.did) {
throw new CredoError(
`DID Rotate attachment's did ${message.did} does not correspond to message did ${message.did}`
)
}
if (signedDid !== message.did) {
throw new CredoError(
`DID Rotate attachment's did ${message.did} does not correspond to message did ${message.did}`
)
}

const { isValid, signerKeys } = await this.jwsService.verifyJws(agentContext, {
jws: {
...jws,
payload: base64UrlPayload,
},
jwkResolver: ({ jws: { header } }) => {
if (typeof header.kid !== 'string' || !isDid(header.kid, 'key')) {
throw new CredoError('JWS header kid must be a did:key DID.')
}
const { isValid, signerKeys } = await this.jwsService.verifyJws(agentContext, {
jws: {
...jws,
payload: base64UrlPayload,
},
jwkResolver: ({ jws: { header } }) => {
if (typeof header.kid !== 'string' || !isDid(header.kid, 'key')) {
throw new CredoError('JWS header kid must be a did:key DID.')
}

const didKey = DidKey.fromDid(header.kid)
return getJwkFromKey(didKey.key)
},
})
const didKey = DidKey.fromDid(header.kid)
return getJwkFromKey(didKey.key)
},
})

if (!isValid || !signerKeys.every((key) => invitationKeysBase58?.includes(key.publicKeyBase58))) {
throw new DidExchangeProblemReportError(
`DID Rotate signature is invalid. isValid: ${isValid} signerKeys: ${JSON.stringify(
signerKeys
)} invitationKeys:${JSON.stringify(invitationKeysBase58)}`,
{
problemCode: DidExchangeProblemReportReason.ResponseNotAccepted,
}
)
if (!isValid || !signerKeys.every((key) => invitationKeysBase58?.includes(key.publicKeyBase58))) {
throw new DidExchangeProblemReportError(
`DID Rotate signature is invalid. isValid: ${isValid} signerKeys: ${JSON.stringify(
signerKeys
)} invitationKeys:${JSON.stringify(invitationKeysBase58)}`,
{
problemCode: DidExchangeProblemReportReason.ResponseNotAccepted,
}
)
}
} else {
this.logger.warn(`Document does not contain didRotate`)
}
Copy link
Contributor

@TimoGlastra TimoGlastra Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this change is correct. extractResolvableDidDocument is called when the did document is not did:peer:1. If we resolve the did, instead of using an diddocument attachment, we REQUIRE the rotate attachment to be present. Could you explain a bit more why this change is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test case I ran into was agents or mediators using did:peer:2 without the intention of doing a did rotate or alsoKnownAs. It also is a part of the didExchange RFC that the value is optional for any reason so for interoperability sake I think this should remain.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the did is not rotated from the invitation we can indeed ignore it. But if the did is rotated, not doing this check means the connection can be hijacked. I could send a response even though you created the invitation, because we don't require a signature from the invitation did. If the did is the same, then we don't have that issue and i'm ok with not requiring did rotate. But the current code doesn't take this into consideration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might be confused but the warn only happens if the attachment is not present, the else block is for the if statement on line 526 where it checks if the didRotateAttachment is defined. The signature is always checked if the rotate attachment is present. The else block with the warn was a suggestion made by @genaris when this PR was initially made.

Copy link
Contributor

@TimoGlastra TimoGlastra Jun 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The check that is now in place always allows didRotate to not be present, meaning we could just remove the check. So we need to be more specific in checking when didRotate needs to be present: when you rotate your did from the invitation. Even though the Aries RFC mentions it's optional, it's super important for security to do this check. did_rotate was added specifically for this reason, and it's only optional to not introduce a breaking change in the RFC. Before did rotate was introduced, it was basically not possible to rotate a did from the invitation if you were not using diddoc~attach and sign it.

If the did is rotated from the invitation, either a signed diddoc~attach must be present OR a signed did_rotate~attach must be present

}

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/modules/credentials/protocol/v2/messages/V2OfferCredentialMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export class V2OfferCredentialMessage extends AgentMessage {

@Expose({ name: 'credential_preview' })
@Type(() => V2CredentialPreview)
@IsOptional()
@ValidateNested()
@IsInstance(V2CredentialPreview)
public credentialPreview?: V2CredentialPreview
Expand Down
14 changes: 11 additions & 3 deletions packages/core/src/modules/dids/domain/verificationMethod/Bls12381G1Key2020.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@ export function isBls12381G1Key2020(verificationMethod: VerificationMethod): ver
* Get a key from a Bls12381G1Key2020 verification method.
*/
export function getKeyFromBls12381G1Key2020(verificationMethod: Bls12381G1Key2020) {
if (!verificationMethod.publicKeyBase58) {
throw new CredoError('verification method is missing publicKeyBase58')
if (verificationMethod.publicKeyBase58) {
return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.Bls12381g1)
}
if (verificationMethod.publicKeyMultibase) {
const key = Key.fromFingerprint(verificationMethod.publicKeyMultibase)
if (key.keyType === KeyType.Bls12381g1) return key
else
throw new CredoError(
`Unexpected key type from resolving multibase encoding, key type was ${key.keyType} but expected ${KeyType.Bls12381g1}}`
)
}

return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.Bls12381g1)
throw new CredoError('verification method is missing publicKeyBase58 or publicKeyMultibase')
}
14 changes: 11 additions & 3 deletions packages/core/src/modules/dids/domain/verificationMethod/Bls12381G2Key2020.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@ export function isBls12381G2Key2020(verificationMethod: VerificationMethod): ver
* Get a key from a Bls12381G2Key2020 verification method.
*/
export function getKeyFromBls12381G2Key2020(verificationMethod: Bls12381G2Key2020) {
if (!verificationMethod.publicKeyBase58) {
throw new CredoError('verification method is missing publicKeyBase58')
if (verificationMethod.publicKeyBase58) {
return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.Bls12381g2)
}
if (verificationMethod.publicKeyMultibase) {
const key = Key.fromFingerprint(verificationMethod.publicKeyMultibase)
if (key.keyType === KeyType.Bls12381g2) return key
else
throw new CredoError(
`Unexpected key type from resolving multibase encoding, key type was ${key.keyType} but expected ${KeyType.Bls12381g2}}`
)
}

return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.Bls12381g2)
throw new CredoError('verification method is missing publicKeyBase58 or publicKeyMultibase')
}
14 changes: 11 additions & 3 deletions ...ages/core/src/modules/dids/domain/verificationMethod/EcdsaSecp256k1VerificationKey2019.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,17 @@ export function isEcdsaSecp256k1VerificationKey2019(
* Get a key from a EcdsaSecp256k1VerificationKey2019 verification method.
*/
export function getKeyFromEcdsaSecp256k1VerificationKey2019(verificationMethod: EcdsaSecp256k1VerificationKey2019) {
if (!verificationMethod.publicKeyBase58) {
throw new CredoError('verification method is missing publicKeyBase58')
if (verificationMethod.publicKeyBase58) {
return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.K256)
}
if (verificationMethod.publicKeyMultibase) {
const key = Key.fromFingerprint(verificationMethod.publicKeyMultibase)
if (key.keyType === KeyType.K256) return key
else
throw new CredoError(
`Unexpected key type from resolving multibase encoding, key type was ${key.keyType} but expected ${KeyType.K256}}`
)
}

return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.K256)
throw new CredoError('verification method is missing publicKeyBase58 or publicKeyMultibase')
}
14 changes: 11 additions & 3 deletions packages/core/src/modules/dids/domain/verificationMethod/Ed25519VerificationKey2018.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,17 @@ export function isEd25519VerificationKey2018(
* Get a key from a Ed25519VerificationKey2018 verification method.
*/
export function getKeyFromEd25519VerificationKey2018(verificationMethod: Ed25519VerificationKey2018) {
if (!verificationMethod.publicKeyBase58) {
throw new CredoError('verification method is missing publicKeyBase58')
if (verificationMethod.publicKeyBase58) {
return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.Ed25519)
}
if (verificationMethod.publicKeyMultibase) {
const key = Key.fromFingerprint(verificationMethod.publicKeyMultibase)
if (key.keyType === KeyType.Ed25519) return key
else
throw new CredoError(
`Unexpected key type from resolving multibase encoding, key type was ${key.keyType} but expected ${KeyType.Ed25519}`
)
}

return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.Ed25519)
throw new CredoError('verification method is missing publicKeyBase58 or publicKeyMultibase')
}
14 changes: 11 additions & 3 deletions packages/core/src/modules/dids/domain/verificationMethod/X25519KeyAgreementKey2019.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,17 @@ export function isX25519KeyAgreementKey2019(
* Get a key from a X25519KeyAgreementKey2019 verification method.
*/
export function getKeyFromX25519KeyAgreementKey2019(verificationMethod: X25519KeyAgreementKey2019) {
if (!verificationMethod.publicKeyBase58) {
throw new CredoError('verification method is missing publicKeyBase58')
if (verificationMethod.publicKeyBase58) {
return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.X25519)
}
if (verificationMethod.publicKeyMultibase) {
const key = Key.fromFingerprint(verificationMethod.publicKeyMultibase)
if (key.keyType === KeyType.X25519) return key
else
throw new CredoError(
`Unexpected key type from resolving multibase encoding, key type was ${key.keyType} but expected ${KeyType.X25519}`
)
}

return Key.fromPublicKeyBase58(verificationMethod.publicKeyBase58, KeyType.X25519)
throw new CredoError('verification method is missing publicKeyBase58 or publicKeyMultibase')
}
2 changes: 1 addition & 1 deletion packages/core/src/modules/dids/methods/peer/didPeer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { CredoError } from '../../../../error'
import { getAlternativeDidsForNumAlgo4Did } from './peerDidNumAlgo4'

const PEER_DID_REGEX = new RegExp(
'^did:peer:(([01](z)([1-9a-km-zA-HJ-NP-Z]{5,200}))|(2((.[AEVID](z)([1-9a-km-zA-HJ-NP-Z]{5,200}))+(.(S)[0-9a-zA-Z=]*)?))|([4](z[1-9a-km-zA-HJ-NP-Z]{46})(:z[1-9a-km-zA-HJ-NP-Z]{6,}){0,1}))$'
'^did:peer:(([01](z)([1-9a-km-zA-HJ-NP-Z]{5,200}))|(2((.[AEVID](z)([1-9a-km-zA-HJ-NP-Z]{5,200}))+(.(S)[0-9a-zA-Z=]*)*))|([4](z[1-9a-km-zA-HJ-NP-Z]{46})(:z[1-9a-km-zA-HJ-NP-Z]{6,}){0,1}))$'
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this change in regex mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This regex is specifically changed for did:peer:2. With the updates to the protocol of did:peer:2 the services on the encoded version are not always a list and can be multiple service appended instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have an example did:peer so I can add a test covering this?

)

export function isValidPeerDid(did: string): boolean {
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/modules/dids/methods/peer/peerDidNumAlgo2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ export function didToNumAlgo2DidDocument(did: string) {
for (let service of services) {
// Expand abbreviations used for service key/values
service = expandServiceAbbreviations(service)

service.id = `${did}#${service.type.toLowerCase()}-${serviceIndex++}`

didDocument.addService(JsonTransformer.fromJSON(service, DidDocumentService))
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/modules/dif-presentation-exchange/DifPresentationExchangeService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ export class DifPresentationExchangeService {
for (const inputDescriptor of pd.input_descriptors) {
for (const schema of inputDescriptor.schema) {
w3cQuery.push({
$or: [{ expandedType: [schema.uri] }, { contexts: [schema.uri] }, { type: [schema.uri] }],
$or: [{ expandedTypes: [schema.uri] }, { contexts: [schema.uri] }, { types: [schema.uri] }],
})
}
}
Expand Down
25 changes: 18 additions & 7 deletions packages/core/src/modules/vc/data-integrity/W3cJsonLdCredentialService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { createWalletKeyPairClass } from '../../../crypto/WalletKeyPair'
import { CredoError } from '../../../error'
import { injectable } from '../../../plugins'
import { asArray, JsonTransformer } from '../../../utils'
import { VerificationMethod } from '../../dids'
import { DidsApi, VerificationMethod } from '../../dids'
import { getKeyFromVerificationMethod } from '../../dids/domain/key-type'
import { W3cCredentialsModuleConfig } from '../W3cCredentialsModuleConfig'
import { w3cDate } from '../util'
Expand Down Expand Up @@ -339,12 +339,23 @@ export class W3cJsonLdCredentialService {
agentContext: AgentContext,
verificationMethod: string
): Promise<Key> {
const documentLoader = this.w3cCredentialsModuleConfig.documentLoader(agentContext)
const verificationMethodObject = await documentLoader(verificationMethod)
const verificationMethodClass = JsonTransformer.fromJSON(verificationMethodObject.document, VerificationMethod)

const key = getKeyFromVerificationMethod(verificationMethodClass)
return key
if (!verificationMethod.startsWith('did:')) {
const documentLoader = this.w3cCredentialsModuleConfig.documentLoader(agentContext)
const verificationMethodObject = await documentLoader(verificationMethod)
const verificationMethodClass = JsonTransformer.fromJSON(verificationMethodObject.document, VerificationMethod)

const key = getKeyFromVerificationMethod(verificationMethodClass)
return key
} else {
const [did, keyid] = verificationMethod.split('#')
const didsApi = agentContext.dependencyManager.resolve(DidsApi)
const doc = await didsApi.resolve(did)
if (doc.didDocument) {
const verificationMethodClass = doc.didDocument.dereferenceKey(keyid)
return getKeyFromVerificationMethod(verificationMethodClass)
}
throw new CredoError(`Could not resolve verification method with id ${verificationMethod}`)
}
Comment on lines +342 to +358
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? The document loader should be able to load did documents, so I'm not sure if we want to bypass it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The document loader would resolve the did but not return the full did Document with the verification information applied. This might have changed since my testing because I added this while testing with 0.5.1

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry, could you elaborate a bit further? What do you mean with "not return the full did Document with the verification information applied"? What does it return?

}

private getSignatureSuitesForCredential(agentContext: AgentContext, credential: W3cJsonLdVerifiableCredential) {
Expand Down
Loading