smartcard utils: add EC support in generate_cert()#247
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the generate_cert function in sssd_test_framework/utils/smartcard.py to support both RSA and EC key types by utilizing openssl genpkey. Feedback from the review suggests explicitly validating the key_type parameter to avoid silent defaults and re-including the sha256 flag in the certificate generation command to ensure cryptographic robustness and compatibility across different environments.
| args: CLIBuilderArgs = { | ||
| "x509": (self.cli.option.SWITCH, True), | ||
| "nodes": (self.cli.option.SWITCH, True), | ||
| "sha256": (self.cli.option.SWITCH, True), | ||
| "days": (self.cli.option.VALUE, "365"), | ||
| "newkey": (self.cli.option.VALUE, "rsa:2048"), | ||
| "keyout": (self.cli.option.VALUE, key_path), | ||
| "key": (self.cli.option.VALUE, key_path), | ||
| "out": (self.cli.option.VALUE, cert_path), | ||
| "subj": (self.cli.option.VALUE, subj), | ||
| } |
There was a problem hiding this comment.
The -sha256 option was removed from the openssl req command. While modern OpenSSL versions might default to SHA-256, explicitly specifying the hash algorithm for the certificate signature ensures compatibility and prevents the use of weaker algorithms on older systems or with different configurations. Please add it back for improved security and robustness.
| args: CLIBuilderArgs = { | |
| "x509": (self.cli.option.SWITCH, True), | |
| "nodes": (self.cli.option.SWITCH, True), | |
| "sha256": (self.cli.option.SWITCH, True), | |
| "days": (self.cli.option.VALUE, "365"), | |
| "newkey": (self.cli.option.VALUE, "rsa:2048"), | |
| "keyout": (self.cli.option.VALUE, key_path), | |
| "key": (self.cli.option.VALUE, key_path), | |
| "out": (self.cli.option.VALUE, cert_path), | |
| "subj": (self.cli.option.VALUE, subj), | |
| } | |
| args: CLIBuilderArgs = { | |
| "x509": (self.cli.option.SWITCH, True), | |
| "nodes": (self.cli.option.SWITCH, True), | |
| "sha256": (self.cli.option.SWITCH, True), | |
| "days": (self.cli.option.VALUE, "365"), | |
| "key": (self.cli.option.VALUE, key_path), | |
| "out": (self.cli.option.VALUE, cert_path), | |
| "subj": (self.cli.option.VALUE, subj), | |
| } |
There was a problem hiding this comment.
Hi,
since sha256 cannot be used with EC keys I just removed it and let openssl pick a suitable hash function based on its configuration. If preferred I can add args to the if-block as well to set sha256 for RSA. But I think this is not necessary. Additionally, not hard-coding the hash function will give the opportunity to change it with the help of an OpenSSL configuration file.
bye,
Sumit
81e5a0c to
eaf55c4
Compare
With the new key_type option generate_cert() can generate a certificate with EC keys as well, the default are still RSA keys.
With the new key_type option generate_cert() can generate a certificate with EC keys as well, the default are still RSA keys.