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

Sign Attachment Protocol #586

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

Conversation

TelegramSam
Copy link
Contributor

This protocol is based off earlier work, and simplifies several things and adds a mechanism to present gathered signatures.

Signed-off-by: Sam Curren telegramsam@gmail.com

Signed-off-by: Sam Curren <telegramsam@gmail.com>
Signed-off-by: Sam Curren <telegramsam@gmail.com>
features/0586-sign-attachment/README.md Show resolved Hide resolved

### Signature Request

This message represents a request from a "coordinator" to a "signer" to provide a signature for the attachment(s) included in the request. The request will indicate the attached message type as well as the requested signature type, and "goal codes" will be included to indicate the "goal" of the coordinator, and stated goal of the signer.
Copy link
Member

Choose a reason for hiding this comment

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

"the stated goal of the coordinator" for balance maybe?

I feel like maybe there should be only one goal - ie. the goal of the coordinator. The goal (or action) of the signer seems like it would always be implied by that goal.

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've adjusted the language to reflect that this is from the opinion of the coordinator. The one goal discussion is a good one, but I didn't want to hold up the other changes for that discussion.

features/0586-sign-attachment/README.md Outdated Show resolved Hide resolved
features/0586-sign-attachment/README.md Show resolved Hide resolved
features/0586-sign-attachment/README.md Outdated Show resolved Hide resolved
features/0586-sign-attachment/README.md Outdated Show resolved Hide resolved
"signature_type": "<requested signature type>",
"signature": { ... structure determined by signature type ... }
}],
"messages~attach": [{
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be ~attach to match the request? Although I would prefer just calling it messages.

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 moved to the attachment append method (as opposed to inline) partially as an experiment to see what it would look like. We are considering only appended attachments in DIDComm V2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But - I did make it consistent.

features/0586-sign-attachment/README.md Outdated Show resolved Hide resolved
Signed-off-by: Sam Curren <telegramsam@gmail.com>
@TelegramSam
Copy link
Contributor Author

@andrewwhitehead small adjustments made. Larger points are good ones, and I've left those open.

Copy link
Member

@dhh1128 dhh1128 left a comment

Choose a reason for hiding this comment

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

LGTM

I would note, however, that I think "Please Sign This" is a better name than "Sign Attachment." My reasoning is that "Sign Attachment" makes me want to assume that the signed artifact is intended to be an attachment (a false assumption). On the other hand, "Please Sign This" makes it clear that what's being signed is an attachment to the message in this protocol -- and expresses no opinion whatsoever about how whether the signed artifact will eventually become an attachment on another message.

Change the name or leave it alone; I'm okay with either choice.

@baha-ai
Copy link
Member

baha-ai commented Apr 14, 2021

is this change related to signed attachment in DIDComm?

I'm asking because this PR is about to remove the signing wording of attachments in DIDComm (ie recipientKeys only used for envelope encryption). If they're related, then we have a conflict.

Or if they're related and both needed, then this PR must specify where the signing key comes from. It cannot be stored in DIDComm's recipientKeys section. It should be resolved from the signer's DID doc's Authentication section.

@Exulansis
Copy link

Can someone point me to where I can find some further info / guidance on the purpose and allowed values for the context, method, and signature_type fields defined in this RFC?

For a bit of context, I am trying to figure out if the protocol defined in this RFC can be used (perhaps in combination with the attachment formats defined in Aries RFC 0593) to request / produce Linked Data Proof signatures.

Thank you!

@pstuermlinger
Copy link
Contributor

Hello,
I want to take the opportunity to propose a change in the allowed values of the signer_goal_code field and associated behavior.

Motivation
Currently, it seems like the goal code ledger.transaction.write is a valid goal only for the author. However, once a transaction/attachment has enough signatures, there's no technical reason why the endorser shouldn't write the transaction to the ledger by itself. If the RFC would allow such a behavior, it would enable author-agents within highly restricted enterprise or government networks to utilize a priviledged endorser not only as signer but also as a messenger.

Proposed solution
Change the type of signer_goal_code from string to array. This allows an author to set the signer goal to either ["transaction.endorse"], ["transaction.endorse", "ledger.transaction.write"] or only ["ledger.transaction.write"].
For the sake of accuracy and possible future developments, I suggest following the order.

In case of ledger.transaction.write as (last) goal, the Signature Response should provide the ledger write result (e.g. the created schema definition) as attachment.

Related discussions / issues

If my proposal is accepted, I'm happy to make the necessary changes in the document.

@Exulansis
Copy link

Exulansis commented Apr 29, 2021

Can someone point me to where I can find some further info / guidance on the purpose and allowed values for the context, method, and signature_type fields defined in this RFC?

For a bit of context, I am trying to figure out if the protocol defined in this RFC can be used (perhaps in combination with the attachment formats defined in Aries RFC 0593) to request / produce Linked Data Proof signatures.

I think this question would be resolved with some guidance on how to define / register application specific protocols based on this RFC document (e.g. for usage with Linked Data Documents / Linked Data Proofs).

The RFC currently states:

The result of this is that any application of this protocol requires another spec for the application of this protocol. This presents an issue for compatibility detection and increases the complexity of handling these messages for agents.

I am curious if examples of such application specific specs already exist (I was so far only able to find this document), or if any are being developed. It might of course be early to ask such questions, given the RFC document is not yet merged in.

Thank you.

@dhh1128
Copy link
Member

dhh1128 commented Apr 29, 2021

I am sensitive to the pending questions in this conversation -- but I don't feel like I can answer them. I think @TelegramSam or one of the folks on the BCGov team needs to speak to the intent and to whether the problem domain of the protocol overlaps with collecting Linked Data Proof signatures enough to justify solving that problem this way.

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.

None yet

7 participants