-
Notifications
You must be signed in to change notification settings - Fork 60
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
Should implementation check consistency of 'from', 'to', 'skid' and similar headers for authcrypt, anoncrypt, sign envelopes and plaintext? #237
Comments
Another example: |
Is it just me or does anyone else feel disturbed that multiple layers need to be considered simultaneously? |
If applications will need to analyse different layers and make it's own trust decisions based on non-defined logic than i don't see any value in DIDComm. We just recommend to use Jose, not define protocol. |
I agree that this is rather an edge case, but the current spec says nothing about it explicitly. The spec should either say that such construction is invalid (and implementations will raise an error), or define how implementations should handle this situation. As for potential example of authcrypting a signed message: Alice has two DIDs: pairwise DID for Alice-to-Bob communication (DID_AB) and a public DID (DID_A) rooted on a blockchain for example. Alice and Bob want to authenticate themselves in the scope of established pairwise connection (so authcrypt is done via DID_AB), but at the same time non-repudiation (for a 3d party) is needed fort a particular message. So, the message is also signed by Alice's public DID (DID_A). |
@vimmerru It makes a lot of sense to compare @ashcherbakov in your example, I would argue that the authcrypt layer should not care that payload it [de]encrypts is signed or not, its only purpose should be to provide authenticated encryption, not judge the payload. |
I think what can be useful to avoid such questions:
otherwise API that application developers expect:
may mean quite different things in different implementations. |
We don't insist on any option/answer here. We just want the spec to provide explicit and clear description of such cases, so that it says what's acceptable and expected, what needs to be verified by implementations, and what's an error or unexpected format, where implementations can raise an exception/error. |
First of all, signing is likely to be unusual. Not weird, just unusual. That's because the major use case for signing in non-DIDComm contexts -- knowing with 100% certainty who sent a message -- is NOT addressed with signing in DIDComm. To know the sender of a message, you use authenticated encryption. It's only if you need non-repudiation (proving to the rest of the world, not just the recipient, who is behind a message) that you add signing. And I believe non-repudiation to happen in a tiny minority of cases. Secondly, I believe By far the most common scenario, when Bob is looking at a message, will be that it was authcrypted by Alice. Just authcrypted -- not authcrypted and signed, not anoncrypted then authcrypted, not anon+auth+sign. This is the 95% case, at least. In the remaining cases, Bob might see On the other hand, by far the most common scenario, when Bob's agency/mediator is looking at a message, is that it will be anoncrypted. Just anoncrypted -- not anoncrypted and anything else. This is the 95% case. It's also possible (not illegal) to send the agency/mediator an authcrypted message. But I don't know why you would want to do this. If Alice DOES send an authcrypted message to Bob's mediator, there doesn't have to be any relationship between the A.did@A:Mediator DID that Alice uses to authcrypt her No other combinations make sense. Ignoring the rewrapping that happens when you are preparing a complex route, authcrypt(authcrypt(payload)) is wrong. anoncrypt(anoncrypt(payload)) is wrong. More layers of encryption are wrong. My suggestion is that implementation code should unpack/decrypt exactly and only one layer at a time, NOT try to pull apart two layers as if they were one. In the 9x% case where Bob gets an authcrypted message from Alice, this single unpack operation will turn the message into plaintext. In the rare case where Bob gets an authcrypt(sign(payload)) message, unwrapping once will produce sign(payload) as output. Bob can then be properly surprised that he got something signed, and call unpack again on the signed stuff to get the original payload. This is more or less what would happen with human mail if you got an envelope and opened it. Usually you'd expect to see a letter inside. But if, instead, you saw another envelope with a special wax seal inside, you'd know that you're dealing with something carrying a special signature, and you'd open the second envelope. Performing a second unpacking operation is not an inconvenience. And in the other rare case, where the message for Bob is in the form anoncrypt(authcrypt(something)) -- and "something" could be either signed or plaintext -- Bob should call the unpack function once and observe an inner authcrypted envelope, then again to find out what's inside that envelope. Libraries that try to understand what's going on with more than one layer at a time are doing too much. Separate from the implementation advice I proposed in the previous paragraph, I think we should update the spec with an explanation of each of these combinations -- why they exist, what they mean, when they should be used. If everybody on this thread agrees with my reasoning, I will open a PR to this effect. |
I have feeling that if we go this way our developers MUST be crypto experts to use this API and specification and getting every message will be uggly loop in a program with a lot of conditions inside. |
@dhh1128 Could you formally answer to questions we asked? Most of them are related to only one layer, not to layer combination. |
I disagree with the word "every". Getting unusual messages will require two calls. Getting normal messages will require one. Where I feel we are not aligned is whether we should try to hide unusual situations. I say "no", because I want the code to look different (follow a different observable codepath) when the semantics are different. Receiving a certified letter in the mail is different from receiving an ordinary letter, because you have to sign for it. But it hardly ever happens -- and when it does, the extra work you are doing teaches you something about the extra guarantees that come with it. I feel like you are preferring to make all codepaths as similar as possible, from the perspective of the application developer, because you feel that would be easiest for them. Is this an accurate summary of your preference? |
The formal answers to many of your questions depends on a concept called message trust contexts (MTC), that is described here: https://github.com/hyperledger/aries-rfcs/blob/master/concepts/0029-message-trust-contexts/README.md. We don't have to implement them exactly as that RFC describes -- but we need some construct like them. See below.
Is it illegal for Alice to mail to Bob a message signed by Carol? I don't think so. But I agree that it's an anomaly. What I would like here is a warning. But I don't see a way to raise warnings. What we should do is return an MTC that describes the anomaly and recommends different trust (trust in Carol, not Alice) as a result.
"additional metadata" = MTC
anoncrypt and authcrypt keys will be different by design; if they aren't different, anoncrypt makes no sense. So I don't understand the question.
No. It is wrong for a sender not to encrypt for all of Bob's keys. But Bob may have chosen not to tell the sender about all of the keys that he has. So it not an error to encrypt a message only to some of Bob's keys.
Keys at the same level in a single authcrypted envelope -- or keys used by different authcrypted envelopes?
Yes. In MTC.
No. I don't think this is an error. I also don't think a single call should look at both of those layers, anyway.
Yes. In MTC. |
You don't now is it 'usual' or 'not'. As a result you need 3 calls and condition always. Call #3 is to check that message is usual. Also it should be inside cycle as there can be multiple unusual envelopes.
What is MTC? |
Message Trust Context. See https://github.com/hyperledger/aries-rfcs/blob/master/concepts/0029-message-trust-contexts/README.md. |
@dhh1128 I like this idea, but what we miss is an instruction how to build MTC based on specific message and it's layers. |
Some example pseudocode for receiving any DIDComm message: # Starting point is an encrypted envelope that just came in over the wire.
inside, mtc = unpack(envelope, MTC())
if inside.is_encrypted:
if not envelope.anoncrypted or not inside.anoncrypted:
raise Exception("The only reason to double-encrypt is if the outer envelope provides anonymity, and the inner one provides the main encryption. Sender is behaving strangely. Message rejected.")
payload, mtc = unpack(inside, mtc)
else:
payload = inside
if payload.is_signed:
plaintext, mtc = unpack(payload, mtc)
else:
plaintext = payload
# Now examine mtc, which has accumulated state over repeated calls. Does the trust match your
# requirements? That will depend on what type of message it is. If the plaintext message is to make
# the next move in a chess game, then you don't care much about the identity of the sender, as long
# as it's a sender you know. But if the plaintext message is to agree to a mortgage, you probably need
# to know that the message was signed. Etc. Here, filter for all error conditions that the application
# cares about.
if plaintext.type == something_needing_nonrepudiable and not mtc.nonrepudiable:
raise Exception("I can't prove to other parties that the {plaintext.type} message came from {mtc.sender_did} because it wasn't signed.")
if plaintext.type == something_needing_authcrypt and not mtc.authcrypted:
raise Exception("I need to know who sent the the {plaintext.type} message, but sender is anonymous." Observations:
|
@dhh1128
Regardless of combination, just one common call can be called by receiver:
|
We should probably address the following items in the Spec: Proposed-Consistency-checks |
@kdenhartog @dhh1128 |
I'd say that's an implementation consideration. A conservative approach would be to always fail and error. A more "fail gracefully" approach would be to return the metadata about what passed and what failed and let the caller decide what to do with it. In the case of which one you should choose I think it depends on how you're architecting your system. For example, if you choose to implement an agent "all in one" I'd think you'd be fine just erroring on the first one. If you're going more in line with a microservice architecture where DIDComm envelope unpacking is a separate service then I'd think the second option is likely a better approach.
I don't think this should ever error anymore. The spec may need to be updated to say this though. it's my opinion that
I don't think that's necessary. Our implementation just uses the first recipient kid recognized and fails if it doesn't fully authenticate for that first one. A more robust approach would be to try the first and if it fails try the next, but that seemed overly complex to implement so we opted against it.
We don't bother checking the second one. I'd say this is more of an implementation consideration. Some people could require more, some might say one is fine. I don't see a reason to check them all in this case. Do you have any that I might be missing?
Again, I think it depends on your architecture here, but in general we do make the assumption that this information is returned. Even when we don't use it, it's been useful to include it because it may provide useful information for generating a response. I think in the original indy-sdk implementation I went with returning this information as we, but you could just as easily manage this without returning the key information and instead use a message management engine which associates a thread with a recipient DID and the response is generated based on all keyAgreement keys in the DID Document of the associated recipient DID.
Hmm, not sure how
I think this falls back on the architectual decision aspects.
My opinion has been that much of this could fall back on the implementation details because many of these questions are more specific to how to process errors. I think it's more so that we just need to say "if this case happens throw this error back to the sender". I may have just misunderstood what you were trying to convey though. |
My opinion was that originally this would be a "yes" this should match. This thread has convinced me there's valid use cases where decoupling these would be useful though. I'm now less certain about this then I was when I wrote the nesting code to process this. Let me think about it. |
Yeah... you're probably right that this may be an architectual conflation that I made. That would explain why our implementation always seemed more complex then it needed to be. |
I think:
I'm a fan of sending an error up when this check fails when decrypting a message. |
Notice this is changing since:
Most notably: when receiving a signed message it is really important to verify we are included in the |
Proposal: We add language that indicates the DIDs present in the encryption and signature layers must match the DIDs in the plaintext layer. |
Discussed WG 20220509. Check must be performed, and inconsistency errors are reported. Recipient MAY discard the message. |
I suggest imagine multiple situations to think and answer:
from
field in plaintext is empty or different.anoncrypt(authcrypt(plaintext))
envelopes, but set ofto
dids/keys in anoncrypt, authcrypt and maybe plaintext is different.authcrypt(plaintext)
message encrypted for multiple keys. Bob owns multiple of this case on this device.anoncrypt(authcrypt(plaintext))
message to Bob. anoncrypt and authcrypt parts have differentto
, but Bob still can decrypt it as he owns keys for both parts.It's just examples of such cases i can imagine significantly more similar corner cases. Ideally specification should define process of unwrapping every envelope and checks it should perform after it.
The text was updated successfully, but these errors were encountered: