From fdb39f9e52248eced63693a1289c72e350791a9b Mon Sep 17 00:00:00 2001 From: Pontus Freyhult Date: Thu, 21 Dec 2023 10:22:33 +0100 Subject: [PATCH 1/3] Bugfix; incorrect length was used to check for discarded areas --- internal/version/version.go | 2 +- streaming/in.go | 8 +++-- streaming/streaming_test.go | 66 +++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/internal/version/version.go b/internal/version/version.go index ad61f50..fbdf783 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -7,7 +7,7 @@ import ( ) // The version in the current branch -var Version = "1.8.6" +var Version = "1.8.7" // If this is "" (empty string) then it means that it is a final release. // Otherwise, this is a pre-release e.g. "dev", "beta", "rc1", etc. diff --git a/streaming/in.go b/streaming/in.go index c1da4a3..d7ace96 100644 --- a/streaming/in.go +++ b/streaming/in.go @@ -264,7 +264,7 @@ func (c *crypt4GHInternalReader) read(p []byte) (n int, err error) { } canRead := len(p[haveRead:]) - remainingInBuffer := c.bufferUse - c.buffer.Len() + remainingInBuffer := c.buffer.Len() if remainingInBuffer < canRead { canRead = remainingInBuffer @@ -297,9 +297,11 @@ func (c *crypt4GHInternalReader) read(p []byte) (n int, err error) { haveRead++ } } else { - // We can just read the rest of the buffer + // Read larger chunk from buffer. As precaution, limit to what we + // should be able to read only, as that is the bit we've checked + // if the discard list imposes any holes in - r, err := c.buffer.Read(p[haveRead:]) + r, err := c.buffer.Read(p[haveRead : haveRead+canRead]) haveRead += r c.streamPos += int64(r) diff --git a/streaming/streaming_test.go b/streaming/streaming_test.go index ebc07c9..bcf11cb 100644 --- a/streaming/streaming_test.go +++ b/streaming/streaming_test.go @@ -399,6 +399,72 @@ func TestGetHeader(t *testing.T) { } } +func TestReencryptionWithDataEditListInCrypt4GHReaderDiscardStart(t *testing.T) { + inFile, err := os.Open("../test/sample.txt") + if err != nil { + t.Error(err) + } + writerPrivateKey, err := keys.ReadPrivateKey(strings.NewReader(sshEd25519SecEnc), []byte("123123")) + if err != nil { + t.Error(err) + } + readerPublicKey, err := keys.ReadPublicKey(strings.NewReader(crypt4ghX25519Pub)) + if err != nil { + t.Error(err) + } + buffer := bytes.Buffer{} + readerPublicKeyList := [][chacha20poly1305.KeySize]byte{} + readerPublicKeyList = append(readerPublicKeyList, readerPublicKey) + writer, err := NewCrypt4GHWriter(&buffer, writerPrivateKey, readerPublicKeyList, nil) + if err != nil { + t.Error(err) + } + _, err = io.Copy(writer, inFile) + if err != nil { + t.Error(err) + } + err = inFile.Close() + if err != nil { + t.Error(err) + } + err = writer.Close() + if err != nil { + t.Error(err) + } + + readerSecretKey, err := keys.ReadPrivateKey(strings.NewReader(crypt4ghX25519Sec), []byte("password")) + if err != nil { + t.Error(err) + } + dataEditListHeaderPacket := headers.DataEditListHeaderPacket{ + PacketType: headers.PacketType{PacketType: headers.DataEditList}, + NumberLengths: 3, + Lengths: []uint64{0, 100, 300}, + } + reader, err := NewCrypt4GHReader(&buffer, readerSecretKey, &dataEditListHeaderPacket) + if err != nil { + t.Error(err) + } + all, err := io.ReadAll(reader) + if err != nil { + t.Error(err) + } + inFile, err = os.Open("../test/sample.txt") + if err != nil { + t.Error(err) + } + inBytes, err := io.ReadAll(inFile) + if err != nil { + t.Error(err) + } + if !bytes.Equal(all[:100], inBytes[:100]) { + t.Errorf("Different data before discard: %v vs %v", all[:100], inBytes[:100]) + } + if !bytes.Equal(all[100:], inBytes[400:]) { + t.Errorf("Different data after discard: %v vs %v (truncated)", all[400:500], inBytes[100:200]) + } +} + func TestNewCrypt4GHWriterWithoutPrivateKey(t *testing.T) { inFile, err := os.Open("../test/sample.txt") if err != nil { From e9e3d68d10e467d11a3040aa0301093f39f7e103 Mon Sep 17 00:00:00 2001 From: Pontus Freyhult Date: Fri, 22 Dec 2023 11:25:08 +0100 Subject: [PATCH 2/3] Fix faulty test condition --- streaming/streaming_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/streaming/streaming_test.go b/streaming/streaming_test.go index bcf11cb..83109ff 100644 --- a/streaming/streaming_test.go +++ b/streaming/streaming_test.go @@ -922,7 +922,7 @@ func TestSeek(t *testing.T) { t.Error(err) } - if r, err := writer.Write(inBytes[:70225]); err != nil || r != len(inBytes) { + if r, err := writer.Write(inBytes[:70225]); err != nil || r != 70225 { t.Errorf("Problem when writing to cryptgh writer, r=%d, err=%v", r, err) } From 8ec0094dbcb556b682eb5e6d4544d2bede7aa43a Mon Sep 17 00:00:00 2001 From: Pontus Freyhult Date: Fri, 22 Dec 2023 11:42:23 +0100 Subject: [PATCH 3/3] Better errors from test TestReencryptionWithDataEditListAndDiscard --- streaming/streaming_test.go | 39 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/streaming/streaming_test.go b/streaming/streaming_test.go index 83109ff..dd54475 100644 --- a/streaming/streaming_test.go +++ b/streaming/streaming_test.go @@ -285,35 +285,35 @@ func TestReencryptionWithDataEditListAndDiscard(t *testing.T) { } writerPrivateKey, err := keys.ReadPrivateKey(strings.NewReader(sshEd25519SecEnc), []byte("123123")) if err != nil { - t.Error(err) + t.Errorf("Reading private key failed with %v", err) } readerPublicKey, err := keys.ReadPublicKey(strings.NewReader(crypt4ghX25519Pub)) if err != nil { - t.Error(err) + t.Errorf("Reading public key failed with %v", err) } buffer := bytes.Buffer{} readerPublicKeyList := [][chacha20poly1305.KeySize]byte{} readerPublicKeyList = append(readerPublicKeyList, readerPublicKey) writer, err := NewCrypt4GHWriter(&buffer, writerPrivateKey, readerPublicKeyList, nil) if err != nil { - t.Error(err) + t.Errorf("Creating writer failed with %v", err) } _, err = io.Copy(writer, inFile) if err != nil { - t.Error(err) + t.Errorf("Copying infile to writer failed with %v", err) } err = inFile.Close() if err != nil { - t.Error(err) + t.Errorf("Closing infile failed with %v", err) } err = writer.Close() if err != nil { - t.Error(err) + t.Errorf("Closing writer failed with %v", err) } readerSecretKey, err := keys.ReadPrivateKey(strings.NewReader(crypt4ghX25519Sec), []byte("password")) if err != nil { - t.Error(err) + t.Errorf("Reading private key failed with %v", err) } dataEditListHeaderPacket := headers.DataEditListHeaderPacket{ PacketType: headers.PacketType{PacketType: headers.DataEditList}, @@ -322,54 +322,55 @@ func TestReencryptionWithDataEditListAndDiscard(t *testing.T) { } reader, err := NewCrypt4GHReader(&buffer, readerSecretKey, &dataEditListHeaderPacket) if err != nil { - t.Error(err) + t.Errorf("Creating reader failed with %v", err) } discarded, err := reader.Discard(toDiscard) if err != nil { - t.Error(err) + t.Errorf("Discarding failed with %v", err) } if discarded != toDiscard { - t.Fail() + t.Errorf("Discarded return doesn't match was asked for %v != %v", discarded, toDiscard) } all, err := io.ReadAll(reader) if err != nil { - t.Error(err) + t.Errorf("Reading all from reader failed with %v", err) } inFile, err = os.Open("../test/sample.txt") if err != nil { - t.Error(err) + t.Errorf("Opening test sample failed with %v", err) } bufioReader := bufio.NewReader(inFile) _, err = bufioReader.Discard(950 + toDiscard) if err != nil { - t.Error(err) + t.Errorf("Discarding failed with %v", err) } firstLine, _, err := bufioReader.ReadLine() if err != nil { - t.Error(err) + t.Errorf("First Readline failed with %v", err) } _, _, err = bufioReader.ReadLine() if err != nil { - t.Error(err) + t.Errorf("First Skipped Readline failed with %v", err) } _, _, err = bufioReader.ReadLine() if err != nil { - t.Error(err) + t.Errorf("Second Skipped Readline failed with %v", err) } _, _, err = bufioReader.ReadLine() if err != nil { - t.Error(err) + t.Errorf("Third Skipped Readline failed with %v", err) } secondLine, _, err := bufioReader.ReadLine() if err != nil { - t.Error(err) + t.Errorf("Second used Readline failed with %v", err) } expectedText := strings.TrimSpace(string(firstLine) + "\n" + string(secondLine)) actualText := strings.TrimSpace(string(all)) if !strings.EqualFold(expectedText, actualText) { - t.Fail() + t.Errorf("Texts didn't match: %v, %v", expectedText, actualText) + } }