protondrive: never reuse the session key of an unverified name on rename or move

Renaming or moving a node whose name signature failed reused the old
name's session key, which a server that made that name knows, so it could
read the new name. The new name now gets a new session key unless the
old name's signature verified.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

backend/protondrive/crypto.go

@@ -567,8 +567,8 @@ func reencryptPassphrase(keys *nodeKeys, newParentKR, signerKR *crypto.KeyRing,
 }
 
 // encryptName returns name encrypted to parentKR and signed by
-// signerKR, without AEAD. A new node passes a nil sk; a rename or move
-// passes the session key of the current name, from nameSessionKey.
+// signerKR, without AEAD, with the session key sk, or a new one if sk
+// is nil.
 func encryptName(name string, sk *crypto.SessionKey, parentKR, signerKR *crypto.KeyRing, now time.Time) (string, error) {
 	enc, err := encryptArmored([]byte(name), sk, parentKR, signerKR, false, now)
 	if err != nil {

backend/protondrive/nodes.go

@@ -136,6 +136,7 @@ type node struct {
 	keys        *nodeKeys
 	hashKey     []byte           // folders only; nil if it can't be decrypted
 	hashKeyErr  error            // why hashKey may not be the folder's; nil if its signature verified or isn't required
+	nameSigned  bool             // whether the name's signature verified
 	keyVerifyKR *crypto.KeyRing  // verifies the node's key and content key signatures
 	link        *api.LinkDetails // the encrypted metadata
 }
@@ -513,6 +514,7 @@ func (c *client) decryptNode(ctx context.Context, d *api.LinkDetails, parent *no
 			return nil, fmt.Errorf("invalid name %q: %w", truncateName(name), err), nil
 		}
 	}
+	nameSigned := nameVerifyErr == nil
 	if isNotSigned(nameVerifyErr) && l.CreateTime < nameSignedSince {
 		nameVerifyErr = nil
 	}
@@ -535,6 +537,7 @@ func (c *client) decryptNode(ctx context.Context, d *api.LinkDetails, parent *no
 		ServerModTime: time.Unix(l.ModifyTime, 0),
 		keys:          keys,
 		keyVerifyKR:   keyVerifyKR,
+		nameSigned:    nameSigned,
 		link:          d,
 	}
 	verifyErrs := []error{annotate("name", nameVerifyErr), annotate("node key", keyVerifyErr)}
@@ -1160,8 +1163,7 @@ func (c *client) rename(ctx context.Context, n *node, newName string) error {
 	if err != nil {
 		return fmt.Errorf("%s: %w", what, err)
 	}
-	// As in the SDK the new name reuses the session key of the old.
-	sk, err := nameSessionKey(n.link.Link.Name, parent.keys.keyRing)
+	sk, err := renameSessionKey(n, parent.keys.keyRing)
 	if err != nil {
 		return fmt.Errorf("%s: %w", what, err)
 	}
@@ -1184,6 +1186,17 @@ func (c *client) rename(ctx context.Context, n *node, newName string) error {
 	return nil
 }
 
+// renameSessionKey returns the session key for n's new name, from the
+// name encrypted to parentKR: as in the SDK the session key of the old
+// name, unless its signature didn't verify, when it is nil for a new
+// one, as whoever made it may know it.
+func renameSessionKey(n *node, parentKR *crypto.KeyRing) (*crypto.SessionKey, error) {
+	if !n.nameSigned {
+		return nil, nil
+	}
+	return nameSessionKey(n.link.Link.Name, parentKR)
+}
+
 // checkVerified returns an error wrapping errIntegrity if n's key is
 // unverified, for op, an operation which would vouch for n. If
 // IgnoreSignatureErrors is set it logs that instead.
@@ -1214,7 +1227,8 @@ type newParentCrypto struct {
 // encryptForNewParent returns the name newName and the passphrase of n
 // encrypted to folder newParent and signed by the client's address,
 // reusing their session keys as the SDK's encryptNodeWithNewParent
-// does, for op, a move or copy.
+// does (the name's only if renameSessionKey allows), for op, a move or
+// copy.
 func (c *client) encryptForNewParent(ctx context.Context, op string, n *node, newParent *node, newName string) (*newParentCrypto, error) {
 	// An anonymous node's passphrase is signed anew as the client's.
 	if err := c.checkVerified(op, n); err != nil {
@@ -1230,7 +1244,7 @@ func (c *client) encryptForNewParent(ctx context.Context, op string, n *node, ne
 		return nil, err
 	}
 	now := c.s.serverTime()
-	sk, err := nameSessionKey(n.link.Link.Name, oldParentKR)
+	sk, err := renameSessionKey(n, oldParentKR)
 	if err != nil {
 		return nil, err
 	}

backend/protondrive/protontest/drive.go

@@ -1130,6 +1130,28 @@ func (u *User) NodeSecrets(id string) (passphrase, sessionKey []byte) {
 	return passphrase, sk.Key
 }
 
+// NameSessionKey returns the session key of the stored name of node
+// id, for tests to check which key a rename or move used. It panics if
+// it can't be decrypted.
+func (u *User) NameSessionKey(id string) []byte {
+	v := u.view()
+	l := v.d.links[id]
+	if l == nil {
+		panic("protontest: no link " + id)
+	}
+	parent, err := v.parentKey(l)
+	if parent == nil {
+		panic(fmt.Sprintf("protontest: parent key of %s: %v", id, err))
+	}
+	msg, err := crypto.NewPGPMessageFromArmored(l.name)
+	mustCrypto("read name", err)
+	dh, err := crypto.PGP().Decryption().DecryptionKey(parent).New()
+	mustCrypto("decrypt name", err)
+	sk, err := dh.DecryptSessionKey(msg.BinaryKeyPacket())
+	mustCrypto("decrypt name session key", err)
+	return sk.Key
+}
+
 // needBlocks panics unless r has at least n blocks.
 func needBlocks(r *revision, n int) {
 	if len(r.blocks) < n {

backend/protondrive/security_test.go

@@ -298,6 +298,38 @@ func TestHostileServer(t *testing.T) {
 		assert.Equal(t, 1, strings.Count(capture.String(), want), "%q logged other than once in:\n%s", want, capture.String())
 	})
 
+	t.Run("renaming or moving a node whose name the server encrypted", func(t *testing.T) {
+		ctx := context.Background()
+		remote, _, u := setupFakeRemote(t)
+		u.SeedFolder(u.RootID(), "dir")
+		renamed := u.SeedFile(u.RootID(), "renamed.txt", []byte("a"), time.Now())
+		moved := u.SeedFile(u.RootID(), "moved.txt", []byte("b"), time.Now())
+		control := u.SeedFile(u.RootID(), "control.txt", []byte("c"), time.Now())
+		serverKeys := map[string][]byte{}
+		for _, id := range []string{renamed, moved} {
+			u.Corrupt(id, "name-unsigned")
+			serverKeys[id] = u.NameSessionKey(id)
+		}
+		controlKey := u.NameSessionKey(control)
+
+		f := newFakeFs(t, remote, "")
+		for _, op := range []struct{ from, to, id string }{
+			{"renamed.txt", "secret-new-name.txt", renamed},
+			{"moved.txt", "dir/secret-moved-name.txt", moved},
+		} {
+			o, err := f.NewObject(ctx, op.from)
+			require.NoError(t, err)
+			_, err = operations.Move(ctx, f, nil, op.to, o)
+			require.NoError(t, err)
+			assert.NotEqual(t, serverKeys[op.id], u.NameSessionKey(op.id), "%s: the new name is under the server's session key", op.to)
+		}
+		o, err := f.NewObject(ctx, "control.txt")
+		require.NoError(t, err)
+		_, err = operations.Move(ctx, f, nil, "control-renamed.txt", o)
+		require.NoError(t, err)
+		assert.Equal(t, controlKey, u.NameSessionKey(control), "a verified name keeps its session key, as in the SDK")
+	})
+
 	t.Run("moving or copying out of a folder with a forged key", func(t *testing.T) {
 		ctx := context.Background()
 		remote, _, u := setupFakeRemote(t)