freeradius-server

freeradius-server copied to clipboard

For FR_TYPE_SECRET, buffers should be allocated from an mlocked pool, which should secure memset the objects to zero when they're freed.

The pool should be page aligned and reuse the same code as the mprotected pages.

Related thoughts

• It also might be a good idea to implement a new API which allows allocation of memory fragments, and tracks the allocations in a linked list, as phase 0 then, allocates a page aligned talloc pool to hold all those allocations, and copies them over. This would allow us to get rid of the explicit mprotect pool size, which was always a dumb hack, and not require an explicit pool size for secret memory.

• We can coarsely intercept memory allocations in OpenSSL that may be dealing with things like private keys. Other allocations may get caught in this, but that's not a huge issue.

• We need page aligned memory for mprotect, madvise, mlock, mmap, mremap.

• We might want to use madvise on the pages used for the ring buffers for packet processing, to allow aggressive readahead.

@terryburton this relates to some of the things we were talking about. Do you have any other thoughts on memory hardening strategies, other than secure memset on free and making sure sensitive memory doesn't get paged? Might be worth having a look at SME, and seeing if we can enable that on the relevant pages, I think it'd prevent cold boot attacks...

There's also memguard in go, but I'm not sure what mechanism it's actually using to do memory encryption and whether it's go specific.

We could:

• add flags->secret to dict.h
• set it in the RADIUS code, for encrypted attributes
• update _fr_pair_free() to check for the flags
• if set, use memset_s() to wipe the secret data

That works for VALUE_PAIR. We would need to add something similar for CONF_PAIR:

• add bool secret, which is set when the CONF_PARSER has FR_TYPE_SECRET
• add _cf_pair_free() function as a talloc destructor
• if cp->secret, call memset_s()

realistically, that's much less useful than the VALUE_PAIR changes. The CONF_PAIRs are generally never freed while the server is running. Because the server usually needs continuous access to the data.

Sure. I added some functions in talloc.c to add an arbitrary number of destructors to a talloc chunk using the linking ctx stuff. We could just add another generic function in that source file to add a destructor which calls memset_s on the chunk when it's freed.

That way it's really easy to sprinkle around the code, and we can apply it at a per-talloc-chunk level, without needing to add explicit destructor functions anywhere.

I think talloc_get_size would work for this.

Function is called talloc_destructor_add

Looks like there's a vestigial declaration in talloc.h for fr_talloc_destructor_t *talloc_add_destructor(TALLOC_CTX *chunk, fr_talloc_free_func_t func, void const *uctx); that should be removed