protondrive: refuse writing into a folder whose name hash key is unsigned
A folder's hash key whose signature failed or was missing was only logged, and the names of everything written into the folder were then hashed with it, so a server which made the key could test guesses at those names. Creating, uploading, renaming, moving or copying into such a folder is now refused unless ignore_signature_errors is set, when an ERROR is logged once per folder. Reading is unaffected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
db7afafc08
commit
b3f5685e62
3 files changed
+61
-3
No files matched your search
@@ -66,7 +66,8 @@ const (
|
||||
|
||||
// nameSignedSince and hashKeySignedSince are the Unix times from
|
||||
// which names and hash keys are signed: as in the SDK, a missing
|
||||
// signature on an older one is not an error.
|
||||
// signature on an older one is not an error. The creation time is
|
||||
// the server's, so a server can exempt a node by back-dating it.
|
||||
nameSignedSince = 1609459200 // 2021-01-01
|
||||
hashKeySignedSince = 1627776000 // 2021-08-01
|
||||
|
||||
@@ -134,6 +135,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
|
||||
keyVerifyKR *crypto.KeyRing // verifies the node's key and content key signatures
|
||||
link *api.LinkDetails // the encrypted metadata
|
||||
}
|
||||
@@ -552,6 +554,9 @@ func (c *client) decryptNode(ctx context.Context, d *api.LinkDetails, parent *no
|
||||
hashKeyVerifyErr = nil
|
||||
}
|
||||
n.hashKey = hashKey
|
||||
if hashKeyVerifyErr != nil {
|
||||
n.hashKeyErr = fmt.Errorf("the hash key of %q isn't signed by its owner: %w", truncateName(name), hashKeyVerifyErr)
|
||||
}
|
||||
decryptErrs = append(decryptErrs, annotate("hash key", err))
|
||||
verifyErrs = append(verifyErrs, annotate("hash key", hashKeyVerifyErr))
|
||||
xattrsVerifyKR := signerKR
|
||||
@@ -960,7 +965,7 @@ func mutationError(what string, err error) error {
|
||||
}
|
||||
|
||||
// targetFolder returns folder id to put or name a node in, which must
|
||||
// have a readable hash key, and a verified key unless
|
||||
// have a readable hash key, and a verified key and hash key unless
|
||||
// IgnoreSignatureErrors is set.
|
||||
func (c *client) targetFolder(ctx context.Context, id string) (*node, error) {
|
||||
n, err := c.getNode(ctx, id)
|
||||
@@ -984,6 +989,16 @@ func (c *client) targetFolder(ctx context.Context, id string) (*node, error) {
|
||||
case n.keys.unverified != nil:
|
||||
c.reportUnverified("writing into", n, "whoever made its key can read what is written")
|
||||
}
|
||||
switch {
|
||||
case n.hashKeyErr != nil && !c.opt.IgnoreSignatureErrors:
|
||||
// Whoever made the hash key could test guesses at the names
|
||||
// written into the folder against their lookup hashes.
|
||||
return nil, fmt.Errorf("folder %q: %w: its hash key isn't verified, so whoever made it could test guesses at the names written into it: %v: set ignore_signature_errors to write anyway", n.Name, errIntegrity, n.hashKeyErr)
|
||||
case n.hashKeyErr != nil:
|
||||
c.reportOnce("unverified hash key writing into", n.ID, func() {
|
||||
fs.Errorf(nil, "protondrive: writing into %q (ID %s) although its hash key isn't verified, so whoever made it can test guesses at the names written: %v", n.Name, n.ID, n.hashKeyErr)
|
||||
})
|
||||
}
|
||||
return n, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -257,6 +257,47 @@ func TestHostileServer(t *testing.T) {
|
||||
require.NoError(t, err, "upload with signature checks off")
|
||||
})
|
||||
|
||||
t.Run("writing into a folder whose hash key the server made", func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
remote, _, u := setupFakeRemote(t)
|
||||
dir := u.SeedFolder(u.RootID(), "dir")
|
||||
u.SeedFile(dir, "existing.txt", []byte("existing"), time.Now())
|
||||
u.SeedFile(u.RootID(), "mine.txt", []byte("mine"), time.Now())
|
||||
u.Corrupt(dir, "hash-key-unsigned")
|
||||
before := len(u.Dump())
|
||||
|
||||
f := newFakeFs(t, remote, "")
|
||||
src := object.NewMemoryObject("dir/tax-return.pdf", time.Now(), []byte("secret"))
|
||||
_, err := operations.Copy(ctx, f, nil, src.Remote(), src)
|
||||
require.Error(t, err, "uploaded")
|
||||
assert.Contains(t, err.Error(), "ignore_signature_errors")
|
||||
assert.Error(t, f.Mkdir(ctx, "dir/sub"), "made a folder")
|
||||
mine, err := f.NewObject(ctx, "mine.txt")
|
||||
require.NoError(t, err)
|
||||
_, err = operations.Copy(ctx, f, nil, "dir/copied.txt", mine)
|
||||
assert.Error(t, err, "copied in")
|
||||
_, err = operations.Move(ctx, f, nil, "dir/moved.txt", mine)
|
||||
assert.Error(t, err, "moved in")
|
||||
existing, err := f.NewObject(ctx, "dir/existing.txt")
|
||||
require.NoError(t, err)
|
||||
_, err = operations.Move(ctx, f, nil, "dir/renamed.txt", existing)
|
||||
assert.Error(t, err, "renamed")
|
||||
assert.Len(t, u.Dump(), before, "something was written")
|
||||
got, err := readAll(ctx, existing)
|
||||
require.NoError(t, err, "reading isn't affected")
|
||||
assert.Equal(t, "existing", string(got))
|
||||
|
||||
f = newFakeFs(t, strings.TrimSuffix(remote, ":")+",ignore_signature_errors=true:", "")
|
||||
capture := captureLog(t, 0)
|
||||
for _, name := range []string{"a.txt", "b.txt"} {
|
||||
src := object.NewMemoryObject("dir/"+name, time.Now(), []byte("allowed"))
|
||||
_, err = operations.Copy(ctx, f, nil, src.Remote(), src)
|
||||
require.NoError(t, err, "upload with signature checks off")
|
||||
}
|
||||
want := fmt.Sprintf("ERROR : protondrive: writing into %q (ID %s) although its hash key isn't verified", "dir", dir)
|
||||
assert.Equal(t, 1, strings.Count(capture.String(), want), "%q logged other than once in:\n%s", want, capture.String())
|
||||
})
|
||||
|
||||
t.Run("moving or copying out of a folder with a forged key", func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
remote, _, u := setupFakeRemote(t)
|
||||
|
||||
@@ -227,7 +227,9 @@ By default rclone refuses to:
|
||||
A content key without a signature counts as one that doesn't verify;
|
||||
- create, upload, rename, move or copy anything into a folder whose key
|
||||
isn't verified, because what is written there would be encrypted to a
|
||||
key someone else may hold;
|
||||
key someone else may hold, or whose name hash key's signature doesn't
|
||||
verify, because whoever made that key could test guesses at the names
|
||||
written there;
|
||||
- rename, move or copy an item whose key isn't verified, because that
|
||||
re-signs it with your keys.
|
||||
|
||||
|
||||
Reference in new issue
Block a user