Updated Handler for appencryptionrequest, added z_functions ported from React Native Verus#228
Updated Handler for appencryptionrequest, added z_functions ported from React Native Verus#228iamahmedshahh wants to merge 12 commits intoVerusCoin:generic-request-changesfrom
Conversation
… decrypt from react-native-zcash
Add validation logic to prevent spoofing attacks in AppEncryptionRequest flow: - If requestSignerID is in wallet: appOrDelegatedID can be any identity (user is delegating) - If requestSignerID is NOT in wallet: appOrDelegatedID must equal requestSignerID or be absent (app can only claim to be itself) New helper functions: - isIdentityInWallet: checks if identity's primaryaddresses include wallet address - validateRequestIdentities: enforces the above rules, returns validated appID - validateResponseSignerID: validates user's responding identity is in wallet Handler does the following: - Validates identities before key derivation - Builds full AppEncryptionResponseDetails (ivk, evk, address, optionally spending_key) - Encrypts response to app's z-address - Pushes result to response array for envelope construction
-- fixed typo in zGet validate response signer in now the main handler of appEncryptionHandler -- changed the top level import on zfunctions from react native zcash (old) to react native verus (new)
… on miketout changes -- added checks in appEncryptionRequestValidator -- requests are handled in appEncryptionRequestHandler -- returns encrypted response and handledIndices
-- returned encrypted results directly -- code clean up
…stVDXFObject function - using the ESK for deriving channel keys instead of seed - removed the app or delegated id check that was in appEncryptionRequestValidator because that would go on envelope level.
.yarn/releases/yarn-classic.cjs
Outdated
There was a problem hiding this comment.
Please remove this file, we do not use yarn classic
There was a problem hiding this comment.
removed, will push when make all changes
There was a problem hiding this comment.
Please undo this change, we should point to the VerusCoin repo and pointing to biz's repo was temporary
There was a problem hiding this comment.
I dont see it pointing to any repo except for the ./m2/repository
| const keys = derivationResult.result; | ||
|
|
||
| // build response details | ||
| const responseDetails = AppEncryptionResponseDetails.fromJson({ |
There was a problem hiding this comment.
Please avoid using the fromJson function, and use the constructor while passing in and formatting the data as needed by the constructor
| // encrypt response to app's z-address | ||
| const encryptTo = request.encryptToZAddress; | ||
|
|
||
| if (!encryptTo) { |
There was a problem hiding this comment.
You don't need to handle encryptToZAddress here, that'll be handled when processing the entire response at the very end
There was a problem hiding this comment.
Hm I got this confused with encryptResponseToAddress on the generic request, I'm actually not sure what to do to handle the encryptToZAddress on the appEncryptionRequestHandler, it still might need to be handled in a different way than what you're doing though, because to return an encrypted AppEncryptionResponse, you probably need to return a DataDescriptorOrdinalVDXFObject instead of an AppEncryptionResponseOrdinalVDXFObject. AppEncryptionResponseOrdinalVDXFObject would only be if the response was unencrypted.
There was a problem hiding this comment.
Looking into the code it seems this encryptToZAddress is not actually optional, I will make it optional with a flag and then you can check something like hasEncryptToZAddress to see if it is present or not
There was a problem hiding this comment.
It was pushed and the libraries were updated a few days ago
| // extract envelope-level info | ||
| const systemID = request.signature.systemID.toIAddress(); | ||
| const requestSignerID = request.signature.identityID.toIAddress(); | ||
| const appOrDelegatedID = getAppOrDelegatedID(request); |
There was a problem hiding this comment.
Please do not do any verification of the appOrDelegatedID in this request, that will be verified when the request comes in, before processing occurs. You can remove things like validateRequestIdentities
.yarnrc.yml
Outdated
There was a problem hiding this comment.
Remove this file please
There was a problem hiding this comment.
Changes to this file should be removed
shim.js
Outdated
There was a problem hiding this comment.
Please do not add shim.js
There was a problem hiding this comment.
Changes to this file should be removed, there is no reason to change the packages or yarn lock files
| VrpcProvider.initEndpoint(coinObj.system_id, coinObj.vrpc_endpoints[0]); | ||
|
|
||
| // process the request | ||
| const encryptedResponse = await processAppEncryptionRequest({ |
There was a problem hiding this comment.
From what I can tell, processAppEncryptionRequest returns hex, this will cause a runtime error when hex is passed into the data field of AppEncryptionResponseOrdinalVDXFObject, as it is expecting AppEncryptionResponseDetails
- Use AppEncryptionResponseDetails constructor instead of fromJson()
- Convert string keys to proper types (Buffer, SaplingExtendedViewingKey,
SaplingPaymentAddress, SaplingExtendedSpendingKey)
- Remove validateRequestIdentities and isIdentityInWallet functions
- Validation now handled at envelope level before handler is called
- Fix response wrapping to prevent runtime error
- Unencrypted: return AppEncryptionResponseOrdinalVDXFObject with
AppEncryptionResponseDetails object (not hex string)
- Encrypted: return DataDescriptorOrdinalVDXFObject with
DataDescriptor containing encrypted data
- Rename encryptToZAddress to encryptResponseToAddress
- Add hasEncryptResponseToAddress() helper with method/property fallback
- Add getEncryptResponseToAddress() helper for address extraction
No description provided.