From 678d54b34bf07c8f562e74a4f20175c2b4e3fbc6 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Tue, 25 Aug 2020 20:42:36 +0300 Subject: [PATCH] Fix bugs --- crypto/keyexport.go | 30 +++++++++++++++++++++--------- crypto/keyimport.go | 22 ++++++++++++---------- crypto/store.go | 2 +- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/crypto/keyexport.go b/crypto/keyexport.go index c582848a..4cdfd9c7 100644 --- a/crypto/keyexport.go +++ b/crypto/keyexport.go @@ -41,8 +41,11 @@ type ExportedSession struct { const defaultPassphraseRounds = 100000 -func computeKey(passphrase string, salt []byte, rounds int) []byte { - return pbkdf2.Key([]byte(passphrase), salt, rounds, 512, sha512.New) +func computeKey(passphrase string, salt []byte, rounds int) (encryptionKey, hashKey []byte) { + key := pbkdf2.Key([]byte(passphrase), salt, rounds, 64, sha512.New) + encryptionKey = key[:32] + hashKey = key[32:] + return } func makeExportIV() ([]byte, error) { @@ -63,9 +66,7 @@ func makeExportKeys(passphrase string) (encryptionKey, hashKey, salt, iv []byte, return } - key := computeKey(passphrase, salt, defaultPassphraseRounds) - encryptionKey = key[:256] - hashKey = key[256:] + encryptionKey, hashKey = computeKey(passphrase, salt, defaultPassphraseRounds) iv, err = makeExportIV() return @@ -105,16 +106,24 @@ const exportLineLengthLimit = 76 const exportHeaderLength = 1 + 16 + 16 + 4 const exportHashLength = 32 +func min(a, b int) int { + if a > b { + return b + } + return a +} + func formatKeyExportData(data []byte) string { dataStr := base64.StdEncoding.EncodeToString(data) - lines := make([]string, math.Ceil(float64(len(dataStr)) / exportLineLengthLimit) + 2) + // Base64 lines + prefix + suffix + empty line at end + lines := make([]string, int(math.Ceil(float64(len(dataStr)) / exportLineLengthLimit)) + 3) lines[0] = exportPrefix line := 1 for ptr := 0; ptr < len(dataStr); ptr += exportLineLengthLimit { - lines[line] = dataStr[ptr:ptr+exportLineLengthLimit] + lines[line] = dataStr[ptr:min(ptr+exportLineLengthLimit, len(dataStr))] line++ } - lines[len(lines)-1] = exportSuffix + lines[len(lines)-2] = exportSuffix return strings.Join(lines, "\n") } @@ -150,7 +159,10 @@ func ExportKeys(passphrase string, sessions []*InboundGroupSession) (string, err binary.BigEndian.PutUint32(exportData[33:37], defaultPassphraseRounds) // Encrypt data with AES-256-CTR - block, _ := aes.NewCipher(encryptionKey) + block, err := aes.NewCipher(encryptionKey) + if err != nil { + return "", err + } cipher.NewCTR(block, iv).XORKeyStream(exportData[exportHeaderLength:dataWithoutHashLength], unencryptedData) // Hash all the data with HMAC-SHA256 and put it at the end diff --git a/crypto/keyimport.go b/crypto/keyimport.go index 93014109..5d26fd19 100644 --- a/crypto/keyimport.go +++ b/crypto/keyimport.go @@ -24,7 +24,8 @@ import ( ) var ( - ErrMissingExportPrefix = errors.New("invalid Matrix key export: missing prefix/suffix") + ErrMissingExportPrefix = errors.New("invalid Matrix key export: missing prefix") + ErrMissingExportSuffix = errors.New("invalid Matrix key export: missing suffix") ErrUnsupportedExportVersion = errors.New("unsupported Matrix key export format version") ErrMismatchingExportHash = errors.New("mismatching hash; incorrect passphrase?") ErrInvalidExportedAlgorithm = errors.New("session has unknown algorithm") @@ -45,9 +46,7 @@ func decryptKeyExport(passphrase string, exportData []byte) ([]ExportedSession, hash := exportData[dataWithoutHashLength:] // Compute the encryption and hash keys from the passphrase and salt - key := computeKey(passphrase, salt, int(passphraseRounds)) - encryptionKey := key[:256] - hashKey := key[256:] + encryptionKey, hashKey := computeKey(passphrase, salt, int(passphraseRounds)) // Compute and verify the hash. If it doesn't match, the passphrase is probably wrong mac := hmac.New(sha256.New, hashKey) @@ -89,6 +88,7 @@ func (mach *OlmMachine) importExportedRoomKey(session ExportedSession) error { // TODO should we add something here to mark the signing key as unverified like key requests do? ForwardingChains: session.ForwardingChains, } + // TODO we probably shouldn't override existing sessions (except maybe if earliest known session is lower than the one in the store?) err = mach.CryptoStore.PutGroupSession(igs.RoomID, igs.SenderKey, igs.ID(), igs) if err != nil { return errors.Wrap(err, "failed to store imported session") @@ -96,17 +96,19 @@ func (mach *OlmMachine) importExportedRoomKey(session ExportedSession) error { return nil } -func (mach *OlmMachine) ImportKeys(passphrase string, data string) (int, error) { - if !strings.HasPrefix(data, exportPrefix) || !strings.HasSuffix(data, exportSuffix) { - return 0, ErrMissingExportPrefix +func (mach *OlmMachine) ImportKeys(passphrase string, data string) (int, int, error) { + if !strings.HasPrefix(data, exportPrefix) { + return 0, 0, ErrMissingExportPrefix + } else if !strings.HasSuffix(data, exportSuffix) && !strings.HasSuffix(data, exportSuffix + "\n") { + return 0, 0, ErrMissingExportSuffix } exportData, err := base64.StdEncoding.DecodeString(data[len(exportPrefix) : len(data)-len(exportPrefix)]) if err != nil { - return 0, err + return 0, 0, err } sessions, err := decryptKeyExport(passphrase, exportData) if err != nil { - return 0, err + return 0, 0, err } count := 0 @@ -119,5 +121,5 @@ func (mach *OlmMachine) ImportKeys(passphrase string, data string) (int, error) count++ } } - return count, nil + return count, len(sessions), nil } diff --git a/crypto/store.go b/crypto/store.go index 674bfb70..b0ede8dd 100644 --- a/crypto/store.go +++ b/crypto/store.go @@ -339,6 +339,7 @@ func (gs *GobStore) GetWithheldGroupSession(roomID id.RoomID, senderKey id.Sende func (gs *GobStore) GetGroupSessionsForRoom(roomID id.RoomID) ([]*InboundGroupSession, error) { gs.lock.Lock() + defer gs.lock.Unlock() room, ok := gs.GroupSessions[roomID] if !ok { return []*InboundGroupSession{}, nil @@ -349,7 +350,6 @@ func (gs *GobStore) GetGroupSessionsForRoom(roomID id.RoomID) ([]*InboundGroupSe result = append(result, session) } } - gs.lock.Unlock() return result, nil }