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
From 9ae8af9a597acd8ca84bdeadee1edb9011fa0c43 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedr...@redhat.com>
Date: Fri, 10 Dec 2021 14:28:13 +0100
Subject: [PATCH 1/2] man: update ifp options for FindByValidCertificate
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.
Signed-off-by: Iker Pedrosa <ipedr...@redhat.com>
---
src/man/sssd-ifp.5.xml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/man/sssd-ifp.5.xml b/src/man/sssd-ifp.5.xml
index c787ef9d4d..d3080537a6 100644
--- a/src/man/sssd-ifp.5.xml
+++ b/src/man/sssd-ifp.5.xml
@@ -38,6 +38,22 @@
to query information about remote users and groups over the
system bus.
</para>
+
+ <refsect2 id='valid_certificate'>
+ <title>FIND BY VALID CERTIFICATE</title>
+ <para>
+ The following options can be used to control how the certificates
+ are validated when using the FindByValidCertificate() API:
+ <itemizedlist>
+ <listitem><para>ca_db</para></listitem>
+ <listitem><para>p11_child_timeout</para></listitem>
+ <listitem><para>certificate_verification</para></listitem>
+ </itemizedlist>
+ For more details about the options see
+ <citerefentry><refentrytitle>sssd.conf</refentrytitle>
+ <manvolnum>5</manvolnum></citerefentry>.
+ </para>
+ </refsect2>
</refsect1>
<refsect1 id='configuration-options'>
From 51e68dedb5ffd0c1a00aee34e2b76dd03306ad6b Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedr...@redhat.com>
Date: Fri, 10 Dec 2021 11:20:23 +0100
Subject: [PATCH 2/2] ifp: improve FindByValidCertificate() error
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.
Finally, a new test case has been created for the certificate authority
file missing situation.
Resolves: https://github.com/SSSD/sssd/issues/5911
Signed-off-by: Iker Pedrosa <ipedr...@redhat.com>
---
src/p11_child/p11_child_common.c | 9 ++++--
src/p11_child/p11_child_openssl.c | 14 +++++++--
src/responder/ifp/ifp_users.c | 15 ++++++++--
src/sbus/sbus_errors.c | 1 +
src/sbus/sbus_errors.h | 1 +
src/tests/intg/test_infopipe.py | 47 ++++++++++++++++++++++++++++++-
src/util/child_common.h | 1 -
src/util/util.h | 6 +++-
src/util/util_errors.c | 2 ++
src/util/util_errors.h | 2 ++
10 files changed, 87 insertions(+), 11 deletions(-)
diff --git a/src/p11_child/p11_child_common.c b/src/p11_child/p11_child_common.c
index 7c8259479d..4bcaaf6e80 100644
--- a/src/p11_child/p11_child_common.c
+++ b/src/p11_child/p11_child_common.c
@@ -147,7 +147,7 @@ int main(int argc, const char *argv[])
poptContext pc;
int debug_fd = -1;
const char *opt_logger = NULL;
- errno_t ret;
+ errno_t ret = 0;
TALLOC_CTX *main_ctx = NULL;
enum op_mode mode = OP_NONE;
enum pin_mode pin_mode = PIN_NONE;
@@ -382,5 +382,10 @@ int main(int argc, const char *argv[])
DEBUG(SSSDBG_CRIT_FAILURE, "p11_child failed!\n");
close(STDOUT_FILENO);
talloc_free(main_ctx);
- return EXIT_FAILURE;
+
+ if (ret == ERR_CA_DB_NOT_FOUND) {
+ return CA_DB_NOT_FOUND_EXIT_CODE;
+ } else {
+ return EXIT_FAILURE;
+ }
}
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index de5d583a0e..d4a890a2b4 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -667,9 +667,17 @@ errno_t init_verification(struct p11_ctx *p11_ctx,
if (!X509_LOOKUP_load_file(lookup, p11_ctx->ca_db, X509_FILETYPE_PEM)) {
err = ERR_get_error();
- DEBUG(SSSDBG_OP_FAILURE, "X509_LOOKUP_load_file failed [%lu][%s].\n",
- err, ERR_error_string(err, NULL));
- ret = EIO;
+ DEBUG(SSSDBG_OP_FAILURE,
+ "X509_LOOKUP_load_file [%s] failed [%lu][%s].\n",
+ p11_ctx->ca_db, err, ERR_error_string(err, NULL));
+
+ if (ERR_GET_LIB(err) == ERR_LIB_SYS &&
+ ERR_GET_REASON(err) == ENOENT) {
+ ret = ERR_CA_DB_NOT_FOUND;
+ } else {
+ ret = EIO;
+ }
+
goto done;
}
diff --git a/src/responder/ifp/ifp_users.c b/src/responder/ifp/ifp_users.c
index 8746b25034..ba9ca5ad4a 100644
--- a/src/responder/ifp/ifp_users.c
+++ b/src/responder/ifp/ifp_users.c
@@ -1254,15 +1254,24 @@ ifp_users_find_by_valid_cert_step(int child_status,
PIPE_FD_CLOSE(state->io->write_to_child_fd);
if (WIFEXITED(child_status)) {
- if (WEXITSTATUS(child_status) != 0) {
+ if (WEXITSTATUS(child_status) == CA_DB_NOT_FOUND_EXIT_CODE) {
DEBUG(SSSDBG_OP_FAILURE,
- P11_CHILD_PATH " failed with status [%d]\n", child_status);
+ P11_CHILD_PATH " failed [%d]: [%s].\n",
+ ERR_CA_DB_NOT_FOUND, sss_strerror(ERR_CA_DB_NOT_FOUND));
+ tevent_req_error(req, ERR_CA_DB_NOT_FOUND);
+ return;
+ } else if (WEXITSTATUS(child_status) != 0) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ P11_CHILD_PATH " failed with status [%d]. Check p11_child"
+ " logs for more information.\n",
+ WEXITSTATUS(child_status));
tevent_req_error(req, ERR_INVALID_CERT);
return;
}
} else if (WIFSIGNALED(child_status)) {
DEBUG(SSSDBG_OP_FAILURE,
- P11_CHILD_PATH " was terminated by signal [%d]\n",
+ P11_CHILD_PATH " was terminated by signal [%d]. Check p11_child"
+ " logs for more information.\n",
WTERMSIG(child_status));
tevent_req_error(req, ECHILD);
return;
diff --git a/src/sbus/sbus_errors.c b/src/sbus/sbus_errors.c
index 63c67dd5c9..c6449dd87e 100644
--- a/src/sbus/sbus_errors.c
+++ b/src/sbus/sbus_errors.c
@@ -34,6 +34,7 @@ static const struct {
{ SBUS_ERROR_INTERNAL, ERR_INTERNAL },
{ SBUS_ERROR_NOT_FOUND, ENOENT },
{ SBUS_ERROR_KILLED, ERR_SBUS_KILL_CONNECTION },
+ { SBUS_ERROR_NO_CA, ERR_CA_DB_NOT_FOUND},
/* D-Bus standard errors. Some errno values may overlap, but when
* finding its D-Bus pair the first match is returned. */
diff --git a/src/sbus/sbus_errors.h b/src/sbus/sbus_errors.h
index f970be7950..e005f05d12 100644
--- a/src/sbus/sbus_errors.h
+++ b/src/sbus/sbus_errors.h
@@ -31,6 +31,7 @@
#define SBUS_ERROR_INTERNAL "sbus.Error.Internal"
#define SBUS_ERROR_NOT_FOUND "sbus.Error.NotFound"
#define SBUS_ERROR_KILLED "sbus.Error.ConnectionKilled"
+#define SBUS_ERROR_NO_CA "sbus.Error.NoCA"
#define SBUS_ERROR_ERRNO "sbus.Error.Errno"
errno_t sbus_error_to_errno(DBusError *error);
diff --git a/src/tests/intg/test_infopipe.py b/src/tests/intg/test_infopipe.py
index d493a94514..1ca2076b27 100644
--- a/src/tests/intg/test_infopipe.py
+++ b/src/tests/intg/test_infopipe.py
@@ -32,6 +32,7 @@
import pytest
import dbus
import base64
+import shutil
import config
import ds_openldap
@@ -313,6 +314,34 @@ def create_sssd_fixture(request):
create_sssd_cleanup(request)
+def backup_ca_db():
+ """Create backup file for ca db"""
+ src = os.path.dirname(config.PAM_CERT_DB_PATH) + "/SSSD_test_CA.pem"
+ dst = os.path.dirname(config.PAM_CERT_DB_PATH) + "/SSSD_test_CA.pem.bp"
+ shutil.copyfile(src, dst)
+
+
+def restore_ca_db():
+ """Restore backup file for ca db"""
+ src = os.path.dirname(config.PAM_CERT_DB_PATH) + "/SSSD_test_CA.pem.bp"
+ dst = os.path.dirname(config.PAM_CERT_DB_PATH) + "/SSSD_test_CA.pem"
+ shutil.copyfile(src, dst)
+ os.remove(src)
+
+
+def create_restore_ca_db(request):
+ """Add teardown for restoring ca_db"""
+ request.addfinalizer(restore_ca_db)
+
+
+def create_ca_db_fixture(request):
+ """
+ Create backup for ca_db and add teardown for restoring it
+ """
+ backup_ca_db()
+ create_restore_ca_db(request)
+
+
@pytest.fixture
def sanity_rfc2307(request, ldap_conn):
ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
@@ -335,6 +364,7 @@ def sanity_rfc2307(request, ldap_conn):
conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307, config)
create_conf_fixture(request, conf)
create_sssd_fixture(request)
+ create_ca_db_fixture(request)
return None
@@ -348,6 +378,7 @@ def simple_rfc2307(request, ldap_conn):
conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307, config)
create_conf_fixture(request, conf)
create_sssd_fixture(request)
+ create_ca_db_fixture(request)
return None
@@ -371,6 +402,7 @@ def auto_private_groups_rfc2307(request, ldap_conn):
""").format(**locals())
create_conf_fixture(request, conf)
create_sssd_fixture(request)
+ create_ca_db_fixture(request)
return None
@@ -398,6 +430,7 @@ def add_user_with_cert(request, ldap_conn):
conf = format_certificate_conf(ldap_conn, SCHEMA_RFC2307_BIS, config)
create_conf_fixture(request, conf)
create_sssd_fixture(request)
+ create_ca_db_fixture(request)
return None
@@ -743,7 +776,6 @@ def test_find_by_valid_certificate(dbus_system_bus,
assert str(ex) == "sbus.Error.NotFound: No such file or directory"
# Valid certificate from another CA
- print(os.environ['ABS_SRCDIR'])
cert_file = os.environ['ABS_SRCDIR'] + \
"/../test_ECC_CA/SSSD_test_ECC_cert_key_0001.pem"
with open(cert_file, "r") as f:
@@ -763,3 +795,16 @@ def test_find_by_valid_certificate(dbus_system_bus,
except dbus.exceptions.DBusException as ex:
error = "org.freedesktop.DBus.Error.IOError: Input/output error"
assert str(ex) == error
+
+ # Remove certificate db
+ cert_db = cert_path + "/SSSD_test_CA.pem"
+ os.remove(cert_db)
+ cert_file = cert_path + "/SSSD_test_cert_x509_0002.pem"
+ with open(cert_file, "r") as f:
+ cert = f.read()
+ try:
+ res = users_iface.FindByValidCertificate(cert)
+ assert False, "Previous call should raise an exception"
+ except dbus.exceptions.DBusException as ex:
+ assert str(ex) == \
+ "sbus.Error.NoCA: Certificate authority file not found"
diff --git a/src/util/child_common.h b/src/util/child_common.h
index 92d66a5009..6dab2f5e91 100644
--- a/src/util/child_common.h
+++ b/src/util/child_common.h
@@ -36,7 +36,6 @@
#define CHILD_MSG_CHUNK 256
#define SIGTERM_TO_SIGKILL_TIME 2
-#define CHILD_TIMEOUT_EXIT_CODE 7
struct response {
uint8_t *buf;
diff --git a/src/util/util.h b/src/util/util.h
index 6dfd2540cc..2d99a29c2b 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -114,7 +114,11 @@ extern int dbus_activated;
#define FLAGS_GEN_CONF 0x0008
#define FLAGS_NO_WATCHDOG 0x0010
-#define SSS_WATCHDOG_EXIT_CODE 70 /* to match EX_SOFTWARE in sysexits.h */
+enum sssd_exit_status {
+ CHILD_TIMEOUT_EXIT_CODE = 7,
+ CA_DB_NOT_FOUND_EXIT_CODE = 50,
+ SSS_WATCHDOG_EXIT_CODE = 70 /* to match EX_SOFTWARE in sysexits.h */
+};
#define PIPE_INIT { -1, -1 }
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index 882c0b4aa3..4132040c48 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -146,6 +146,8 @@ struct err_string error_to_str[] = {
{ "TLS handshake was interrupted"}, /* ERR_TLS_HANDSHAKE_INTERRUPTED */
+ { "Certificate authority file not found"}, /* ERR_CA_DB_NOT_FOUND */
+
{ "ERR_LAST" } /* ERR_LAST */
};
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index 49d3982cc3..df366e769f 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -167,6 +167,8 @@ enum sssd_errors {
ERR_TLS_HANDSHAKE_INTERRUPTED,
+ ERR_CA_DB_NOT_FOUND,
+
ERR_LAST /* ALWAYS LAST */
};
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
