Bug 52406 - openldap: Multiple issues (4.4)
openldap: Multiple issues (4.4)
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Security updates
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-7-errata
Assigned To: Felix Botner
Erik Damrose
:
Depends on:
Blocks: 52747 54643
  Show dependency treegraph
 
Reported: 2020-11-20 17:34 CET by Erik Damrose
Modified: 2022-04-06 13:03 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Security Issue
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score: 7.5 (CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Erik Damrose univentionstaff 2020-11-20 17:34:14 CET
CVE-2020-25692 null-ptr dereference for unauthenticated packet in OpenLDAP slapd
CVE-2020-25709 assertion failure in Certificate List syntax validation
CVE-2020-25710 assertion failure in CSN normalization with invalid input

https://security-tracker.debian.org/tracker/source-package/openldap
Comment 1 Felix Botner univentionstaff 2020-12-16 15:39:24 CET
CVE-2020-25692 - https://bugs.openldap.org/show_bug.cgi?id=9370
CVE-2020-25709 - https://bugs.openldap.org/show_bug.cgi?id=9383
CVE-2020-25710 - https://bugs.openldap.org/show_bug.cgi?id=9384

Added upstream patches (2.4.44+dfsg-5+deb9u6)

 * ITS-9370-check-for-equality-rule-on-old_rdn.patch
 * ITS-9383-remove-assert-in-certificateListValidate.patch
 * ITS-9384-remove-assert-in-obsolete-csnNormalize23.patch

as

 * 99_ITS-9370-check-for-equality-rule-on-old_rdn.quilt
 * 99_ITS-9383-remove-assert-in-certificateListValidate.quilt
 * 99_ITS-9384-remove-assert-in-obsolete-csnNormalize23.quilt

to 

 * svn/patches/openldap/4.4-0-0-ucs/2.4.45+dfsg-1~bpo9+1-errata4.4-7
 * svn/patches/openldap/5.0-0-0-ucs/2.4.47+dfsg-3+deb10u2

successful build
Package: openldap
Version: 2.4.45+dfsg-1~bpo9+1A~4.4.0.202012151059
Branch: ucs_4.4-0-errata4.4-7
Scope: errata4.4-7

Successful build
Package: openldap
Version: 2.4.47+dfsg-3+deb10u2A~5.0.0.202012161230
Branch: ucs_5.0-0

ucs-test-ldap without problems in 4.4-7 and 5.0-0
Jenkins Test for 4.4-7 OK 

yaml: openldap.yaml (4.4-7)
Comment 2 Erik Damrose univentionstaff 2020-12-16 19:39:04 CET
To make sure the depending bug is not missed by QA, we have to make sure the pending issue at bug 51910 is cleared before verifying this bug.
Comment 3 Philipp Hahn univentionstaff 2020-12-17 12:37:29 CET
(In reply to Erik Damrose from comment #2)
> To make sure the depending bug is not missed by QA, we have to make sure the
> pending issue at bug 51910 is cleared before verifying this bug.

For Bug #51910 I had to extract the code for translog.c, which only existed in the patch. As it was still using the old format of "the outer patch adds an inner patch to debian/patches/"-format, I took the liberty to convert it to the new .quilt mechanism built-into repo-ng since 2016 (Bug #42238) which greatly simplifies maintaining "translog.c". Any change to "translog.c" would otherwise require much more work for Bug #51910 again.

No functional changes where done, which are still pending in Bug #51910. I did two builds with the old .patch and the new .quilt version and compared the result: there was no change in the source code and the resulting binaries.

The commit to convert from .patch to .quilt only carries the Bug #51910 number because it was done for that bug.
Comment 4 Arvid Requate univentionstaff 2020-12-17 17:22:35 CET
> No functional changes where done, which are still pending in Bug #51910. I did two builds with the old .patch and the new .quilt version and compared the result: there was no change in the source code and the resulting binaries.

Ok, cool, but QA has o be done by a second person. And commits should not be done under bugs that are neither scheduled nor assigned. Finally, some hint should be given to the UCS-4 team. I'm sure you agree to all three points.
Comment 5 Philipp Hahn univentionstaff 2020-12-17 17:55:33 CET
(In reply to Arvid Requate from comment #4)
> > No functional changes where done, which are still pending in Bug #51910. I did two builds with the old .patch and the new .quilt version and compared the result: there was no change in the source code and the resulting binaries.
> 
> Ok, cool, but QA has to be done by a second person. And commits should not be
> done under bugs that are neither scheduled nor assigned. Finally, some hint
> should be given to the UCS-4 team. I'm sure you agree to all three points.

Computers are better/faster/cheaper for comparing things to be bit-identical: I did the diff and recompile exactly for this reason to NOT plant a time bomb for later when the next real change for OpenLDAP happens - feel free to redo the same work again if you don't trust others...
Comment 6 Arvid Requate univentionstaff 2020-12-17 22:51:58 CET
Fine, then we should establish your proposal into a commonly accepted procedure. Since you are a fan of zero trust, you can imagine my answer. We need to agree upon some way to cooperate, and we should follow that.
Comment 7 Philipp Hahn univentionstaff 2020-12-18 09:34:21 CET
(In reply to Arvid Requate from comment #6)
> Fine, then we should establish your proposal into a commonly accepted
> procedure. Since you are a fan of zero trust,

Quiet the contrary: I DO trust you and all other colleges to do a great job, but I did NOT trust myself and therefore re-built OpenLDAP locally to make sure, that r19128 and r19129 only contained formatting changes, but NO code change.

> you can imagine my answer. We
> need to agree upon some way to cooperate, and we should follow that.

Our job is to bring value to our customers. Bug #51910 is still NOT shipped to our customers, so that value is currently on hold.

Basically you're telling that the value must be released only completely or never.
I released part of that value at least to my fellow colleges to save them from doing the work again I had to do under pullcord conditions:
- I had to painfully extract the code for `translog.c` from our nested patch
- I had to find the origin of several patches lacking authorship and description, which complicated my task of ruling them out as being responsible for Bug #51910
This was already a wast of my time as @fbest did exactly the same work for UCS 5.0-0. So you also could see my work as making further maintenance easier because 4.4-7 and 5.0-0 are synchronized again.

Processes are good if they work as they guide people in their decision making.
If people use them for blocking other people from getting their work done they become a problem.
Even more so if they are only used by "people in power" to cement their power over others.

I recently read this proverb from Gene Kim: “Improving daily work is even more important than doing daily work.”
So if my work improves the work of others, I see it as a win.

So our goal should be:
- bring value to our customers.
- if this is not possible, at least bring value to our fellow colleges.
Comment 8 Arvid Requate univentionstaff 2020-12-18 14:04:39 CET
Maybe I didn't make myself clear: It's totally ok to convert the patches, but changes to production code must not be made without following the QA procedures we agreed upon. If you create a new simplified process, that's fine, just tell everyone on the team to agree upon. Don't silently change things, as that becomes dangerous if you extrapolate it to everyone on the team behaving like that.
Comment 9 Philipp Hahn univentionstaff 2020-12-19 06:24:40 CET
(In reply to Felix Botner from comment #1)
> Added upstream patches (2.4.44+dfsg-5+deb9u6)
...
> to 
...
>  * svn/patches/openldap/5.0-0-0-ucs/2.4.47+dfsg-3+deb10u2
...
> Package: openldap
> Version: 2.4.47+dfsg-3+deb10u2A~5.0.0.202012161230
> Branch: ucs_5.0-0

For 5.0-0 please check for pending updates from Debian Security first.
I reverted your change r19244 with r19258 and instead imported the pending Debian Security update 2.4.47+dfsg-3+deb10u4 as part of my work to import the latest Debian 10.7 point update, which happened last weekend.

The rational for importing from Debian instead of patching is that we want to be as close to Debian as possible: Each patch in svn/patches/ moves us away one more step from that and leads to additional work in the future when the next import from Debian must happen (r19254 failed).
Comment 10 Erik Damrose univentionstaff 2021-01-04 16:53:24 CET
OK: Patches
 * 99_ITS-9370-check-for-equality-rule-on-old_rdn.quilt
 * 99_ITS-9383-remove-assert-in-certificateListValidate.quilt
 * 99_ITS-9384-remove-assert-in-obsolete-csnNormalize23.quilt

OK: package build with patches
OK: yaml
OK: openldap 2.4.47+dfsg-3+deb10u4 in UCS 5 already contains the issues fixed here
Verified