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

[FIX] #1245 DIDExchange request handling should require invitation key rotation #1278

Merged
merged 7 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,13 @@ impl HarnessAgent {

let request_thread = &request.inner().decorators.thread;

let inviter_key = request_thread
.as_ref()
.and_then(|th| self.inviter_keys.read().unwrap().get(&th.thid).cloned())
.ok_or_else(|| {
HarnessError::from_msg(HarnessErrorType::InvalidState, "Inviter key not found")
})?;

let opt_invitation = match request_thread.as_ref().and_then(|th| th.pthid.as_ref()) {
Some(pthid) => {
let invitation = self.aries_agent.out_of_band().get_invitation(pthid)?;
Expand All @@ -188,7 +195,7 @@ impl HarnessAgent {
let (thid, pthid, my_did, their_did) = self
.aries_agent
.did_exchange()
.handle_msg_request(request, opt_invitation)
.handle_msg_request(request, inviter_key, opt_invitation)
.await?;

if let Some(pthid) = pthid {
Expand Down
12 changes: 11 additions & 1 deletion aries/agents/aath-backchannel/src/controllers/didcomm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl HarnessAgent {
}

pub async fn receive_message(&self, payload: Vec<u8>) -> HarnessResult<HttpResponse> {
let (message, sender_vk) = EncryptionEnvelope::anon_unpack_aries_msg(
let (message, sender_vk, recipient_vk) = EncryptionEnvelope::anon_unpack_aries_msg(
self.aries_agent.wallet().as_ref(),
payload.clone(),
)
Expand All @@ -222,6 +222,16 @@ impl HarnessAgent {
"Received anoncrypted message",
)
})?;

let connection_id = self
.aries_agent
.connections()
.get_by_sender_vk(sender_vk.clone())?;
self.inviter_keys
.write()
.unwrap()
.insert(connection_id.clone(), recipient_vk);

info!("Received message: {}", message);
match message {
AriesMessage::Notification(msg) => {
Expand Down
2 changes: 2 additions & 0 deletions aries/agents/aath-backchannel/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ pub struct HarnessAgent {
// todo: extra didx specific AATH service
didx_msg_buffer: RwLock<Vec<AriesMessage>>,
didx_pthid_to_thid: Mutex<HashMap<String, String>>,
inviter_keys: RwLock<HashMap<String, String>>,
}

#[macro_export]
Expand Down Expand Up @@ -121,6 +122,7 @@ async fn main() -> std::io::Result<()> {
status: Status::Active,
didx_msg_buffer: Default::default(),
didx_pthid_to_thid: Mutex::new(Default::default()),
inviter_keys: RwLock::new(HashMap::new()),
})))
.app_data(web::Data::new(RwLock::new(Vec::<AriesMessage>::new())))
.service(
Expand Down
14 changes: 6 additions & 8 deletions aries/agents/aries-vcx-agent/src/handlers/did_exchange.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use aries_vcx::{
use aries_vcx_wallet::wallet::base_wallet::BaseWallet;
use did_resolver_registry::ResolverRegistry;
use did_resolver_sov::did_resolver::did_doc::schema::did_doc::DidDocument;
use public_key::{Key, KeyType};
use url::Url;

use crate::{
Expand Down Expand Up @@ -125,6 +126,7 @@ impl<T: BaseWallet> DidcommHandlerDidExchange<T> {
pub async fn handle_msg_request(
&self,
request: AnyRequest,
inviter_key: String,
invitation: Option<OobInvitation>,
) -> AgentResult<(String, Option<String>, String, String)> {
// todo: type the return type
Expand All @@ -142,16 +144,12 @@ impl<T: BaseWallet> DidcommHandlerDidExchange<T> {
.thid
.clone();

// Todo: "invitation_key" should not be None; see the todo inside this scope
let inviter_key = Key::from_base58(&inviter_key, KeyType::Ed25519)?;

let invitation_key = match invitation {
None => {
// TODO: Case for "implicit invitations", where request is sent on basis of
// knowledge of public DID However in that cases we should
// probably use the Recipient Verkey which was used to anoncrypt the Request msg
None
}
None => inviter_key,
Some(invitation) => {
Some(resolve_enc_key_from_invitation(&invitation, &self.resolver_registry).await?)
resolve_enc_key_from_invitation(&invitation, &self.resolver_registry).await?
}
};

Expand Down
21 changes: 21 additions & 0 deletions aries/aries_vcx/src/protocols/did_exchange/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,27 @@ pub async fn resolve_enc_key_from_invitation(
}
}

pub async fn resolve_enc_key_from_did(
did: &str,
resolver_registry: &Arc<ResolverRegistry>,
) -> Result<Key, AriesVcxError> {
let output = resolver_registry
.resolve(&did.try_into()?, &Default::default())
.await
.map_err(|err| {
AriesVcxError::from_msg(
AriesVcxErrorKind::InvalidDid,
format!("DID resolution failed: {err}"),
)
})?;
info!(
"resolve_enc_key_from_did >> Resolved did document {}",
output.did_document
);
let did_doc = output.did_document;
resolve_enc_key_from_did_doc(&did_doc)
}

/// Attempts to resolve a [Key] in the [DidDocument] that can be used for sending encrypted
/// messages. The approach is:
/// * check the service for a recipient key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl GenericDidExchange {
resolver_registry: &Arc<ResolverRegistry>,
request: AnyRequest,
our_peer_did: &PeerDid<Numalgo4>,
invitation_key: Option<Key>,
invitation_key: Key,
) -> Result<(Self, AnyResponse), AriesVcxError> {
let TransitionResult { state, output } =
DidExchangeResponder::<ResponseSent>::receive_request(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl DidExchangeResponder<ResponseSent> {
resolver_registry: &Arc<ResolverRegistry>,
request: AnyRequest,
our_peer_did: &PeerDid<Numalgo4>,
invitation_key: Option<Key>,
invitation_key: Key,
) -> Result<TransitionResult<DidExchangeResponder<ResponseSent>, AnyResponse>, AriesVcxError>
{
debug!(
Expand All @@ -51,21 +51,7 @@ impl DidExchangeResponder<ResponseSent> {
DidExchangeTypeV1::V1_1(_) => assemble_did_rotate_attachment(our_peer_did.did()),
DidExchangeTypeV1::V1_0(_) => ddo_to_attach(our_did_document.clone())?,
};
let attachment = match invitation_key {
Some(invitation_key) => {
// TODO: this must happen only if we rotate DID; We currently do that always
// can skip signing if we don't rotate did document (unique p2p invitations
// with peer DIDs)
jws_sign_attach(unsigned_attachment, invitation_key, wallet).await?
}
None => {
// TODO: not signing if invitation_key is not provided, that would be case for
// implicit invitations. However we should probably sign with
// the key the request used as recipient_vk to anoncrypt the request
// So argument "invitation_key" should be required
unsigned_attachment
}
};
let attachment = jws_sign_attach(unsigned_attachment, invitation_key, wallet).await?;

let request_id = request.id.clone();
let request_pthid = request.decorators.thread.and_then(|thid| thid.pthid);
Expand Down
29 changes: 17 additions & 12 deletions aries/aries_vcx/src/utils/encryption_envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,33 +184,37 @@ impl EncryptionEnvelope {
async fn _unpack_a2a_message(
wallet: &impl BaseWallet,
encrypted_data: Vec<u8>,
) -> VcxResult<(String, Option<String>)> {
) -> VcxResult<(String, Option<String>, String)> {
trace!(
"EncryptionEnvelope::_unpack_a2a_message >>> processing payload of {} bytes",
encrypted_data.len()
);
let unpacked_msg = wallet.unpack_message(&encrypted_data).await?;
Ok((unpacked_msg.message, unpacked_msg.sender_verkey))
Ok((
unpacked_msg.message,
unpacked_msg.sender_verkey,
unpacked_msg.recipient_verkey,
))
}

pub async fn anon_unpack_aries_msg(
wallet: &impl BaseWallet,
encrypted_data: Vec<u8>,
) -> VcxResult<(AriesMessage, Option<String>)> {
let (message, sender_vk) = Self::anon_unpack(wallet, encrypted_data).await?;
) -> VcxResult<(AriesMessage, Option<String>, String)> {
let (message, sender_vk, recipient_vk) = Self::anon_unpack(wallet, encrypted_data).await?;
let a2a_message = serde_json::from_str(&message).map_err(|err| {
AriesVcxError::from_msg(
AriesVcxErrorKind::InvalidJson,
format!("Cannot deserialize A2A message: {}", err),
)
})?;
Ok((a2a_message, sender_vk))
Ok((a2a_message, sender_vk, recipient_vk))
}

pub async fn anon_unpack(
wallet: &impl BaseWallet,
encrypted_data: Vec<u8>,
) -> VcxResult<(String, Option<String>)> {
) -> VcxResult<(String, Option<String>, String)> {
trace!(
"EncryptionEnvelope::anon_unpack >>> processing payload of {} bytes",
encrypted_data.len()
Expand Down Expand Up @@ -244,7 +248,7 @@ impl EncryptionEnvelope {
expected_vk
);

let (a2a_message, sender_vk) = Self::_unpack_a2a_message(wallet, encrypted_data).await?;
let (a2a_message, sender_vk, _) = Self::_unpack_a2a_message(wallet, encrypted_data).await?;
trace!(
"anon_unpack >> a2a_msg: {:?}, sender_vk: {:?}",
a2a_message,
Expand Down Expand Up @@ -309,7 +313,7 @@ pub mod unit_tests {
.await
.unwrap();

let (data_unpacked, sender_verkey) =
let (data_unpacked, sender_verkey, _) =
EncryptionEnvelope::anon_unpack(&setup.wallet, envelope.0)
.await
.unwrap();
Expand Down Expand Up @@ -386,17 +390,18 @@ pub mod unit_tests {
.await
.unwrap();

let (fwd_msg, _) = EncryptionEnvelope::anon_unpack(&setup.wallet, envelope.0)
let (fwd_msg, _, _) = EncryptionEnvelope::anon_unpack(&setup.wallet, envelope.0)
.await
.unwrap();
let fwd_payload = serde_json::from_str::<Value>(&fwd_msg)
.unwrap()
.get("msg")
.unwrap()
.to_string();
let (core_payload, _) = EncryptionEnvelope::anon_unpack(&setup.wallet, fwd_payload.into())
.await
.unwrap();
let (core_payload, _, _) =
EncryptionEnvelope::anon_unpack(&setup.wallet, fwd_payload.into())
.await
.unwrap();

assert_eq!(data_original, core_payload);
}
Expand Down
4 changes: 2 additions & 2 deletions aries/aries_vcx/tests/test_did_exchange.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ async fn did_exchange_test(
&resolver_registry,
request,
&responders_peer_did,
Some(invitation_key.clone()),
invitation_key.clone(),
)
.await
.unwrap();
Expand Down Expand Up @@ -337,7 +337,7 @@ async fn did_exchange_test_with_invalid_rotation_signature() -> Result<(), Box<d
request,
&responders_peer_did,
// sign with NOT the invitation key
Some(incorrect_invitation_key),
incorrect_invitation_key,
)
.await?;

Expand Down
Loading