Automatically use system openssl configuration on rpm builds#13625
Automatically use system openssl configuration on rpm builds#13625adamlazik1 wants to merge 1 commit into
Conversation
|
Why are we adding a feature in the main repo, just to patch it in packaging? |
To differentiate between deb in rpm builds I believe. |
|
Differentiate how? |
|
I believe that the argument was that an edge case can happen in which in the opensslcnf.config file is missing, but we should assume that crypto-policies are always applied on an rpm system. |
|
Then check for the existence of /etc/crypto-policies/ directly? Patching at the packaging level is automatically buying you future build problems (including Packit problems), which I really would recommend against. |
| - logger.debug "No crypto-policies detected, using HIGH cipher string as default." | ||
| - 'HIGH' | ||
| - end | ||
| + 'PROFILE=SYSTEM' |
There was a problem hiding this comment.
On upstream, this either logs crypto-policies are used, or not.
On Satellite, this deletes any logging - there will be no information about crypto-policies.
There was a problem hiding this comment.
On Satellite, this deletes any logging - there will be no information about crypto-policies.
Would it make sense to log something that has no alternatives?
There was a problem hiding this comment.
I think it won't leave the user wondering. It's probably nice to see "yes, if I set crypto policies, they will be taken into account".
There was a problem hiding this comment.
Based on what @evgeni said in the comments I am reevaluating whether we should have this patch at all. If a RHEL system has crypto-policies enabled but no config file, that's a broken system and probably not something we should deal with. Opinions?
There was a problem hiding this comment.
@ekohl also said that the patch might not be necessary in here theforeman/smart-proxy#947 (comment)
There was a problem hiding this comment.
I thought the reason for this patch was to avoid autodetection during startup every time. If it isn't a problem (and I don't think it is), I also think this patch doesn't need to exist at all. A system either has crypto-policies or it doesn't, and we don't really care whether it's rpm based or something else based.
If a system is broken in a way that it does have crypto policies and doesn't have the file, it's nice to have this exception caught and communicated somehow, but for this case you don't need to explain things, just say that the file doesn't exist.
4 participants
Requires theforeman/smart-proxy#947