diff --git a/lib/model/model.go b/lib/model/model.go index 8f61f0e90..259ded135 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -2079,7 +2079,7 @@ func (m *model) Request(conn protocol.Connection, req *protocol.Request) (out pr return nil, protocol.ErrGeneric } - if folderCfg.Type != config.FolderTypeReceiveEncrypted && len(req.Hash) > 0 && !scanner.Validate(res.data[:n], req.Hash) { + if folderCfg.Type != config.FolderTypeReceiveEncrypted && !scanner.Validate(res.data[:n], req.Hash) { m.recheckFile(deviceID, req.Folder, req.Name, req.Offset, req.Hash) l.Debugf("%v REQ(in) failed validating data: %s: %q / %q o=%d s=%d", m, deviceID.Short(), req.Folder, req.Name, req.Offset, req.Size) return nil, protocol.ErrNoSuchFile diff --git a/lib/model/model_test.go b/lib/model/model_test.go index de3e90342..3eb65fd25 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -9,6 +9,7 @@ package model import ( "bytes" "context" + "crypto/sha256" "encoding/json" "errors" "fmt" @@ -93,8 +94,10 @@ func TestRequest(t *testing.T) { m.ScanFolder("default") + foobarHash := sha256.Sum256([]byte("foobar")) + // Existing, shared file - res, err := m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 6}) + res, err := m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 6, Hash: foobarHash[:]}) if err != nil { t.Fatal(err) } @@ -104,35 +107,42 @@ func TestRequest(t *testing.T) { } // Existing, nonshared file - _, err = m.Request(device2Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 6}) + _, err = m.Request(device2Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 6, Hash: foobarHash[:]}) if err == nil { t.Error("Unexpected nil error on insecure file read") } // Nonexistent file - _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "nonexistent", Size: 6}) + _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "nonexistent", Size: 6, Hash: foobarHash[:]}) if err == nil { t.Error("Unexpected nil error on insecure file read") } // Shared folder, but disallowed file name - _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "../walk.go", Size: 6}) + _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "../walk.go", Size: 6, Hash: foobarHash[:]}) if err == nil { t.Error("Unexpected nil error on insecure file read") } // Negative size - _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: -4}) + _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: -4, Hash: foobarHash[:]}) if err == nil { t.Error("Unexpected nil error on insecure file read") } - // Larger block than available + // Missing hash + _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 6}) + if err == nil { + t.Error("Unexpected nil error on request without hash") + } + + // Larger block than available, with a mismatched hash _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 42, Hash: []byte("hash necessary but not checked")}) if err == nil { t.Error("Unexpected nil error on read past end of file") } - _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 42}) + // Larger block than available, with the matching hash of the short read + _, err = m.Request(device1Conn, &protocol.Request{Folder: "default", Name: "foo", Size: 42, Hash: foobarHash[:]}) if err != nil { t.Error("Unexpected error when large read should be permitted") } @@ -2979,15 +2989,16 @@ func TestRequestLimit(t *testing.T) { defer cleanupModel(m) m.ScanFolder("default") + emptyHash := sha256.Sum256(nil) befReq := time.Now() - first, err := m.Request(conn, &protocol.Request{Folder: "default", Name: file, Size: 2000}) + first, err := m.Request(conn, &protocol.Request{Folder: "default", Name: file, Size: 2000, Hash: emptyHash[:]}) if err != nil { t.Fatalf("First request failed: %v", err) } reqDur := time.Since(befReq) returned := make(chan struct{}) go func() { - second, err := m.Request(conn, &protocol.Request{Folder: "default", Name: file, Size: 2000}) + second, err := m.Request(conn, &protocol.Request{Folder: "default", Name: file, Size: 2000, Hash: emptyHash[:]}) if err != nil { t.Errorf("Second request failed: %v", err) } diff --git a/lib/protocol/encryption.go b/lib/protocol/encryption.go index d662ddd35..7c1ccaa65 100644 --- a/lib/protocol/encryption.go +++ b/lib/protocol/encryption.go @@ -93,31 +93,25 @@ func (e encryptedModel) Request(req *Request) (RequestResponse, error) { } realSize := req.Size - blockOverhead realOffset := req.Offset - int64(req.BlockNo*blockOverhead) + if realOffset < 0 { + panic("bug: realOffset underflow") + } if req.Size < minPaddedSize { return nil, errors.New("short request") } - // Attempt to decrypt the block hash; it may be nil depending on what - // type of device the request comes from. Trusted devices with - // encryption enabled know the hash but don't bother to encrypt & send - // it to us. Untrusted devices have the hash from the encrypted index - // data and do send it. The model knows to only verify the hash if it - // actually gets one. - - var realHash []byte + // Decrypt the block hash. fileKey := e.keyGen.FileKey(realName, folderKey) - if len(req.Hash) > 0 { - var additional [8]byte - binary.BigEndian.PutUint64(additional[:], uint64(realOffset)) - realHash, err = decryptDeterministic(req.Hash, fileKey, additional[:]) - if err != nil { - // "Legacy", no offset additional data? - realHash, err = decryptDeterministic(req.Hash, fileKey, nil) - } - if err != nil { - return nil, fmt.Errorf("decrypting block hash: %w", err) - } + var additional [8]byte + binary.BigEndian.PutUint64(additional[:], uint64(realOffset)) + realHash, err := decryptDeterministic(req.Hash, fileKey, additional[:]) + if err != nil { + // "Legacy", no offset additional data? + realHash, err = decryptDeterministic(req.Hash, fileKey, nil) + } + if err != nil { + return nil, fmt.Errorf("decrypting block hash: %w", err) } // Perform that request and grab the data. diff --git a/lib/protocol/protocol.go b/lib/protocol/protocol.go index 077cc2702..486fcc7e8 100644 --- a/lib/protocol/protocol.go +++ b/lib/protocol/protocol.go @@ -489,6 +489,12 @@ func (c *rawConnection) dispatcherLoop() (err error) { if msg.Size > MaxRequestSize { return newProtocolError(fmt.Errorf("request size %d exceeds maximum allowed", msg.Size), msgContext) } + if len(msg.Hash) == 0 { + // Syncthing versions older than v1.28.1 omit the hash in + // encrypted requests from trusted devices (a rare config) + // and will run into this. + return newProtocolError(errors.New("request missing block hash"), msgContext) + } go c.handleRequest(requestFromWire(msg)) case *bep.Response: diff --git a/lib/protocol/protocol_test.go b/lib/protocol/protocol_test.go index 743312ad4..dda4ba86a 100644 --- a/lib/protocol/protocol_test.go +++ b/lib/protocol/protocol_test.go @@ -560,6 +560,7 @@ func TestRequestMaxSize(t *testing.T) { Id: 1, Name: "valid", Size: MaxRequestSize, + Hash: []byte{42}, } res := <-c.outbox @@ -573,6 +574,7 @@ func TestRequestMaxSize(t *testing.T) { Id: 2, Name: "invalid", Size: int32(s), + Hash: []byte{42}, } select { @@ -606,6 +608,7 @@ func TestRequestZeroSize(t *testing.T) { Id: 1, Name: "valid", Size: 0, + Hash: []byte{42}, } select { @@ -632,6 +635,7 @@ func TestRequestInvalidFilename(t *testing.T) { Id: 1, Name: "../escape", Size: 1024, + Hash: []byte{42}, } select { diff --git a/lib/scanner/blocks.go b/lib/scanner/blocks.go index 7aaf0eb97..3b34d5b7c 100644 --- a/lib/scanner/blocks.go +++ b/lib/scanner/blocks.go @@ -120,25 +120,12 @@ func Blocks(ctx context.Context, r io.Reader, blocksize int, sizehint int64, cou return blocks, nil } -// Validate validates the hash, if len(hash)>0. +// Validate validates the hash. func Validate(buf, hash []byte) bool { - if len(hash) > 0 { - hbuf := sha256.Sum256(buf) - return bytes.Equal(hbuf[:], hash) - } - - return true + hbuf := sha256.Sum256(buf) + return bytes.Equal(hbuf[:], hash) } -type noopHash struct{} - -func (noopHash) Sum32() uint32 { return 0 } -func (noopHash) BlockSize() int { return 0 } -func (noopHash) Size() int { return 0 } -func (noopHash) Reset() {} -func (noopHash) Sum([]byte) []byte { return nil } -func (noopHash) Write([]byte) (int, error) { return 0, nil } - type noopCounter struct{} func (*noopCounter) Update(_ int64) {}