diff --git a/e2ee/omemo/envelope.go b/e2ee/omemo/envelope.go index b0eb325..3b69bab 100644 --- a/e2ee/omemo/envelope.go +++ b/e2ee/omemo/envelope.go @@ -32,7 +32,11 @@ import ( // plaintext body. The GCM tag is stripped from the ciphertext and // instead appended to the key (key||tag, 32 bytes) before that blob is // Signal-encrypted per recipient; carries ciphertext only, -// carries the GCM nonce. +// carries the GCM nonce. This is what we (and current real +// clients) produce; decryptPayloadOmemo0 additionally accepts an +// older, non-conformant 16-byte-key-only layout some real-world +// senders still use (see its own doc comment) - real-world interop, +// confirmed against actual client behavior/history, not assumed. // // - Omemo1 and Omemo2 share the same payload crypto (introduced at // Omemo1/XEP-0384 0.4.0, unchanged since): the plaintext is first @@ -152,12 +156,41 @@ func encryptPayloadOmemo0(plaintext []byte) (ciphertext, iv, contentKey []byte, return ciphertext, iv, contentKey, nil } +// decryptPayloadOmemo0 accepts two real-world wire layouts for the +// per-device content key, dispatching on its length: +// +// - 32 bytes (omemo0KeyLen+omemo0TagLen): the spec-correct, current +// layout this package also produces (see encryptPayloadOmemo0) - key +// and GCM tag concatenated in , is ciphertext only. +// Confirmed as what current real clients use (python-oldmemo, the +// backend behind Gajim's OMEMO plugin and slixmpp-omemo; Profanity; +// the xmpp-parsers crate's doc comment) - not assumed. +// - 16 bytes (omemo0KeyLen): an older, non-conformant layout some +// real-world senders still use, where is just the raw AES key +// and the GCM tag instead rides along appended to +// (ciphertext||tag - the natural output of a plain AEAD Seal call). +// Confirmed via Conversations' own git history (commit e38a9cd729bf, +// "no longer accept auth tag appended to payload", ~2020-01 - meaning +// Conversations itself accepted exactly this for years before that) +// and Dino's dino#303 (a bug from mishandling this same split) - not +// a hypothetical. Accepted here defensively for interop, the same way +// Conversations itself did. func decryptPayloadOmemo0(ciphertext, iv, contentKey []byte) ([]byte, error) { - if len(contentKey) != omemo0KeyLen+omemo0TagLen { - return nil, fmt.Errorf("omemo: omemo0 content key must be %d bytes, got %d", omemo0KeyLen+omemo0TagLen, len(contentKey)) + switch len(contentKey) { + case omemo0KeyLen + omemo0TagLen: + return openOmemo0GCM(ciphertext, iv, contentKey[:omemo0KeyLen], contentKey[omemo0KeyLen:]) + case omemo0KeyLen: + if len(ciphertext) < omemo0TagLen { + return nil, fmt.Errorf("omemo: omemo0 payload too short to contain a trailing GCM tag: %d bytes", len(ciphertext)) + } + tag := ciphertext[len(ciphertext)-omemo0TagLen:] + return openOmemo0GCM(ciphertext[:len(ciphertext)-omemo0TagLen], iv, contentKey, tag) + default: + return nil, fmt.Errorf("omemo: omemo0 content key must be %d or %d bytes, got %d", omemo0KeyLen, omemo0KeyLen+omemo0TagLen, len(contentKey)) } - key := contentKey[:omemo0KeyLen] - tag := contentKey[omemo0KeyLen:] +} + +func openOmemo0GCM(ciphertext, iv, key, tag []byte) ([]byte, error) { block, err := aes.NewCipher(key) if err != nil { return nil, err diff --git a/e2ee/omemo/envelope_test.go b/e2ee/omemo/envelope_test.go index 9995a40..db168a7 100644 --- a/e2ee/omemo/envelope_test.go +++ b/e2ee/omemo/envelope_test.go @@ -2,10 +2,53 @@ package omemo import ( "bytes" + "crypto/aes" + "crypto/cipher" + "crypto/rand" "reflect" "testing" ) +// TestDecryptPayloadOmemo0LegacyKeyLayout covers the real-world interop +// fallback in decryptPayloadOmemo0: some senders (confirmed via +// Conversations' own git history and dino#303 - see that function's doc +// comment) put only the raw 16-byte AES key in and let the GCM tag +// ride along appended to instead, rather than the current +// key||tag (32-byte) layout this package itself produces. This constructs +// that legacy layout directly (bypassing encryptPayloadOmemo0, which never +// produces it) to prove decryptPayloadOmemo0 still accepts it. +func TestDecryptPayloadOmemo0LegacyKeyLayout(t *testing.T) { + plaintext := []byte("hello from an older client") + + key := make([]byte, omemo0KeyLen) + if _, err := rand.Read(key); err != nil { + t.Fatalf("rand.Read key: %v", err) + } + iv := make([]byte, omemo0IVLen) + if _, err := rand.Read(iv); err != nil { + t.Fatalf("rand.Read iv: %v", err) + } + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("aes.NewCipher: %v", err) + } + gcm, err := cipher.NewGCM(block) + if err != nil { + t.Fatalf("cipher.NewGCM: %v", err) + } + // The legacy layout: = ciphertext||tag (GCM's Seal output, + // kept whole), = the raw 16-byte key (no tag appended). + sealed := gcm.Seal(nil, iv, plaintext, nil) + + got, err := decryptPayloadOmemo0(sealed, iv, key) + if err != nil { + t.Fatalf("decryptPayloadOmemo0: %v", err) + } + if !bytes.Equal(got, plaintext) { + t.Fatalf("got %q, want %q", got, plaintext) + } +} + func TestEncryptOuterRoundTrip(t *testing.T) { for _, tc := range []struct { name string