Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16658 )
Change subject: KUDU-3210 Increase key size in tests and EMC ...................................................................... Patch Set 3: (3 comments) 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 w Right: smaller key sizes result in less CPU resources consumed not only during key generation, but also during signature signing tokens, signature verification, encryption on the wire, etc. We kept the key sizes as small as possible in tests because the majority of our tests are about functionality not directly related to crypto stuff, so the idea is to have bare minimum at the crypto side, when possible. I guess we should switch to bigger keys if that complexity is something substantial, but I don't see why it would be so. What complexity do you see regarding using different assertions in various tests based on the gate flag? E.g., I don't see any changes in assertions in this changelist, so I'm not sure why would they appear once we start using different key length based on the proposed gate flag? We don't assume to run different parts of our tests suite with different value of the FIPS mode gate flag. I'm cautious about bumping up key sizes because I don't know how much impact that would be for our tests. If taking about speed and what part of overall costs of benchmarks crypto-related stuff is taking, I'm curious how much is that if running the whole suite of the tests on the same machine with short and FIPS-required key length? Or at least, maybe the the following tests: * token-test * registration-test * tls_socket-test * token_signer-itest * master_cert_authority-itest I guess we are interested to see not only time taken, but also CPU consumed (so, running some of the tests above under perf-stat might be a good idea as well). I'd think that if the overall increase is less than 3%, we probably should switch to bigger keys. http://gerrit.cloudera.org:8080/#/c/16658/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16658/4//COMMIT_MSG@10 PS4, Line 10: these tests > These aren't required to make the tests pass in a FIPS environment, I only I see; I thought those tweaks were necessary to pass (I missed the important 'even' part). http://gerrit.cloudera.org:8080/#/c/16658/3/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/16658/3/src/kudu/mini-cluster/external_mini_cluster.cc@a990 PS3, Line 990: I'd suggest you run the tests on RHEL/CentOS 8.1 if you are about to remove this (security level 2 mandates size for RSA keys to be at least 2048). See https://github.com/apache/kudu/commit/93e85876f472b2668604ce5c15eafb17ce303989 for details. -- 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: 3 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: Fri, 30 Oct 2020 08:25:47 +0000 Gerrit-HasComments: Yes
