I finally got around to outlining the next steps we discussed. # The two stories that should be done next https://pulp.plan.io/issues/7700 https://pulp.plan.io/issues/7701
# I closed this story because we don't have a plan for it. We can reopen it later when we do https://pulp.plan.io/issues/6291 On Thu, Sep 17, 2020 at 4:38 AM Quirin Pamp <p...@atix.de> wrote: > The result of our discussion was to pursue a (somewhat) different > approach to the one from the PR. > It therefore makes sense to close the PR. > > I am a bit fuzzy on what the next steps were (adding a public key to the > SigningService base class?), but I suppose I will just have to re-watch the > meeting. > I also get the feeling, continuing SigningService work isn't at the top of > anyone's To-Do list right now. We will see who blinks first. > ------------------------------ > *From:* Brian Bouterse <bmbou...@redhat.com> > *Sent:* 16 September 2020 22:55 > *To:* Quirin Pamp <p...@atix.de> > *Cc:* pulp-dev@redhat.com <pulp-dev@redhat.com> > *Subject:* Re: Unblocking SigningService hash check PR > > At the pulpcore meeting we noticed the signing service PR [0] had stalled > and instead we needed to work towards the next-steps identified in the > video call and discussed on this thread. I closed that PR with a note > asking that we focus on the next steps we discussed in the meeting (also > linked to [0]). I can help review and guide that contribution if someone is > interested. > > [0]: https://github.com/pulp/pulpcore/pull/659#issuecomment-693658607 > > > > On Tue, Jun 30, 2020 at 4:48 PM Brian Bouterse <bmbou...@redhat.com> > wrote: > > Sorry for the long delay in reply. Thank you for the email. I also want to > unblock that PR. I put some replies in line. > > On Wed, Jun 24, 2020 at 7:03 AM Quirin Pamp <p...@atix.de> wrote: > > Hi, > > > However we decide to continue with the SigningService topic in the medium > and longrun, I wanted to have one more go at unblocking the following PR in > the short run: > > > https://github.com/pulp/pulpcore/pull/659 > > > Currently, this PR issues a warning whenever the hash of a signing service > script has changed on disk (compared to when it was first validated). > > > I think we are all in agreement that this is a bad compromise between > doing nothing at all (since the script might have changed for legitimate > reasons), and issuing a full on Error in cases where things are broken. > > > Yes I believe all parties agree the warning is the compromise no-one likes. > > > My proposal is the following: > > > Instead of issuing a warning, a changed hash value on disk would trigger > an automatic re-validation of the script on disk. > > If the validation fails, it will throw a hard error (which would certainly > be the correct course of action for a script that does not perform what the > SigningService promises). > > If the validation succeeds, the SigningService is updated with the new > hash value, and everything continues as it nothing had happened (we just > assume the script was changed for legitimate reasons). > > Overall this sounds ok to me, but I want to confirm, is the validation > this code does? > https://github.com/pulp/pulpcore/blob/bce4bee8124502ecd183fc664b904f5af5be97db/pulpcore/app/models/content.py#L453-L490 > > If so, moving the `public_key` and ^ `signing service` attributes from > AsciiArmoredDetachedSigningservice in pulp_rpm to SigningService in > pulpcore would be a good first step. I believe this will need to happen > first and is consistent with the last discussion of our video call. This > would be a good first step in its own PR I think. > > > The only thing I can come up with where this approach might be > problematic, is if users want to have different versions of the signing > service script on different workers (for some reason). > > However, in such cases it would still be possible to work around the > problem by having a single signing service script call a secondary script > that differs on different workers. > > If each script has its own public key it will fail. If each script on each > worker with a different sha256 shares one public key it would be > revalidated-and-modified each time it's used, which is strange but ok. Also > I don't think this use case is really valid so I'm ok with this. > > > If you are worried that the possibility of such a workaround defeats the > whole purpose of hashing the script in the first place, consider the > following: > > This is not intended as a security feature against some kind of malicious > attacker scenario, it is intended to provide some more meaningful error > reporting, for operational mistakes. > In this context I almost consider it a bonus if Sysadmin users who want to > do something rather unusual and complicated (different signing service > scripts on different workers) are forced to think about this carefully. > > The use case doesn't resonate with me, but that's ok. If others see value > in it, it doesn't harm existing use cases, and ya'll are willing to > contribute it, let's do this. > > Here is one idea I want to pitch as a possible counterproposal: What if > we don't store the sha256 and instead, we perform validation on the signed > data all the time? The sha256 is nice, but it won't catch the case when the > script is interacting with a key server, and that key rotates but the > script doesn't. In that case the signatures will still be returned, pulp > will be unaware, no error will occur, yet it should. Validating data in all > cases I think would get us everything the sha256 script proposes and even > more. > > What do you think about this as an option? > > > > Where to go from here: > > If we can get some kind of agreement that we would be willing to merge the > version of the above PR that I have proposed, I would ask Manisha to make > the relevant changes and they could be reviewed and merged. > This would not prevent us from taking SigningServices into an entirely > different direction in the future. > > Agreed, but what do you think about moving the public_key and validate > methods to SigningService first? > > > thanks, > Quirin (quba42) > >
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev