<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div>The result of our discussion was to pursue a (somewhat) different approach to the one from the PR.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
It therefore makes sense to close the PR.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
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.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
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.<br>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Brian Bouterse <bmbouter@redhat.com><br>
<b>Sent:</b> 16 September 2020 22:55<br>
<b>To:</b> Quirin Pamp <pamp@atix.de><br>
<b>Cc:</b> pulp-dev@redhat.com <pulp-dev@redhat.com><br>
<b>Subject:</b> Re: Unblocking SigningService hash check PR</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div>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.<br>
</div>
<div><br>
</div>
<div>[0]: <a href="https://github.com/pulp/pulpcore/pull/659#issuecomment-693658607">
https://github.com/pulp/pulpcore/pull/659#issuecomment-693658607</a></div>
<div><br>
</div>
<br>
</div>
<br>
<div class="x_gmail_quote">
<div dir="ltr" class="x_gmail_attr">On Tue, Jun 30, 2020 at 4:48 PM Brian Bouterse <<a href="mailto:bmbouter@redhat.com">bmbouter@redhat.com</a>> wrote:<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div dir="ltr">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.<br>
</div>
<br>
<div class="x_gmail_quote">
<div dir="ltr" class="x_gmail_attr">On Wed, Jun 24, 2020 at 7:03 AM Quirin Pamp <<a href="mailto:pamp@atix.de" target="_blank">pamp@atix.de</a>> wrote:<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<p style="margin-top:0px; margin-bottom:0px">Hi,<br>
</p>
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
<p style="margin-top:0px; margin-bottom:0px">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:</p>
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
<p style="margin-top:0px; margin-bottom:0px"><a href="https://github.com/pulp/pulpcore/pull/659" id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787LPlnk526330" target="_blank">https://github.com/pulp/pulpcore/pull/659</a></p>
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
<p style="margin-top:0px; margin-bottom:0px">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).</p>
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
<p style="margin-top:0px; margin-bottom:0px">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.<br>
</p>
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
</div>
</div>
</div>
</blockquote>
<div>Yes I believe all parties agree the warning is the compromise no-one likes.<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<p style="margin-top:0px; margin-bottom:0px"></p>
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
<p style="margin-top:0px; margin-bottom:0px">My proposal is the following:</p>
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
<p style="margin-top:0px; margin-bottom:0px">Instead of issuing a warning, a changed hash value on disk would trigger an automatic re-validation of the script on disk.</p>
<p style="margin-top:0px; margin-bottom:0px">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).</p>
<p style="margin-top:0px; margin-bottom:0px">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).<br>
</p>
</div>
</div>
</div>
</blockquote>
<div>Overall this sounds ok to me, but I want to confirm, is the validation this code does?
<a href="https://github.com/pulp/pulpcore/blob/bce4bee8124502ecd183fc664b904f5af5be97db/pulpcore/app/models/content.py#L453-L490" target="_blank">
https://github.com/pulp/pulpcore/blob/bce4bee8124502ecd183fc664b904f5af5be97db/pulpcore/app/models/content.py#L453-L490</a></div>
<div><br>
</div>
<div>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.<br>
</div>
<div><br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<p style="margin-top:0px; margin-bottom:0px"></p>
<p style="margin-top:0px; margin-bottom:0px"><br>
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).</p>
<p style="margin-top:0px; margin-bottom:0px">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.</p>
</div>
</div>
</div>
</blockquote>
<div>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.<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<p style="margin-top:0px; margin-bottom:0px"><br>
</p>
<p style="margin-top:0px; margin-bottom:0px">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:</p>
<p style="margin-top:0px; margin-bottom:0px">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.<br>
</p>
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.</div>
</div>
</div>
</blockquote>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>What do you think about this as an option?<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif">
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<br>
</div>
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<br>
</div>
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
Where to go from here:</div>
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<br>
</div>
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
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.</div>
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
This would not prevent us from taking SigningServices into an entirely different direction in the future.</div>
</div>
</div>
</blockquote>
<div>Agreed, but what do you think about moving the public_key and validate methods to SigningService first?</div>
<div><br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div id="x_gmail-m_-8177176549571854860gmail-m_-6440372971759489787divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif">
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<br>
</div>
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
thanks,</div>
<div dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
Quirin (quba42)<br>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</body>
</html>