Merge pull request #15 from Retengart/refix-rand-read

Refix rand read && HMAC instead of just hash

.gitignore

@@ -20,5 +20,5 @@
 # Go workspace file
 go.work
 go.work.sum
-
+.idea/
 TODO

dist/snapcraft/snapcraft.yaml

@@ -1,7 +1,7 @@
 name: picocrypt
 summary: A very small, very simple, yet very secure encryption tool.
 description: Picocrypt is a very small, very simple, yet very secure encryption tool that you can use to protect your files. It's designed to be the go-to tool for encryption, with a focus on security, simplicity, and reliability. Picocrypt uses the secure XChaCha20 cipher and the Argon2id key derivation function to provide a high level of security, even from three-letter agencies like the NSA. Your privacy and security is under attack. Take it back with confidence by protecting your files with Picocrypt.
-version: "1.49"
+version: "2.00"
 confinement: strict
 base: core22
 grade: stable

dist/windows/versioninfo.rc

@@ -1,6 +1,6 @@
 1 VERSIONINFO
-FILEVERSION 1,49,0,0
-PRODUCTVERSION 1,49,0,0
+FILEVERSION 2,00,0,0
+PRODUCTVERSION 2,00,0,0
 FILEOS 0x40004
 FILETYPE 0x1
 {
@@ -8,7 +8,7 @@ BLOCK "StringFileInfo"
 {
 	BLOCK "040904B0"
 	{
-		VALUE "FileVersion", "1.49"
+		VALUE "FileVersion", "2.00"
 		VALUE "LegalCopyright", "\xA9 Evan Su & contributors, GPLv3"
 		VALUE "ProductName", "Picocrypt"
 	}

rec.md

@@ -0,0 +1,35 @@
+### Summary of Findings
+
+1. **PCC-001**
+   - **Type:** Design decision
+   - **Description:** Data encrypted with Picocrypt is stored in a custom file format, including a header which is unauthenticated. As a result, any changes made by an attacker would go undetected.
+   - **Threat level:** Low
+
+2. **PCC-004**
+   - **Type:** Cryptographic implementation
+   - **Description:** Picocrypt offers both encryption and authentication. When a user wants to decrypt a volume, it starts with decrypting and only verifies the signature afterwards. As a result a user may unknowingly use their private key on attacker-controlled material.
+   - **Threat level:** Low
+
+3. **PCC-006**
+   - **Type:** Design decision
+   - **Description:** Picocrypt uses a memory-hard key derivation function to limit brute force attacks on the password. The output is then hashed using SHA3-512 and stored in the header, to verify the key before attempting to decrypt. As a result there are two algorithms that can be attacked individually which would both lead to the key.
+   - **Threat level:** N/A
+
+### Summary of Recommendations
+
+1. **PCC-001**
+   - **Type:** Design decision
+   - **Recommendation:**
+     - Authenticate data before processing. Since part of the header is required for key derivation, this is not possible. Adding authentication to the header would allow users to be informed if the header was tampered with (after key derivation, but before decryption).
+
+2. **PCC-004**
+   - **Type:** Cryptographic implementation
+   - **Recommendation:**
+     - Authenticate the ciphertext before decrypting it.
+
+3. **PCC-006**
+   - **Type:** Design decision
+   - **Recommendation:**
+     - Do not store the hash of the key in the header.
+     - Verify the signature before attempting to decrypt the volume.
+     - Consider replacing the hash with a MAC of the header.

src/Picocrypt.go

@@ -2,7 +2,7 @@ package main
 
 /*
 
-Picocrypt v1.49
+Picocrypt v2.00
 Copyright (c) Evan Su
 Released under GPL-3.0-only
 https://github.com/Picocrypt/Picocrypt
@@ -61,7 +61,7 @@ var TRANSPARENT = color.RGBA{0x00, 0x00, 0x00, 0x00}
 
 // Generic variables
 var window *giu.MasterWindow
-var version = "v1.49"
+var version = "v2.00"
 var dpi float32
 var mode string
 var working bool
@@ -1186,13 +1186,19 @@ func work() {
 	var hkdfSalt []byte                // HKDF-SHA3 salt, 32 bytes
 	var serpentIV []byte               // Serpent IV, 16 bytes
 	var nonce []byte                   // 24-byte XChaCha20 nonce
-	var keyHash []byte                 // SHA3-512 hash of encryption key
+	var keyHash []byte                 // HMAC-SHA3-512 of header
 	var keyHashRef []byte              // Same as 'keyHash', but used for comparison
 	var keyfileKey []byte              // The SHA3-256 hashes of keyfiles
 	var keyfileHash = make([]byte, 32) // The SHA3-256 of 'keyfileKey'
 	var keyfileHashRef []byte          // Same as 'keyfileHash', but used for comparison
 	var authTag []byte                 // 64-byte authentication tag (BLAKE2b or HMAC-SHA3)
 
+	// Header fields decoded (used for MAC verification)
+	var headerVersion []byte
+	var headerComments []byte
+	var headerCommentsLen int
+	var headerFlags []byte
+
 	var tempZipCipherW *chacha20.Cipher
 	var tempZipCipherR *chacha20.Cipher
 	var tempZipInUse bool = false
@@ -1637,17 +1643,17 @@ func work() {
 		_, errs[3] = fout.Write(rsEncode(rs5, flags))
 
 		// Fill values with Go's CSPRNG
-		if _, err := rand.Read(salt); err != nil {
-			panic(err)
+		if n, err := rand.Read(salt); err != nil || n != 16 {
+			panic(errors.New("failed to read 16 bytes from crypto/rand"))
 		}
-		if _, err := rand.Read(hkdfSalt); err != nil {
-			panic(err)
+		if n, err := rand.Read(hkdfSalt); err != nil || n != 32 {
+			panic(errors.New("failed to read 32 bytes from crypto/rand"))
 		}
-		if _, err := rand.Read(serpentIV); err != nil {
-			panic(err)
+		if n, err := rand.Read(serpentIV); err != nil || n != 16 {
+			panic(errors.New("failed to read 16 bytes from crypto/rand"))
 		}
-		if _, err := rand.Read(nonce); err != nil {
-			panic(err)
+		if n, err := rand.Read(nonce); err != nil || n != 24 {
+			panic(errors.New("failed to read 24 bytes from crypto/rand"))
 		}
 		if bytes.Equal(salt, make([]byte, 16)) {
 			panic(errors.New("fatal crypto/rand error"))
@@ -1690,9 +1696,9 @@ func work() {
 		// Stores any Reed-Solomon decoding errors
 		errs := make([]error, 10)
 
-		version := make([]byte, 15)
-		fin.Read(version)
-		_, errs[0] = rsDecode(rs5, version)
+		versionEnc := make([]byte, 15)
+		fin.Read(versionEnc)
+		headerVersion, errs[0] = rsDecode(rs5, versionEnc)
 
 		tmp := make([]byte, 15)
 		fin.Read(tmp)
@@ -1704,12 +1710,23 @@ func work() {
 		}
 
 		commentsLength, _ := strconv.Atoi(string(tmp))
-		fin.Read(make([]byte, commentsLength*3))
+		headerCommentsLen = commentsLength
+		headerComments = make([]byte, 0, commentsLength)
+		for i := 0; i < commentsLength; i++ {
+			cEnc := make([]byte, 3)
+			fin.Read(cEnc)
+			cDec, err := rsDecode(rs1, cEnc)
+			if err != nil {
+				errs[1] = err
+			}
+			headerComments = append(headerComments, cDec...)
+		}
 		total -= int64(commentsLength) * 3
 
 		flags := make([]byte, 15)
 		fin.Read(flags)
 		flags, errs[2] = rsDecode(rs5, flags)
+		headerFlags = flags
 		paranoid = flags[0] == 1
 		reedsolo = flags[3] == 1
 		padded = flags[4] == 1
@@ -1893,59 +1910,94 @@ func work() {
 	popupStatus = "Calculating values..."
 	giu.Update()
 
-	// Hash the encryption key for comparison when decrypting
-	tmp := sha3.New512()
-	if _, err := tmp.Write(key); err != nil {
-		panic(err)
-	}
-	keyHash = tmp.Sum(nil)
-
-	// Validate the password and/or keyfiles
-	if mode == "decrypt" {
-		keyCorrect := subtle.ConstantTimeCompare(keyHash, keyHashRef) == 1
-		keyfileCorrect := subtle.ConstantTimeCompare(keyfileHash, keyfileHashRef) == 1
-		incorrect := !keyCorrect
-		if keyfile || len(keyfiles) > 0 {
-			incorrect = !keyCorrect || !keyfileCorrect
+	// Compute or verify header MAC (HMAC-SHA3-512)
+	{
+		// Derive a subkey for the header MAC
+		subkeyHeader := make([]byte, 64)
+		hk := hkdf.New(sha3.New512, key, hkdfSalt, nil)
+		if n, err := hk.Read(subkeyHeader); err != nil || n != 64 {
+			panic(errors.New("fatal hkdf.Read error"))
 		}
 
-		// If something is incorrect
-		if incorrect {
-			if keep {
-				kept = true
-			} else {
-				if !keyCorrect {
-					mainStatus = "The provided password is incorrect"
+		macHeader := hmac.New(sha3.New512, subkeyHeader)
+
+		if mode == "encrypt" {
+			// Reconstruct flags
+			flagsHeader := make([]byte, 5)
+			if paranoid { flagsHeader[0] = 1 }
+			if len(keyfiles) > 0 { flagsHeader[1] = 1 }
+			if keyfileOrdered { flagsHeader[2] = 1 }
+			if reedsolo { flagsHeader[3] = 1 }
+			if total%int64(MiB) >= int64(MiB)-128 { flagsHeader[4] = 1 }
+
+			macHeader.Write([]byte(version))
+			macHeader.Write([]byte(fmt.Sprintf("%05d", len(comments))))
+			macHeader.Write([]byte(comments))
+			macHeader.Write(flagsHeader)
+			macHeader.Write(salt)
+			macHeader.Write(hkdfSalt)
+			macHeader.Write(serpentIV)
+			macHeader.Write(nonce)
+			macHeader.Write(keyfileHash)
+
+			keyHash = macHeader.Sum(nil)
+		} else { // decrypt
+			macHeader.Write(headerVersion)
+			macHeader.Write([]byte(fmt.Sprintf("%05d", headerCommentsLen)))
+			macHeader.Write(headerComments)
+			macHeader.Write(headerFlags)
+			macHeader.Write(salt)
+			macHeader.Write(hkdfSalt)
+			macHeader.Write(serpentIV)
+			macHeader.Write(nonce)
+			macHeader.Write(keyfileHash)
+
+			keyHash = macHeader.Sum(nil)
+
+			headerValid := subtle.ConstantTimeCompare(keyHash, keyHashRef) == 1
+			keyfileCorrect := subtle.ConstantTimeCompare(keyfileHash, keyfileHashRef) == 1
+			incorrect := !headerValid
+			if keyfile || len(keyfiles) > 0 {
+				incorrect = !headerValid || !keyfileCorrect
+			}
+
+			if incorrect {
+				if keep {
+					kept = true
 				} else {
-					if keyfileOrdered {
-						mainStatus = "Incorrect keyfiles or ordering"
+					if !headerValid {
+						mainStatus = "The password is incorrect or header is tampered"
 					} else {
-						mainStatus = "Incorrect keyfiles"
+						if keyfileOrdered {
+							mainStatus = "Incorrect keyfiles or ordering"
+						} else {
+							mainStatus = "Incorrect keyfiles"
+						}
+						if deniability {
+							fin.Close()
+							os.Remove(inputFile)
+							inputFile = strings.TrimSuffix(inputFile, ".tmp")
+						}
 					}
-					if deniability {
-						fin.Close()
-						os.Remove(inputFile)
-						inputFile = strings.TrimSuffix(inputFile, ".tmp")
+					broken(fin, nil, mainStatus, true)
+					if recombine {
+						inputFile = inputFileOld
 					}
+					return
 				}
-				broken(fin, nil, mainStatus, true)
+			}
+
+			// Create the output file for decryption (moved here after validation)
+			fout, err = os.Create(outputFile + ".incomplete")
+			if err != nil {
+				fin.Close()
 				if recombine {
-					inputFile = inputFileOld
+					os.Remove(inputFile)
 				}
+				accessDenied("Write")
 				return
 			}
 		}
-
-		// Create the output file for decryption
-		fout, err = os.Create(outputFile + ".incomplete")
-		if err != nil {
-			fin.Close()
-			if recombine {
-				os.Remove(inputFile)
-			}
-			accessDenied("Write")
-			return
-		}
 	}
 
 	if len(keyfiles) > 0 || keyfile {