On 11/5/21 3:59 PM, Dmitry Safonov wrote:
On 11/5/21 09:54, Leonard Crestez wrote:
On 11/5/21 3:49 AM, Dmitry Safonov wrote:
Convert tcp_md5sig_pool to more generic tcp_sig_pool.
Now tcp_sig_pool_alloc(const char *alg) can be used to allocate per-cpu
ahash request for different hashing algorithms besides md5.
tcp_sig_pool_get() and tcp_sig_pool_put() should be used to get
ahash_request and scratch area.
This pool pattern is a workaround for crypto-api only being able to
allocate transforms from user context.
It would be useful for this "one-transform-per-cpu" object to be part of
crypto api itself, there is nothing TCP-specific here other than the
size of scratch buffer.
Agree, it would be nice to have something like this as a part of crypto.
The intention here is to reuse md5 sig pool, rather than introduce
another similar one.
Make tcp_sig_pool reusable for TCP Authentication Option support
(TCP-AO, RFC5925), where RFC5926[1] requires HMAC-SHA1 and AES-128_CMAC
hashing at least.
Additional work would be required to support options of arbitrary size
and I don't think anyone would use non-standard crypto algorithms.
Is RFC5926 conformance really insufficient?
For the resulting hash, the scratch buffer can be used.
Honestly, I just don't see much benefit in introducing more code and
structures in order to limit hash algorithms. If anything,
:if (strcmp("hmac(sha1)", opts.algo) && strcmp("cmac(aes)", opts.algo))
: return -EPROTONOSUPPORT;
and passing the string straight to crypto seems to be better than adding
new structures.
On the other side, those two hashes MUST be supported to comply with
RFC, other may. As user can already configure conflicting receive/send
ids for MKTs, I don't see a point not allowing any hash algorithm
supported by crypto.
The algorithm enum controls not just the algorithm but the length in
bytes of the traffic key and signatures. Supporting arbitrary algorithms
requires supporting arbitrary byte lengths everywhere and increasing a
couple of stack allocations (including some in the TCP core).
An earlier version actually had u8 fields for the length but those were
later removed.
Adding this is not difficult but requires careful testing of the new
corner cases. All this will be for functionality is never going to be used.
My knowledge of cryptography doesn't go much beyond "data goes in
signature goes out" but there are many recent arguments from that cipher
agility is outright harmful and recent protocols like WireGuard don't
support any algorithm choices.
You already limit usage when root-enabled sysctl is triggered, I don't
see big concerns here.
+#define TCP_SIG_POOL_MAX 8
+static struct tcp_sig_pool_priv_t {
+ struct tcp_sig_crypto cryptos[TCP_SIG_POOL_MAX];
+ unsigned int cryptos_nr;
+} tcp_sig_pool_priv = {
+ .cryptos_nr = 1,
+ .cryptos[TCP_MD5_SIG_ID].alg = "md5",
+};
Why an array of 8? Better to use an arbitrary list.
Some reasonable limit, may be 16 or whatever in order to avoid
dynamically (re-)allocating the array and keeping O(1) lookups.
Defining an arbitrary array length limit is an underhanded way of making
lookup O(1).
