Skip to content

Commit

Permalink
pf_tamper fix (DEBUG)
Browse files Browse the repository at this point in the history
  • Loading branch information
@g2flyer
g2flyer committed Aug 22, 2024
1 parent 5d593d2 commit 39e2dc4
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 78 deletions.
20 changes: 15 additions & 5 deletions libos/test/fs/test_enc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ def setUpClass(cls):
os.mkdir(cls.ENCRYPTED_DIR)
cls.OUTPUT_DIR = os.path.join(cls.TEST_DIR, 'enc_output')
cls.OUTPUT_FILES = [os.path.join(cls.OUTPUT_DIR, str(x)) for x in cls.FILE_SIZES]
cls.DEC_OUTPUT_DIR = os.path.join(cls.TEST_DIR, 'dec_output')
if not os.path.exists(cls.DEC_OUTPUT_DIR):
os.mkdir(cls.DEC_OUTPUT_DIR)
# create encrypted files
cls.__set_default_key(cls)
for i in cls.INDEXES:
Expand Down Expand Up @@ -179,31 +182,38 @@ def test_500_invalid(self):
# prepare valid encrypted file (largest one for maximum possible corruptions)
original_input = self.OUTPUT_FILES[-1]
self.__encrypt_file(self.INPUT_FILES[-1], original_input)
shutil.copy(original_input, original_input+".save")
# generate invalid files based on the above
self.__corrupt_file(original_input, invalid_dir)

# try to decrypt invalid files
failed = False
for name in os.listdir(invalid_dir):
invalid = os.path.join(invalid_dir, name)
output_path = os.path.join(self.OUTPUT_DIR, name)
input_path = os.path.join(invalid_dir, os.path.basename(original_input))
output_path = os.path.join(self.DEC_OUTPUT_DIR, name)
input_path = original_input
# copy the file so it has the original file name (for allowed path check)
shutil.copy(invalid, input_path)

# test decryption using decryption utility
try:
args = ['decrypt', '-V', '-w', self.WRAP_KEY, '-i', input_path, '-o', output_path]
print(f"DDEBUG: pf_tamper args: %s" % args)
self.__pf_crypt(args)
except subprocess.CalledProcessError as exc:
# decryption of invalid file must fail with -1 (wrapped to 255)
self.assertEqual(exc.returncode, 255)
else:
print('[!] Fail: successfully decrypted file: ' + name)
self.fail()
failed = True

if failed:
self.fail()

# TODO: re-enable me eventually once above is fixed
# test decryption as part of reading file in program running with gramine
stdout, stderr = self.run_binary(['pf_tamper', input_path])
self.assertIn('ERROR: ', stdout)
# stdout, stderr = self.run_binary(['pf_tamper', input_path])
# self.assertIn('ERROR: ', stdout)
# TODO: check also that we updated map in trace/stderr?
# DEBUG: self.assertIn('truncate(' + path_1 + ') to ' + str(size_out) + ' OK', stdout)

Expand Down
5 changes: 3 additions & 2 deletions libos/test/fs/test_fs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ def setUpClass(cls):
file.write(os.urandom(cls.FILE_SIZES[i]))

@classmethod
def tearDownClass(cls):
shutil.rmtree(cls.TEST_DIR)
# DEBUG
# def tearDownClass(cls):
# shutil.rmtree(cls.TEST_DIR)

def setUp(self):
# clean output for each test
Expand Down
145 changes: 74 additions & 71 deletions tools/sgx/pf_tamper/pf_tamper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ static void tamper_truncate(void) {
/* extend */
truncate_file("extend_0", g_input_size + 1);
truncate_file("extend_1", g_input_size + PF_NODE_SIZE / 2);
// TODO: tampering below goes undetected during decryption!!
truncate_file("extend_2", g_input_size + PF_NODE_SIZE);
truncate_file("extend_3", g_input_size + PF_NODE_SIZE + PF_NODE_SIZE / 2);
}
Expand Down Expand Up @@ -271,23 +272,36 @@ static void pf_encrypt(const void* decrypted, size_t size, const pf_key_t* key,
} while (0)

/* if update is true, also create a file with correct metadata MAC */
#define BREAK_PF(suffix, update, ...) do { \
__BREAK_PF(suffix, __VA_ARGS__); \
if (update) { \
__BREAK_PF(suffix "_fixed", __VA_ARGS__ { \
pf_encrypt(meta_dec, sizeof(*meta_dec), &g_meta_key, \
&meta->plaintext_part.metadata_mac, meta->encrypted_part, \
"metadata"); \
} ); \
} \
} while (0)

#define BREAK_MHT(suffix, ...) do { \
__BREAK_PF(suffix, __VA_ARGS__ { \
pf_encrypt(mht_dec, sizeof(*mht_dec), &meta_dec->root_mht_node_key, \
&meta_dec->root_mht_node_mac, mht_enc, "mht"); \
} ); \
} while (0)
#define BREAK_PLN(suffix, update, ...) \
do { \
__BREAK_PF(suffix, __VA_ARGS__); \
if (update) { \
__BREAK_PF( \
suffix "_fixed", __VA_ARGS__ { \
pf_encrypt(meta_dec, sizeof(*meta_dec), &g_meta_key, \
&meta->plaintext_part.metadata_mac, meta->encrypted_part, \
"metadata"); \
}); \
} \
} while (0)

#define BREAK_DEC(suffix, ...) \
do { \
__BREAK_PF( \
suffix, __VA_ARGS__ { \
pf_encrypt(meta_dec, sizeof(*meta_dec), &g_meta_key, \
&meta->plaintext_part.metadata_mac, meta->encrypted_part, "metadata"); \
}); \
} while (0)

#define BREAK_MHT(suffix, ...) \
do { \
__BREAK_PF( \
suffix, __VA_ARGS__ { \
pf_encrypt(mht_dec, sizeof(*mht_dec), &meta_dec->root_mht_node_key, \
&meta_dec->root_mht_node_mac, mht_enc, "mht"); \
}); \
} while (0)

#define LAST_BYTE(array) (((uint8_t*)&array)[sizeof(array) - 1])

Expand All @@ -303,60 +317,51 @@ static void tamper_modify(void) {
FATAL("Out of memory\n");

/* plain part of the metadata isn't covered by the MAC so no point updating it */
BREAK_PF("meta_plain_id_0", /*update=*/false,
{ meta->plaintext_part.file_id = 0; });
BREAK_PF("meta_plain_id_1", /*update=*/false,
{ meta->plaintext_part.file_id = UINT64_MAX; });
BREAK_PF("meta_plain_version_0", /*update=*/false,
{ meta->plaintext_part.major_version = 0; });
BREAK_PF("meta_plain_version_1", /*update=*/false,
{ meta->plaintext_part.major_version = 0xff; });
BREAK_PF("meta_plain_version_2", /*update=*/false,
{ meta->plaintext_part.minor_version = 0xff; });
BREAK_PLN("meta_plain_id_0", /*update=*/false, { meta->plaintext_part.file_id = 0; });
BREAK_PLN("meta_plain_id_1", /*update=*/false, { meta->plaintext_part.file_id = UINT64_MAX; });
BREAK_PLN("meta_plain_version_0", /*update=*/false,
{ meta->plaintext_part.major_version = 0; });
BREAK_PLN("meta_plain_version_1", /*update=*/false,
{ meta->plaintext_part.major_version = 0xff; });
// TODO: tampering below goes undetected during decryption!!
// -> currently minor_version is not looked at all during processing of files!!
BREAK_PLN("meta_plain_version_2", /*update=*/false,
{ meta->plaintext_part.minor_version = 0xff; });

/* metadata_key_nonce is the keying material for encrypted metadata key derivation, so create
* also PFs with updated MACs */
BREAK_PF("meta_plain_nonce_0", /*update=*/true,
{ meta->plaintext_part.metadata_key_nonce[0] ^= 1; });
BREAK_PF("meta_plain_nonce_1", /*update=*/true,
{ LAST_BYTE(meta->plaintext_part.metadata_key_nonce) ^= 0xfe; });
BREAK_PF("meta_plain_mac_0", /*update=*/true,
{ meta->plaintext_part.metadata_mac[0] ^= 0xfe; });
BREAK_PF("meta_plain_mac_1", /*update=*/true,
{ LAST_BYTE(meta->plaintext_part.metadata_mac) &= 1; });

BREAK_PF("meta_enc_filename_0", /*update=*/true,
{ meta_dec->file_path[0] = 0; });
BREAK_PF("meta_enc_filename_1", /*update=*/true,
{ meta_dec->file_path[0] ^= 1; });
BREAK_PF("meta_enc_filename_2", /*update=*/true,
{ LAST_BYTE(meta_dec->file_path) ^= 0xfe; });
BREAK_PF("meta_enc_size_0", /*update=*/true,
{ meta_dec->file_size = 0; });
BREAK_PF("meta_enc_size_1", /*update=*/true,
{ meta_dec->file_size = g_input_size - 1; });
BREAK_PF("meta_enc_size_2", /*update=*/true,
{ meta_dec->file_size = g_input_size + 1; });
BREAK_PF("meta_enc_size_3", /*update=*/true,
{ meta_dec->file_size = UINT64_MAX; });
BREAK_PF("meta_enc_mht_key_0", /*update=*/true,
{ meta_dec->root_mht_node_key[0] ^= 1; });
BREAK_PF("meta_enc_mht_key_1", /*update=*/true,
{ LAST_BYTE(meta_dec->root_mht_node_key) ^= 0xfe; });
BREAK_PF("meta_enc_mht_mac_0", /*update=*/true,
{ meta_dec->root_mht_node_mac[0] ^= 1; });
BREAK_PF("meta_enc_mht_mac_1", /*update=*/true,
{ LAST_BYTE(meta_dec->root_mht_node_mac) ^= 0xfe; });
BREAK_PF("meta_enc_data_0", /*update=*/true,
{ meta_dec->file_data[0] ^= 0xfe; });
BREAK_PF("meta_enc_data_1", /*update=*/true,
{ LAST_BYTE(meta_dec->file_data) ^= 1; });
BREAK_PLN("meta_plain_nonce_0", /*update=*/true,
{ meta->plaintext_part.metadata_key_nonce[0] ^= 1; });
BREAK_PLN("meta_plain_nonce_1", /*update=*/true,
{ LAST_BYTE(meta->plaintext_part.metadata_key_nonce) ^= 0xfe; });
BREAK_PLN("meta_plain_mac_0", /*update=*/false, // update would overwrite the tampering
{ meta->plaintext_part.metadata_mac[0] ^= 0xfe; });
BREAK_PLN("meta_plain_mac_1", /*update=*/false, // update would overwrite the tampering
{ LAST_BYTE(meta->plaintext_part.metadata_mac) ^= 1; });

BREAK_DEC("meta_enc_filename_0", { meta_dec->file_path[0] = 0; });
BREAK_DEC("meta_enc_filename_1", { meta_dec->file_path[0] ^= 1; });
// TODO: tampering below goes undetected during decryption!!
BREAK_DEC("meta_enc_filename_2", { LAST_BYTE(meta_dec->file_path) ^= 0xfe; });
// TODO: tampering below goes undetected during decryption!!
BREAK_DEC("meta_enc_size_0", { meta_dec->file_size = 0; });
BREAK_DEC("meta_enc_size_1", { meta_dec->file_size = g_input_size - 1; });
BREAK_DEC("meta_enc_size_2", { meta_dec->file_size = g_input_size + 1; });
BREAK_DEC("meta_enc_size_3", { meta_dec->file_size = UINT64_MAX; });
BREAK_DEC("meta_enc_mht_key_0", { meta_dec->root_mht_node_key[0] ^= 1; });
BREAK_DEC("meta_enc_mht_key_1", { LAST_BYTE(meta_dec->root_mht_node_key) ^= 0xfe; });
BREAK_DEC("meta_enc_mht_mac_0", { meta_dec->root_mht_node_mac[0] ^= 1; });
BREAK_DEC("meta_enc_mht_mac_1", { LAST_BYTE(meta_dec->root_mht_node_mac) ^= 0xfe; });
// TODO: tampering in below two goes undetected during decryption.
// However, as we re-encrypt the data it is properly encrypted just undetectably different from
// the original version?! I.e., below two cases do not make sense and should be removed?!
BREAK_DEC("meta_enc_data_0", { meta_dec->file_data[0] ^= 0xfe; });
BREAK_DEC("meta_enc_data_1", { LAST_BYTE(meta_dec->file_data) ^= 1; });

/* padding is ignored */
BREAK_PF("meta_padding_0", /*update=*/false,
{ meta->padding[0] ^= 1; });
BREAK_PF("meta_padding_1", /*update=*/false,
{ LAST_BYTE(meta->padding) ^= 0xfe; });
// TODO: remove as as they are ignored decryption always works?
BREAK_DEC("meta_padding_0", { meta->padding[0] ^= 1; });
BREAK_DEC("meta_padding_1", { LAST_BYTE(meta->padding) ^= 0xfe; });

BREAK_MHT("mht_0", { mht_dec->data_nodes_crypto[0].key[0] ^= 1; });
BREAK_MHT("mht_1", { mht_dec->data_nodes_crypto[0].mac[0] ^= 1; });
Expand All @@ -380,11 +385,9 @@ static void tamper_modify(void) {
});

/* data nodes start from node #2 */
BREAK_PF("data_0", /*update=*/false,
{ *(out + 2 * PF_NODE_SIZE) ^= 1; });
BREAK_PF("data_1", /*update=*/false,
{ *(out + 3 * PF_NODE_SIZE - 1) ^= 1; });
BREAK_PF("data_2", /*update=*/false, {
BREAK_PLN("data_0", /*update=*/false, { *(out + 2 * PF_NODE_SIZE) ^= 1; });
BREAK_PLN("data_1", /*update=*/false, { *(out + 3 * PF_NODE_SIZE - 1) ^= 1; });
BREAK_PLN("data_2", /*update=*/false, {
/* swap data nodes */
memcpy(out + 2 * PF_NODE_SIZE, g_input_data + 3 * PF_NODE_SIZE, PF_NODE_SIZE);
memcpy(out + 3 * PF_NODE_SIZE, g_input_data + 2 * PF_NODE_SIZE, PF_NODE_SIZE);
Expand Down

0 comments on commit 39e2dc4

Please sign in to comment.