Skip to content

Commit d978358

Browse files
g2flyerg2flyer
committed
pf_tamper fix (DEBUG)
1 parent dc453a6 commit d978358

File tree

3 files changed

+92
-78
lines changed

3 files changed

+92
-78
lines changed

‎libos/test/fs/test_enc.py

+15-5
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ def setUpClass(cls):
3333
os.mkdir(cls.ENCRYPTED_DIR)
3434
cls.OUTPUT_DIR = os.path.join(cls.TEST_DIR, 'enc_output')
3535
cls.OUTPUT_FILES = [os.path.join(cls.OUTPUT_DIR, str(x)) for x in cls.FILE_SIZES]
36+
cls.DEC_OUTPUT_DIR = os.path.join(cls.TEST_DIR, 'dec_output')
37+
if not os.path.exists(cls.DEC_OUTPUT_DIR):
38+
os.mkdir(cls.DEC_OUTPUT_DIR)
3639
# create encrypted files
3740
cls.__set_default_key(cls)
3841
for i in cls.INDEXES:
@@ -179,31 +182,38 @@ def test_500_invalid(self):
179182
# prepare valid encrypted file (largest one for maximum possible corruptions)
180183
original_input = self.OUTPUT_FILES[-1]
181184
self.__encrypt_file(self.INPUT_FILES[-1], original_input)
185+
shutil.copy(original_input, original_input+".save")
182186
# generate invalid files based on the above
183187
self.__corrupt_file(original_input, invalid_dir)
184188

185189
# try to decrypt invalid files
190+
failed = False
186191
for name in os.listdir(invalid_dir):
187192
invalid = os.path.join(invalid_dir, name)
188-
output_path = os.path.join(self.OUTPUT_DIR, name)
189-
input_path = os.path.join(invalid_dir, os.path.basename(original_input))
193+
output_path = os.path.join(self.DEC_OUTPUT_DIR, name)
194+
input_path = original_input
190195
# copy the file so it has the original file name (for allowed path check)
191196
shutil.copy(invalid, input_path)
192197

193198
# test decryption using decryption utility
194199
try:
195200
args = ['decrypt', '-V', '-w', self.WRAP_KEY, '-i', input_path, '-o', output_path]
201+
print(f"DDEBUG: pf_tamper args: %s" % args)
196202
self.__pf_crypt(args)
197203
except subprocess.CalledProcessError as exc:
198204
# decryption of invalid file must fail with -1 (wrapped to 255)
199205
self.assertEqual(exc.returncode, 255)
200206
else:
201207
print('[!] Fail: successfully decrypted file: ' + name)
202-
self.fail()
208+
failed = True
209+
210+
if failed:
211+
self.fail()
203212

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

‎libos/test/fs/test_fs.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ def setUpClass(cls):
3030
file.write(os.urandom(cls.FILE_SIZES[i]))
3131

3232
@classmethod
33-
def tearDownClass(cls):
34-
shutil.rmtree(cls.TEST_DIR)
33+
# DEBUG
34+
# def tearDownClass(cls):
35+
# shutil.rmtree(cls.TEST_DIR)
3536

3637
def setUp(self):
3738
# clean output for each test

‎tools/sgx/pf_tamper/pf_tamper.c

+74-71
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ static void tamper_truncate(void) {
214214
/* extend */
215215
truncate_file("extend_0", g_input_size + 1);
216216
truncate_file("extend_1", g_input_size + PF_NODE_SIZE / 2);
217+
// TODO: tampering below goes undetected during decryption!!
217218
truncate_file("extend_2", g_input_size + PF_NODE_SIZE);
218219
truncate_file("extend_3", g_input_size + PF_NODE_SIZE + PF_NODE_SIZE / 2);
219220
}
@@ -271,23 +272,36 @@ static void pf_encrypt(const void* decrypted, size_t size, const pf_key_t* key,
271272
} while (0)
272273

273274
/* if update is true, also create a file with correct metadata MAC */
274-
#define BREAK_PF(suffix, update, ...) do { \
275-
__BREAK_PF(suffix, __VA_ARGS__); \
276-
if (update) { \
277-
__BREAK_PF(suffix "_fixed", __VA_ARGS__ { \
278-
pf_encrypt(meta_dec, sizeof(*meta_dec), &g_meta_key, \
279-
&meta->plaintext_part.metadata_mac, meta->encrypted_part, \
280-
"metadata"); \
281-
} ); \
282-
} \
283-
} while (0)
284-
285-
#define BREAK_MHT(suffix, ...) do { \
286-
__BREAK_PF(suffix, __VA_ARGS__ { \
287-
pf_encrypt(mht_dec, sizeof(*mht_dec), &meta_dec->root_mht_node_key, \
288-
&meta_dec->root_mht_node_mac, mht_enc, "mht"); \
289-
} ); \
290-
} while (0)
275+
#define BREAK_PLN(suffix, update, ...) \
276+
do { \
277+
__BREAK_PF(suffix, __VA_ARGS__); \
278+
if (update) { \
279+
__BREAK_PF( \
280+
suffix "_fixed", __VA_ARGS__ { \
281+
pf_encrypt(meta_dec, sizeof(*meta_dec), &g_meta_key, \
282+
&meta->plaintext_part.metadata_mac, meta->encrypted_part, \
283+
"metadata"); \
284+
}); \
285+
} \
286+
} while (0)
287+
288+
#define BREAK_DEC(suffix, ...) \
289+
do { \
290+
__BREAK_PF( \
291+
suffix, __VA_ARGS__ { \
292+
pf_encrypt(meta_dec, sizeof(*meta_dec), &g_meta_key, \
293+
&meta->plaintext_part.metadata_mac, meta->encrypted_part, "metadata"); \
294+
}); \
295+
} while (0)
296+
297+
#define BREAK_MHT(suffix, ...) \
298+
do { \
299+
__BREAK_PF( \
300+
suffix, __VA_ARGS__ { \
301+
pf_encrypt(mht_dec, sizeof(*mht_dec), &meta_dec->root_mht_node_key, \
302+
&meta_dec->root_mht_node_mac, mht_enc, "mht"); \
303+
}); \
304+
} while (0)
291305

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

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

305319
/* plain part of the metadata isn't covered by the MAC so no point updating it */
306-
BREAK_PF("meta_plain_id_0", /*update=*/false,
307-
{ meta->plaintext_part.file_id = 0; });
308-
BREAK_PF("meta_plain_id_1", /*update=*/false,
309-
{ meta->plaintext_part.file_id = UINT64_MAX; });
310-
BREAK_PF("meta_plain_version_0", /*update=*/false,
311-
{ meta->plaintext_part.major_version = 0; });
312-
BREAK_PF("meta_plain_version_1", /*update=*/false,
313-
{ meta->plaintext_part.major_version = 0xff; });
314-
BREAK_PF("meta_plain_version_2", /*update=*/false,
315-
{ meta->plaintext_part.minor_version = 0xff; });
320+
BREAK_PLN("meta_plain_id_0", /*update=*/false, { meta->plaintext_part.file_id = 0; });
321+
BREAK_PLN("meta_plain_id_1", /*update=*/false, { meta->plaintext_part.file_id = UINT64_MAX; });
322+
BREAK_PLN("meta_plain_version_0", /*update=*/false,
323+
{ meta->plaintext_part.major_version = 0; });
324+
BREAK_PLN("meta_plain_version_1", /*update=*/false,
325+
{ meta->plaintext_part.major_version = 0xff; });
326+
// TODO: tampering below goes undetected during decryption!!
327+
// -> currently minor_version is not looked at all during processing of files!!
328+
BREAK_PLN("meta_plain_version_2", /*update=*/false,
329+
{ meta->plaintext_part.minor_version = 0xff; });
316330

317331
/* metadata_key_nonce is the keying material for encrypted metadata key derivation, so create
318332
* also PFs with updated MACs */
319-
BREAK_PF("meta_plain_nonce_0", /*update=*/true,
320-
{ meta->plaintext_part.metadata_key_nonce[0] ^= 1; });
321-
BREAK_PF("meta_plain_nonce_1", /*update=*/true,
322-
{ LAST_BYTE(meta->plaintext_part.metadata_key_nonce) ^= 0xfe; });
323-
BREAK_PF("meta_plain_mac_0", /*update=*/true,
324-
{ meta->plaintext_part.metadata_mac[0] ^= 0xfe; });
325-
BREAK_PF("meta_plain_mac_1", /*update=*/true,
326-
{ LAST_BYTE(meta->plaintext_part.metadata_mac) &= 1; });
327-
328-
BREAK_PF("meta_enc_filename_0", /*update=*/true,
329-
{ meta_dec->file_path[0] = 0; });
330-
BREAK_PF("meta_enc_filename_1", /*update=*/true,
331-
{ meta_dec->file_path[0] ^= 1; });
332-
BREAK_PF("meta_enc_filename_2", /*update=*/true,
333-
{ LAST_BYTE(meta_dec->file_path) ^= 0xfe; });
334-
BREAK_PF("meta_enc_size_0", /*update=*/true,
335-
{ meta_dec->file_size = 0; });
336-
BREAK_PF("meta_enc_size_1", /*update=*/true,
337-
{ meta_dec->file_size = g_input_size - 1; });
338-
BREAK_PF("meta_enc_size_2", /*update=*/true,
339-
{ meta_dec->file_size = g_input_size + 1; });
340-
BREAK_PF("meta_enc_size_3", /*update=*/true,
341-
{ meta_dec->file_size = UINT64_MAX; });
342-
BREAK_PF("meta_enc_mht_key_0", /*update=*/true,
343-
{ meta_dec->root_mht_node_key[0] ^= 1; });
344-
BREAK_PF("meta_enc_mht_key_1", /*update=*/true,
345-
{ LAST_BYTE(meta_dec->root_mht_node_key) ^= 0xfe; });
346-
BREAK_PF("meta_enc_mht_mac_0", /*update=*/true,
347-
{ meta_dec->root_mht_node_mac[0] ^= 1; });
348-
BREAK_PF("meta_enc_mht_mac_1", /*update=*/true,
349-
{ LAST_BYTE(meta_dec->root_mht_node_mac) ^= 0xfe; });
350-
BREAK_PF("meta_enc_data_0", /*update=*/true,
351-
{ meta_dec->file_data[0] ^= 0xfe; });
352-
BREAK_PF("meta_enc_data_1", /*update=*/true,
353-
{ LAST_BYTE(meta_dec->file_data) ^= 1; });
333+
BREAK_PLN("meta_plain_nonce_0", /*update=*/true,
334+
{ meta->plaintext_part.metadata_key_nonce[0] ^= 1; });
335+
BREAK_PLN("meta_plain_nonce_1", /*update=*/true,
336+
{ LAST_BYTE(meta->plaintext_part.metadata_key_nonce) ^= 0xfe; });
337+
BREAK_PLN("meta_plain_mac_0", /*update=*/false, // update would overwrite the tampering
338+
{ meta->plaintext_part.metadata_mac[0] ^= 0xfe; });
339+
BREAK_PLN("meta_plain_mac_1", /*update=*/false, // update would overwrite the tampering
340+
{ LAST_BYTE(meta->plaintext_part.metadata_mac) ^= 1; });
341+
342+
BREAK_DEC("meta_enc_filename_0", { meta_dec->file_path[0] = 0; });
343+
BREAK_DEC("meta_enc_filename_1", { meta_dec->file_path[0] ^= 1; });
344+
// TODO: tampering below goes undetected during decryption!!
345+
BREAK_DEC("meta_enc_filename_2", { LAST_BYTE(meta_dec->file_path) ^= 0xfe; });
346+
// TODO: tampering below goes undetected during decryption!!
347+
BREAK_DEC("meta_enc_size_0", { meta_dec->file_size = 0; });
348+
BREAK_DEC("meta_enc_size_1", { meta_dec->file_size = g_input_size - 1; });
349+
BREAK_DEC("meta_enc_size_2", { meta_dec->file_size = g_input_size + 1; });
350+
BREAK_DEC("meta_enc_size_3", { meta_dec->file_size = UINT64_MAX; });
351+
BREAK_DEC("meta_enc_mht_key_0", { meta_dec->root_mht_node_key[0] ^= 1; });
352+
BREAK_DEC("meta_enc_mht_key_1", { LAST_BYTE(meta_dec->root_mht_node_key) ^= 0xfe; });
353+
BREAK_DEC("meta_enc_mht_mac_0", { meta_dec->root_mht_node_mac[0] ^= 1; });
354+
BREAK_DEC("meta_enc_mht_mac_1", { LAST_BYTE(meta_dec->root_mht_node_mac) ^= 0xfe; });
355+
// TODO: tampering in below two goes undetected during decryption.
356+
// However, as we re-encrypt the data it is properly encrypted just undetectably different from
357+
// the original version?! I.e., below two cases do not make sense and should be removed?!
358+
BREAK_DEC("meta_enc_data_0", { meta_dec->file_data[0] ^= 0xfe; });
359+
BREAK_DEC("meta_enc_data_1", { LAST_BYTE(meta_dec->file_data) ^= 1; });
354360

355361
/* padding is ignored */
356-
BREAK_PF("meta_padding_0", /*update=*/false,
357-
{ meta->padding[0] ^= 1; });
358-
BREAK_PF("meta_padding_1", /*update=*/false,
359-
{ LAST_BYTE(meta->padding) ^= 0xfe; });
362+
// TODO: remove as as they are ignored decryption always works?
363+
BREAK_DEC("meta_padding_0", { meta->padding[0] ^= 1; });
364+
BREAK_DEC("meta_padding_1", { LAST_BYTE(meta->padding) ^= 0xfe; });
360365

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

382387
/* data nodes start from node #2 */
383-
BREAK_PF("data_0", /*update=*/false,
384-
{ *(out + 2 * PF_NODE_SIZE) ^= 1; });
385-
BREAK_PF("data_1", /*update=*/false,
386-
{ *(out + 3 * PF_NODE_SIZE - 1) ^= 1; });
387-
BREAK_PF("data_2", /*update=*/false, {
388+
BREAK_PLN("data_0", /*update=*/false, { *(out + 2 * PF_NODE_SIZE) ^= 1; });
389+
BREAK_PLN("data_1", /*update=*/false, { *(out + 3 * PF_NODE_SIZE - 1) ^= 1; });
390+
BREAK_PLN("data_2", /*update=*/false, {
388391
/* swap data nodes */
389392
memcpy(out + 2 * PF_NODE_SIZE, g_input_data + 3 * PF_NODE_SIZE, PF_NODE_SIZE);
390393
memcpy(out + 3 * PF_NODE_SIZE, g_input_data + 2 * PF_NODE_SIZE, PF_NODE_SIZE);

0 commit comments

Comments
 (0)