Skip to contentSkip to conversation

ecryptfs: split packet parser length fixes

PATCH In review Yichong Chen 2 participants 5 messages Jul 15 – Jul 21 Ecrypt file system

lore
Share & tools
lore

Yichong Chen

author
#0 lore raw

v2 cover ecryptfs: split packet parser length fixes

This v2 splits the packet parser hardening into three separate fixes as requested by Tyler Hicks:

1. pass the actual packet set buffer size into ecryptfs_parse_packet_set()
2. fix the tag 11 exact-fit size check
3. reject too-small tag 70 packet bodies before size subtraction

Changes in v2:
- Split the original patch into three independent fixes.
- Dropped the unrelated trailing blank line removal in keystore.c.

Yichong Chen (3):
ecryptfs: pass packet set buffer size to parser
ecryptfs: fix tag 11 packet exact-fit size check
ecryptfs: reject too-small tag 70 packets

 fs/ecryptfs/crypto.c          |  2 +-
 fs/ecryptfs/ecryptfs_kernel.h |  3 ++-
 fs/ecryptfs/keystore.c        | 31 +++++++++++++++++++++++++++----
 3 files changed, 30 insertions(+), 6 deletions(-)
Signature · 1 line
2.51.0
author
#1 lore raw

PATCH v2 1/3 ecryptfs: pass packet set buffer size to parser

ecryptfs_parse_packet_set() receives a pointer into the file header, but it calculates the remaining packet buffer size from PAGE_SIZE - 8. For version 1 headers the packet set starts later in the header, so this can overstate the available buffer.

Pass the actual packet set buffer length from the caller and calculate per-packet limits from the remaining bytes in that buffer. Recompute the remaining length after consuming a tag 3 packet before parsing the following tag 11 packet.

Fixes 237fead61998 ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
Signed-off-by Yichong Chen <[email protected]>
---
 fs/ecryptfs/crypto.c          |  2 +-
 fs/ecryptfs/ecryptfs_kernel.h |  3 ++-
 fs/ecryptfs/keystore.c        | 23 ++++++++++++++++++++---
 3 files changed, 23 insertions(+), 5 deletions(-)
fs/ecryptfs/crypto.c +1 -1
index 74b02b5..e67119b 100644
@@ -1197,7 +1197,7 @@ static int ecryptfs_read_headers_virt(char *page_virt,
 	} else
 		set_default_header_data(crypt_stat);
 	rc = ecryptfs_parse_packet_set(crypt_stat, (page_virt + offset),
-				       ecryptfs_dentry);
+				       PAGE_SIZE - offset, ecryptfs_dentry);
 out:
 	return rc;
 }
fs/ecryptfs/ecryptfs_kernel.h +2 -1
index f4f56a9..7d2488a 100644
@@ -580,7 +580,8 @@ int ecryptfs_generate_key_packet_set(char *dest_base,
 				     size_t *len, size_t max);
 int
 ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
-			  unsigned char *src, struct dentry *ecryptfs_dentry);
+			  unsigned char *src, size_t src_size,
+			  struct dentry *ecryptfs_dentry);
 int ecryptfs_truncate(struct dentry *dentry, loff_t new_length);
 ssize_t
 ecryptfs_getxattr_lower(struct dentry *lower_dentry, struct inode *lower_inode,
fs/ecryptfs/keystore.c +20 -3
index ebebc95..90c2b80 100644
@@ -1704,6 +1704,7 @@ out:
  * ecryptfs_parse_packet_set
  * @crypt_stat: The cryptographic context
  * @src: Virtual address of region of memory containing the packets
+ * @src_size: Size of the packet set buffer
  * @ecryptfs_dentry: The eCryptfs dentry associated with the packet set
  *
  * Get crypt_stat to have the file's session key if the requisite key

@@ -1714,7 +1715,7 @@ out:
  * conditions.
  */
 int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
-			      unsigned char *src,
+			      unsigned char *src, size_t src_size,
 			      struct dentry *ecryptfs_dentry)
 {
 	size_t i = 0;

@@ -1736,7 +1737,11 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
 	 * added the our &auth_tok_list */
 	next_packet_is_auth_tok_packet = 1;
 	while (next_packet_is_auth_tok_packet) {
-		size_t max_packet_size = ((PAGE_SIZE - 8) - i);
+		size_t max_packet_size;
+
+		if (i >= src_size)
+			break;
+		max_packet_size = src_size - i;
 
 		switch (src[i]) {
 		case ECRYPTFS_TAG_3_PACKET_TYPE:

@@ -1751,12 +1756,16 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
 				goto out_wipe_list;
 			}
 			i += packet_size;
+			if (i > src_size) {
+				rc = -EIO;
+				goto out_wipe_list;
+			}
 			rc = parse_tag_11_packet((unsigned char *)&src[i],
 						 sig_tmp_space,
 						 ECRYPTFS_SIG_SIZE,
 						 &tag_11_contents_size,
 						 &tag_11_packet_size,
-						 max_packet_size);
+						 src_size - i);
 			if (rc) {
 				ecryptfs_printk(KERN_ERR, "No valid "
 						"(ecryptfs-specific) literal "

@@ -1768,6 +1777,10 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
 				goto out_wipe_list;
 			}
 			i += tag_11_packet_size;
+			if (i > src_size) {
+				rc = -EIO;
+				goto out_wipe_list;
+			}
 			if (ECRYPTFS_SIG_SIZE != tag_11_contents_size) {
 				ecryptfs_printk(KERN_ERR, "Expected "
 						"signature of size [%d]; "

@@ -1793,6 +1806,10 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
 				goto out_wipe_list;
 			}
 			i += packet_size;
+			if (i > src_size) {
+				rc = -EIO;
+				goto out_wipe_list;
+			}
 			crypt_stat->flags |= ECRYPTFS_ENCRYPTED;
 			break;
 		case ECRYPTFS_TAG_11_PACKET_TYPE:
Signature · 1 line
2.51.0
author
#2 lore raw

PATCH v2 2/3 ecryptfs: fix tag 11 packet exact-fit size check

In reply to Yichong Chen

parse_tag_11_packet() rejects a packet when the already-consumed tag and length bytes plus the packet body exceed the caller supplied maximum packet size. The check currently adds one extra byte, even though *packet_size already includes the tag byte before the length is parsed.

Remove the extra byte so a tag 11 packet that exactly fits the available buffer is accepted while oversized packets are still rejected.

Fixes 237fead61998 ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
Signed-off-by Yichong Chen <[email protected]>
---
 fs/ecryptfs/keystore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ecryptfs/keystore.c +1 -1
index 90c2b80..02b5d77 100644
@@ -1537,7 +1537,7 @@ parse_tag_11_packet(unsigned char *data, unsigned char *contents,
 	}
 	(*packet_size) += length_size;
 	(*tag_11_contents_size) = (body_size - 14);
-	if (unlikely((*packet_size) + body_size + 1 > max_packet_size)) {
+	if (unlikely((*packet_size) + body_size > max_packet_size)) {
 		printk(KERN_ERR "Packet size exceeds max\n");
 		rc = -EINVAL;
 		goto out;
Signature · 1 line
2.51.0
author
#3 lore raw

PATCH v2 3/3 ecryptfs: reject too-small tag 70 packets

In reply to Yichong Chen

ecryptfs_parse_tag_70_packet() subtracts fixed metadata fields from the parsed packet body size to derive the encrypted filename size. A malformed packet with a body smaller than those fixed fields can underflow that size calculation.

Reject tag 70 packets before the subtraction unless the body contains the signature, cipher code, and at least one byte of encrypted filename data.

Fixes 9c79f34f7ee7 ("eCryptfs: Filename Encryption: Tag 70 packets")
Signed-off-by Yichong Chen <[email protected]>
---
 fs/ecryptfs/keystore.c | 6 ++++++
 1 file changed, 6 insertions(+)
fs/ecryptfs/keystore.c +6 -0
index 02b5d77..a525a8c 100644
@@ -894,6 +894,12 @@ ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size,
 		       "rc = [%d]\n", __func__, rc);
 		goto out;
 	}
+	if (s->parsed_tag_70_packet_size < (ECRYPTFS_SIG_SIZE + 2)) {
+		ecryptfs_printk(KERN_WARNING, "Invalid packet size [%zd]\n",
+				s->parsed_tag_70_packet_size);
+		rc = -EINVAL;
+		goto out;
+	}
 	s->block_aligned_filename_size = (s->parsed_tag_70_packet_size
 					  - ECRYPTFS_SIG_SIZE - 1);
 	if ((1 + s->packet_size_len + s->parsed_tag_70_packet_size)
Signature · 1 line
2.51.0
maintainer
#4 lore raw
In reply to Yichong Chen
Quoted 8 lines · Yichong Chen

This v2 splits the packet parser hardening into three separate fixes as requested by Tyler Hicks:

1. pass the actual packet set buffer size into ecryptfs_parse_packet_set()
2. fix the tag 11 exact-fit size check
3. reject too-small tag 70 packet bodies before size subtraction

[...]

Thank you! They've been applied to the next branch of the tyhicks/ecryptfs.git tree.

You can find a direct link below but please be aware that the commit hash is unstable and, therefore, the URL may not be valid in the future.

[1/3] ecryptfs: pass packet set buffer size to parser

      https://git.kernel.org/tyhicks/ecryptfs/c/2602b79c5b3e2f6fce12e38a670f8e3fda4e46a2
[2/3] ecryptfs: fix tag 11 packet exact-fit size check
      https://git.kernel.org/tyhicks/ecryptfs/c/8b2ec0f56f55477f547d332526c9ae2a8fabc0a5
[3/3] ecryptfs: reject too-small tag 70 packets
      https://git.kernel.org/tyhicks/ecryptfs/c/e97bbe1b2bd82ec2ae37ad2e4965b4d3e78bbf7f

Tyler