Skip to content

prefer system crypto policy #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

remicollet
Copy link
Contributor

@remicollet remicollet commented Jan 16, 2019

Mandatory for official repository
See

Perhaps better to have a configure option for this (like the --with-system-ciphers of PHP°

@remicollet
Copy link
Contributor Author

To summarize, it is easier for a sysadmin to alter a configuration than to change an hardcoded value in sources ;)

@VBart VBart requested a review from igorsysoev January 23, 2019 15:14
@remicollet
Copy link
Contributor Author

More explanation about why this change is needed
https://www.redhat.com/en/blog/consistent-security-crypto-policies-red-hat-enterprise-linux-8

@alejandro-colomar alejandro-colomar requested review from alejandro-colomar and removed request for igorsysoev October 20, 2022 09:41
@alejandro-colomar alejandro-colomar self-assigned this Oct 20, 2022
Comment on lines +306 to 371
if (conf->ciphers) { /* else use system crypto policy */
if (SSL_CTX_set_cipher_list(ctx, conf->ciphers) == 0) {
nxt_openssl_log_error(task, NXT_LOG_ALERT,
"SSL_CTX_set_cipher_list(\"%s\") failed",
ciphers);
goto fail;
conf->ciphers);
goto fail;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Remi!

Please detail in the commit message the mechanism by which system crypto policy is being used in the "else" case. Is it by not calling SSL_CTX_set_cipher_list(), or how?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't call SSL_CTX_set_cipher_list, then you use system default. The goal of this PR

@alejandro-colomar
Copy link
Contributor

If this PR is still necessary, please rebase it against the master branch.

@remicollet
Copy link
Contributor Author

Rebased

@alejandro-colomar
Copy link
Contributor

alejandro-colomar commented Oct 20, 2022

Looks good to me. Please include something like "If you don't call SSL_CTX_set_cipher_list, then you use system default." in the commit message, and sign the patch (signed-off-by), and I'll merge it.

Thanks!

@alejandro-colomar
Copy link
Contributor

BTW, it would also be nice if you add a changelog line into docs/changes.xml.

If you don't call SSL_CTX_set_cipher_list, then you use system default.

Signed-off-by: Remi Collet <remi@remirepo.net>
@alejandro-colomar
Copy link
Contributor

Great. I'll push it in a moment.

Thanks!

@alejandro-colomar
Copy link
Contributor

Merged.

@remicollet remicollet deleted the issue-systempolicy branch October 21, 2022 04:57
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.

2 participants