From a531c0e9609abaa4c661ada98e4cf09e42ace5d8 Mon Sep 17 00:00:00 2001 From: Jeremy Lakeman Date: Tue, 20 Sep 2016 16:27:49 +0930 Subject: [PATCH] Use a random walk to find free keyring slots Always round the file size up to the nearest 16 slots --- keyring.c | 109 +++++++++++++++++++++++++++++++++++++++----------- keyring.h | 4 ++ tests/keyring | 9 +++-- 3 files changed, 96 insertions(+), 26 deletions(-) diff --git a/keyring.c b/keyring.c index 8a2c4b1a..2d312914 100644 --- a/keyring.c +++ b/keyring.c @@ -1350,14 +1350,19 @@ static void set_slot(keyring_file *k, unsigned slot, int bitvalue) /* Find free slot in keyring. Slot 0 in any slab is the BAM and possible keyring salt, so only * search for space in slots 1 and above. TODO: Extend to handle more than one slab! - * TODO: random search to avoid predictability of used slots! */ static unsigned find_free_slot(const keyring_file *k) { + unsigned i; unsigned slot; - for (slot = 1; slot < KEYRING_BAM_BITS; ++slot) - if (!test_slot(k, slot)) + // walk the list of slots, randomising the low order bits of the index + unsigned mask = randombytes_uniform(KEYRING_ALLOC_CHUNK); + for (i = 0; i < KEYRING_BAM_BITS; ++i){ + slot = 1+(i ^ mask); + DEBUGF(keyring, "Check %u, is slot %u free?", i, slot); + if (slot < KEYRING_BAM_BITS && !test_slot(k, slot)) return slot; + } return 0; } @@ -1437,6 +1442,7 @@ keyring_identity *keyring_create_identity(keyring_file *k, const char *pin) WHY("no free slots in first slab (no support for more than one slab)"); goto kci_safeexit; } + DEBUGF(keyring, "Allocate identity into slot %u", id->slot); /* Mark slot as occupied and internalise new identity. */ if (keyring_commit_identity(k, id)!=1) @@ -1452,6 +1458,28 @@ keyring_identity *keyring_create_identity(keyring_file *k, const char *pin) return NULL; } +static int write_random_slot(keyring_file *k, unsigned slot) +{ + if (test_slot(k, slot)!=0) + return 0; + + DEBUGF(keyring, "Fill slot %u with randomness", slot); + uint8_t random_data[KEYRING_PAGE_SIZE]; + randombytes_buf(random_data, sizeof random_data); + + off_t file_offset = KEYRING_PAGE_SIZE * slot; + + if (fseeko(k->file, k->file_size, SEEK_SET) == -1) + return WHYF_perror("fseeko(%d, %ld, SEEK_SET)", fileno(k->file), (long)file_offset); + if (fwrite(random_data, sizeof random_data, 1, k->file) != 1) + return WHYF_perror("fwrite(%p, %ld, 1, %d)", random_data, sizeof random_data, fileno(k->file)); + + if (k->file_size < file_offset + KEYRING_PAGE_SIZE) + k->file_size = file_offset + KEYRING_PAGE_SIZE; + + return 0; +} + int keyring_commit(keyring_file *k) { DEBUGF(keyring, "k=%p", k); @@ -1478,32 +1506,66 @@ int keyring_commit(keyring_file *k) keyring_iterator it; keyring_iterator_start(k, &it); while(keyring_next_identity(&it)){ + if (it.identity->slot == 0){ + it.identity->slot = find_free_slot(k); + DEBUGF(keyring, "Allocate identity into slot %u", it.identity->slot); + } unsigned char pkr[KEYRING_PAGE_SIZE]; - if (keyring_pack_identity(it.identity, pkr)) + + if (keyring_pack_identity(it.identity, pkr)){ errorCount++; - else { - /* Now crypt and store block */ - /* Crypt */ - if (keyring_munge_block(pkr, KEYRING_PAGE_SIZE, - it.file->KeyRingSalt, it.file->KeyRingSaltLen, - it.file->KeyRingPin, it.identity->PKRPin)) { - WHY("keyring_munge_block() failed"); + continue; + } + /* Now crypt and store block */ + /* Crypt */ + if (keyring_munge_block(pkr, KEYRING_PAGE_SIZE, + it.file->KeyRingSalt, it.file->KeyRingSaltLen, + it.file->KeyRingPin, it.identity->PKRPin)) { + WHY("keyring_munge_block() failed"); + errorCount++; + continue; + } + + /* Store */ + off_t file_offset = KEYRING_PAGE_SIZE * it.identity->slot; + + while ((off_t)k->file_size < file_offset){ + // write randomness into any blank keyring entries + // ignoring any range we are about to write anyway + unsigned slot = k->file_size / KEYRING_PAGE_SIZE; + if (write_random_slot(k, slot)!=0){ errorCount++; - } else { - /* Store */ - off_t file_offset = KEYRING_PAGE_SIZE * it.identity->slot; - if (file_offset == 0) { - DEBUGF(keyring, "ID id=%p has slot=0", it.identity); - } else if (fseeko(k->file, file_offset, SEEK_SET) == -1) { - WHYF_perror("fseeko(%d, %ld, SEEK_SET)", fileno(k->file), (long)file_offset); - errorCount++; - } else if (fwrite(pkr, KEYRING_PAGE_SIZE, 1, k->file) != 1) { - WHYF_perror("fwrite(%p, %ld, 1, %d)", pkr, (long)KEYRING_PAGE_SIZE, fileno(k->file)); - errorCount++; - } + break; } } + + DEBUGF(keyring, "Write identity to slot %u", it.identity->slot); + + if (fseeko(k->file, file_offset, SEEK_SET) == -1) { + WHYF_perror("fseeko(%d, %ld, SEEK_SET)", fileno(k->file), (long)file_offset); + errorCount++; + continue; + } + + if (fwrite(pkr, KEYRING_PAGE_SIZE, 1, k->file) != 1) { + WHYF_perror("fwrite(%p, %ld, 1, %d)", pkr, (long)KEYRING_PAGE_SIZE, fileno(k->file)); + errorCount++; + continue; + } + + if (k->file_size < file_offset + KEYRING_PAGE_SIZE) + k->file_size = file_offset + KEYRING_PAGE_SIZE; } + + // keep writing random bytes until the number of slots that might be used is an exact multiple of KEYRING_ALLOC_CHUNK + while(1){ + unsigned slot_count = k->file_size / KEYRING_PAGE_SIZE; + if ((slot_count % KEYRING_ALLOC_CHUNK) == 0) + break; + if (write_random_slot(k, slot_count)!=0) + break; + } + if (fflush(k->file) == -1) { WHYF_perror("fflush(%d)", fileno(k->file)); errorCount++; @@ -2207,6 +2269,7 @@ int keyring_load_from_dump(keyring_file *k, unsigned entry_pinc, const char **en keyring_free_identity(id); return WHY("no free slot"); } + DEBUGF(keyring, "Allocate identity into slot %u", id->slot); } if (!keyring_identity_add_keypair(id, kp)) keyring_free_keypair(kp); diff --git a/keyring.h b/keyring.h index ad139721..5ef997dd 100644 --- a/keyring.h +++ b/keyring.h @@ -60,6 +60,10 @@ typedef struct keyring_identity { #define KEYRING_BAM_BYTES ((size_t)2048) #define KEYRING_BAM_BITS (KEYRING_BAM_BYTES<<3) #define KEYRING_SLAB_SIZE (KEYRING_PAGE_SIZE*KEYRING_BAM_BITS) + +// should be a power of 2 +#define KEYRING_ALLOC_CHUNK (16) + typedef struct keyring_bam { size_t file_offset; unsigned char bitmap[KEYRING_BAM_BYTES]; diff --git a/tests/keyring b/tests/keyring index 87711a28..1a76ee6b 100755 --- a/tests/keyring +++ b/tests/keyring @@ -49,10 +49,13 @@ setup_instances() { doc_KeyringCreate="Create keyring destroys existing keys" test_KeyringCreate() { - executeOk_servald keyring add '' - executeOk_servald keyring add '' + for i in {1..20} + do + executeOk_servald keyring add '' + tfw_cat --stderr + done executeOk_servald keyring list - assert_keyring_list 2 + assert_keyring_list 20 executeOk_servald keyring create executeOk_servald keyring list assert_keyring_list 0