Bug 263379
| Summary: | [regression] [ipsec] compatibility broken between stable/12 and stable/13 opencrypto in AEAD mode | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Eugene Grosbein <eugen> |
| Component: | kern | Assignee: | John Baldwin <jhb> |
| Status: | Closed FIXED | ||
| Severity: | Affects Some People | CC: | jhb, markj, net, zlei |
| Priority: | --- | Flags: | eugen:
mfc-stable13?
|
| Version: | 13.1-STABLE | ||
| Hardware: | Any | ||
| OS: | Any | ||
To be clear, what do you think using AES-GCM with a separate SHA-512 HMAC should do? From the RFCs I have read, there is no defined standard for using a separate MAC with an AEAD cipher. For example, in the combinations listed in section 4 of RFC 8221, there is no defined method for combining a distinct MAC with an AEAD cipher. About the best I could imagine is that perhaps you intend to use AES-GCM with an ESP header and then a separate SHA-512 HMAC with an AH header? I can find no mention anywhere of such a construct for IPsec, and the closest thing I can find (using a non-AEAD cipher with ESP and a separate MAC with AH) is deprecated in RFC 8221 (the recommended approach is to use the non-AEAD cipher and MAC together as a combined ETA cipher with ESP, e.g. AES-CBC+SHA-512-HMAC). I suspect the fact that stable/12 does not fail this is probably a weird bug due to lack of argument validation in stable/12, and it is probably putting packets on the wire that no other OS can handle (perhaps it is basically using the AES-CTR side of AES-GCM but with the IV for AES-CTR constructed using AES-GCM rules and then generating the SHA-512 HMAC using ETA rules). That is, I suspect what you kind of have is a broken, non-standard version of AES-CTR + SHA-512 HMAC (which is a defined combined algorithm you can use). But really you are better of just using stock AES-GCM anyway and using GMAC rather than SHA512-HMAC. Your changes to OCF are all incorrect. If you want to combine AES-GCM with a separate HMAC, you would need to construct separate crypto sessions and crypto requests for a given buffer, the first to do AES-GCM and the second to deal with the HMAC. (In reply to John Baldwin from comment #1) I'm not an IPSec expert in any way. I tend to agree that this setup may be broken, if you say so. I'm talking about setkey(8) manual page that still states: ALGORITHMS The following list shows the supported algorithms. The protocol and algorithm are almost completely orthogonal. And about compatibility issued when you have multiple stable/12 peers and want to upgrade to stable/13 sequentially not breaking encrypted links. So the issue with the setkey manpage is it was written before AEAD algorithms were a thing and when you always had to use Encrypt-then-Auth (ETA) combinations of distinct ciphers and MACs. In general when using an AEAD algorithm like AES-GCM, AES-CCM, or Chacha20-Poly1305, you provide a single key that is then used to generate keys for both the cipher (AES-CTR for AES-GCM/CCM and Chacha20 for Chacha20-Poly1305) and the MAC (GMAC, CBC-MAC, and Poly1305, respectively). For those three AEAD ciphers, the cipher is a stream cipher and the key stream block for a given counter value is used as the key of the MAC.
The fact that stable/12 let you construct this combination is a long-standing bug in cryptosoft.c and there's no way to express the combination of the AES-CTR "side" of AES-GCM combined with some other MAC in stable/13 and later. For the purposes of upgrading, you could switch your existing 12.x hosts to use a supported cipher first prior to upgrading. I would suggest just using -E aes-gcm-16 and dropping the -A entirely.
Unfortunately, the AES-CTR cipher defined for IPsec does not permit setting all 16 bytes of the nonce and instead mandates that the low 4 bytes define a 32-bit counter starting at 1 (whereas AES-GCM uses block 1 internally and starts encrypting with the low 4 bytes set to block 2 for the AES-CTR "part"), so there isn't a way to recreate the thing stable/12 is doing by using -E aes-ctr combined with the existing -A. It might be possible to do this though using a local hack that would let you use -E aes-ctr in place of -E aes-gcm-16. You would keep the same key for -E, but just change it to use 'aes-ctr' and then apply this patch:
diff --git a/sys/netipsec/xform_esp.c b/sys/netipsec/xform_esp.c
index ee363a7c911a..465716c3ea7c 100644
--- a/sys/netipsec/xform_esp.c
+++ b/sys/netipsec/xform_esp.c
@@ -455,7 +455,7 @@ esp_input(struct mbuf *m, struct secasvar *sav, int skip, int protoff)
_KEYLEN(sav->key_enc) - 4, 4);
m_copydata(m, skip + hlen - sav->ivlen, sav->ivlen, &ivp[4]);
if (SAV_ISCTR(sav)) {
- be32enc(&ivp[sav->ivlen + 4], 1);
+ be32enc(&ivp[sav->ivlen + 4], 2);
}
crp->crp_flags |= CRYPTO_F_IV_SEPARATE;
} else if (sav->ivlen != 0)
@@ -880,7 +880,7 @@ esp_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav,
_KEYLEN(sav->key_enc) - 4, 4);
be64enc(&ivp[4], cntr);
if (SAV_ISCTR(sav)) {
- be32enc(&ivp[sav->ivlen + 4], 1);
+ be32enc(&ivp[sav->ivlen + 4], 2);
}
m_copyback(m, skip + hlen - sav->ivlen, sav->ivlen, &ivp[4]);
crp->crp_flags |= CRYPTO_F_IV_SEPARATE;
This breaks the "official" AES-CTR defined in RFC 3686 (so it can't be committed), but you could try applying this locally and keeping it as a local patch if it works until you are able to migrate existing hosts to using a supported cipher suite.
I can work on clarifying the language in the setkey(8) manual page to clarify that -A should not be used with AEAD ciphers.
(In reply to John Baldwin from comment #3) I personally can deal with the problem in multiple ways but this PR is about migration ways suitable for most users. Yes, please correct setkey(8) manual page and perhaps we need post-13.1-RELEASE Errata Note explaining how users should behave in process of migration if they have unsupported setup. I have a review for the manpage at https://reviews.freebsd.org/D34947 (Eugene, I couldn't add you on the review via your freebsd.org username for some reason?) In terms of how to inform other users, we could perhaps add a check to stable/12 to warn users about using an explicit -A in combination with AES-GCM just as we have warnings in place now for older cipher suites deprecated in FreeBSD 13. I suspect such uses are somewhat rare though given it's not a standard combination? (There's no support in IKE for specifying a combination like this for example.) (In reply to John Baldwin from comment #3) I was told that aes-gcm-16 should not be used with static keys for anything other then debugging, so I'm experimenting with -E aes-cbc ... -A hmac-sha2-512 and it still fails with EINVAL (same way). And same way it fails replacing hmac-sha2-512 with hmac-sha2-256 or with hmac-sha1. Does stable/13 support Encrypt-then-Auth (ETA) mode and if so, which algos? (In reply to John Baldwin from comment #5) > In terms of how to inform other users, we could perhaps add a check to stable/12 to warn users about using an explicit -A in combination with AES-GCM just as we have warnings in place now for older cipher suites deprecated in FreeBSD 13. I'm afraid it could be a bit late as there will be no 12.4-RELEASE (?) so the warning may not reach most users. (In reply to Eugene Grosbein from comment #6) Static keys are not good for AES-GCM or AES-CTR as the sequence number can rollover yes. Even for AES-CBC I would be hesitant to rely on static keys rather than using an IKE daemon to permit dynamic keys. stable/13 should work fine with ETA combos such as AES-CBC with SHA1/256/512 HMACs. Note that the key for AES-CBC is shorter than for AES-CTR/GCM as it is "only" the actual AES key (so 16, 24, or 32 bytes) and doesn't include the extra 4 bytes for the implicit part of the IV. (And setkey just reports "EINVAL" for all manner of errors, so it's rather hard to figure out why setkey fails in my experience, so my best guess is you are reusing the GCM key but need to remove the last 4 bytes.) The kyua tests test AES-CBC (both 128 and 256 bit keys) with SHA1-HMAC and SHA2-256-HMAC. In regards to stable/12, yes, I think it is also late and a warning might not be seen by many users (and almost said as much). stable/12 is still supported until 2024 so a 12.4 doesn't seem completely unlikely however. (In reply to John Baldwin from comment #8) > Static keys are not good for AES-GCM or AES-CTR as the sequence number can rollover yes. Maybe it's worth mentioning in the setkey(8), too. > stable/13 should work fine with ETA combos such as AES-CBC with SHA1/256/512 HMACs. But it does not for me. > Note that the key for AES-CBC is shorter than for AES-CTR/GCM as it is "only" the actual AES key (so 16, 24, or 32 bytes) and doesn't include the extra 4 bytes for the implicit part of the IV. I am aware of this. > (And setkey just reports "EINVAL" for all manner of errors, so it's rather hard to figure out why setkey fails in my experience, so my best guess is you are reusing the GCM key but need to remove the last 4 bytes.) First, I did modify the key to shorter it. Second, setkey utility has its own syntax checks that include checks for the length and it does not even try to sent ADD request to the kernel for wrong key length issuing a bit more readable error with line number of /etc/ipsec.conf > The kyua tests test AES-CBC (both 128 and 256 bit keys) with SHA1-HMAC and SHA2-256-HMAC. Would you please point me to the corresponding kyua test? The kyua tests are src/tests/sys/netipsec/tunnel. There are several layers of indirection though that can make it a bit hard to see the actual config files used. When doing testing on both 12.x and later I have used setkey config files from https://github.com/freebsd-net/netperf/tree/master/IPSEC/Configs (specifically the dut-* and source-* configs with one end using dut-<foo> and the other end using source-<foo>). The only one that I have not been able to make work is *-ipcomp.conf. (In reply to John Baldwin from comment #10) Thank you very much. I've found that EINVAL is also returned if I forget to kldload ipsec.ko. Pilot error. OTOH, maybe it is time for our setkey(8) to kldload ipsec.ko automatically just like ifconfig(8) loads needed modules. We have ipsec.ko in all supported branches (since 11.1-RELEASE). Something like this maybe? --- sbin/setkey.c.orig 2022-02-04 20:36:27.627040000 +0000 +++ sbin/setkey.c 2022-04-19 17:22:46.193421000 +0000 @@ -34,6 +34,8 @@ #include <sys/types.h> #include <sys/param.h> +#include <sys/linker.h> +#include <sys/module.h> #include <sys/socket.h> #include <sys/time.h> #include <err.h> @@ -102,6 +104,17 @@ usage() exit(1); } +static int +modload(const char *name) +{ + if (modfind(name) < 0) + if (kldload(name) < 0 || modfind(name) < 0) { + warn("%s: module not found", name); + return 0; + } + return 1; +} + int main(ac, av) int ac; @@ -167,6 +180,7 @@ main(ac, av) } } + modload("ipsec"); so = pfkey_open(); if (so < 0) { perror("pfkey_open"); A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=e6dede145616ed8f98c629c23a2ba206b812c921 commit e6dede145616ed8f98c629c23a2ba206b812c921 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-04-27 19:18:52 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-04-27 19:23:18 +0000 setkey(8): Clarify language around AEAD ciphers. AEAD ciphers for IPsec combine both encryption and authentication. As such, ESP configurations using an AEAD cipher should not use a seperate authentication algorithm via -A. However, this was not apparent from the setkey manpage and 12.x and earlier did not perform sufficient argument validation permitting users to pair an explicit -A such as SHA256-HMAC with AES-GCM. (The result was a non-standard combination of AES-CTR with the specified MAC, but with the wrong initial block counter (and thus different keystream) compared to using AES-CTR as the cipher.) Attempt to clarify this in the manpage by explicitly calling out AEAD ciphers (currently only AES-GCM) and noting that AEAD ciphers should not use -A. While here, explicitly note which authentication algorithms can be used with esp vs esp-old. Also add subsection headings for the different algorithm lists and tidy some language. I did not convert the tables to column lists (Bl -column) though that would probably be more correct than using literal blocks (Bd -literal). PR: 263379 Reviewed by: Pau Amma <pauamma@gundo.com>, markj Differential Revision: https://reviews.freebsd.org/D34947 sbin/setkey/setkey.8 | 58 +++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 26 deletions(-) A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=0aef8628458a7d03e3c7e63ae05e228191167eec commit 0aef8628458a7d03e3c7e63ae05e228191167eec Author: Eugene Grosbein <eugen@FreeBSD.org> AuthorDate: 2022-05-05 12:02:29 +0000 Commit: Eugene Grosbein <eugen@FreeBSD.org> CommitDate: 2022-05-05 12:02:29 +0000 If setkey(8) is used without ipsec.ko loaded beforehand, its attempt to install SA/SPD into the kernel results in cryptic EINVAL error code. Let it be a bit more user-friendly and try to load ipsec.ko automatically if it is not loaded, just like ifconfig(8) does it for modules it needs. PR: 263379 MFC after: 2 weeks sbin/setkey/setkey.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=9132c793cb014e321c92ed502d20df4426492562 commit 9132c793cb014e321c92ed502d20df4426492562 Author: Eugene Grosbein <eugen@FreeBSD.org> AuthorDate: 2022-05-05 12:02:29 +0000 Commit: Eugene Grosbein <eugen@FreeBSD.org> CommitDate: 2022-05-19 06:05:30 +0000 setkey(8): MFC: load ipsec.ko automatically If setkey(8) is used without ipsec.ko loaded beforehand, its attempt to install SA/SPD into the kernel results in cryptic EINVAL error code. Let it be a bit more user-friendly and try to load ipsec.ko automatically if it is not loaded, just like ifconfig(8) does it for modules it needs. PR: 263379 (cherry picked from commit 0aef8628458a7d03e3c7e63ae05e228191167eec) sbin/setkey/setkey.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=dc72e1b86f8ae396215418cdeda037e37df11e98 commit dc72e1b86f8ae396215418cdeda037e37df11e98 Author: Eugene Grosbein <eugen@FreeBSD.org> AuthorDate: 2022-05-05 12:02:29 +0000 Commit: Eugene Grosbein <eugen@FreeBSD.org> CommitDate: 2022-05-19 06:11:34 +0000 setkey: MFC: load ipsec.ko automatically If setkey(8) is used without ipsec.ko loaded beforehand, its attempt to install SA/SPD into the kernel results in cryptic EINVAL error code. Let it be a bit more user-friendly and try to load ipsec.ko automatically if it is not loaded, just like ifconfig(8) does it for modules it needs. PR: 263379 (cherry picked from commit 0aef8628458a7d03e3c7e63ae05e228191167eec) sbin/setkey/setkey.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6835ace580917ec512eb96cf9c456f4acc161247 commit 6835ace580917ec512eb96cf9c456f4acc161247 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-04-27 19:18:52 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-05-20 00:35:34 +0000 setkey(8): Clarify language around AEAD ciphers. AEAD ciphers for IPsec combine both encryption and authentication. As such, ESP configurations using an AEAD cipher should not use a seperate authentication algorithm via -A. However, this was not apparent from the setkey manpage and 12.x and earlier did not perform sufficient argument validation permitting users to pair an explicit -A such as SHA256-HMAC with AES-GCM. (The result was a non-standard combination of AES-CTR with the specified MAC, but with the wrong initial block counter (and thus different keystream) compared to using AES-CTR as the cipher.) Attempt to clarify this in the manpage by explicitly calling out AEAD ciphers (currently only AES-GCM) and noting that AEAD ciphers should not use -A. While here, explicitly note which authentication algorithms can be used with esp vs esp-old. Also add subsection headings for the different algorithm lists and tidy some language. I did not convert the tables to column lists (Bl -column) though that would probably be more correct than using literal blocks (Bd -literal). PR: 263379 Reviewed by: Pau Amma <pauamma@gundo.com>, markj Differential Revision: https://reviews.freebsd.org/D34947 (cherry picked from commit e6dede145616ed8f98c629c23a2ba206b812c921) sbin/setkey/setkey.8 | 58 +++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 26 deletions(-) A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6aaf8a8b1bcf500aa7342043d43007ff9c52cd65 commit 6aaf8a8b1bcf500aa7342043d43007ff9c52cd65 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-04-27 19:18:52 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-05-20 00:42:24 +0000 setkey(8): Clarify language around AEAD ciphers. AEAD ciphers for IPsec combine both encryption and authentication. As such, ESP configurations using an AEAD cipher should not use a seperate authentication algorithm via -A. However, this was not apparent from the setkey manpage and 12.x and earlier did not perform sufficient argument validation permitting users to pair an explicit -A such as SHA256-HMAC with AES-GCM. (The result was a non-standard combination of AES-CTR with the specified MAC, but with the wrong initial block counter (and thus different keystream) compared to using AES-CTR as the cipher.) Attempt to clarify this in the manpage by explicitly calling out AEAD ciphers (currently only AES-GCM) and noting that AEAD ciphers should not use -A. While here, explicitly note which authentication algorithms can be used with esp vs esp-old. Also add subsection headings for the different algorithm lists and tidy some language. I did not convert the tables to column lists (Bl -column) though that would probably be more correct than using literal blocks (Bd -literal). PR: 263379 Reviewed by: Pau Amma <pauamma@gundo.com>, markj Differential Revision: https://reviews.freebsd.org/D34947 (cherry picked from commit e6dede145616ed8f98c629c23a2ba206b812c921) sbin/setkey/setkey.8 | 74 ++++++++++++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 34 deletions(-) setkey(8) binary and manual page fixed in all supported branches. |
Hi! Assume we have the following in the /etc/ipsec.conf: add 1.1.1.1 2.2.2.2 esp 7888 -m transport -E aes-gcm-16 "3#&*f738@?>=_fOH<D30z%WV&*^>@0D+n1{c" -A hmac-sha2-512 "M@-#@k9?NWiuj4f04jJE(dm{4/B=p9d5@7v}naW,[3!_1}4.]n-t;99L0+/14004"; Exact key values do not matter but their lenght do matter. "/sbin/setkey -f /etc/ipsec.conf" parses this just fine for both of stable/12 and stable/13 and sends a message over a socket to the kernel to add new SA. stable/12 kernel accepts it (and then it works) but stable/13 of 17 April 2022 rejects with EINVAL. Note that stock stable/13 kernel accepts it if you change hmac-sha2-512 to hmac-sha2-256 and halve the length of the key. Here is some preliminary and incomplete patch for stable/13 opencrypto that makes stable/13 to accept this and install new SA: http://www.grosbein.net/freebsd/opencrypto/opencrypto.diff Still, the patch does not solve the problem completely as ESP packets sent from patched stable/13 system get dropped by stable/12 increasing error counter "packets dropped; no transform" in the output of "netstat -sp esp". Note again, that stable/12 system has another stable/12 peer with same configuration and encrypted traffic flows just fine. There is no IKE daemon in the picture intentionally, to simplify debugging.