Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16658 )
Change subject: Increase key size in tests and EMC ...................................................................... Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/16658/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16658/3//COMMIT_MSG@11 PS3, Line 11: This commit removes the test-only key size I've been pondering over this and think it's still weird to gate this. We wouldn't need to gate only some startup flags, but we would need to use different assertions based on this in various tests. I feel this would make things needlessly complicated. > And as you mentioned, running test with different key length might show > potential issues, so adding a gate for different key length is a win-win, IMO. We don't need to care about small key sizes though, we only need to make sure that we support, and default to key sizes that and algorithms that are considered secure by modern standards. As I understand, the main concern here, at least based on comments such as the one below is speed: // Generate smaller RSA keys -- generating a 768-bit key is faster // than generating the default 2048-bit key, and we don't care about // strong encryption in tests. Setting it lower (e.g. 512 bits) results // in OpenSSL errors RSA_sign:digest too big for rsa key:rsa_sign.c:122 // since we are using strong/high TLS v1.2 cipher suites, so the minimum // size of TLS-related RSA key is 768 bits (due to the usage of // the ECDHE-RSA-AES256-GCM-SHA384 suite). While it is true that generating and using larger keys is technically more expensive, it doesn't seem to cause any problems for us, and it shouldn't either, as it's still fast enough[1], and the resources spent on crypto is only a tiny part of the overall costs of tests and benchmarks. I'd be surprised if the difference would be statistically significant over our benchmark and test suites. The above comment also mentions that it's set to the minimum key size required by TLS v1.2 cipher suites that we support. We can think of this as raising the tested key sizes to the minimum as required by FIPS 140-2, which is also something that we should support. [1] https://blog.cloudflare.com/how-expensive-is-crypto-anyway/ -- To view, visit http://gerrit.cloudera.org:8080/16658 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45b96e0b4499ea1d80db5871a529c732ad41220e Gerrit-Change-Number: 16658 Gerrit-PatchSet: 7 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Thu, 29 Oct 2020 07:40:56 +0000 Gerrit-HasComments: Yes
