Skip to content

Commit d20eed8

Browse files
committed
fix: keep E2E ciphertext and is_encrypted correct on message edits
1 parent 558f5e7 commit d20eed8

8 files changed

Lines changed: 90 additions & 32 deletions

File tree

ARCHITECTURE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ See [PROTOCOL.md](PROTOCOL.md) for the full message format specification.
274274
3. **Message encryption**: Inner JSON payload is sealed with ChaCha20-Poly1305; nonce ‖ ciphertext is base64-encoded into `content` with `encrypted: true` (see **PROTOCOL.md**)
275275
4. **Transport**: Standard WebSocket JSON; server does not decrypt
276276
5. **Storage**: Server persists opaque ciphertext in the database
277+
6. **Edits**: For `type: edit`, the reference client sends the replacement body ciphertext when E2E is enabled; the server updates `content` and persists `is_encrypted` from the wire `encrypted` field so reconnects and message metadata stay correct
277278

278279
## Database Schema
279280

@@ -299,6 +300,8 @@ CREATE TABLE messages (
299300
);
300301
```
301302

303+
Edits update `content` and `edited`; `is_encrypted` follows the `encrypted` field on the edit message so ciphertext is not downgraded to plain text in the database after an edit.
304+
302305
#### `user_message_state`
303306
```sql
304307
CREATE TABLE user_message_state (

PROTOCOL.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ These values of `type` extend the core chat protocol:
122122

123123
| `type` | Purpose |
124124
|--------|---------|
125-
| `edit` | Replace the text of an existing message. Requires `message_id` and new text in `content`. Only the original author may edit (server-enforced). |
125+
| `edit` | Replace the text of an existing message. Requires `message_id` and new body in `content`. Set `encrypted` to `true` when `content` is E2E ciphertext (same base64 **nonce ‖ ciphertext** layout as `text`); the server persists that flag to `is_encrypted` so history and reconnects stay consistent. Only the original sender may edit; admins cannot edit someone else's message (unlike `delete`, where an admin may remove any message). Enforced by matching WebSocket `sender` to the stored row. |
126126
| `delete` | Soft-delete a message. Requires `message_id`. Authors may delete their own messages; admins may delete any message. |
127127
| `typing` | Typing indicator; `content` is not required. The server sets `sender` and broadcasts to clients (see [Channels](#channels) for delivery scope when `channel` is set). |
128128
| `reaction` | Add or remove a reaction. Requires the `reaction` object (`emoji`, `target_id`, optional `is_removal`). |
@@ -158,6 +158,7 @@ Optional chat encryption uses a **shared symmetric key** for all participants (g
158158
- **Algorithm**: ChaCha20-Poly1305 (RFC 8439). Each encrypted payload uses a random **12-byte nonce** (typical for this AEAD).
159159
- **Text messages on the wire**: Same JSON message shape as plaintext chat; set `encrypted` to `true`. The `content` field is **standard base64** encoding of **nonce ‖ ciphertext** (nonce first, 12 bytes, then the Poly1305-sealed ciphertext). The plaintext decrypted by the AEAD is a JSON object representing the inner chat message (e.g. sender, content, type, timestamp) as produced by the reference client.
160160
- **Files**: When `type` is `file` and E2E is enabled, the reference client encrypts file bytes with the same global key; see client implementation for the exact binary layout (nonce-prefixed ciphertext).
161+
- **Edits** (`type`: `edit`): When E2E is enabled, the reference client encrypts the new plaintext the same way as a normal `text` message and sets `encrypted` accordingly. The server stores the new opaque `content` and updates `is_encrypted` from the incoming `encrypted` field (it does not force plaintext).
161162

162163
The server stores and relays opaque `content` (and encrypted file blobs) without performing decryption.
163164

@@ -175,6 +176,7 @@ The server stores and relays opaque `content` (and encrypted file blobs) without
175176
- Delivers to all connected clients **or** only to members of a channel when `channel` is non-empty and `sender` is not `System` (see [Channels](#channels)). Direct messages use a separate path (sender and recipient only).
176177
- Reactions, read receipts, and last channel per user may be persisted server-side and replayed to reconnecting clients.
177178
- Message history is capped at 1000 messages.
179+
- On successful `type`: `edit`, the server updates `content`, sets `edited`, and sets stored encryption metadata from the incoming `encrypted` field (so edited ciphertext rows remain ciphertext with `is_encrypted` aligned to the wire).
178180

179181
---
180182

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ Run **`./marchat-client -doctor`** or **`./marchat-server -doctor`** for a text
347347
### Messaging
348348
| Command | Description |
349349
|---------|-------------|
350-
| `:edit <id> <text>` | Edit a message by its ID |
350+
| `:edit <id> <text>` | Edit your own message by ID (admins cannot edit others' messages; with E2E on, the new text is encrypted like normal chat and the server keeps `is_encrypted` in sync) |
351351
| `:delete <id>` | Delete a message by its ID |
352352
| `:dm [user] [msg]` | Send a DM or toggle DM mode (no args exits DM mode) |
353353
| `:search <query>` | Search message history on the server |

client/main.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,7 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
595595
if m2.MessageID == v.MessageID {
596596
m.messages[i].Content = v.Content
597597
m.messages[i].Edited = true
598+
m.messages[i].Encrypted = v.Encrypted
598599
break
599600
}
600601
}
@@ -1560,8 +1561,23 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
15601561
m.messages = append(m.messages, shared.Message{Sender: "System", Content: "Invalid message ID", CreatedAt: time.Now(), Type: shared.TextMessage})
15611562
} else {
15621563
newContent := strings.Join(parts[2:], " ")
1563-
editMsg := shared.Message{Type: shared.EditMessageType, MessageID: id, Content: newContent, Sender: m.cfg.Username}
1564-
if m.conn != nil {
1564+
editMsg := shared.Message{Type: shared.EditMessageType, MessageID: id, Sender: m.cfg.Username}
1565+
okToSend := true
1566+
if m.useE2E && m.keystore != nil && m.keystore.GetSessionKey("global") != nil {
1567+
if err := verifyKeystoreUnlocked(m.keystore); err != nil {
1568+
m.messages = append(m.messages, shared.Message{Sender: "System", Content: "[ERROR] Keystore not unlocked: " + err.Error(), CreatedAt: time.Now(), Type: shared.TextMessage})
1569+
okToSend = false
1570+
} else if wire, encErr := encryptGlobalTextWireContent(m.keystore, m.cfg.Username, newContent); encErr != nil {
1571+
m.messages = append(m.messages, shared.Message{Sender: "System", Content: "[ERROR] Failed to encrypt edit: " + encErr.Error(), CreatedAt: time.Now(), Type: shared.TextMessage})
1572+
okToSend = false
1573+
} else {
1574+
editMsg.Content = wire
1575+
editMsg.Encrypted = true
1576+
}
1577+
} else {
1578+
editMsg.Content = newContent
1579+
}
1580+
if okToSend && m.conn != nil {
15651581
_ = m.conn.WriteJSON(editMsg)
15661582
}
15671583
}

client/websocket.go

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,32 @@ type UserList struct {
3939

4040
type quitMsg struct{}
4141

42+
// encryptGlobalTextWireContent returns base64(nonce ‖ ciphertext) for global chat E2E text,
43+
// matching the wire format produced for normal encrypted messages.
44+
func encryptGlobalTextWireContent(keystore *crypto.KeyStore, username, plaintext string) (string, error) {
45+
if keystore == nil {
46+
return "", fmt.Errorf("keystore not initialized")
47+
}
48+
if keystore.GetSessionKey("global") == nil {
49+
return "", fmt.Errorf("global key not available - global E2E encryption not initialized")
50+
}
51+
encryptedMsg, err := keystore.EncryptMessage(username, plaintext, "global")
52+
if err != nil {
53+
return "", fmt.Errorf("global encryption failed: %w", err)
54+
}
55+
if len(encryptedMsg.Encrypted) == 0 {
56+
return "", fmt.Errorf("encryption returned empty ciphertext")
57+
}
58+
combinedData := make([]byte, 0, len(encryptedMsg.Nonce)+len(encryptedMsg.Encrypted))
59+
combinedData = append(combinedData, encryptedMsg.Nonce...)
60+
combinedData = append(combinedData, encryptedMsg.Encrypted...)
61+
finalContent := base64.StdEncoding.EncodeToString(combinedData)
62+
if len(finalContent) == 0 {
63+
return "", fmt.Errorf("final content is empty after encoding")
64+
}
65+
return finalContent, nil
66+
}
67+
4268
func debugEncryptAndSend(recipients []string, plaintext string, ws *websocket.Conn, keystore *crypto.KeyStore, username string) error {
4369
log.Printf("DEBUG: Starting global encryption for %d recipients", len(recipients))
4470
log.Printf("DEBUG: Plaintext length: %d", len(plaintext))
@@ -56,32 +82,13 @@ func debugEncryptAndSend(recipients []string, plaintext string, ws *websocket.Co
5682
}
5783
log.Printf("DEBUG: Global key available (ID: %s)", globalKey.KeyID)
5884

59-
conversationID := "global"
60-
encryptedMsg, err := keystore.EncryptMessage(username, plaintext, conversationID)
85+
finalContent, err := encryptGlobalTextWireContent(keystore, username, plaintext)
6186
if err != nil {
6287
log.Printf("ERROR: Global encryption failed: %v", err)
63-
return fmt.Errorf("global encryption failed: %v", err)
64-
}
65-
66-
log.Printf("DEBUG: Global encryption successful - encrypted length: %d", len(encryptedMsg.Encrypted))
67-
68-
if len(encryptedMsg.Encrypted) == 0 {
69-
log.Printf("ERROR: Encryption returned empty ciphertext")
70-
return fmt.Errorf("encryption returned empty ciphertext; aborting send")
88+
return err
7189
}
72-
73-
combinedData := make([]byte, 0, len(encryptedMsg.Nonce)+len(encryptedMsg.Encrypted))
74-
combinedData = append(combinedData, encryptedMsg.Nonce...)
75-
combinedData = append(combinedData, encryptedMsg.Encrypted...)
76-
77-
finalContent := base64.StdEncoding.EncodeToString(combinedData)
7890
log.Printf("DEBUG: Base64 encoded nonce+ciphertext - length: %d", len(finalContent))
7991

80-
if len(finalContent) == 0 {
81-
log.Printf("ERROR: Final content is empty after encoding")
82-
return fmt.Errorf("final content is empty after encoding")
83-
}
84-
8592
msg := shared.Message{
8693
Content: finalContent,
8794
Sender: username,

server/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func (c *Client) readPump() {
107107
}
108108

109109
if msg.Type == shared.EditMessageType && msg.MessageID > 0 {
110-
if err := EditMessage(c.db, msg.MessageID, c.username, msg.Content); err != nil {
110+
if err := EditMessage(c.db, msg.MessageID, c.username, msg.Content, msg.Encrypted); err != nil {
111111
c.send <- shared.Message{
112112
Sender: "System",
113113
Content: "Edit failed: " + err.Error(),

server/handlers.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -697,10 +697,10 @@ func ClearMessages(db *sql.DB) error {
697697
return err
698698
}
699699

700-
func EditMessage(db *sql.DB, messageID int64, sender, newContent string) error {
700+
func EditMessage(db *sql.DB, messageID int64, sender, newContent string, encrypted bool) error {
701701
result, err := dbExec(db,
702-
`UPDATE messages SET content = ?, edited = 1, is_encrypted = 0 WHERE message_id = ? AND sender = ?`,
703-
newContent, messageID, sender)
702+
`UPDATE messages SET content = ?, edited = 1, is_encrypted = ? WHERE message_id = ? AND sender = ?`,
703+
newContent, encrypted, messageID, sender)
704704
if err != nil {
705705
return fmt.Errorf("edit message: %w", err)
706706
}

server/handlers_test.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,12 +458,13 @@ func TestEditMessage(t *testing.T) {
458458
}
459459

460460
t.Run("success", func(t *testing.T) {
461-
if err := EditMessage(db, id, "alice", "updated"); err != nil {
461+
if err := EditMessage(db, id, "alice", "updated", false); err != nil {
462462
t.Fatalf("EditMessage failed: %v", err)
463463
}
464464
var content string
465465
var edited bool
466-
if err := db.QueryRow(`SELECT content, edited FROM messages WHERE message_id = ?`, id).Scan(&content, &edited); err != nil {
466+
var isEnc bool
467+
if err := db.QueryRow(`SELECT content, edited, is_encrypted FROM messages WHERE message_id = ?`, id).Scan(&content, &edited, &isEnc); err != nil {
467468
t.Fatalf("query row: %v", err)
468469
}
469470
if content != "updated" {
@@ -472,6 +473,35 @@ func TestEditMessage(t *testing.T) {
472473
if !edited {
473474
t.Error("edited flag not set")
474475
}
476+
if isEnc {
477+
t.Error("plain message edit should leave is_encrypted false")
478+
}
479+
})
480+
481+
t.Run("encrypted_content_flag", func(t *testing.T) {
482+
eid, err := InsertMessage(db, shared.Message{
483+
Sender: "alice",
484+
Content: "original-cipher",
485+
CreatedAt: now.Add(2 * time.Minute),
486+
Encrypted: true,
487+
})
488+
if err != nil {
489+
t.Fatalf("InsertMessage failed: %v", err)
490+
}
491+
if err := EditMessage(db, eid, "alice", "updated-cipher", true); err != nil {
492+
t.Fatalf("EditMessage failed: %v", err)
493+
}
494+
var content string
495+
var isEnc bool
496+
if err := db.QueryRow(`SELECT content, is_encrypted FROM messages WHERE message_id = ?`, eid).Scan(&content, &isEnc); err != nil {
497+
t.Fatalf("query row: %v", err)
498+
}
499+
if content != "updated-cipher" {
500+
t.Errorf("content = %q, want updated-cipher", content)
501+
}
502+
if !isEnc {
503+
t.Error("encrypted edit should set is_encrypted true in DB")
504+
}
475505
})
476506

477507
t.Run("wrong user", func(t *testing.T) {
@@ -484,7 +514,7 @@ func TestEditMessage(t *testing.T) {
484514
if err != nil {
485515
t.Fatalf("InsertMessage failed: %v", err)
486516
}
487-
err = EditMessage(db, id2, "alice", "hijack")
517+
err = EditMessage(db, id2, "alice", "hijack", false)
488518
if err == nil {
489519
t.Fatal("expected error for wrong sender")
490520
}

0 commit comments

Comments
 (0)