Add support for RFC 0434: Out-of-Band

[TimoGlastra]

Spec: https://github.com/hyperledger/aries-rfcs/blob/b3a3942ef052039e73cd23d847f42947f8287da2/features/0434-outofband/README.md

[jakubkoci]

I wrote down a few notes I would like to share with you here.

Inbound Message processing rules

1. Unpack if it’s packed message (JweLikeMessage)
   • Packed message can by
     • anoncrypt (unpack returns recipient key)
     • authcrypt (unpack returns recipient key and sender key)
2. Transform JSON message to AgentMessage instance
3. Validate message
4. Validate if an agent is capable to respond to the message
   • agent has a connection identified by both recipient and sender key, we can’t identify connection just by recipient key because we cannot be sure that the sender is the same, therefore sender key must be the same as connection.theirKey
   • message has a ~service decorator -> deprecated when a new OOB is in use
   • it’s an old OOB message with did or service-related attributes
   • it’s a new OOB message and has a service attribute (maybe this can or should be validated in OOB module 🤷‍♂️ or receiveInvitation from OOB module moves to MessageReceiver and keeps just createInvitation)
5. What if we don’t have information on how to respond at the moment of receiving a message?
   • Throw an error immediately and stop message processing.
   • Throw an error immediately and store the message for later. Yes but for how long?
   • Dispatch the message and leave the problem to MessageSender. MessageSender can store the messages which wasn't able to process for later… Hmm that doesn’t make much sense
6. Dispatch message inside InboundMessageContext instance

Implementation Ideas

I started with an idea to create a separate OOB module which would create and process OOB messages. The problem is that I needed to copy-paste a lot of code from ConnectionModule and MessageReceiver. There is a similarity with ConnectionModule because we need to create connections and with MessageReceiver because we need to check if we have a connection and dispatch messages. Also, OOB module oob.receiveInvitation method is quite similar to messageSender.receiveMessage:

MessageReceiver

• isPacked -> unpack
• connectionExists -> reuse connection
• transformMessage
• validateMessage
• saveSession
• dispatch one message

OOB

• connectionExists -> create or reuse connection
• transformMessage
• validateMessage
• dispatch one or more messages

Currently, we assume there are three types of messages. The connection invitation which we pass to connections.receiveInvitation. Packed message and other OOB messages with ~service decorator, both passed to agent.receiveMessage. New OOB also has three types but is organized in a bit different way. There is an OOB message containing one or more JsonMessage.

agent.connections.receiveInvitation(invitation: InvitationJsonMessage)
agent.receiveMessage(message: PackedMessage | DecoratedJsonMessage)

To me, it makes sense to have just one method which would accept all

agent.receiveMessage(message: OobMessage | PackedMessage | JsonMessage)

The problem is that JsonMessage does not have ~service decorator and that processing of OobMessage could recursively call receiveMessage.

I suggest to implement OOB as part of ConnectionsModule for start.

connections.receiveOobInvitation(message: OobMessage)
connections.receiveInvitation(message: OobMessage | InvitationJsonMessage)
connections.createOobInvitation(...)

oob.receiveMessage(OobMessage)
messageReceive.receiveMessage(PackedMessage | JsonMessage+service)

This allows us gradual update according to Aries RFC 0496: Transition to the Out of Band and DID Exchange Protocols, create a new connection, call MessageReceiver.receiveMessage(PackedMessage | DecoratedJsonMessage) many times. We can also add ~service decorator to JsonMessage for now and remove this when we get rid of the current OOB implementation relying on ~service.

[jakubkoci]

I found a problem with processing the connection request when the connection already exists.

The OOB spec says: “if a connection they have was created from an out-of-band invitation from the same services DID of a new invitation message, the connection MAY be reused. The receiver may choose to not reuse the existing connection for privacy purposes and repeat a handshake protocol to receive a redundant connection.”

Let’s say that a connection request is sent from Bob (invitee) to Alice (inviter), encrypted with Alice’s key (recipientKey), signed by Bob’s key (senderKey). Current implementation finds a connection based on Alice’s key (recipientKey) and because Bob is trying to make a new connection (by sending packed Connection Request message) with a different key (senderKey), it fails with the following error:

AriesFrameworkError: Inbound message senderKey '4Bxpr2BAeZUAAGw6bg4aKWc9XWDh7oFaBf4wVxCMteJe' is different from connection.theirKey '8aZJYVzp8TyfjY5Eh9rXwzpUNGjsoUwrVAYqTEL7JE83'

I guess we should not throw an error and pass the message forward. If the new connection is successfully made, the previous connection became stale. I’m not sure how to handle that.

I tried to change the logic in MessageReceiver.receiveMessage method to this:

if (senderKey && recipientKey) {
  connection = await this.connectionService.findByTheirKey(senderKey)
}

But there are failing mediation tests with that. I’m digging into it and trying to understand what’s the problem, but I just wanted to share it with you here if you have any ideas.

[TimoGlastra]

Let’s say that a connection request is sent from Bob (invitee) to Alice (inviter), encrypted with Alice’s key (recipientKey), signed by Bob’s key (senderKey). Current implementation finds a connection based on Alice’s key (recipientKey) and because Bob is trying to make a new connection (by sending packed Connection Request message) with a different key (senderKey), it fails with the following error:

I'm not following you 100%. When alice receives the request the connection doesn't have theirKey yet, so how can this happen?

[jakubkoci]

Yeah, sorry I didn't explain it well. I try it again :)

Let’s say that there is an existing connection between Alice (A.pk1, B.pk1) and Bob (B.pk1, A.pk1).

• Alice (inviter) sends OOB invitation with A.pk1.
• Bob (invitee) receives the invitation but decides to not reuse the connection. Bob creates a connection request with new B.pk2 and encrypts it with A.pk1
• Alice receives the connection request. Unpack returns A.pk1 as recipientKey and B.pk2 as senderKey
• Alice already has a connection where recipientKey is A.pk1, therefore theirKey is not null but is equal to B.pk1, therefor it's different than senderKey B.pk2.

[jakubkoci]

Aha, if I remove the condition connection && connection.theirKey !== null && connection.theirKey !== senderKey it passes. But that's probably something we don't want to do :)

[TimoGlastra]

I understand now. Let's talk about it during the WG call. There are some things to consider here, but I think we could loosen the restriction a bit and allow it to pass through as you said. I think this is related to the work we will need to do for connections using public dids, as it would also create the scenario where the same key is used for multiple connections. Difference is that we would still like to create a new key for each interaction, we just associate it with the public did.

If we loosen the restrictions, it will work something like: Alice receives a connection request. We look at the recipientKey and senderKey to retrieve the connection.

We could still check then if the request is valid based on the following checks (in the connection response handler):

• Is the key used to connect associated with a public did, and is that public did allowed to be used as an implicit invitation?
• Did we create a connection invitation / OOB invitation that uses this key, that hasn't been completed yet (could be a multi-use invitation)

In general protocol implementation themselves check whether a connection is attached and whether that connection is complete. This can be easily done using context.assertReadyConnection(). The most important aspect here is that we don't want to associate a connection to the context that is not related.

[TimoGlastra]

As a practical note. We can remove the check, but need to modify the connection assignment a few lines above to only be assigned to the upper scope connection property.

So something like this:

    let connection: ConnectionRecord | null = null

    // Only fetch connection if recipientKey and senderKey are present (AuthCrypt)
    if (senderKey && recipientKey) {
     connection = await this.connectionService.findByQuery({ verkey: recipientKey, theirKey: senderKey })
    }

[TimoGlastra]

Fixed in #717, outstanding issues tracked in #764