- 
                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
          
        
      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