3 ms·
The primary benefit of AF_ALG is IMHO when it's combined with kernel keyrings, i.e. ALG_SET_KEY_BY_KEY_SERIAL. To steal from the sibling post: > * When using
by eqvinox 6mo ago
The primary benefit of AF_ALG is IMHO when it's combined with kernel keyrings, i.e. ALG_SET_KEY_BY_KEY_SERIAL.
To steal from the sibling post:
> * When using user space libraries, all key material and other cryptographic sensitive parameters remains in the calling application's memory even when the application supplied the information to the library. When using AF_ALG, the key material and other sensitive parameters are handed to the kernel. The calling application now can reliably erase that information [...]
It's even more than this: you can do crypto ops in user space without ever even having the key to begin with.
[Ed.: that said, maybe AF_ALG should be locked behind some CAP_*]
[Ed.#2: that said^2, I'm putting this one on authencesn, not AF_ALG. It's the extended sequence number juggling that went poorly, not AF_ALG at large. I bet this might even blow up in some strange hardware scenarios, "network packet on PCIe memory" or something like that - I'm speculating, though.]
- angry_octet 6mo agoBetter implemented as another user space process than in the kernel.
- eqvinox 6mo agoYou can't access TPMs that way.
- wetpaws 6mo ago[dead]
- angry_octet 6mo agoMost of the Linux kernel crypto is not touching the TPM. If there is a TPM task, only that code should be in kernel, and it should be accessed from user space by a process with the appropriate token.
- eqvinox 6mo agoYes, AF_ALG is exposing too many things, like authencesn, which has zero reason for being userspace accessible. It's a crypto mode specific to IPsec. However, > it should be accessed from user space by a process with the appropriate token. That is AF_ALG. The operations it offers are what you need for full coverage. The issues with it are two: - usage specific crypto in the kernel implements the same interfaces, and it doesn't have a filter for that, as mentioned above. It's not offering too many operations, it's offering too many algorithms. - it's trying to be fast. I guess people also want to use crypto accelerators through it. (Which is kinda related to TPMs, there is accelerator hardware with built-in protected key storage...) The CVE we're looking at here is in the intersection of both of these.
- angry_octet 6mo agoAll the uses of vmsplice etc are a bit tricky, and that points to the need for a better interface. But given you're using splice, why not do the crypto in user space? A belief that it is better to be fast and buggy than safe and slower?
- eqvinox 6mo agoIf neither a hardware component nor kernel key management is involved, crypto should be done in userspace, end of sentence. The more I think about it, the more I think it should be behind CAP_SYS_ADMIN, or a new CAP_KCRYPT (better name TBD. CAP_CRYPT_OFFLOAD?)
- angry_octet 6mo agoYes it should definitely require a capability. Still a risk that some admin-enabled method (like enabling an IPsec VPN) provides a path to it, but would reduce the potential for crafting weird inputs.
- angry_octet 6mo agoI'm also wondering if it couldn't be rewritten to use io_uring interfaces.
- kasabali 6mo agoGood
- eqvinox 6mo agoCheesecake Now, is your comment contributing more to this discussion, or mine?
- ebiggers 6mo agoIt doesn't seem to actually get used that way in practice. ALG_SET_KEY_BY_KEY_SERIAL didn't even appear until just a few years ago. And either way, if the interface allows you to overwrite the su binary, whether it theoretically could provide some other security benefit becomes kind of irrelevant.
- eqvinox 6mo agoIt is being used that way: https://github.com/opensourcerouting/frr/blob/2b48e4f97fb02133f3a09db067dc8249ed41e968/lib/keyctl.c#L593 https://github.com/opensourcerouting/frr/blob/2b48e4f97fb021... And, sure, if it breaks system security it's pointless. But so did "dirty pipe". I do agree the number of issues in AF_ALG is annoying, which is why I suggested a CAP_* restriction. Maybe CAP_SYS_ADMIN in init_ns, that's kinda the big hammer.