Skip to content

smartcard utils: add EC support in generate_cert()#247

Open
sumit-bose wants to merge 1 commit into
SSSD:masterfrom
sumit-bose:ec_cert
Open

smartcard utils: add EC support in generate_cert()#247
sumit-bose wants to merge 1 commit into
SSSD:masterfrom
sumit-bose:ec_cert

Conversation

@sumit-bose
Copy link
Copy Markdown
Contributor

With the new key_type option generate_cert() can generate a certificate with EC keys as well, the default are still RSA keys.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread sssd_test_framework/utils/smartcard.py
Comment on lines 200 to 207
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),
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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),
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@sumit-bose sumit-bose force-pushed the ec_cert branch 3 times, most recently from 81e5a0c to eaf55c4 Compare May 20, 2026 14:06
With the new key_type option generate_cert() can generate a certificate
with EC keys as well, the default are still RSA keys.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants