URL: https://github.com/SSSD/sssd/pull/5918 Author: ikerexxe Title: #5918: Client ca validation error Action: opened
PR body: """ Improve the error handling for FindByValidCertificate() by returning a specific exception ID when the certificate authority file is missing. Moreover, the log lines have been changed to point to p11_child logs when an unknown error happens.
Moreover, a new test case has been created for the certificate authority file missing situation.
Finally, include a reference to ca_db, p11_child_timeout and certificate_verification in sssd-ifp man page. These options can used be to control how the certificates are validated with FindByValidCertificate() API.
Resolves: https://github.com/SSSD/sssd/issues/5911 """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5918/head:pr5918 git checkout pr5918
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
ikerexxe commented: """ @sumit-bose the first commit of this PR mentions `ca_db`, `p11_child_timeout` and `certificate_verification` as you suggested in an email. """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-994512784
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
sumit-bose commented: """ Hi,
there is a pep8 issue:
``` Start: Wed Dec 15 08:53:56 UTC 2021 + find . -path ./src/config -prune -o -name '*.py' -exec pycodestyle --ignore=E121,E123,E126,E226,E24,E704,W503,W504,W605,E722 '{}' + ./src/tests/intg/test_infopipe.py:809:80: E501 line too long (81 > 79 characters) End: Wed Dec 15 08:54:01 UTC 2021 ```
bye, Sumit """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-994744210
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/5918 Author: ikerexxe Title: #5918: Client ca validation error Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5918/head:pr5918 git checkout pr5918
URL: https://github.com/SSSD/sssd/pull/5918 Author: ikerexxe Title: #5918: Client ca validation error Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5918/head:pr5918 git checkout pr5918
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
sumit-bose commented: """ Hi,
I'm fine with the patches, I would only recommend to use a higher value then `2` for the exit code since e.g. the monitor is already using `2` for other purposes.
@ikerexxe, have you checked with @martinpitt if the fix works for him?
It would be good if someone else can have a look as well since Iker and I developed the idea for the fix together.
bye, Sumit """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-995644563
URL: https://github.com/SSSD/sssd/pull/5918 Author: ikerexxe Title: #5918: Client ca validation error Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5918/head:pr5918 git checkout pr5918
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
ikerexxe commented: """ Debian and RHEL8 failures aren't related with this PR. In any case, I have a fix proposal for that in #5922 """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-995832498
URL: https://github.com/SSSD/sssd/pull/5918 Author: ikerexxe Title: #5918: Client ca validation error Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5918/head:pr5918 git checkout pr5918
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
alexey-tikhonov commented: """ @ikerexxe, is this still "Changes requested"? """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996082266
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
ikerexxe commented: """
@ikerexxe, is this still "Changes requested"?
Yes, I'd like to run the CI and see that everything works correctly before changing its status. But apparently this is not going to happen soon because there's some outage that I'm unable to understand. """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996531449
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
martinpitt commented: """ I looked at the diff, and the new exception looks very reasonable/useful for me, thanks @ikerexxe ! I didn't test it yet -- is there some packit-like rpm being produced in CI which I could check? """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996554368
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
ikerexxe commented: """
I looked at the diff, and the new exception looks very reasonable/useful for me, thanks @ikerexxe ! I didn't test it yet -- is there some packit-like rpm being produced in CI which I could check?
No, but I could create a COPR repository or scratch-build if you share the distribution that you are using. """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996556784
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
martinpitt commented: """ @ikerexxe : I'm happy to test this on Fedora 35, rhel 8.5, 8.6 or 9.0, or centos stream 8. Thanks! """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996579410
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
ikerexxe commented: """
@ikerexxe : I'm happy to test this on Fedora 35, rhel 8.5, 8.6 or 9.0, or centos stream 8. Thanks!
Fedora 35 can be tested using the following [repo](https://copr.fedorainfracloud.org/coprs/ipedrosa/test_ca_validation_error/).
Example usage: ``` dnf copr enable ipedrosa/test_ca_validation_error dnf update sssd ```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996657989
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
martinpitt commented: """ Thanks @ikerexxe ! Just FTR, last day (few hours) of work for this year, will check this in January. Happy EOY holidays! """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996700404
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: +Waiting for review
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
alexey-tikhonov commented: """ All CI targets are green besides Rawhide that seems to be stuck on multihost tests: ``` configure: success 00:00:45 ci-build-debug/ci-configure.log make-tests: success 00:36:55 ci-build-debug/ci-make-tests.log make-check-valgrind: success 00:10:52 ci-build-debug/ci-make-check-valgrind.log make-intgcheck: success 00:48:23 ci-build-debug/ci-make-intgcheck.log make-distcheck: success 00:39:47 ci-build-debug/ci-make-distcheck.log make-srpm: success 00:00:20 ci-build-debug/ci-make-srpm.log ``` -- I will stop it. """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996741631
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
alexey-tikhonov commented: """ Overall looks good to me. Just a couple of nitpicks. """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996793581
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: -Waiting for review
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/5918 Author: ikerexxe Title: #5918: Client ca validation error Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5918/head:pr5918 git checkout pr5918
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: +Waiting for review
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
alexey-tikhonov commented: """ Thanks, ACK. """
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-996824912
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: -Waiting for review
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: +Ready to push
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
alexey-tikhonov commented: """ Pushed PR: https://github.com/SSSD/sssd/pull/5918
* `master` * fd0f087ada2f4a59d4b22846d06f1b19fd522e11 - ifp: improve FindByValidCertificate() error * 8d54b8c029d276801b323dfe045a155a4311ee49 - man: update ifp options for FindByValidCertificate
"""
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-997050340
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: -Accepted
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
Label: -Ready to push
URL: https://github.com/SSSD/sssd/pull/5918 Author: ikerexxe Title: #5918: Client ca validation error Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5918/head:pr5918 git checkout pr5918
URL: https://github.com/SSSD/sssd/pull/5918 Title: #5918: Client ca validation error
martinpitt commented: """ @ikerexxe : Sorry for the delay, holidays came in between. I finally tested the COPR at last, and it works great now: ``` [root@x0 ~]# busctl call org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users FindByValidCertificate s -- "$(cat /var/tmp/alice.pem)" o "/org/freedesktop/sssd/infopipe/Users/cockpit_2elan/200400001"
[root@x0 ~]# mv /etc/sssd/pki/sssd_auth_ca_db.pem{,.disabled} [root@x0 ~]# busctl call org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users FindByValidCertificate s -- "$(cat /var/tmp/alice.pem)" Call failed: Certificate authority file not found
[root@x0 ~]# echo > /etc/sssd/pki/sssd_auth_ca_db.pem [root@x0 ~]# busctl call org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users FindByValidCertificate s -- "$(cat /var/tmp/alice.pem)" Call failed: 1432158296: Invalid certificate provided ```
The reply details in `busctl monitor` for the second call:
``` ‣ Type=error Endian=l Flags=1 Version=1 Cookie=23 ReplyCookie=2 Timestamp="Tue 2022-01-04 10:37:40.619498 UTC" Sender=:1.56 Destination=:1.101 ErrorName=sbus.Error.NoCA ErrorMessage="Certificate authority file not found" UniqueName=:1.56 MESSAGE "s" { STRING "Certificate authority file not found"; }; ```
So, perfect :+1: Thank you, and happy new year!
"""
See the full comment at https://github.com/SSSD/sssd/pull/5918#issuecomment-1004699015
sssd-devel@lists.fedorahosted.org