ltp: new test for crypto_user information leak bug
v2 cover ltp: new test for crypto_user information leak bug
This series adds a test for an information leak bug in the crypto user configuration API which was recently fixed.
Changed since v1:
- Check all attributes, not just the first of each type
- Work around dependency bug in old kernel headers
- Rename __check_for_leaks() to do_check_for_leaks()
- Use SPDX license identifier
Eric Biggers (3):
lapi/cryptouser.h: add more declarations
tst_netlink: inline functions in header crypto/crypto_user01.c: new test for information leak bug
include/lapi/cryptouser.h | 73 +++++++++ include/tst_netlink.h | 11 +- runtest/crypto | 1 + runtest/cve | 2 + testcases/kernel/crypto/.gitignore | 1 + testcases/kernel/crypto/crypto_user01.c | 195 ++++++++++++++++++++++++ 6 files changed, 278 insertions(+), 5 deletions(-) create mode 100644 testcases/kernel/crypto/crypto_user01.c
Signature · 1 line
2.19.2
PATCH v2 1/3 lapi/cryptouser.h: add more declarations
From: Eric Biggers <[email protected]>
Add some missing declarations for the crypto user configuration API, a.k.a. NETLINK_CRYPTO.
--- include/lapi/cryptouser.h | 73 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+)
include/lapi/cryptouser.h +73 -0
@@ -34,6 +34,24 @@ enum {__CRYPTO_MSG_MAX }; +enum crypto_attr_type_t { +CRYPTOCFGA_UNSPEC , +CRYPTOCFGA_PRIORITY_VAL ,/* uint32_t */ +CRYPTOCFGA_REPORT_LARVAL ,/* struct crypto_report_larval */ +CRYPTOCFGA_REPORT_HASH ,/* struct crypto_report_hash */ +CRYPTOCFGA_REPORT_BLKCIPHER ,/* struct crypto_report_blkcipher */ +CRYPTOCFGA_REPORT_AEAD ,/* struct crypto_report_aead */ +CRYPTOCFGA_REPORT_COMPRESS ,/* struct crypto_report_comp */ +CRYPTOCFGA_REPORT_RNG ,/* struct crypto_report_rng */ +CRYPTOCFGA_REPORT_CIPHER ,/* struct crypto_report_cipher */ +CRYPTOCFGA_REPORT_AKCIPHER ,/* struct crypto_report_akcipher */ +CRYPTOCFGA_REPORT_KPP ,/* struct crypto_report_kpp */ +CRYPTOCFGA_REPORT_ACOMP ,/* struct crypto_report_acomp */ + __CRYPTOCFGA_MAX + +#define CRYPTOCFGA_MAX (__CRYPTOCFGA_MAX - 1) +}; +struct crypto_user_alg {char cru_name[CRYPTO_MAX_NAME ];char cru_driver_name[CRYPTO_MAX_NAME ];
@@ -44,6 +62,61 @@ struct crypto_user_alg {uint32_t cru_flags; }; +struct crypto_report_larval { +char type[CRYPTO_MAX_NAME ]; +}; + +struct crypto_report_hash { +char type[CRYPTO_MAX_NAME ]; +unsigned int blocksize; +unsigned int digestsize; +}; + +struct crypto_report_cipher { +char type[CRYPTO_MAX_NAME ]; +unsigned int blocksize; +unsigned int min_keysize; +unsigned int max_keysize; +}; + +struct crypto_report_blkcipher { +char type[CRYPTO_MAX_NAME ]; +char geniv[CRYPTO_MAX_NAME ]; +unsigned int blocksize; +unsigned int min_keysize; +unsigned int max_keysize; +unsigned int ivsize; +}; + +struct crypto_report_aead { +char type[CRYPTO_MAX_NAME ]; +char geniv[CRYPTO_MAX_NAME ]; +unsigned int blocksize; +unsigned int maxauthsize; +unsigned int ivsize; +}; + +struct crypto_report_comp { +char type[CRYPTO_MAX_NAME ]; +}; + +struct crypto_report_rng { +char type[CRYPTO_MAX_NAME ]; +unsigned int seedsize; +}; + +struct crypto_report_akcipher { +char type[CRYPTO_MAX_NAME ]; +}; + +struct crypto_report_kpp { +char type[CRYPTO_MAX_NAME ]; +}; + +struct crypto_report_acomp { +char type[CRYPTO_MAX_NAME ]; +}; +#endif /* HAVE_LINUX_CRYPTOUSER_H */ /* These are taken from include/crypto.h in the kernel tree. They are not
Signature · 1 line
2.19.2
PATCH v2 2/3 tst_netlink: inline functions in header
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
safe_netlink_send() and safe_netlink_recv() need to be static inline, since they're defined in a .h file.
--- include/tst_netlink.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
include/tst_netlink.h +6 -5
@@ -33,9 +33,9 @@#endif /** @private */ -ssize_t safe_netlink_send (const char *file,const int lineno, -int fd,const struct nlmsghdr *nh, -const void *payload) +static inline ssize_t safe_netlink_send (const char *file,const int lineno, +int fd,const struct nlmsghdr *nh, +const void *payload) {struct sockaddr_nl sa = { .nl_family =AF_NETLINK };struct iovec iov[2 ] = {
@@ -68,8 +68,9 @@ ssize_t safe_netlink_send(const char *file, const int lineno,safe_netlink_send (__FILE__, __LINE__, fd, nl_header, payload) /** @private */ -ssize_t safe_netlink_recv (const char *file,const int lineno, -int fd,char *nl_headers_buf,size_t buf_len) +static inline ssize_t safe_netlink_recv (const char *file,const int lineno, +int fd,char *nl_headers_buf, +size_t buf_len) {struct iovec iov = { nl_headers_buf, buf_len };struct sockaddr_nl sa;
Signature · 1 line
2.19.2
PATCH v2 3/3 crypto/crypto_user01.c: new test for information leak bug
In reply to Eric BiggersFrom: Eric Biggers <[email protected]>
Test for a bug in the crypto user configuration API (NETLINK_CRYPTO) that leaked uninitialized memory to userspace. This bug was assigned CVE-2018-19854, and it was also a re-introduction of CVE-2013-2547.
--- runtest/crypto | 1 + runtest/cve | 2 + testcases/kernel/crypto/.gitignore | 1 + testcases/kernel/crypto/crypto_user01.c | 195 ++++++++++++++++++++++++ 4 files changed, 199 insertions(+) create mode 100644 testcases/kernel/crypto/crypto_user01.c
runtest/crypto +1 -0
@@ -1 +1,2 @@ pcrypt_aead01 pcrypt_aead01 +crypto_user01 crypto_user01
runtest/cve +2 -0
@@ -3,6 +3,7 @@ cve-2011-0999 thp01 -I 120 cve-2011-2183 ksm05 -I 10 cve-2011-2496 vma03 cve-2012-0957 uname04 +cve-2013-2547 crypto_user01 cve-2014-0196 cve-2014-0196 cve-2015-0235 gethostbyname_r01 cve-2015-7550 keyctl02
@@ -36,3 +37,4 @@ cve-2017-17053 cve-2017-17053 cve-2017-18075 pcrypt_aead01 cve-2018-5803 sctp_big_chunk cve-2018-1000001 realpath01 +cve-2018-19854 crypto_user01
testcases/kernel/crypto/.gitignore +1 -0
@@ -1 +1,2 @@ pcrypt_aead01 +crypto_user01
testcases/kernel/crypto/crypto_user01.c new file +195 -0
195 lines not on this page · raw message
Signature · 1 line
2.19.2
PATCH v2 3/3 crypto/crypto_user01.c: new test for information leak bug
Hi!
Quoted 18 lines
runtest/crypto +1
-0@@ -1 +1,2 @@ pcrypt_aead01 pcrypt_aead01 +crypto_user01 crypto_user01runtest/cve +1
-0@@ -3,6 +3,7 @@ cve-2011-0999 thp01 -I 120 cve-2011-2183 ksm05 -I 10 cve-2011-2496 vma03 cve-2012-0957 uname04 +cve-2013-2547 crypto_user01 cve-2014-0196 cve-2014-0196 cve-2015-0235 gethostbyname_r01 cve-2015-7550 keyctl02
runtest/cve cve-2017-17053 cve-2017-17053 · line 40
Quoted 4 lines
runtest/cve +1
-0@@ -36,3 +37,4 @@ cve-2017-17053 cve-2017-17053 cve-2017-18075 pcrypt_aead01 cve-2018-5803 sctp_big_chunk cve-2018-1000001 realpath01 +cve-2018-19854 crypto_user01
Does it really make sense to run this test twice just under different name?
BTW: I'm working on test tags and metadata patches that would make failed tests print list of CVEs and kernel commits that fixed them on a failure and also under the -h help switch, which should fix this once for all.
See:
http://lists.linux.it/pipermail/ltp/2018-November/009895.html
http://lists.linux.it/pipermail/ltp/2018-November/010118.html
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v2 3/3 crypto/crypto_user01.c: new test for information leak bug
Hi Cyril,
Quoted 32 lines · Cyril Hrubis
Hi!
Quoted 18 lines
runtest/crypto +1
-0@@ -1 +1,2 @@ pcrypt_aead01 pcrypt_aead01 +crypto_user01 crypto_user01runtest/cve +1
-0@@ -3,6 +3,7 @@ cve-2011-0999 thp01 -I 120 cve-2011-2183 ksm05 -I 10 cve-2011-2496 vma03 cve-2012-0957 uname04 +cve-2013-2547 crypto_user01 cve-2014-0196 cve-2014-0196 cve-2015-0235 gethostbyname_r01 cve-2015-7550 keyctl02runtest/cve cve-2017-17053 cve-2017-17053 · line 40
Quoted 4 lines
runtest/cve +1
-0@@ -36,3 +37,4 @@ cve-2017-17053 cve-2017-17053 cve-2017-18075 pcrypt_aead01 cve-2018-5803 sctp_big_chunk cve-2018-1000001 realpath01 +cve-2018-19854 crypto_user01Does it really make sense to run this test twice just under different name?
BTW: I'm working on test tags and metadata patches that would make failed tests print list of CVEs and kernel commits that fixed them on a failure and also under the -h help switch, which should fix this once for all.
Quoted 5 lines
See:
http://lists.linux.it/pipermail/ltp/2018-November/009895.html
http://lists.linux.it/pipermail/ltp/2018-November/010118.html
No it doesn't really make sense to run a test twice, but this has two associated CVE numbers, resulting in two runs when tests are run by CVE number. Are you saying you'd prefer that it be listed under just the more recent CVE? Or are you saying the runtest/cve file will be going away and replaced by something else anyway?
- Eric
PATCH v2 3/3 crypto/crypto_user01.c: new test for information leak bug
Hi!
Quoted 5 lines
No it doesn't really make sense to run a test twice, but this has two associated CVE numbers, resulting in two runs when tests are run by CVE number. Are you saying you'd prefer that it be listed under just the more recent CVE? Or are you saying the runtest/cve file will be going away and replaced by something else anyway?
For now I would just put it under the newer CVE number.
For a future I would like to be able to export all the metadata from the tests to the tesrunner, so the testrunner would know, among other things, which tests are associated with CVEs. That, in turn, would eliminate the need to maintain various different sets of runtest files.
But given that I just started to work on that I do not expect it to be finished anytime soon.
Does that sounds reasonable to you?
Also no need to resent the patchset, I can remove the runtest entry when applying the patch.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v2 3/3 crypto/crypto_user01.c: new test for information leak bug
Hi Cyril,
Quoted 14 lines · Cyril Hrubis
Hi!
Quoted 5 lines
No it doesn't really make sense to run a test twice, but this has two associated CVE numbers, resulting in two runs when tests are run by CVE number. Are you saying you'd prefer that it be listed under just the more recent CVE? Or are you saying the runtest/cve file will be going away and replaced by something else anyway?
For now I would just put it under the newer CVE number.
For a future I would like to be able to export all the metadata from the tests to the tesrunner, so the testrunner would know, among other things, which tests are associated with CVEs. That, in turn, would eliminate the need to maintain various different sets of runtest files.
Quoted 8 lines
But given that I just started to work on that I do not expect it to be finished anytime soon.
Does that sounds reasonable to you?
Also no need to resent the patchset, I can remove the runtest entry when applying the patch.
Yes that sounds reasonable to me, please feel free to just put it under the newer CVE number. Thanks!
- Eric
PATCH v2 3/3 crypto/crypto_user01.c: new test for information leak bug
Hi!
Quoted 2 lines
Yes that sounds reasonable to me, please feel free to just put it under the newer CVE number. Thanks!
Pushed, thanks.
Signature · 2 lines
Cyril Hrubis [email protected]
v2 cover ltp: new test for crypto_user information leak bug
In reply to Eric BiggersHello Eric,
Quoted 8 lines · Eric Biggers
This series adds a test for an information leak bug in the crypto user configuration API which was recently fixed.
Changed since v1:
- Check all attributes, not just the first of each type
- Work around dependency bug in old kernel headers
- Rename__check_for_leaks()todo_check_for_leaks()
- Use SPDX license identifier
Thanks, I tested it and it LGTM.
Signature · 2 lines
Thank you, Richard.