<div dir="ltr"><div> I believe pretty much everything is said here, and I just want to add a single thought.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div></div></blockquote></blockquote><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>However, I do worry that this new default behaviour violates the 
basic user expectation that deb repositories published by pulp will 
include the same metadata fields as the upstream repository that was 
synced.</div></blockquote>
<div>(In fact I have an open issue to ensure this is the case for non-checksum fields).</div><div>Here i really see a difference between "checksum fields" and "other metadata fields". Checksum fields are supposed to ensure the integrity of the packages, and since multiple checksum algorithms are used, this information is there with a certain multiplicity. All the other fields are in a way unique. Therefore i do not see a problem omitting some hashing algorithms. Obviously this needs to be verified against all apt-repo consumers that are widespread.<br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 11, 2021 at 4:20 AM Brian Bouterse <<a href="mailto:bmbouter@redhat.com">bmbouter@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Thanks Quirin for the questions. I put my understanding and recommendations inline. Other devs please share your perspectives and advice, especially if they differ from what is written here. More questions and discussion are welcome. This is complicated stuff, but we want to be here to help.<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 10, 2021 at 11:40 AM Quirin Pamp <<a href="mailto:pamp@atix.de" target="_blank">pamp@atix.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">




<div dir="ltr">
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
The pulp_deb plugin currently makes use of md5, sha1, sha256, and sha512.
<div>Using ALLOWED_CONTENT_CHECKSUMS to "prohibit" one or more of these checksum types currently simply breaks the plugin.</div>
<div>This is one (of several) reasons why the pulp_deb CI tests are currently broken against pulpcore master (they use the default configuration for pulpcore 3.11, prohibiting md5 and sha1).</div></div></div></div></blockquote><div>Putting something back in place temporarily to get the CI working prior to release would make sense to me. Something like what pulpcore used to have:   <a href="https://github.com/pulp/pulpcore/pull/1123/files#diff-48fd5085612376b649898c2408233a202481ac724dd377c1369cf20ee34649ddL33-L38" target="_blank">https://github.com/pulp/pulpcore/pull/1123/files#diff-48fd5085612376b649898c2408233a202481ac724dd377c1369cf20ee34649ddL33-L38</a></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div><br>
</div>
<div>I could adjust the sync and publish, to only store and retrive checksums permitted by ALLOWED_CONTENT_CHECKSUMS.</div>
<div>I am pretty sure current APT implementations can get by with (and indeed prefer) sha256.</div></div></div></div></blockquote><div>With 3.11 at least, after the user runs the `handle-content-checksums` command, the Artifacts will only contain those checksums in ALLOWED_CONTENT_CHECKSUMS. 3.11 also ships a migration which auto-runs this command so if a user already has sha1 and md5, and they want to keep those sha1 and md5 in the db on Artifacts, they need to add back sha1 and/or md5 to ALLOWED_CONTENT_CHECKSUMS prior to running the migrations with 3.11 or the md5 and sha1 checksums will be removed from all artifacts.<br></div><div><br></div><div>FYI Pulp will also refuse to start if there are additional or missing checksums on Artifacts that differ from the set in ALLOWED_CONTENT_CHECKSUMS. This is actually for plugin writer's benefits (we think) so that as plugin code interacts with Artifacts, whatever is there aligns with the administrators config of the ALLOWED_CONTENT_CHECKSUMS config. If checksums are added to ALLOWED_CONTENT_CHECKSUMS the user would have to re-run the `handle-content-checksums` command and Pulp will recalculate the checksums, but this is an expensive operation (obviously).<br></div><div><br></div><div>3.11 does not make changes/enforcement with RemoteArtifacts, only Artifacts. More changes addressing RemoteArtifacts are coming in 3.12, which should finish this epic code push around this setting.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div><br>
</div>
<div>However, I do worry that this new default behaviour violates the basic user expectation that deb repositories published by pulp will include the same metadata fields as the upstream repository that was synced.</div></div></div></div></blockquote><div>This concern makes sense to me. It's a balancing act; users rightfully may want to use all checksums for a content type like pulp_deb, as is typical for that content type, but admins deploying pulp in an organization may feel differently about what checksums should be allowed in their org. This change in 3.11 is really about defaults; any given system can be configured anyway they want to.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div>(In fact I have an open issue to ensure this is the case for non-checksum fields).</div>
<div>Publishing md5 and sha1 checksums (in addition to sha256 and possibly others) is widespread practice. The official Debian and Ubuntu repos all do so.</div>
<div>I have no idea to what extent real world APT usage depends on these fields. <br></div></div></div></div></blockquote><div>I hear what you're saying here. I don't really have advice in this area. RPM is more or less in a similar state, where repos use sha1 broadly, but some environments where Pulp is used want to prohibit that. My goal is to facilitate the choice to be made in one place, the ALLOWED_CONTENT_CHECKSUMS setting.</div><div><br></div><div>In the most straightforward aspect, the ALLOWED_CONTENT_CHECKSUM setting should prevent checksum calculation for the purposes of integrity verification. For that, we recommend plugin writers use pulpcore.plugin.pulp_hashlib.new instead of hashlib.new which is a thin wrapper around hashlib.new allowing only "enabled" checksums to be instantiated. The less straightforward aspect is that these checksums should also not available for clients to ultimately do the same thing. Our intention with this setting is that pulpcore and plugins adhere to both of these practices, but the former is more straightforward at implementing than the latter. We documented both of these goals (and the machinery we recommend using) here in the plugin writer docs: <a href="https://github.com/pulp/pulpcore/blob/master/docs/plugins/plugin-writer/concepts/index.rst#checksum-use-in-plugins" target="_blank">https://github.com/pulp/pulpcore/blob/master/docs/plugins/plugin-writer/concepts/index.rst#checksum-use-in-plugins</a></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div><br>
</div>
<div>I could of course punt to users, and simply tell them in documentation to re-add md5 and sha1 to ALLOWED_CONTENT_CHECKSUMS if they want the corresponding metadata fields.</div></div></div></div></blockquote><div>Doing it with docs is more or less the strategy most plugins are pursuing. md5 isn't used much anymore so that one isn't typically as big a problem. Sha1 you may want to think about prompting your users to think about via docs.<br></div><div><br></div><div>Note that programmatically changing this setting can get tricky (practically speaking) if multiple plugins start to try to modify it on behalf of their users. Ultimately though it's up to you as a plugin writer to decide for users, or have them decide. We've opted mostly for letting users having to take action if they want md5 and sha1, but whatever you decide is ok with me. Generally I think your options are a) document adding them back, b) having a setting that can enable/disable checksums to default-re-enable one or both checksums when pulp_deb is installed, or c) doing nothing and defaulting to disincluding sha1 and md5.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div>I don't feel particularly happy about this since this ammounts to an indefinite recommendation against the default pulpcore configuration for all users who want their pulp_deb repositories to resemble official Debian/Ubuntu repositories.</div>
<div>(And for a security relevant configuration at that.)</div></div></div></div></blockquote><div>I can understand being uneasy about setting pulp_deb to have a default that is not aligned with pulpcore; once it's put in place like that I don't think it'll be easy or possible to practically change it. This is one motivation for going the docs route overall, which keeps the defaults aligned, but users on the hook to opt-back-in.</div><div><br></div><div>Note that Katello (I think) is re-enabling all of them by default on their installs, so also consider the choice in front of pulp_deb in the context of just independent pulp_deb users, not Katello users. Please confirm this with katello also, but this is my understanding.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div><br>
</div>
<div>Alternatively, I could store the unloved md5 and sha1 hashes in the content models themselves instead of the artifacts, but this is ugly for a whole host of reasons:</div>
<div><br>
</div>
<div>1) It is a lot more work</div>
<div>2) For artifacts pulpcore automatically does the work of actually checking the checksums against the artifacts.</div>
<div>3) It duplicates the source of truth for checksums (the horror!) for users that do not go along with the new default pulpcore configuration.</div>
<div><br>
</div>
<div>So that approach is probably a non-starter.</div></div></div></div></blockquote><div>I agree this is a non-starter. Our goal is to not have them available at all if admins don't want them to be, so storing them to the side I wouldn't recommend. As an alternative to this idea, I think it would be better for your users to add back sha1 and md5 to ALLOWED_CONTENT_CHECKSUMS instead.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div><br>
</div>
<div>From the point of view of pulp_deb it would be better if pulpcore did not so much refuse to handle md5 and sha1, but rather would guarantee that at least one strong checksum is also present and used for integrity checking.</div>
<div>Which I believe is the case anyway since we absolutely require sha256 to be present, no?</div></div></div></div></blockquote><div>The data model always require sha256, in fact Pulp will not start if this is removed from ALLOWED_CONTENT_CHECKSUM. It probably would be easier on users to do what you're describing, but we also have to consider the organizations who choose to use Pulp and have different expectations. We ended up with a pretty strict behavior of removing checksums that are no longer trusted from the db because it was the simplest way of causing plugin writers to have to honor the requirements of the organization deploying Pulp who decide at the organization level what they feel is trustworthy regarding package integrity checking. I understand this makes the life of a plugin writer more difficult. I also know plugin writers dealing with that can be a pain. We've been spending a lot of effort on this recently, so for sure, we feel the pain also.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div><br>
</div>
<div>To summarize: I am uncertain how best to proceed, but perhaps I am overthinking this and simply respecting ALLOWED_CONTENT_CHECKSUMS and letting users decide is best.</div></div></div></div></blockquote><div>The question I'll ask to help answer yours is: how much does pulp_deb break with 3.11's defaults? This would be good to know. Want to run a few tests and let us know? Maybe we can help give more info with that.</div><div><br></div><div>Aside from that, my general advice is to expect that pulp_deb users will change this setting, and to have the pulp_deb code work with the checksums it has available and error when it cannot fulfill their request due to not having the checksums it would need to do so.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><div style="font-size:12pt;font-family:Calibri,Arial,Helvetica,sans-serif;color:rgb(0,0,0)">
<div><br>
</div>
<div>regards,</div>
Quirin<br>
</div>
</div>
<div id="gmail-m_-8459554599438042811m_1803424099316091926gmail-m_-7483328092459562246appendonsend"></div>
<hr style="display:inline-block;width:98%">
<div id="gmail-m_-8459554599438042811m_1803424099316091926gmail-m_-7483328092459562246divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> <a href="mailto:pulp-dev-bounces@redhat.com" target="_blank">pulp-dev-bounces@redhat.com</a> <<a href="mailto:pulp-dev-bounces@redhat.com" target="_blank">pulp-dev-bounces@redhat.com</a>> on behalf of Brian Bouterse <<a href="mailto:bmbouter@redhat.com" target="_blank">bmbouter@redhat.com</a>><br>
<b>Sent:</b> 12 February 2021 21:13<br>
<b>To:</b> Pulp-dev <<a href="mailto:pulp-dev@redhat.com" target="_blank">pulp-dev@redhat.com</a>>; pulp-list <<a href="mailto:pulp-list@redhat.com" target="_blank">pulp-list@redhat.com</a>><br>
<b>Subject:</b> [Pulp-dev] Removing MD5 and SHA-1 as default available checksums in 3.11</font>
<div> </div>
</div>
<div>
<div dir="ltr">tl;dr With pulpcore 3.11, the plan is to remove MD5 and SHA-1 from the list of default available checksums.  RPM and Migration plugin users will need to add this back in at 3.11 upgrade time for your systems to continue working. Please give on-list
 feedback on this change.<br>
<br>
## Background<br>
<br>
Pulp has the ALLOWED_CONTENT_CHECKSUMS setting [0] which, by default, currently includes md5, sha-1, sha-224, sha-256, sha-384, and sha-512. Pulp code is restricted to only using hashers from this list. This feature gives admins the ability to prohibit hashers
 they do not trust. Pulp uses these checksums for package integrity verification purposes when syncing and publishing content.<br>
<br>
## Motivation<br>
<br>
We need to make Pulp secure by default. MD5 is known to be insecure, and therefore it is unsafe for Pulp to allow its use for calculating package integrity by default. SHA-1 is widely believed to be insecure, or will be soon, and should not be allowed by default
 for the same reason.<br>
<br>
## Proposal<br>
<br>
Pulpcore 3.11 would remove md5 and sha-1 from the default list of allowed checksums, leaving sha-224..sha-512. Specifically this change is occuring in the `ALLOWED_CONTENT_CHECKSUMS` setting [0]. This is only a change to the default settings; any specific system
 can be configured as desired. Nothing is "being taken away".<br>
<br>
## Required User Action with 3.11<br>
<br>
We believe both RPM plugin users and Migration plugin users will be impacted by this and mostly from the SHA-1 removal. SHA-1 is still used on a variety of CDNs including Red Hat's. Also as data is migrated from Pulp2 systems, this also likely uses SHA-1 and
 MD5 as the migration plugin runs.<br>
<br>
If users are using the defaults for `ALLOWED_CONTENT_CHECKSUMS` and want to continue using SHA-1, they will need to update `ALLOWED_CONTENT_CHECKSUMS` in their settings file. Alternatively, users will need to run `pulpcore-manager handle-artifact-checksums`
 after upgrade to update any existing artifacts after upgrading.<br>
<br>
## Why not automate this?<br>
<br>
We do not take manual user action at upgrade time lightly. However, this is a security change, and we believe we need each Pulp system to opt-in for themselves.<br>
<br>
[0]: <a href="https://docs.pulpproject.org/pulpcore/settings.html#allowed-content-checksums" target="_blank">
https://docs.pulpproject.org/pulpcore/settings.html#allowed-content-checksums</a><br>
<br>
Thanks!<br>
The Pulpcore Team</div>
</div>
</div>

</blockquote></div></div>
_______________________________________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
<a href="https://listman.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://listman.redhat.com/mailman/listinfo/pulp-dev</a><br>
</blockquote></div>