-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Issue #1671: Only set SecHashKey et al. when SecHashEngine is On #1690
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
Issue #1671: Only set SecHashKey et al. when SecHashEngine is On #1690
Conversation
cfb5b5d
to
a987e79
Compare
apache2/apache2_config.c
Outdated
if (dcfg->crypto_hash_location_pm == NOT_SET) dcfg->crypto_hash_location_pm = 0; | ||
if (dcfg->crypto_hash_iframesrc_pm == NOT_SET) dcfg->crypto_hash_iframesrc_pm = 0; | ||
if (dcfg->crypto_hash_framesrc_pm == NOT_SET) dcfg->crypto_hash_framesrc_pm = 0; | ||
if (dcfg->hash_is_enabled == HASH_ENABLED) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey,
For the sake of keep things initialized, what do you think about:
if (dcfg->hash_is_enabled == HASH_ENABLED) {
if (dcfg->crypto_key == NOT_SET_P) dcfg->crypto_key = getkey(dcfg->mp);
if (dcfg->crypto_key_len == NOT_SET) dcfg->crypto_key_len = strlen(dcfg->crypto_key);
} else {
if (dcfg->crypto_key == NOT_SET_P) dcfg->crypto_key = "";
if (dcfg->crypto_key_len == NOT_SET) dcfg->crypto_key_len = 0;
}
if (dcfg->crypto_key_add == NOT_SET) dcfg->crypto_key_add = HASH_KEYONLY;
if (dcfg->crypto_param_name == NOT_SET_P) dcfg->crypto_param_name = "crypt";
if (dcfg->hash_is_enabled == NOT_SET) dcfg->hash_is_enabled = HASH_DISABLED;
if (dcfg->hash_enforcement == NOT_SET) dcfg->hash_enforcement = HASH_DISABLED;
if (dcfg->crypto_hash_href_rx == NOT_SET) dcfg->crypto_hash_href_rx = 0;
if (dcfg->crypto_hash_faction_rx == NOT_SET) dcfg->crypto_hash_faction_rx = 0;
if (dcfg->crypto_hash_location_rx == NOT_SET) dcfg->crypto_hash_location_rx = 0;
if (dcfg->crypto_hash_iframesrc_rx == NOT_SET) dcfg->crypto_hash_iframesrc_rx = 0;
if (dcfg->crypto_hash_framesrc_rx == NOT_SET) dcfg->crypto_hash_framesrc_rx = 0;
if (dcfg->crypto_hash_href_pm == NOT_SET) dcfg->crypto_hash_href_pm = 0;
if (dcfg->crypto_hash_faction_pm == NOT_SET) dcfg->crypto_hash_faction_pm = 0;
if (dcfg->crypto_hash_location_pm == NOT_SET) dcfg->crypto_hash_location_pm = 0;
if (dcfg->crypto_hash_iframesrc_pm == NOT_SET) dcfg->crypto_hash_iframesrc_pm = 0;
if (dcfg->crypto_hash_framesrc_pm == NOT_SET) dcfg->crypto_hash_framesrc_pm = 0;
Just in case people access this data regardless of hash_is_enabled state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @zimmerle, that was exactly what we were thinking on round 2, will update pull request here once our local one is working ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
a987e79
to
ab9d183
Compare
ab9d183
to
43b8490
Compare
This looks good to me. Tests are passing and SecHash is working as expected. SecHashEngine On |
Merged as of a677456 Thanks! :) |
As requested in issue #1671 by @victorhora