On Thu, Mar 14, 2024 at 04:52:47AM -0700, James Prestwood wrote: > IWD uses AF_ALG/keyctl for _all_ its crypto, cipher, and checksum needs. > Anything that wifi requires as far as crypto goes IWD uses the kernel, > except ECC is the only exception. The entire list of crypto requirements > (for full support at least) for IWD is here: > > https://git.kernel.org/pub/scm/network/wireless/iwd.git/tree/tools/test_runner_kernel_config That's quite an extensive list, and it's not documented in the iwd README. Don't you get bug reports from users who are running a kernel that's missing one of those options? > For KEYCTL_PKEY_* specifically we use it for all asymmetric crypto > operations, (query), encrypt, decrypt, sign, verify. > > I'll be honest, the AF_ALG/keyctl support in ELL was mostly done by the time > I started working on IWD so I was not aware the documentation was so poor. > That is an entirely separate issue than this IMO, and I'm happy to help with > getting docs updated to include a proper list of supported features. In > addition maybe some automated testing that gets run on kernel builds which > actually exercises this API so it doesn't get accidentally get broken in the > future? Docs/tests IMO are the proper "fix" here, not telling someone to > stop using an API that has existed a long time. I looked into the history, and it seems the KEYCTL_PKEY_* APIs were added as a collaboration between the iwd developers and the kernel keyrings maintainer. So, as far as I can tell, it's not that the kernel had an existing API that iwd started using. It's that iwd got some APIs added to the kernel for themselves. KEYCTL_PKEY_* don't seem to have been adopted elsewhere; Debian Code Search doesn't return any notable results. keyctl does provide a command-line interface to them, but I can't find any users of the keyctl commands either. Then, everyone disappeared and it got dumped on the next generation of kernel developers, who often don't know that this API even exists. And since the API is also poorly specified and difficult to maintain (e.g., changing a seemingly unrelated part of the kernel can break it), the results are predictable... And of course the only thing that breaks is iwd, since it's the only user. It would be worth taking a step back and looking at the overall system architecture here. Is this the best way to ensure a reliable wireless experience for Linux users? Maybe it's time to admit that KEYCTL_PKEY_* was basically an experiment, and a different direction (e.g. using OpenSSL) should be taken... (Another issue with the kernel keyrings stuff is that provides a significant attack surface for the kernel to be exploited.) If you do decide to continue with the status quo, it may be necessary for the iwd developers to take a more active role in maintaining this API in order to ensure it continues working properly for you. AF_ALG is on *slightly* firmer ground since it's been around for longer, is properly part of the crypto subsystem, and has a few other users. Unfortunately it still suffers from the same issues though, just to a slightly lesser degree. > I'm also not entirely sure why this stuff continues to be removed from the > kernel. First MD4, then it got reverted, then this (now reverted, thanks). > Both cases there was not clear justification of why it was being removed. These algorithms are insecure, and it's likely that the author of these commits thought that there were no remaining users and nothing would break. Removing them is a worthy goal for code maintenance purposes and to avoid providing insecure options that could accidentally be used. The AF_ALG and KEYCTL_PKEY_* APIs are very easy to overlook and I suspect that the author of these commits did not know about them. These APIs are rarely used, not well specified, the availability of them and specific algorithms varies by kernel configuration, and userspace only uses a subset of the algorithms in the kernel's museum of crypto primitives anyway. So it's plausible that there are algorithms that no one is using or that at least there is a fallback for, so can be safely removed... - Eric
