openssl: [RFC] alternative AF_ALG engine #1547
Conversation
|
Current master, patched with this PR, 1.1.1 data. Everything tested to this point appears OK. |
Little tutorialThese are some basic instructions to help me test this. By default, only hardware-accelerated drivers are enabled. If you wish to test it with software drivers, or to change the cipher or digest lists, you can edit Then, add the following sections anywhere past the last of the unsectioned commands (I add this to the very end of the file)--notice that you don't need the CIPHERS or DIGESTS lines to enable all algos, I've added them here for reference: The following line from my example above shows the algorithms currently enabled: I'd measure the speed of the algorithms to determine if it's worth to leave them enabled. Use You need to use
So, in my case, with software-only, /dev/crypto is considerably slower across the board. Please share some results here so I can get a feeling for what the speed is like. |
|
Cannot build wget with OpenSSL 1.1.1 |
Ye, I have encountered the same problem |
|
At first sight, it looks like wget depends on openssl engine support. I get a slightly different error message, but it works with engines turned on. |
|
@cotequeiroz I submitted a patch upstream patching it out but have not backported. |
|
Does IPQ806x (EA8500) and Kirkwood (NSA325) supports hardware crypto? |
|
Kirkwood does. |
|
Is the wget-ssl ld build issue not an openssl build deprecated APIs issue, like stubby (actually getdns) and possibly unbound. Regardless, it builds and WFM. |
|
OpenSSL 1.1.x has built-in support for AF_ALG kernel interfaces for crypto acceleration, so why not to just enable AF_ALG in kernel config and forget about cryptodev and /dev/crypto? |
|
It's slower. |
|
I had posted some numbers somewhere (previous thread ?) where, at least with the device I am testing on, /dev/crypto appeared significantly faster. |
|
Yes, and /dev/crypto is not very stable. |
|
I'm working on /dev/crypto right now. I'm trying to add GCM support. After I'm finished, I'll tackle AF_ALG. No promises at either end, though. |
If you look at the cryptodev-linux code, the main crypto function uses zero-copy interface. |
|
Yes, that is my understanding for the reason of /dev/crypto better performance. |
|
With OpenSSL 1.1 zero copy is supported but it is on the #ifdef for some reason, probably because not all versions of kernel support it. Here is the commit: Bottom line it seems if right configurations are enabled AF_ALG performance should be similar to /dev/crypto |
cotequeiroz
force-pushed
the
openssl-1.1_devcrypto
branch
from
6f3a1f3
to
65b01e4
Compare
|
I wasn't able to do AES-GCM with /dev/crypto, as it doesn't work with partial message (multiple update + one final call) as openssl requires. /dev/crypto only works with a single call containing the whole message. So, I'll see what I can do with the afalg engine next. It will take a while. |
|
@borkra Second run, with The third set is with /dev/crypto: It might have to do with page alignment:
|
|
IIRC this was one of the reasons for WireGuard avoiding AF_ALG and using its own thing. @zx2c4 |
Well, you still get huge gain over software only implementation. |
|
@borkra Here's my branch that has it. All you need is to replace One of the problems I found with AF_ALG is the high number of open file descriptors kept open, especially after enabling ecb mode. It is used for RNG, apparently, and generating a RSA key keeps a large number of ecb cipher contexts open. Most implementations use two fds, keeping the original one open. I found that it works with that fd closed, but such behaviour is undocumented (most of the AF_ALG stuff is poorly documented). Zero-copy support needs at least 3 fds (one for the main operation, two for the zero-copy pipes) per context. I might implement zero-copy if I can find some time; openssl's async io is useless in opewrt, as it will not compile with musl. The current AF_ALG engine uses AIO. Tell me if you're able to test it. |
|
FWIW, that RNG seeds itself using get_random_bytes, which is the equivalent of /dev/urandom. Not sure if that invalidates what you're after, if you don't think /dev/urandom is sufficient for that. |
|
@zx2c4 /dev/urandom, or /dev/random are not FIPS compliant entropy sources, and as such cannot be used for seeds or any other use in FIPS compliant design. There is jitter entropy generator is FIPS compliant in kernel on some CPUs, and some of the CPU. |
|
Right, that's what I assumed. So, look at crypto/drbg.c below. It might not actually satisfy whatever you're after.
|
|
The last time I compared afalg and devcrypto I only looked at the mamba device, but the above made me take another look. The following would appear to indicate an inverse correlation between device capabilities and performance differential of the two. From the above numbers and new data from a run on a rango and mamba.
edit: cut and paste wrong heading |
|
@cotequeiroz i would like to report that nginx is broken if the engine support is disabled in openssl (needs it even with no engine at all)) |
|
@Ansuel |
|
@Ansuel, can you give me some details of the failure? Is it a build failure, or run-time failure? Tell me the steps to reproduce the issue. |
|
@cotequeiroz just build openssl with no engine support also other package suffer from this (can't remember them but i notice them reading the forum) |
|
Any chance the crypto.mk change can be upstreamed? I have a similar yet different change in my tree that I would like to get in: https://github.com/neheb/source/commit/aaa4215aa8c39ca5abf42fd05da9e2608c8fcf9c |
|
I imagine you mean to submit a standalone patch for |
|
I don't remember if my patch was for openssl or cryptsetup. In any case, I rebased and submitted. |
cotequeiroz
force-pushed
the
openssl-1.1_devcrypto
branch
from
26a2fbb
to
e115983
Compare
|
The latest openssl push has broken this. |
cotequeiroz
force-pushed
the
openssl-1.1_devcrypto
branch
from
e115983
to
fe4aa63
Compare
This adds engine configuration sections to openssl.cnf, with a commented list of engines. To enable an engine, all you have to do is uncomment the engine line. It also adds some useful comments to the devcrypto engine configuration section. Other engines currently don't have configuration commands. Signed-off-by: Eneas U de Queiroz <cotequeiroz@gmail.com>
Use an engine based on the /dev/crytpo engine, supporting more algorithms, configuration options, and not requiring (nor supporting) async io. Signed-off-by: Eneas U de Queiroz <cote2004-github@yahoo.com>
This is needed to export crypto information to netfilter, allowing openssl afalg engine to obtain information about the drivers being used. Signed-off-by: Eneas U de Queiroz <cote2004-github@yahoo.com>
cotequeiroz
force-pushed
the
openssl-1.1_devcrypto
branch
from
fe4aa63
to
0dbed74
Compare
|
Just added the "needs reviewer" tag when I realized this has "RFC" in its title. So, is this in RFC state? |
|
TLDR: yes, it is still RFC, but it does need feedback. I've created this an alternative to the upstream engine; this one uses a simpler API, with synchronous calls, but supports more algorithms. Upstream has a different approach, using the async API, which requires AIO kernel support (so it does not get built by the bots), and, at least in the most used scenario, is much slower. I've been maintaining it since it does have its users, but I'm not sure it is worth to add it to the main repository. My doubt, is not really maintenance--I'm currently maintaining the openssl package, and it can always be dropped if I fail to maintain it--but that that we already have the devcrypto engine, which is an upstream-supported, faster alternative. I haven't received feedback from the core developers, but we can remove the RFC label, and add "needs reviewer" if it helps, or is the right thing to do. |
Well, that sounds to me like you should make sure whether there is demand. Normally, having a PR open that long without any core-developer comments tends to mean that demand is not big. Maybe it would be a good idea to discuss this matter of general interest on the mailing list, and then decide about whether the PR is kept open or closed? |
|
Let me try to compile this out of the openssl tree; then I can package it separately and add it to the packages feed. That would make more sense to me. I'll close this if able; if not, I'll post to the mailing list. If you feel it should be closed now, go ahead--I'm counting on being able to take this out of the openssl tree. |
|
I'm not in a hurry, just picking up loose ends.... |
|
It has served its purpose--I would not have come up with that idea right, had you not poked me. Thanks. I'll close this as soon as I'm done. |
|
I've moved this to a separate package in the packages feed openwrt/packages#10423. |
|
Perfect, thanks for taking care! |
I've written an AF_ALG engine for openssl based on the /dev/crypto engine. It does not require kernel AIO support. Please test this especially if you have hardware crypto acceleration.
Signed-off-by: Eneas U de Queiroz cote2004-github@yahoo.com
PS: This was originally a /dev/crypto PR, but the patches were submitted to the openwrt mailing list.