From 4c58b82813051d134abdc3e2a47fe2dbb08f1c14 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Thu, 20 Feb 2025 14:13:42 -0700 Subject: [PATCH] verificationhelper/sas: don't trust keys until both MAC events are sent Signed-off-by: Sumner Evans --- crypto/verificationhelper/sas.go | 73 +++++++++++++++++++------------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/crypto/verificationhelper/sas.go b/crypto/verificationhelper/sas.go index a78b4b57..70b77759 100644 --- a/crypto/verificationhelper/sas.go +++ b/crypto/verificationhelper/sas.go @@ -111,6 +111,7 @@ func (vh *VerificationHelper) ConfirmSAS(ctx context.Context, txnID id.Verificat keys := map[id.KeyID]jsonbytes.UnpaddedBytes{} log.Info().Msg("Signing keys") + var masterKey string // My device key myDevice := vh.mach.OwnIdentity() @@ -123,8 +124,9 @@ func (vh *VerificationHelper) ConfirmSAS(ctx context.Context, txnID id.Verificat // Master signing key crossSigningKeys := vh.mach.GetOwnCrossSigningPublicKeys(ctx) if crossSigningKeys != nil { - crossSigningKeyID := id.NewKeyID(id.KeyAlgorithmEd25519, crossSigningKeys.MasterKey.String()) - keys[crossSigningKeyID], err = vh.verificationMACHKDF(txn, vh.client.UserID, vh.client.DeviceID, txn.TheirUserID, txn.TheirDeviceID, crossSigningKeyID.String(), crossSigningKeys.MasterKey.String()) + masterKey = crossSigningKeys.MasterKey.String() + crossSigningKeyID := id.NewKeyID(id.KeyAlgorithmEd25519, masterKey) + keys[crossSigningKeyID], err = vh.verificationMACHKDF(txn, vh.client.UserID, vh.client.DeviceID, txn.TheirUserID, txn.TheirDeviceID, crossSigningKeyID.String(), masterKey) if err != nil { return err } @@ -148,10 +150,16 @@ func (vh *VerificationHelper) ConfirmSAS(ctx context.Context, txnID id.Verificat if err != nil { return err } + log.Info().Msg("Sent our MAC event") txn.SentOurMAC = true if txn.ReceivedTheirMAC { txn.VerificationState = VerificationStateSASMACExchanged + + if err := vh.trustKeysAfterMACCheck(ctx, txn, masterKey); err != nil { + return fmt.Errorf("failed to trust keys: %w", err) + } + err := vh.sendVerificationEvent(ctx, txn, event.InRoomVerificationDone, &event.VerificationDoneEventContent{}) if err != nil { return err @@ -731,12 +739,38 @@ func (vh *VerificationHelper) onVerificationMAC(ctx context.Context, txn Verific } log.Info().Msg("All MACs verified") + txn.ReceivedTheirMAC = true + if txn.SentOurMAC { + txn.VerificationState = VerificationStateSASMACExchanged + + if err := vh.trustKeysAfterMACCheck(ctx, txn, masterKey); err != nil { + vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUser, "failed to trust keys: %w", err) + return + } + + err := vh.sendVerificationEvent(ctx, txn, event.InRoomVerificationDone, &event.VerificationDoneEventContent{}) + if err != nil { + vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUser, "failed to send verification done event: %w", err) + return + } + txn.SentOurDone = true + } + + if err := vh.store.SaveVerificationTransaction(ctx, txn); err != nil { + log.Err(err).Msg("failed to save verification transaction") + } +} + +func (vh *VerificationHelper) trustKeysAfterMACCheck(ctx context.Context, txn VerificationTransaction, masterKey string) error { + theirDevice, err := vh.mach.GetOrFetchDevice(ctx, txn.TheirUserID, txn.TheirDeviceID) + if err != nil { + return fmt.Errorf("failed to fetch their device: %w", err) + } // Trust their device theirDevice.Trust = id.TrustStateVerified err = vh.mach.CryptoStore.PutDevice(ctx, txn.TheirUserID, theirDevice) if err != nil { - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUser, "failed to update device trust state after verifying: %w", err) - return + return fmt.Errorf("failed to update device trust state after verifying: %w", err) } if txn.TheirUserID == vh.client.UserID { @@ -748,14 +782,12 @@ func (vh *VerificationHelper) onVerificationMAC(ctx context.Context, txn Verific if vh.mach.CrossSigningKeys != nil { err = vh.mach.SignOwnDevice(ctx, theirDevice) if err != nil { - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUser, "failed to sign our own new device: %w", err) - return + return fmt.Errorf("failed to sign our own new device: %w", err) } } else { err = vh.mach.SignOwnMasterKey(ctx) if err != nil { - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUser, "failed to sign our own master key: %w", err) - return + return fmt.Errorf("failed to sign our own master key: %w", err) } } } else if masterKey != "" { @@ -766,31 +798,14 @@ func (vh *VerificationHelper) onVerificationMAC(ctx context.Context, txn Verific // user-signing key. theirSigningKeys, err := vh.mach.GetCrossSigningPublicKeys(ctx, txn.TheirUserID) if err != nil { - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeKeyMismatch, "couldn't get %s's cross-signing keys: %w", txn.TheirUserID, err) - return + return fmt.Errorf("couldn't get %s's cross-signing keys: %w", txn.TheirUserID, err) } else if theirSigningKeys.MasterKey.String() != masterKey { - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeKeyMismatch, "master keys do not match") - return + return fmt.Errorf("master keys do not match") } if err := vh.mach.SignUser(ctx, txn.TheirUserID, theirSigningKeys.MasterKey); err != nil { - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeInternalError, "failed to sign their master key: %w", err) - return + return fmt.Errorf("failed to sign %s's master key: %w", txn.TheirUserID, err) } } - - txn.ReceivedTheirMAC = true - if txn.SentOurMAC { - txn.VerificationState = VerificationStateSASMACExchanged - err := vh.sendVerificationEvent(ctx, txn, event.InRoomVerificationDone, &event.VerificationDoneEventContent{}) - if err != nil { - vh.cancelVerificationTxn(ctx, txn, event.VerificationCancelCodeUser, "failed to send verification done event: %w", err) - return - } - txn.SentOurDone = true - } - - if err := vh.store.SaveVerificationTransaction(ctx, txn); err != nil { - log.Err(err).Msg("failed to save verification transaction") - } + return nil }