Merge pull request #103 from neicnordic/del_bugfix

Bugfix; incorrect length was used to check for discarded areas
neicnordic · Jan 2, 2024 · 4449f2c · 4449f2c
2 parents 7c6f163 + 8ec0094
commit 4449f2c
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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)
+
 	}
 }
 
@@ -399,6 +400,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 {
@@ -856,7 +923,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)
 	}