protondrive: ignore modification times and hashes the owner didn't sign
Extended attributes whose signature failed or was missing were used, so a server could set a file's modification time, size and SHA-1 at will: for example make `rclone sync --update` skip every changed file. Without ignore_signature_errors they are now not used and an ERROR is logged: the modification time is the server's, there is no SHA-1, and the size comes from the recorded block sizes, which reads check. The file stays readable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
67e4cfeb7a
commit
94eedc942f
4 files changed
+84
-4
No files matched your search
@@ -869,6 +869,26 @@ type xattrs struct {
|
||||
NoSize bool // set if read attributes record no usable size
|
||||
}
|
||||
|
||||
// blockSizesOnly returns x with only its block sizes, which reads
|
||||
// check, and the size they add up to.
|
||||
func (x *xattrs) blockSizesOnly() *xattrs {
|
||||
out := &xattrs{NoSize: true}
|
||||
if x == nil {
|
||||
return out
|
||||
}
|
||||
var total int64
|
||||
for _, size := range x.BlockSizes {
|
||||
if size > maxBlockSize {
|
||||
return out
|
||||
}
|
||||
total += size
|
||||
}
|
||||
if x.BlockSizes != nil {
|
||||
out.BlockSizes, out.Size, out.NoSize = x.BlockSizes, total, false
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// xattrsTimeLayout is the layout of ModificationTime, as JavaScript's
|
||||
// Date.toISOString writes it.
|
||||
const xattrsTimeLayout = "2006-01-02T15:04:05.000Z07:00"
|
||||
|
||||
@@ -549,6 +549,7 @@ func (c *client) decryptNode(ctx context.Context, d *api.LinkDetails, parent *no
|
||||
if l.SignatureEmail != "" {
|
||||
signerKR = keyVerifyKR
|
||||
}
|
||||
var xattrsVerifyErr error
|
||||
switch {
|
||||
case l.Type == api.LinkTypeFolder && d.Folder != nil:
|
||||
n.IsFolder = true
|
||||
@@ -566,10 +567,8 @@ func (c *client) decryptNode(ctx context.Context, d *api.LinkDetails, parent *no
|
||||
if xattrsVerifyKR == nil {
|
||||
xattrsVerifyKR = keys.keyRing
|
||||
}
|
||||
var xattrsVerifyErr error
|
||||
n.X, xattrsVerifyErr, err = decryptXAttrs(d.Folder.XAttr, keys.keyRing, xattrsVerifyKR, now)
|
||||
decryptErrs = append(decryptErrs, annotate("extended attributes", err))
|
||||
verifyErrs = append(verifyErrs, annotate("extended attributes", xattrsVerifyErr))
|
||||
case l.Type == api.LinkTypeFile && d.File != nil:
|
||||
n.MIMEType = d.File.MediaType
|
||||
if rev := d.File.ActiveRevision; rev != nil {
|
||||
@@ -582,14 +581,21 @@ func (c *client) decryptNode(ctx context.Context, d *api.LinkDetails, parent *no
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
var xattrsVerifyErr error
|
||||
n.X, xattrsVerifyErr, err = decryptXAttrs(rev.XAttr, keys.keyRing, revVerifyKR, now)
|
||||
decryptErrs = append(decryptErrs, annotate("extended attributes", err))
|
||||
verifyErrs = append(verifyErrs, annotate("extended attributes", xattrsVerifyErr))
|
||||
}
|
||||
default:
|
||||
return nil, fmt.Errorf("%q: unknown node type %d", truncateName(name), l.Type), nil
|
||||
}
|
||||
if xattrsVerifyErr != nil && !c.opt.IgnoreSignatureErrors {
|
||||
// Anyone with the node's public key can write attributes.
|
||||
// Block sizes are checked as blocks are read; nothing checks
|
||||
// the rest.
|
||||
n.X = n.X.blockSizesOnly()
|
||||
verifyErrs = append(verifyErrs, annotate("extended attributes, so their modification time, size and SHA-1 are not used", xattrsVerifyErr))
|
||||
} else {
|
||||
verifyErrs = append(verifyErrs, annotate("extended attributes", xattrsVerifyErr))
|
||||
}
|
||||
if err := errors.Join(decryptErrs...); err != nil {
|
||||
c.reportOnce("decrypt", n.ID, func() {
|
||||
fs.Errorf(nil, "protondrive: %q (ID %s): can't decrypt: %v", n.Name, n.ID, err)
|
||||
|
||||
@@ -24,6 +24,7 @@ import (
|
||||
"github.com/rclone/rclone/fs/config/configfile"
|
||||
"github.com/rclone/rclone/fs/config/configmap"
|
||||
"github.com/rclone/rclone/fs/config/obscure"
|
||||
"github.com/rclone/rclone/fs/hash"
|
||||
"github.com/rclone/rclone/fs/log"
|
||||
"github.com/rclone/rclone/fs/object"
|
||||
"github.com/rclone/rclone/fs/operations"
|
||||
@@ -330,6 +331,52 @@ func TestHostileServer(t *testing.T) {
|
||||
assert.Equal(t, controlKey, u.NameSessionKey(control), "a verified name keeps its session key, as in the SDK")
|
||||
})
|
||||
|
||||
t.Run("unsigned extended attributes", func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
remote, _, u := setupFakeRemote(t)
|
||||
content := patternedContent(blockSize+10, 18)
|
||||
modTime := time.Date(2025, 3, 4, 5, 6, 7, 0, time.UTC)
|
||||
id := u.SeedFile(u.RootID(), "victim.bin", content, modTime)
|
||||
dir := u.SeedFolder(u.RootID(), "dir")
|
||||
u.Corrupt(id, "xattrs-unsigned")
|
||||
u.Corrupt(dir, "xattrs-unsigned")
|
||||
forged, err := time.Parse(time.RFC3339, protontest.ForgedModTime)
|
||||
require.NoError(t, err)
|
||||
|
||||
capture := captureLog(t, 0)
|
||||
f := newFakeFs(t, remote, "")
|
||||
o, err := f.NewObject(ctx, "victim.bin")
|
||||
require.NoError(t, err)
|
||||
assert.False(t, o.ModTime(ctx).Equal(forged), "the forged modification time is used")
|
||||
sum, err := o.Hash(ctx, hash.SHA1)
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, sum, "the forged SHA-1 is used")
|
||||
assert.Equal(t, int64(len(content)), o.Size(), "the size comes from the block sizes, which reads check")
|
||||
got, err := readAll(ctx, o)
|
||||
require.NoError(t, err, "the file is readable")
|
||||
assert.True(t, bytes.Equal(content, got))
|
||||
rc, err := o.Open(ctx, &fs.RangeOption{Start: blockSize, End: blockSize + 9})
|
||||
require.NoError(t, err)
|
||||
got, err = io.ReadAll(rc)
|
||||
require.NoError(t, rc.Close())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, content[blockSize:], got, "a ranged read")
|
||||
for _, entry := range mustList(ctx, t, f, "") {
|
||||
if entry.Remote() == "dir" {
|
||||
assert.False(t, entry.ModTime(ctx).Equal(forged), "the folder's forged modification time is used")
|
||||
}
|
||||
}
|
||||
assert.Contains(t, capture.String(), "not used")
|
||||
|
||||
f = newFakeFs(t, strings.TrimSuffix(remote, ":")+",ignore_signature_errors=true:", "")
|
||||
o, err = f.NewObject(ctx, "victim.bin")
|
||||
require.NoError(t, err)
|
||||
assert.True(t, o.ModTime(ctx).Equal(forged), "with signature checks off the attributes are used")
|
||||
sum, err = o.Hash(ctx, hash.SHA1)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, protontest.ForgedSHA1, sum)
|
||||
})
|
||||
|
||||
t.Run("moving or copying out of a folder with a forged key", func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
remote, _, u := setupFakeRemote(t)
|
||||
|
||||
@@ -243,6 +243,13 @@ on. The option never turns off the block hash checks.
|
||||
A file or folder name whose signature doesn't verify is listed anyway,
|
||||
with an `ERROR` logged, as Proton's apps do.
|
||||
|
||||
The modification time, size and SHA-1 of an item whose extended
|
||||
attributes' signature doesn't verify aren't used unless
|
||||
`ignore_signature_errors` is set: rclone logs an `ERROR` and shows the
|
||||
server's modification time and no SHA-1. A file's size then comes from
|
||||
its recorded block sizes, which reads check, or is the encrypted size if
|
||||
it has none. The file stays readable.
|
||||
|
||||
Two limits of the Proton Drive protocol, which Proton's SDK shares,
|
||||
can't be detected by rclone:
|
||||
|
||||
|
||||
Reference in new issue
Block a user