Re: [PATCH v2] crypto: prefix module autoloading with crypto-
On 18 November 2014 01:45, Kees Cook keesc...@chromium.org wrote: On Mon, Nov 17, 2014 at 3:20 PM, Mathias Krause mini...@googlemail.com wrote: On 17 November 2014 21:02, Kees Cook keesc...@chromium.org wrote: This prefixes all crypto module loading with crypto- so we never run the risk of exposing module auto-loading to userspace via a crypto API, as demonstrated by Mathias Krause: https://lkml.org/lkml/2013/3/4/70 Signed-off-by: Kees Cook keesc...@chromium.org --- v2: - added missing #include, thanks to minipli - built with allmodconfig [...snip...] diff --git a/include/linux/crypto.h b/include/linux/crypto.h index d45e949699ea..d14230f6e977 100644 --- a/include/linux/crypto.h +++ b/include/linux/crypto.h @@ -26,6 +26,13 @@ #include linux/uaccess.h /* + * Autoloaded crypto modules should only use a prefixed name to avoid allowing + * arbitrary modules to be loaded. + */ +#define MODULE_ALIAS_CRYPTO(name) \ + MODULE_ALIAS(crypto- name) This would break userland relying on the old aliases, e.g. 'modprobe aes' no longer works. Why not have both aliases, one with the crypto- prefix for on-demand loading within the crypto API and one without for manual loading from userland? E.g., something like this: #define MODULE_ALIAS_CRYPTO(name) \ MODULE_ALIAS(name); \ MODULE_ALIAS(crypto- name) That would prevent the userland breakage and still achieves the goal of restricting the request_module() call offered by the means of the AF_ALG API. That was my intention originally, and I should go back to it. The trouble is with the use of __UNIQUE_ID in the MODULE_ALIAS macro. It uses __LINE__ to produce the id, so the suggested macro expansion (which is what I started with) won't work on non-gcc compilers. I haven't found any solutions for C89 version of gcc's __COUNTER__, and I haven't found any C89 ways to force a macro to be expanded as being multi-line. Well, clang should support it as well, according to [1]. But still, a compiler independent solution would be nice. Anyway, the __COUNTER__ support is gcc = 4.3 only. So, according to Documentation/Changes, stating gcc 3.2 is the minimum supported version for compiling the kernel, this would be a no-go, too. [1] http://clang.llvm.org/docs/LanguageExtensions.html#builtin-macros I'd like to avoid having to open-code both MODULE_ALIAS and MODULE_ALIAS_CRYPTO in each module's source. Anyone see some sneaky way to accomplish this? Unfortunately, I do not ... beside this, maybe: #define MODULE_ALIAS_CRYPTO(name) \ __MODULE_INFO(alias, alias_userland, name); \ __MODULE_INFO(alias, alias_crypto, crypto- name) Looks ugly, but works. ;) Regards, Mathias -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] crypto: prefix module autoloading with crypto-
On Tue, Nov 18, 2014 at 3:12 PM, Mathias Krause mini...@googlemail.com wrote: On 18 November 2014 01:45, Kees Cook keesc...@chromium.org wrote: On Mon, Nov 17, 2014 at 3:20 PM, Mathias Krause mini...@googlemail.com wrote: On 17 November 2014 21:02, Kees Cook keesc...@chromium.org wrote: This prefixes all crypto module loading with crypto- so we never run the risk of exposing module auto-loading to userspace via a crypto API, as demonstrated by Mathias Krause: https://lkml.org/lkml/2013/3/4/70 Signed-off-by: Kees Cook keesc...@chromium.org --- v2: - added missing #include, thanks to minipli - built with allmodconfig [...snip...] diff --git a/include/linux/crypto.h b/include/linux/crypto.h index d45e949699ea..d14230f6e977 100644 --- a/include/linux/crypto.h +++ b/include/linux/crypto.h @@ -26,6 +26,13 @@ #include linux/uaccess.h /* + * Autoloaded crypto modules should only use a prefixed name to avoid allowing + * arbitrary modules to be loaded. + */ +#define MODULE_ALIAS_CRYPTO(name) \ + MODULE_ALIAS(crypto- name) This would break userland relying on the old aliases, e.g. 'modprobe aes' no longer works. Why not have both aliases, one with the crypto- prefix for on-demand loading within the crypto API and one without for manual loading from userland? E.g., something like this: #define MODULE_ALIAS_CRYPTO(name) \ MODULE_ALIAS(name); \ MODULE_ALIAS(crypto- name) That would prevent the userland breakage and still achieves the goal of restricting the request_module() call offered by the means of the AF_ALG API. That was my intention originally, and I should go back to it. The trouble is with the use of __UNIQUE_ID in the MODULE_ALIAS macro. It uses __LINE__ to produce the id, so the suggested macro expansion (which is what I started with) won't work on non-gcc compilers. I haven't found any solutions for C89 version of gcc's __COUNTER__, and I haven't found any C89 ways to force a macro to be expanded as being multi-line. Well, clang should support it as well, according to [1]. But still, a compiler independent solution would be nice. Anyway, the __COUNTER__ support is gcc = 4.3 only. So, according to Documentation/Changes, stating gcc 3.2 is the minimum supported version for compiling the kernel, this would be a no-go, too. [1] http://clang.llvm.org/docs/LanguageExtensions.html#builtin-macros Yeah, it's the avr32 auto builder that throws the warnings, so I assume it's using a pre-4.3 gcc. I'd like to avoid having to open-code both MODULE_ALIAS and MODULE_ALIAS_CRYPTO in each module's source. Anyone see some sneaky way to accomplish this? Unfortunately, I do not ... beside this, maybe: #define MODULE_ALIAS_CRYPTO(name) \ __MODULE_INFO(alias, alias_userland, name); \ __MODULE_INFO(alias, alias_crypto, crypto- name) Looks ugly, but works. ;) AAh! Yes, that's perfect! I will spin it this way. Thank you thank you! -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] crypto: prefix module autoloading with crypto-
This prefixes all crypto module loading with crypto- so we never run the risk of exposing module auto-loading to userspace via a crypto API, as demonstrated by Mathias Krause: https://lkml.org/lkml/2013/3/4/70 Signed-off-by: Kees Cook keesc...@chromium.org --- v2: - added missing #include, thanks to minipli - built with allmodconfig --- arch/arm/crypto/aes_glue.c | 4 ++-- arch/arm/crypto/sha1_glue.c | 2 +- arch/arm/crypto/sha1_neon_glue.c| 2 +- arch/arm/crypto/sha512_neon_glue.c | 4 ++-- arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +- arch/arm64/crypto/aes-glue.c| 8 arch/powerpc/crypto/sha1.c | 2 +- arch/s390/crypto/aes_s390.c | 2 +- arch/s390/crypto/des_s390.c | 4 ++-- arch/s390/crypto/ghash_s390.c | 2 +- arch/s390/crypto/sha1_s390.c| 2 +- arch/s390/crypto/sha256_s390.c | 4 ++-- arch/s390/crypto/sha512_s390.c | 4 ++-- arch/sparc/crypto/aes_glue.c| 2 +- arch/sparc/crypto/camellia_glue.c | 2 +- arch/sparc/crypto/crc32c_glue.c | 2 +- arch/sparc/crypto/des_glue.c| 2 +- arch/sparc/crypto/md5_glue.c| 2 +- arch/sparc/crypto/sha1_glue.c | 2 +- arch/sparc/crypto/sha256_glue.c | 4 ++-- arch/sparc/crypto/sha512_glue.c | 4 ++-- arch/x86/crypto/aes_glue.c | 4 ++-- arch/x86/crypto/aesni-intel_glue.c | 2 +- arch/x86/crypto/blowfish_glue.c | 4 ++-- arch/x86/crypto/camellia_aesni_avx2_glue.c | 4 ++-- arch/x86/crypto/camellia_aesni_avx_glue.c | 4 ++-- arch/x86/crypto/camellia_glue.c | 4 ++-- arch/x86/crypto/cast5_avx_glue.c| 2 +- arch/x86/crypto/cast6_avx_glue.c| 2 +- arch/x86/crypto/crc32-pclmul_glue.c | 4 ++-- arch/x86/crypto/crc32c-intel_glue.c | 4 ++-- arch/x86/crypto/crct10dif-pclmul_glue.c | 4 ++-- arch/x86/crypto/des3_ede_glue.c | 8 arch/x86/crypto/ghash-clmulni-intel_glue.c | 2 +- arch/x86/crypto/salsa20_glue.c | 4 ++-- arch/x86/crypto/serpent_avx2_glue.c | 4 ++-- arch/x86/crypto/serpent_avx_glue.c | 2 +- arch/x86/crypto/serpent_sse2_glue.c | 2 +- arch/x86/crypto/sha1_ssse3_glue.c | 2 +- arch/x86/crypto/sha256_ssse3_glue.c | 4 ++-- arch/x86/crypto/sha512_ssse3_glue.c | 4 ++-- arch/x86/crypto/twofish_avx_glue.c | 2 +- arch/x86/crypto/twofish_glue.c | 4 ++-- arch/x86/crypto/twofish_glue_3way.c | 4 ++-- crypto/842.c| 1 + crypto/aes_generic.c| 2 +- crypto/ansi_cprng.c | 2 +- crypto/anubis.c | 1 + crypto/api.c| 4 ++-- crypto/arc4.c | 1 + crypto/blowfish_generic.c | 2 +- crypto/camellia_generic.c | 2 +- crypto/cast5_generic.c | 2 +- crypto/cast6_generic.c | 2 +- crypto/ccm.c| 4 ++-- crypto/crc32.c | 1 + crypto/crc32c_generic.c | 2 +- crypto/crct10dif_generic.c | 2 +- crypto/crypto_null.c| 6 +++--- crypto/ctr.c| 2 +- crypto/deflate.c| 2 +- crypto/des_generic.c| 2 +- crypto/fcrypt.c | 1 + crypto/gcm.c| 6 +++--- crypto/ghash-generic.c | 2 +- crypto/khazad.c | 1 + crypto/krng.c | 2 +- crypto/lz4.c| 1 + crypto/lz4hc.c | 1 + crypto/lzo.c| 1 + crypto/md4.c| 2 +- crypto/md5.c| 1 + crypto/michael_mic.c| 1 + crypto/rmd128.c | 1 + crypto/rmd160.c | 1 + crypto/rmd256.c | 1 + crypto/rmd320.c | 1 + crypto/salsa20_generic.c| 2 +- crypto/seed.c | 1 + crypto/serpent_generic.c| 4 ++-- crypto/sha1_generic.c | 2 +- crypto/sha256_generic.c | 4 ++-- crypto/sha512_generic.c | 4 ++-- crypto/tea.c| 4 ++-- crypto/tgr192.c | 4 ++-- crypto/twofish_generic.c| 2 +- crypto/wp512.c | 4 ++-- crypto/zlib.c | 1 +
Re: [PATCH v2] crypto: prefix module autoloading with crypto-
On 17 November 2014 21:02, Kees Cook keesc...@chromium.org wrote: This prefixes all crypto module loading with crypto- so we never run the risk of exposing module auto-loading to userspace via a crypto API, as demonstrated by Mathias Krause: https://lkml.org/lkml/2013/3/4/70 Signed-off-by: Kees Cook keesc...@chromium.org --- v2: - added missing #include, thanks to minipli - built with allmodconfig --- arch/arm/crypto/aes_glue.c | 4 ++-- arch/arm/crypto/sha1_glue.c | 2 +- arch/arm/crypto/sha1_neon_glue.c| 2 +- arch/arm/crypto/sha512_neon_glue.c | 4 ++-- arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +- arch/arm64/crypto/aes-glue.c| 8 arch/powerpc/crypto/sha1.c | 2 +- arch/s390/crypto/aes_s390.c | 2 +- arch/s390/crypto/des_s390.c | 4 ++-- arch/s390/crypto/ghash_s390.c | 2 +- arch/s390/crypto/sha1_s390.c| 2 +- arch/s390/crypto/sha256_s390.c | 4 ++-- arch/s390/crypto/sha512_s390.c | 4 ++-- arch/sparc/crypto/aes_glue.c| 2 +- arch/sparc/crypto/camellia_glue.c | 2 +- arch/sparc/crypto/crc32c_glue.c | 2 +- arch/sparc/crypto/des_glue.c| 2 +- arch/sparc/crypto/md5_glue.c| 2 +- arch/sparc/crypto/sha1_glue.c | 2 +- arch/sparc/crypto/sha256_glue.c | 4 ++-- arch/sparc/crypto/sha512_glue.c | 4 ++-- arch/x86/crypto/aes_glue.c | 4 ++-- arch/x86/crypto/aesni-intel_glue.c | 2 +- arch/x86/crypto/blowfish_glue.c | 4 ++-- arch/x86/crypto/camellia_aesni_avx2_glue.c | 4 ++-- arch/x86/crypto/camellia_aesni_avx_glue.c | 4 ++-- arch/x86/crypto/camellia_glue.c | 4 ++-- arch/x86/crypto/cast5_avx_glue.c| 2 +- arch/x86/crypto/cast6_avx_glue.c| 2 +- arch/x86/crypto/crc32-pclmul_glue.c | 4 ++-- arch/x86/crypto/crc32c-intel_glue.c | 4 ++-- arch/x86/crypto/crct10dif-pclmul_glue.c | 4 ++-- arch/x86/crypto/des3_ede_glue.c | 8 arch/x86/crypto/ghash-clmulni-intel_glue.c | 2 +- arch/x86/crypto/salsa20_glue.c | 4 ++-- arch/x86/crypto/serpent_avx2_glue.c | 4 ++-- arch/x86/crypto/serpent_avx_glue.c | 2 +- arch/x86/crypto/serpent_sse2_glue.c | 2 +- arch/x86/crypto/sha1_ssse3_glue.c | 2 +- arch/x86/crypto/sha256_ssse3_glue.c | 4 ++-- arch/x86/crypto/sha512_ssse3_glue.c | 4 ++-- arch/x86/crypto/twofish_avx_glue.c | 2 +- arch/x86/crypto/twofish_glue.c | 4 ++-- arch/x86/crypto/twofish_glue_3way.c | 4 ++-- crypto/842.c| 1 + crypto/aes_generic.c| 2 +- crypto/ansi_cprng.c | 2 +- crypto/anubis.c | 1 + crypto/api.c| 4 ++-- crypto/arc4.c | 1 + crypto/blowfish_generic.c | 2 +- crypto/camellia_generic.c | 2 +- crypto/cast5_generic.c | 2 +- crypto/cast6_generic.c | 2 +- crypto/ccm.c| 4 ++-- crypto/crc32.c | 1 + crypto/crc32c_generic.c | 2 +- crypto/crct10dif_generic.c | 2 +- crypto/crypto_null.c| 6 +++--- crypto/ctr.c| 2 +- crypto/deflate.c| 2 +- crypto/des_generic.c| 2 +- crypto/fcrypt.c | 1 + crypto/gcm.c| 6 +++--- crypto/ghash-generic.c | 2 +- crypto/khazad.c | 1 + crypto/krng.c | 2 +- crypto/lz4.c| 1 + crypto/lz4hc.c | 1 + crypto/lzo.c| 1 + crypto/md4.c| 2 +- crypto/md5.c| 1 + crypto/michael_mic.c| 1 + crypto/rmd128.c | 1 + crypto/rmd160.c | 1 + crypto/rmd256.c | 1 + crypto/rmd320.c | 1 + crypto/salsa20_generic.c| 2 +- crypto/seed.c | 1 + crypto/serpent_generic.c| 4 ++-- crypto/sha1_generic.c | 2 +- crypto/sha256_generic.c | 4 ++-- crypto/sha512_generic.c | 4 ++-- crypto/tea.c| 4 ++-- crypto/tgr192.c | 4 ++--
Re: [PATCH v2] crypto: prefix module autoloading with crypto-
On Mon, Nov 17, 2014 at 3:20 PM, Mathias Krause mini...@googlemail.com wrote: On 17 November 2014 21:02, Kees Cook keesc...@chromium.org wrote: This prefixes all crypto module loading with crypto- so we never run the risk of exposing module auto-loading to userspace via a crypto API, as demonstrated by Mathias Krause: https://lkml.org/lkml/2013/3/4/70 Signed-off-by: Kees Cook keesc...@chromium.org --- v2: - added missing #include, thanks to minipli - built with allmodconfig [...snip...] diff --git a/include/linux/crypto.h b/include/linux/crypto.h index d45e949699ea..d14230f6e977 100644 --- a/include/linux/crypto.h +++ b/include/linux/crypto.h @@ -26,6 +26,13 @@ #include linux/uaccess.h /* + * Autoloaded crypto modules should only use a prefixed name to avoid allowing + * arbitrary modules to be loaded. + */ +#define MODULE_ALIAS_CRYPTO(name) \ + MODULE_ALIAS(crypto- name) This would break userland relying on the old aliases, e.g. 'modprobe aes' no longer works. Why not have both aliases, one with the crypto- prefix for on-demand loading within the crypto API and one without for manual loading from userland? E.g., something like this: #define MODULE_ALIAS_CRYPTO(name) \ MODULE_ALIAS(name); \ MODULE_ALIAS(crypto- name) That would prevent the userland breakage and still achieves the goal of restricting the request_module() call offered by the means of the AF_ALG API. That was my intention originally, and I should go back to it. The trouble is with the use of __UNIQUE_ID in the MODULE_ALIAS macro. It uses __LINE__ to produce the id, so the suggested macro expansion (which is what I started with) won't work on non-gcc compilers. I haven't found any solutions for C89 version of gcc's __COUNTER__, and I haven't found any C89 ways to force a macro to be expanded as being multi-line. I'd like to avoid having to open-code both MODULE_ALIAS and MODULE_ALIAS_CRYPTO in each module's source. Anyone see some sneaky way to accomplish this? -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
