nuts-foundation / nuts-node

The reference implementation of the Nuts specification. A decentralized identity network based on the w3c ssi concepts with practical functionality for the healthcare domain.
https://nuts-foundation.gitbook.io
GNU General Public License v3.0
23 stars 15 forks source link

Decryption race condition #1688

Closed woutslakhorst closed 1 year ago

woutslakhorst commented 1 year ago

Decrypting the PAL header (or other future decryption actions) is done by getting the latest DID Document. If a key rotation has occurred, an older key might be able to decode but this is currently not done.

This is a broader issue with encryption/decryption and legacy keys. We might have to say that encryption/decryption with asymmetric keys from the DID Document can only be used in runtime (timewise) use-cases?

reinkrul commented 1 year ago

We currently don't remove rotated keys (https://github.com/nuts-foundation/nuts-node/issues/1660) but if we do, you can't decrypt the PAL header after key rotation has occurred (because now the private key is gone).

We can currently only solve this by saying PAL is a runtime-property, which can only be checked in the present (with some delay to account for network latency and key rotation). But that means it can't be used to exchange historic private TX payload.

It's really just a transport-level ACL: to determine whether the other party is allowed to receive it. Would DIDComm be able to solve this?

woutslakhorst commented 1 year ago

Even a 1 second delay could become a problem when a key rotation occurs within that window. No feedback will be available when this happens.

woutslakhorst commented 1 year ago

Backport to V5

woutslakhorst commented 1 year ago

It's really just a transport-level ACL: to determine whether the other party is allowed to receive it. Would DIDComm be able to solve this?

no

woutslakhorst commented 1 year ago

If you rotate an asymmetric encryption key, you usually decrypt all symmetric keys and then re-encrypt them. In our case this would not be possible since encrypted data is on-chain. The encryptor could detect new keys and re-issue any VC. This really feels like a bandaid solution. A cleaner solution would be to reverse the responsibility of who must guarantee the delivery. This is currently the case for the wallet owner.

woutslakhorst commented 1 year ago

We could migrate everything to OIDC4VCI with the pre-authorized code flow.

This would eliminate the need for authenticated grpc connections, ECIES encryption, PAL headers and maybe even the client certificates (at some point).

An issuer can initiate an Issuance Initiation Request which can be delivered synchronous to a wallet endpoint. When successful the issuer doesn't need to retry it. Any downtime of the wallet would then have to be handled by the issuer (but not the nuts-node).

Specification is currently in progress. Lots of stuff to fix though: https://bitbucket.org/openid/connect/issues?status=new&status=open&component=Credential%20Issuance Also DIDs not supported yet

woutslakhorst commented 1 year ago

OIDVCI has gotten an update on 30th of December 2022. DIDs are now mentioned and supported.

Making the credential issuance a synchronous process would make this a non-issue

gerardsn commented 1 year ago

OpenID4VCI is being implemented and will obsolete the private transactions. Do the DidComm messages (kik-v) have the same problem? If not we can close the issue

woutslakhorst commented 1 year ago

OpenID4VCI is being implemented and will obsolete the private transactions. Do the DidComm messages (kik-v) have the same problem? If not we can close the issue

DidComm messaging is runtime so during a key rotation "a message" could fail but when attempted again it succeeds. Nothing will be permanently broken. So could be closed I guess.