Comment 5 for bug 1926254

Revision history for this message
Matthew Ruffell (mruffell) wrote :

Hi Seth,

Thanks for the review.

I read the commit you found:

commit 1e41dadfa7b9f792ed0f4714a3d3d36f070cf30e
Author: Dr. David von Oheimb <email address hidden>
Date: Sat Jun 27 16:16:12 2020 +0200
Subject: Extend X509 cert checks and error reporting in v3_{purp,crld}.c and x509_{set,vfy}.c
Link: https://github.com/openssl/openssl/commit/1e41dadfa7b9f792ed0f4714a3d3d36f070cf30e

Firstly, yes, you are right, this commit does refactor the code I am suggesting we SRU to focal and groovy, but upon further inspection, this commit was not backported to the 1.1.1 stable series, as it is missing from the OpenSSL_1_1_1-stable branch. As you mentioned, it is a fairly invasive change and modifies a lot of different x509 components, it isn't suitable to be backported to 1.1.1 stable anyway, and much less be acceptable for SRU to focal or groovy.

I think we should stick to the small targeted commits I suggested for this SRU, since they are a part of 1.1.1 stable, and are already in hirsute onward.

To test that the logic from the suggested commits to SRU matches this new refactor commit from version 3.0alpha, I went and built the master branch of openssl, which had commit d1a770414acd34c774248ce8efbe202fd7a44041 at HEAD.

$ env LD_LIBRARY_PATH="/home/ubuntu/openssl/" ../openssl/apps/openssl version
OpenSSL 3.0.0-alpha16-dev (Library: OpenSSL 3.0.0-alpha16-dev )

$ env LD_LIBRARY_PATH="/home/ubuntu/openssl/" ../openssl/apps/openssl verify -CAfile CA/rootCA_cert.pem -untrusted CA/subCA_cert.pem user1_cert.pem
user1_cert.pem: OK

The logic matches and the reproducer certificates verify OK. This confirms we aren't backporting a short lived change, and that this behaviour is the desired and accepted outcome.

@ddstreet Please go ahead and sponsor the SRU to -updates, thanks.