[01/12] common/encrypt: separate data and inode nonces

Commit Message

Josef Bacik Oct. 10, 2023, 8:25 p.m. UTC

From: Sweet Tea Dorminy <[e-mail]>

btrfs will have different inode and data nonces, so we need to be
specific about which nonce each use needs. For now, there is no
difference in the two functions.

Signed-off-by: Sweet Tea Dorminy <[e-mail]>
---
 common/encrypt    | 33 ++++++++++++++++++++++++++-------
 tests/f2fs/002    |  2 +-
 tests/generic/613 |  4 ++--
 3 files changed, 29 insertions(+), 10 deletions(-)

Comments

Eric Biggers Oct. 17, 2023, 5:20 a.m. UTC | #1

On Tue, Oct 10, 2023 at 04:25:54PM -0400, Josef Bacik wrote:
> From: Sweet Tea Dorminy <[e-mail]>
> 
> btrfs will have different inode and data nonces, so we need to be
> specific about which nonce each use needs. For now, there is no
> difference in the two functions.
> 
> Signed-off-by: Sweet Tea Dorminy <[e-mail]>
> ---
>  common/encrypt    | 33 ++++++++++++++++++++++++++-------
>  tests/f2fs/002    |  2 +-
>  tests/generic/613 |  4 ++--
>  3 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/common/encrypt b/common/encrypt
> index 1a77e23b..04b6e5ac 100644
> --- a/common/encrypt
> +++ b/common/encrypt
> @@ -488,7 +488,7 @@ _add_fscrypt_provisioning_key()
>  # Retrieve the encryption nonce of the given inode as a hex string.  The nonce
>  # was randomly generated by the filesystem and isn't exposed directly to
>  # userspace.  But it can be read using the filesystem's debugging tools.
> -_get_encryption_nonce()
> +_get_encryption_file_nonce()
>  {
>  	local device=$1
>  	local inode=$2
> @@ -532,15 +532,34 @@ _get_encryption_nonce()
>  			}'
>  		;;
>  	*)
> -		_fail "_get_encryption_nonce() isn't implemented on $FSTYP"
> +		_fail "_get_encryption_file_nonce() isn't implemented on $FSTYP"
>  		;;
>  	esac
>  }
>  
> -# Require support for _get_encryption_nonce()
> +# Retrieve the encryption nonce used to encrypt the data of the given inode as
> +# a hex string.  The nonce was randomly generated by the filesystem and isn't
> +# exposed directly to userspace.  But it can be read using the filesystem's
> +# debugging tools.
> +_get_encryption_data_nonce()
> +{
> +	local device=$1
> +	local inode=$2
> +
> +	case $FSTYP in
> +	ext4|f2fs)
> +		_get_encryption_file_nonce $device $inode
> +		;;
> +	*)
> +		_fail "_get_encryption_data_nonce() isn't implemented on $FSTYP"
> +		;;
> +	esac
> +}

Shouldn't this be _get_encryption_extent_nonce(), taking the offset of the
extent as a parameter?

Also I think it would sound better as _get_extent_encryption_nonce(), and
likewise _get_file_encryption_nonce().

- Eric

Anand Jain Oct. 31, 2023, 2:13 p.m. UTC | #2

On 11/10/2023 04:25, Josef Bacik wrote:
> From: Sweet Tea Dorminy <[e-mail]>
> 
> btrfs will have different inode and data nonces, so we need to be
> specific about which nonce each use needs. For now, there is no
> difference in the two functions.
> 
> Signed-off-by: Sweet Tea Dorminy <[e-mail]>
> ---
Looks good.

Reviewed-by: Anand Jain <[e-mail]>

And, as Eric pointed out, the naming can be more intuitive. Keywords 
such as 'inode' and 'extent' will make them more intuitive, rather than 
'file' and 'data,' IMO.

Thanks, Anand

Patch

diff --git a/common/encrypt b/common/encrypt
index 1a77e23b..04b6e5ac 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -488,7 +488,7 @@  _add_fscrypt_provisioning_key()
 # Retrieve the encryption nonce of the given inode as a hex string.  The nonce
 # was randomly generated by the filesystem and isn't exposed directly to
 # userspace.  But it can be read using the filesystem's debugging tools.
-_get_encryption_nonce()
+_get_encryption_file_nonce()
 {
 	local device=$1
 	local inode=$2
@@ -532,15 +532,34 @@  _get_encryption_nonce()
 			}'
 		;;
 	*)
-		_fail "_get_encryption_nonce() isn't implemented on $FSTYP"
+		_fail "_get_encryption_file_nonce() isn't implemented on $FSTYP"
 		;;
 	esac
 }
 
-# Require support for _get_encryption_nonce()
+# Retrieve the encryption nonce used to encrypt the data of the given inode as
+# a hex string.  The nonce was randomly generated by the filesystem and isn't
+# exposed directly to userspace.  But it can be read using the filesystem's
+# debugging tools.
+_get_encryption_data_nonce()
+{
+	local device=$1
+	local inode=$2
+
+	case $FSTYP in
+	ext4|f2fs)
+		_get_encryption_file_nonce $device $inode
+		;;
+	*)
+		_fail "_get_encryption_data_nonce() isn't implemented on $FSTYP"
+		;;
+	esac
+}
+
+# Require support for _get_encryption_*nonce()
 _require_get_encryption_nonce_support()
 {
-	echo "Checking for _get_encryption_nonce() support for $FSTYP" >> $seqres.full
+	echo "Checking for _get_encryption_*nonce() support for $FSTYP" >> $seqres.full
 	case $FSTYP in
 	ext4)
 		_require_command "$DEBUGFS_PROG" debugfs
@@ -554,7 +573,7 @@  _require_get_encryption_nonce_support()
 		# the test fail in that case, as it was an f2fs-tools bug...
 		;;
 	*)
-		_notrun "_get_encryption_nonce() isn't implemented on $FSTYP"
+		_notrun "_get_encryption_*nonce() isn't implemented on $FSTYP"
 		;;
 	esac
 }
@@ -760,7 +779,7 @@  _do_verify_ciphertext_for_encryption_policy()
 	echo "Verifying encrypted file contents" >> $seqres.full
 	for f in "${test_contents_files[@]}"; do
 		read -r src inode blocklist <<< "$f"
-		nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
+		nonce=$(_get_encryption_data_nonce $SCRATCH_DEV $inode)
 		_dump_ciphertext_blocks $SCRATCH_DEV $blocklist > $tmp.actual_contents
 		$crypt_contents_cmd $contents_encryption_mode $raw_key_hex \
 			--file-nonce=$nonce --block-size=$blocksize \
@@ -780,7 +799,7 @@  _do_verify_ciphertext_for_encryption_policy()
 	echo "Verifying encrypted file names" >> $seqres.full
 	for f in "${test_filenames_files[@]}"; do
 		read -r name inode dir_inode padding <<< "$f"
-		nonce=$(_get_encryption_nonce $SCRATCH_DEV $dir_inode)
+		nonce=$(_get_encryption_file_nonce $SCRATCH_DEV $dir_inode)
 		_get_ciphertext_filename $SCRATCH_DEV $inode $dir_inode \
 			> $tmp.actual_name
 		echo -n "$name" | \
diff --git a/tests/f2fs/002 b/tests/f2fs/002
index 8235d88a..a51ddf22 100755
--- a/tests/f2fs/002
+++ b/tests/f2fs/002
@@ -129,7 +129,7 @@  blocklist=$(_get_ciphertext_block_list $file)
 _scratch_unmount
 
 echo -e "\n# Getting file's encryption nonce"
-nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
+nonce=$(_get_encryption_data_nonce $SCRATCH_DEV $inode)
 
 echo -e "\n# Dumping the file's raw data"
 _dump_ciphertext_blocks $SCRATCH_DEV $blocklist > $tmp.raw
diff --git a/tests/generic/613 b/tests/generic/613
index 4cf5ccc6..47c60e9c 100755
--- a/tests/generic/613
+++ b/tests/generic/613
@@ -68,10 +68,10 @@  echo -e "\n# Getting encryption nonces from inodes"
 echo -n > $tmp.nonces_hex
 echo -n > $tmp.nonces_bin
 for inode in "${inodes[@]}"; do
-	nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
+	nonce=$(_get_encryption_data_nonce $SCRATCH_DEV $inode)
 	if (( ${#nonce} != 32 )) || [ -n "$(echo "$nonce" | tr -d 0-9a-fA-F)" ]
 	then
-		_fail "Expected nonce to be 16 bytes (32 hex characters), but got \"$nonce\""
+		_fail "Expected nonce for inode $inode to be 16 bytes (32 hex characters), but got \"$nonce\""
 	fi
 	echo $nonce >> $tmp.nonces_hex
 	echo -ne "$(echo $nonce | sed 's/[0-9a-fA-F]\{2\}/\\x\0/g')" \