This makes it easier to maintain these functions as algorithms are
added or removed.
Details
- Reviewers
cem jmg - Commits
- rS360642: Use a lookup table of algorithm types for alg_is_* helpers.
- used test program I'll post in a bit to verify identical behavior before/after
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Test program:
#include <sys/param.h> #include "sys/opencrypto/cryptodev.h" #include <stdbool.h> #include <stdio.h> #define ALG_COMPRESSION 0x1 #define ALG_CIPHER 0x2 #define ALG_DIGEST 0x4 #define ALG_KEYED_DIGEST 0x8 #define ALG_AEAD 0x10 static int alg_flags[] = { [CRYPTO_DES_CBC] = ALG_CIPHER, [CRYPTO_3DES_CBC] = ALG_CIPHER, [CRYPTO_BLF_CBC] = ALG_CIPHER, [CRYPTO_CAST_CBC] = ALG_CIPHER, [CRYPTO_SKIPJACK_CBC] = ALG_CIPHER, [CRYPTO_MD5_HMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_SHA1_HMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_RIPEMD160_HMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_MD5_KPDK] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_SHA1_KPDK] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_AES_CBC] = ALG_CIPHER, [CRYPTO_ARC4] = ALG_CIPHER, [CRYPTO_MD5] = ALG_DIGEST, [CRYPTO_SHA1] = ALG_DIGEST, [CRYPTO_NULL_HMAC] = ALG_DIGEST, [CRYPTO_NULL_CBC] = ALG_CIPHER, [CRYPTO_DEFLATE_COMP] = ALG_COMPRESSION, [CRYPTO_SHA2_256_HMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_SHA2_384_HMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_SHA2_512_HMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_CAMELLIA_CBC] = ALG_CIPHER, [CRYPTO_AES_XTS] = ALG_CIPHER, [CRYPTO_AES_ICM] = ALG_CIPHER, [CRYPTO_AES_NIST_GMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_AES_NIST_GCM_16] = ALG_AEAD, [CRYPTO_BLAKE2B] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_BLAKE2S] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_CHACHA20] = ALG_CIPHER, [CRYPTO_SHA2_224_HMAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_RIPEMD160] = ALG_DIGEST, [CRYPTO_SHA2_224] = ALG_DIGEST, [CRYPTO_SHA2_256] = ALG_DIGEST, [CRYPTO_SHA2_384] = ALG_DIGEST, [CRYPTO_SHA2_512] = ALG_DIGEST, [CRYPTO_POLY1305] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_AES_CCM_CBC_MAC] = ALG_DIGEST | ALG_KEYED_DIGEST, [CRYPTO_AES_CCM_16] = ALG_AEAD, }; static bool alg_is_compression(int alg) { if (alg == CRYPTO_DEFLATE_COMP) return (true); return (false); } static bool alg_is_cipher(int alg) { if (alg >= CRYPTO_DES_CBC && alg <= CRYPTO_SKIPJACK_CBC) return (true); if (alg >= CRYPTO_AES_CBC && alg <= CRYPTO_ARC4) return (true); if (alg == CRYPTO_NULL_CBC) return (true); if (alg >= CRYPTO_CAMELLIA_CBC && alg <= CRYPTO_AES_ICM) return (true); if (alg == CRYPTO_CHACHA20) return (true); return (false); } static bool alg_is_digest(int alg) { if (alg >= CRYPTO_MD5_HMAC && alg <= CRYPTO_SHA1_KPDK) return (true); if (alg >= CRYPTO_MD5 && alg <= CRYPTO_SHA1) return (true); if (alg == CRYPTO_NULL_HMAC) return (true); if (alg >= CRYPTO_SHA2_256_HMAC && alg <= CRYPTO_SHA2_512_HMAC) return (true); if (alg == CRYPTO_AES_NIST_GMAC) return (true); if (alg >= CRYPTO_BLAKE2B && alg <= CRYPTO_BLAKE2S) return (true); if (alg >= CRYPTO_SHA2_224_HMAC && alg <= CRYPTO_POLY1305) return (true); if (alg == CRYPTO_AES_CCM_CBC_MAC) return (true); return (false); } static bool alg_is_keyed_digest(int alg) { if (alg >= CRYPTO_MD5_HMAC && alg <= CRYPTO_SHA1_KPDK) return (true); if (alg >= CRYPTO_SHA2_256_HMAC && alg <= CRYPTO_SHA2_512_HMAC) return (true); if (alg == CRYPTO_AES_NIST_GMAC) return (true); if (alg >= CRYPTO_BLAKE2B && alg <= CRYPTO_BLAKE2S) return (true); if (alg == CRYPTO_SHA2_224_HMAC) return (true); if (alg == CRYPTO_POLY1305) return (true); if (alg == CRYPTO_AES_CCM_CBC_MAC) return (true); return (false); } static bool alg_is_aead(int alg) { if (alg == CRYPTO_AES_NIST_GCM_16) return (true); if (alg == CRYPTO_AES_CCM_16) return (true); return (false); } static bool alg_has_flag(int alg, int flag) { if (alg < nitems(alg_flags)) return ((alg_flags[alg] & flag) == flag); return (false); } int main(void) { int i; bool ok; for (i = 0; i <= CRYPTO_ALGORITHM_MAX; i++) { ok = true; if (alg_is_compression(i) != alg_has_flag(i, ALG_COMPRESSION)) { printf("%d: compression mismatch\n", i); ok = false; } if (alg_is_cipher(i) != alg_has_flag(i, ALG_CIPHER)) { printf("%d: cipher mismatch\n", i); ok = false; } if (alg_is_digest(i) != alg_has_flag(i, ALG_DIGEST)) { printf("%d: digest mismatch\n", i); ok = false; } if (alg_is_keyed_digest(i) != alg_has_flag(i, ALG_KEYED_DIGEST)) { printf("%d: keyed digest mismatch\n", i); ok = false; } if (alg_is_aead(i) != alg_has_flag(i, ALG_AEAD)) { printf("%d: aead mismatch\n", i); ok = false; } if (ok) printf("%d: all matched\n", i); } }
sys/opencrypto/crypto.c | ||
---|---|---|
98 ↗ | (On Diff #71289) | Do these need to be flags? Is it possible for something to be a keyed digest and not a plain digest, in the sense we use for the latter? If they don’t have to be flags, it is harder to specify an invalid combination. |
717 ↗ | (On Diff #71289) | I don’t know if this is mostly convention or true definition, but usually digest is used to refer to a hash function. The non-HMAC MAC algorithms (tags) are substantially different from hashes. I don’t know if it matters here; this is probably more about input parameters and output than anything else, and those are similar. |
738 ↗ | (On Diff #71289) | If this can’t be replaced by return (alg_flags... & flag) maybe the function should be named “flags” plural? |
sys/opencrypto/crypto.c | ||
---|---|---|
98 ↗ | (On Diff #71289) | I suppose they could be an enum instead and 'alg_is_digest' could check for both types. It is true that all "keyed" digests are always digests. I didn't want to use something else like "mac" since just being a digest that uses a key doesn't necessarily make an algorithm a mac. |
717 ↗ | (On Diff #71289) | Yes, it is mostly for the session modes (CSP_MODE_DIGEST). OTOH, it's debatable if we would really support GMAC and CBC_MAC as distinct algorithms. In theory IPsec supports a GMAC-only ESP mode, though I think we implement that as GCM with an empty payload perhaps? Actually, I probably need to go retest that as I might have broken it, and I probably should do it by using a GMAC-only mode. |
LGTM, thanks!
sys/opencrypto/crypto.c | ||
---|---|---|
95–99 ↗ | (On Diff #71376) | Probably want to remove these names now. |