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

DID Rotate Protocol #794

Merged
merged 9 commits into from
Dec 6, 2023
Merged

DID Rotate Protocol #794

merged 9 commits into from
Dec 6, 2023

Conversation

TelegramSam
Copy link
Contributor

No description provided.

Signed-off-by: Sam Curren <[email protected]>
Signed-off-by: Sam Curren <[email protected]>
{
"@id": "123456780",
"@type": "https://didcomm.org/did-rotate/1.0/ack",
"to_did": "did:example:newdid",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed to repeat the to_did field to confirm the reception of it, or should this message follow the convention including status and thread id? For example:

"status": "OK",
  "~thread": {
    "thid": "rotate-did-message-id",
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good topic for discussion in the Aries WG call. I included it as a bit of redundant info, but maybe it shouldn't be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

using only thid avoids error conditions where the returned to_did doesn't match the one sent in the rotate message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in next commit

@TelegramSam
Copy link
Contributor Author

note about reasonable time for observing party

@TelegramSam
Copy link
Contributor Author

notes about stored messages and state, and what should be updated.

@TelegramSam
Copy link
Contributor Author

discussed WG 20230830. ok to merge after discussed changes.

@genaris
Copy link
Contributor

genaris commented Aug 30, 2023

I'm sorry if you've discussed this today and I wasn't paying attention but I noticed from a Discord message from @swcurran something that seems to me a very good idea: to be able to set to_did field to a null value in order to terminate the connection (pretty much the same as supported natively DIDComm V2).

I think this may add a little bit of complexity, but certainly would give to this protocol more value and solve an interesting use-case in an elegant way.

@swcurran
Copy link
Member

Thanks for raising that @genaris — I forgot to raise it. I think it is an excellent idea. The Discord use case was for ending a mediator connection, but there are many other use cases, I’m sure.

Signed-off-by: Sam Curren <[email protected]>
Signed-off-by: Sam Curren <[email protected]>
@@ -19,6 +19,10 @@ This protocol is only applicable to DIDComm v1 - in DIDComm v2 use the more effi
This mechanism allows a party in a relationship to change the DID in use for the relationship. This may be used to switch DID methods,
but also to switch to a new DID within the same DID method. Inspired by (but different from) the DID rotation feature of the DIDComm Messaging (DIDComm v2) spec.
Copy link
Member

Choose a reason for hiding this comment

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

“new DID within the same DID method” to “new DID within the same DID method, such as to update the mediator relationship in the DIDDoc.”

I think it would be helpful to include that example.

Copy link
Member

Choose a reason for hiding this comment

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

Super minor, and not necessary, but I realize there are other motivators for adding/using this protocol, and perhaps a list is needed:

  • change mediators
  • change the key in an non-updatable DID method.
  • hanging up
  • Switch from a deprecated DID Method, such as unqualified DIDs or did:peer numalogos 1 and 2/3 to did:peer:4

@@ -19,6 +19,10 @@ This protocol is only applicable to DIDComm v1 - in DIDComm v2 use the more effi
This mechanism allows a party in a relationship to change the DID in use for the relationship. This may be used to switch DID methods,
but also to switch to a new DID within the same DID method. Inspired by (but different from) the DID rotation feature of the DIDComm Messaging (DIDComm v2) spec.

## Implications for Software Implementations

This protocol enables the identifiers used in a relationship to change. Implementations will need to consider how data related to the relationship is managed. If the relationship DIDs are used as identifiers, those identifiers may need to be updated during the rotation to maintain data integrity.
Copy link
Member

Choose a reason for hiding this comment

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

I think this comment is pretty subtle, and useful only for those “deep in the know”. I’m not sure this is any better, so take this with a grain of salt...

Implementations will need to consider how data (public keys, DIDs and the ID for the relationship) related to the relationship is managed. If the relationship DIDs are used as identifiers, those identifiers may need to be updated during the rotation to maintain data integrity. For example, both parties might have to retain and be able to use as identifiers for the relationship the existing DID and the rotated to DID, and their related keys for a period of time until the rotation is complete.

@@ -62,13 +66,17 @@ Message Type URI: https://didcomm.org/did-rotate/1.0/ack

This message is still sent to the prior DID to acknowledge the receipt of the rotation. Following messages will be sent to the new DID.

`to_did`: The new DID to be used to identify the **rotating_party**
In order to correctly process out of order messages, the The **observing_party** may choose to receive messages from the old did for a reasonable period. This allows messages sent before rotation but received after rotation in the case of out of order message delivery.
Copy link
Member

Choose a reason for hiding this comment

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

Consistency — “old did” —> “old DID"

@@ -62,13 +66,17 @@ Message Type URI: https://didcomm.org/did-rotate/1.0/ack

This message is still sent to the prior DID to acknowledge the receipt of the rotation. Following messages will be sent to the new DID.

`to_did`: The new DID to be used to identify the **rotating_party**
Copy link
Member

Choose a reason for hiding this comment

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

For this ACK and Problem Report section it is typical to talk about the two messages being “adopted” by the ACK and Report Problem messages, to include a link and to ensure that the usage is consistent. I didn’t check on the consistency of use, but I’m guessing it was done :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added


The **rotating_party** is expected to receive messages on both the existing and new DIDs and their associated keys for a reasonable period that MUST extend at least until the following `ack` message has been received.

This message MUST be sent using authcrypt or as a signed message in order to establish the provenance of the new DID. Proper provenance prevents injection attacks that seek to take over a relationship. Any rotate message received without being authcrypted or signed MUST be discarded and not processed.
Copy link
Member

Choose a reason for hiding this comment

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

I would add something like so that readers unfamiliar with the DIDComm message handling will not go down a rabbit hole of trying to figure out if a message is AuthCrypt or not: “In Aries implementations, messages sent in the context of an existing relationship are by default sent using authcrypt.”

@swcurran
Copy link
Member

swcurran commented Sep 1, 2023

Looks good to me. A few suggestions, but very minor — I won’t be upset if they aren’t made. Let me know when you have looked at them to your satisfaction, and I’ll approve.

Nice work!

TelegramSam and others added 2 commits September 6, 2023 07:58
Co-authored-by: Daniel Bluhm <[email protected]>
Signed-off-by: Sam Curren <[email protected]>
@dbluhm
Copy link
Member

dbluhm commented Sep 12, 2023

In a quick attempt to feel out any unexpected gotchas, I'm going to take a quick pass at an implementation of the proposed protocol. I'll report back with my findings.

## Motivation

This mechanism allows a party in a relationship to change the DID they use to identify themselves in that relationship. This may be used to switch DID methods,
but also to switch to a new DID within the same DID method. For non-updatable DID methods, this allows updating DID Doc attributes such as service endpoints. Inspired by (but different from) the DID rotation feature of the DIDComm Messaging (DIDComm v2) spec.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think updating DID Doc attributes for non-resolvable DIDs would need to also attach the DID Document, like we do in DID Exchange protocol.

Do you think we can add an attachment for those cases to allow a broader range of use cases of this protocol, or limit it to those DIDs that are publicly resolvable or embed the DID Document on its identifier?

Copy link
Member

Choose a reason for hiding this comment

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

Given the timing of this and the availability/desirability of did:peer:4, that we should not add this. We don’t want implementations using either unqualified DIDs or did:peer:1, which are the only options AFAIK that would need this. We could add a note about this, but I don’t think we should add the attachment option.

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'm not sure I understand the question. Let's discuss in the Aries WG.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

per discussion in WG 20230920: Would prefer not to add support in favor of did:peer:4, but willing to add if this presents a significant hurdle for the community using did:peer:1.

Copy link
Member

@swcurran swcurran left a comment

Choose a reason for hiding this comment

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

Discussed and agreed to be merged at Aries Working Group Call 2023.12.06.

@swcurran swcurran merged commit 5300d2c into hyperledger:main Dec 6, 2023
1 check passed
@TelegramSam TelegramSam deleted the did_rotate branch December 12, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants