Story #6291
closed
SigningService should issue a warning if the signing script has changed on disk
0%
Description
Currently, signing services like AsciiArmoredDetachedSigningService which inherit from SigningService must verify that the signing script provided by the user produces valid signatures as expected by the signing service before the signing service can be saved.
However, there is no guarantee that the signing script is not changed (for one that is potentially broken) after the signing service has been saved.
The proposal is to store the hash of the signing service when saving the service and checking the actual hash of the script against this stored value whenever the sign() function is called.
After some discussion on irc we concluded that an incorrect hash should merely produce a warning, since there may be legitimate reasons for the script to change and since this check is insufficient to guard against a malicious actor (only against accidental breakage).
One possibility would be to rerun the verification if a hash value mismatch is found.
Updated by mihai.ibanescu@gmail.com over 4 years ago
Been there - ansible playbook failed to distribute the script on all the worker nodes. To me, this is an operational concern, not a security concern.
Letting the user know is good.
My concern with a warning is that it won't be noticed, and metadata will be signed with the wrong signature, or not signed correctly.
To protect the consumer (user fetching packages from a repository), I think I'd prefer to serve an older publication instead of a broken/incorrect one.
Is there a downside to failing the publication altogether if the signing service changed its signature?
Yes, there are legitimate reasons the script may change on disk, but whoever changes them should update the signing service in the database as well.
Updated by mdellweg over 4 years ago
mihai.ibanescu@gmail.com wrote: [...]
Yes, there are legitimate reasons the script may change on disk, but whoever changes them should update the signing service in the database as well.
I agree. Even more so, if the change happens via automation.
The next question that comes to my mind is, whether we need to protect from changing it while a publication is using it? Should we add locking on that resource? Or should we make it read only and in case you need to change the script, you better create a new one?
Updated by mihai.ibanescu@gmail.com over 4 years ago
Preventing publications from attempting to sign while the script is being deployed on disk and updated in the DB would be nice.
I like the idea of asking the user to deploy a new signing service (at a new path) instead of modifying the existing script, but if you (like I) have this automated with Ansible, it makes things harder.
There is a "create signing service" management script that was discussed. Maybe the "update signing service" is slightly different:
- automation deploys script not in the final location, but with a .tmp extension
- "update signing service" moves the file from .tmp to the final location and update the database in a single step. It won't be atomic, but the the race window is closer.
Updated by fao89 over 4 years ago
- Tracker changed from Issue to Story
- % Done set to 0
Updated by bmbouter over 4 years ago
I think having to go back and update the db object is reasonable, but I do not think updating any other object, e.g. RPMRepository to foreign key to the new one is.
I am not fully convinced this is creating much value for our users. If you want to implement it though and leave it as a warning I'm not opposed.
Updated by pulpbot over 4 years ago
- Status changed from NEW to POST
Updated by bmbouter about 4 years ago
- Status changed from POST to CLOSED - WONTFIX
We're trying to pick up the SigningService improvements. We currently plan to make the two below, and then we should discuss this one again. I'm closing for now until we can plan it better.
https://pulp.plan.io/issues/7700 https://pulp.plan.io/issues/7701
.