From eb70ba19e944cd765dfaccfff4f560b97c35383d Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Fri, 2 Jun 2023 13:27:01 +0000 Subject: [PATCH] EX-7331 sec: add support for encryption plus compression For compression efficiency, encryption must be carried out on write after data has been compressed. Otherwise, encrypted data would be almost incompressible. And on read, decryption must occur before data is decompressed. This means encryption is called on pages produced as a result of compression. However, for encryption to work, pages need to have a proper mapping and index. So we need to manually copy index and mapping from the original page cache pages, to the pages used to store compressed data. In case of Direct IO, we leverage information available from the cl_page. Add sanity-sec test_66 to exercise encryption+compression. Signed-off-by: Sebastien Buisson Change-Id: Id54d41365d5a21c54611b8e4af5059088ef87183 Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/51216 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Patrick Farrell --- lustre/osc/osc_compress.c | 28 ++++++++- lustre/osc/osc_request.c | 36 ++++++------ lustre/tests/sanity-sec.sh | 114 +++++++++++++++++++++++++++++++------ lustre/tests/test-framework.sh | 14 +++++ lustre/utils/liblustreapi_layout.c | 2 +- 5 files changed, 154 insertions(+), 40 deletions(-) diff --git a/lustre/osc/osc_compress.c b/lustre/osc/osc_compress.c index 1ee7ce6..3c62553 100644 --- a/lustre/osc/osc_compress.c +++ b/lustre/osc/osc_compress.c @@ -37,6 +37,11 @@ static bool tried_llz4_load = false; static bool tried_lgzip_load = false; +enum cpga_fill_bits { + CPGA_FILL_DIRECTIO = 0x0001, + CPGA_FILL_ENCRYPTED = 0x0002, +}; + static void merge_chunk(struct brw_page **pga, int first, int count, char *merged, unsigned int *size) { @@ -251,13 +256,15 @@ void free_cpga(struct brw_page **cpga, u32 page_count) } int fill_cpga(struct brw_page **cpga, struct brw_page **pga, - char *dst, int src_from, int dst_from, size_t dst_size) + char *dst, int src_from, int dst_from, size_t dst_size, + enum cpga_fill_bits fill_bits) { int chunk_offset; int dst_page; int src_page; struct brw_page *pg; struct osc_async_page *oap; + struct cl_page *clpage; for (chunk_offset = 0, dst_page = dst_from, src_page = src_from; chunk_offset < dst_size; @@ -288,6 +295,16 @@ int fill_cpga(struct brw_page **cpga, struct brw_page **pga, else pg->pg = pga[src_page]->pg; + if (fill_bits & CPGA_FILL_ENCRYPTED) { + if (fill_bits & CPGA_FILL_DIRECTIO) { + clpage = oap2cl_page( + brw_page2oap(pga[src_page])); + pg->pg->index = clpage->cp_page_index; + } else { + pg->pg->index = pga[src_page]->pg->index; + } + } + CDEBUG(D_SEC, "off 0x%llx, flag %x, pg %p, count %u\n", pg->off, pg->flag, pg->pg, pg->count); } @@ -313,6 +330,7 @@ int compress_request(const char *obd_name, struct obdo *oa, int done = 0; int rc = 0; int count = 0; + enum cpga_fill_bits fill_bits = 0; struct cl_page *clpage; unsigned int applied_type = LL_COMPR_TYPE_UNCHANGED; @@ -322,6 +340,11 @@ int compress_request(const char *obd_name, struct obdo *oa, chunk_size = (1 << (clpage->cp_chunk_log_bits + COMPR_CHUNK_MIN_BITS)); pages_in_chunk = chunk_size / PAGE_SIZE; + if (clpage->cp_type == CPT_TRANSIENT) + fill_bits |= CPGA_FILL_DIRECTIO; + if (clpage->cp_inode && IS_ENCRYPTED(clpage->cp_inode)) + fill_bits |= CPGA_FILL_ENCRYPTED; + OBD_ALLOC(*cpga, page_count * sizeof(**cpga)); sptlrpc_enc_pool_get_buf(&src, clpage->cp_chunk_log_bits + COMPR_CHUNK_MIN_BITS); @@ -371,7 +394,8 @@ int compress_request(const char *obd_name, struct obdo *oa, } rc = fill_cpga(*cpga, pga, done ? dst : NULL, - chunk_start, cpga_i, dst_size); + chunk_start, cpga_i, dst_size, + fill_bits); if (!done) { sptlrpc_enc_pool_put_buf(&dst, diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index 0a06ece..6ebda6e 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -1556,15 +1556,15 @@ osc_brw_prep_request(int cmd, struct client_obd *cli, struct obdo *oa, if (req == NULL) RETURN(-ENOMEM); - if (opc == OST_WRITE && compressed) { - if (inode && IS_ENCRYPTED(inode)) { - CWARN("%s: Encrypted, can not be compressed: "DFID"\n", - obd_name, oa->o_parent_seq, oa->o_parent_oid, - oa->o_parent_ver); - compressed = 0; - goto skip_compression; - } + if (opc == OST_WRITE && inode && IS_ENCRYPTED(inode) && + llcrypt_has_encryption_key(inode)) { + struct osc_async_page *oap = brw_page2oap(pga[page_count - 1]); + + oa->o_size = oap->oap_count + oap->oap_obj_off + + oap->oap_page_off; + } + if (opc == OST_WRITE && compressed) { /* * If compression disabled for the file -1 is set to * all pages, so it is enough to check only one @@ -1639,9 +1639,15 @@ retry_encrypt: * which means only once the page is fully processed. */ lockedbymyself = trylock_page(brwpg->pg); - if (directio) { + if (directio || compressed) { map_orig = brwpg->pg->mapping; brwpg->pg->mapping = inode->i_mapping; + } + /* Here, only change page index for the Direct IO case. + * For the compressed case, this is done in fill_cpga() + * when we still have access to original (brw)pages. + */ + if (directio && !compressed) { index_orig = brwpg->pg->index; clpage = oap2cl_page(brw_page2oap(brwpg)); brwpg->pg->index = clpage->cp_page_index; @@ -1651,10 +1657,10 @@ retry_encrypt: pa ? pa[i] : NULL, nunits, 0, GFP_NOFS); - if (directio) { + if (directio || compressed) brwpg->pg->mapping = map_orig; + if (directio && !compressed) brwpg->pg->index = index_orig; - } if (lockedbymyself) unlock_page(brwpg->pg); if (IS_ERR(data_page)) { @@ -1678,14 +1684,6 @@ retry_encrypt: */ SetPageChecked(data_page); brwpg->pg = data_page; - /* there should be no gap in the middle of page array */ - if (i == page_count - 1) { - struct osc_async_page *oap = - brw_page2oap(brwpg); - - oa->o_size = oap->oap_count + - oap->oap_obj_off + oap->oap_page_off; - } /* len is forced to nunits, and relative offset to 0 * so store the old, clear text info */ diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh index 0a6309c..3403bc4 100755 --- a/lustre/tests/sanity-sec.sh +++ b/lustre/tests/sanity-sec.sh @@ -4397,12 +4397,7 @@ test_54() { which fscrypt || skip "This test needs fscrypt userspace tool" - yes | fscrypt setup --force --verbose || - error "fscrypt global setup failed" - sed -i 's/\(.*\)policy_version\(.*\):\(.*\)\"[0-9]*\"\(.*\)/\1policy_version\2:\3"2"\4/' \ - /etc/fscrypt.conf - yes | fscrypt setup --verbose $MOUNT || - error "fscrypt setup $MOUNT failed" + fscrypt_setup $MOUNT 1 mkdir -p $testdir chown -R $ID0:$ID0 $testdir @@ -5318,12 +5313,7 @@ test_63() { which fscrypt || skip "This test needs fscrypt userspace tool" - yes | fscrypt setup --force --verbose || - echo "fscrypt global setup already done" - sed -i 's/\(.*\)policy_version\(.*\):\(.*\)\"[0-9]*\"\(.*\)/\1policy_version\2:\3"2"\4/' \ - /etc/fscrypt.conf - yes | fscrypt setup --verbose $MOUNT || - echo "fscrypt setup $MOUNT already done" + fscrypt_setup $MOUNT 0 # enable_filename_encryption tunable only available for client # built against embedded llcrypt. If client is built against in-kernel @@ -5754,12 +5744,7 @@ test_64f() { mkdir -p $DIR/$tdir || error "mkdir $DIR/$tdir failed" setup_64 - yes | fscrypt setup --force --verbose || - echo "fscrypt global setup already done" - sed -i 's/\(.*\)policy_version\(.*\):\(.*\)\"[0-9]*\"\(.*\)/\1policy_version\2:\3"2"\4/' \ - /etc/fscrypt.conf - yes | fscrypt setup --verbose $MOUNT || - echo "fscrypt setup $MOUNT already done" + fscrypt_setup $MOUNT 0 stack_trap "rm -rf $MOUNT/.fscrypt" # file_perms is required because fscrypt uses chmod/chown @@ -5926,6 +5911,99 @@ test_65() { } run_test 65 "lfs find -printf %La and --attrs support" +test_66() { + local p="$TMP/$TESTSUITE-$TESTNAME.parameters" + local pagesz=$(getconf PAGESIZE) + local vaultdir=$DIR/$tdir/vault + local tmpfile=$TMP/$tfile + local resfile=$TMP/resfile + + (( MDS1_VERSION >= $(version_code 2.14.0.101) )) || + skip "Need MDS >= 2.14.0.101 for compression support" + + cli_enc=$($LCTL get_param mdc.*.import | grep client_encryption) + [ -n "$cli_enc" ] || skip "Need enc support" + which fscrypt || skip "Need fscrypt" + + cp $LUSTRE/tests/sanity-sec.sh $tmpfile + mkdir -p $DIR/$tdir || error "mkdir $DIR/$tdir failed" + + fscrypt_setup $MOUNT 0 + stack_trap "rm -rf $MOUNT/.fscrypt" + + save_lustre_params client "llite.*.enable_compression" > $p + $LCTL set_param -n llite.*.enable_compression 1 + stack_trap "restore_lustre_params < $p; rm -f $p" EXIT + mkdir -p $vaultdir + stack_trap "rm -rf $vaultdir" + $LFS setstripe -E eof -Z lz4 --compress-chunk=64 $vaultdir || + error "cannot set $vaultdir compressed" + echo -e 'mypass\nmypass' | fscrypt encrypt --verbose \ + --source=custom_passphrase --name=protector_65 $vaultdir || + error "fscrypt encrypt $vaultdir failed" + + # simple file + echo test_65 > $vaultdir/file1 + echo test_65 > $TMP/file1 + # text file content, aligned on chunk size (64k) + dd if=$tmpfile of=$vaultdir/file2 bs=64k count=2 conv=fsync + dd if=$tmpfile of=$TMP/file2 bs=64k count=2 conv=fsync + # text file content, smaller than chunk size, not page aligned + dd if=$tmpfile of=$vaultdir/file3 bs=25k count=1 conv=fsync + dd if=$tmpfile of=$TMP/file3 bs=25k count=1 conv=fsync + # text file content, larger than chunk size, not page aligned + dd if=$tmpfile of=$vaultdir/file4 bs=75k count=1 conv=fsync + dd if=$tmpfile of=$TMP/file4 bs=75k count=1 conv=fsync + + fscrypt lock $vaultdir || error "fscrypt lock $vaultdir failed (1)" + cancel_lru_locks + echo mypass | fscrypt unlock $vaultdir || + error "fscrypt unlock $vaultdir failed (1)" + + # read by 4k blocks and populate page cache + for F in file{1..4}; do + dd if=$vaultdir/$F of=/dev/null bs=4k + cmp -bl $vaultdir/$F $TMP/$F || error "$F corrupted" + done + + # exercise DIO + if [ $OSTCOUNT -ge 2 ]; then + dd if=/dev/urandom of=$tmpfile bs=$pagesz count=1 conv=fsync + $LFS setstripe -S 256k -c2 $vaultdir/file5 + + # write at beginning of first stripe, buffered IO + dd if=$tmpfile of=$vaultdir/file5 bs=$pagesz count=1 \ + conv=fsync,notrunc + + # write at beginning of second stripe, direct IO + dd if=$tmpfile of=$vaultdir/file5 bs=$pagesz count=1 seek=256k \ + oflag=seek_bytes,direct conv=fsync,notrunc + + fscrypt lock $vaultdir || + error "fscrypt lock $vaultdir failed (2)" + cancel_lru_locks + echo mypass | fscrypt unlock $vaultdir || + error "fscrypt unlock $vaultdir failed (2)" + + # read at beginning of first stripe, direct IO + dd if=$vaultdir/file5 of=$resfile bs=$pagesz count=1 \ + iflag=direct conv=fsync + + cmp -bl $tmpfile $resfile || + error "file $vaultdir/file5 is corrupted (1)" + + # read at beginning of second stripe, buffered IO + dd if=$vaultdir/file5 of=$resfile bs=$pagesz count=1 skip=256k \ + iflag=skip_bytes conv=fsync + + cmp -bl $tmpfile $resfile || + error "file $vaultdir/file5 is corrupted (2)" + + rm -f $tmpfile $resfile + fi +} +run_test 66 "Encryption + compression" + log "cleanup: ======================================================" sec_unsetup() { diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh index 047581c..2513687 100755 --- a/lustre/tests/test-framework.sh +++ b/lustre/tests/test-framework.sh @@ -1254,6 +1254,20 @@ cleanup_sk() { fi } +fscrypt_setup() { + local mount=$1 + local exit_on_error=$2 + local action=echo + + (( exit_on_error == 0 )) || action=error + yes | fscrypt setup --force --verbose || + $action "fscrypt global setup failed" + sed -i 's/\(.*\)policy_version\(.*\):\(.*\)\"[0-9]*\"\(.*\)/\1policy_version\2:\3"2"\4/' \ + /etc/fscrypt.conf + yes | fscrypt setup --verbose $mount || + $action "fscrypt setup $MOUNT failed" +} + facet_svc() { local facet=$1 local var=${facet}_svc diff --git a/lustre/utils/liblustreapi_layout.c b/lustre/utils/liblustreapi_layout.c index e185787..68c9537 100644 --- a/lustre/utils/liblustreapi_layout.c +++ b/lustre/utils/liblustreapi_layout.c @@ -1593,7 +1593,7 @@ int llapi_layout_compress_set(struct llapi_layout *layout, if (getenv("LFS_SETSTRIPE_COMPR_OK") == NULL) { fprintf(stderr, - "WARNING: File compression is a technology preview feature and is not yet intended for production use. Compression is skipped for encrypted files.\n"); + "WARNING: File compression is a technology preview feature and is not yet intended for production use.\n"); } for (i = 0; i < ARRAY_SIZE(compr_type_table); i++) { -- 1.8.3.1