URL: https://github.com/SSSD/sssd/pull/1001 Author: sumit-bose Title: #1001: ssh: fix matching rules default Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/1001/head:pr1001 git checkout pr1001
From 9aaad44855945d5f8f7b0079e2c18c341ff9dd0f Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Mon, 9 Mar 2020 13:39:47 +0100 Subject: [PATCH] ssh: fix matching rules default Before the ssh_use_certificate_matching_rules option was added the ssh responder returned ssh keys derived from all valid certificates. Since the default of the ssh_use_certificate_matching_rules option is 'all_rules' in a case where no matching rules are defined all certificated will be filtered out and no ssh keys are returned. The intention of the default was to allow the same same certificates which are allowed in the PAM responder for authentication. The missing default matching rule which is currently use by the PAM responder if no other rules are available is added by this patch. There might still be a small regression in case certificates without the extended key usage (EKU) clientAuth were used for ssh. In this case 'ssh_use_certificate_matching_rules = no_rules' or a suitable matching rule must be added to the configuration. Related to https://pagure.io/SSSD/sssd/issue/4121 --- src/man/sssd.conf.5.xml | 9 ++++- src/responder/pam/pam_helpers.h | 2 ++ src/responder/pam/pamsrv_p11.c | 3 +- src/responder/ssh/ssh_cmd.c | 30 +++++++++++++---- src/tests/cmocka/test_ssh_srv.c | 58 +++++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 9 deletions(-) diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 58383579c0..a2567f5acb 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -1766,6 +1766,13 @@ p11_uri = library-description=OpenSC%20smartcard%20framework;slot-id=2 will be filtered out and ssh keys will be generated from all valid certificates. </para> + <para> + If no rules are configured using 'all_rules' will + enable a default rule which enables all + certificates suitable for client authentication. + This is the same behavior as for the PAM responder + if certificate authentication is enabled. + </para> <para> A non-existing rule name is considered an error. If as a result no rule is selected all certificates @@ -1773,7 +1780,7 @@ p11_uri = library-description=OpenSC%20smartcard%20framework;slot-id=2 </para> <para> Default: not set, equivalent to 'all_rules, - all found rules are used + all found rules or the default rule are used </para> </listitem> </varlistentry> diff --git a/src/responder/pam/pam_helpers.h b/src/responder/pam/pam_helpers.h index 614389706d..23fd308bb4 100644 --- a/src/responder/pam/pam_helpers.h +++ b/src/responder/pam/pam_helpers.h @@ -25,6 +25,8 @@ #include "util/util.h" +#define CERT_AUTH_DEFAULT_MATCHING_RULE "KRB5:<EKU>clientAuth" + errno_t pam_initgr_cache_set(struct tevent_context *ev, hash_table_t *id_table, char *name, diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c index 0dc53a826d..8e276b2001 100644 --- a/src/responder/pam/pamsrv_p11.c +++ b/src/responder/pam/pamsrv_p11.c @@ -26,13 +26,12 @@ #include "util/child_common.h" #include "util/strtonum.h" #include "responder/pam/pamsrv.h" +#include "responder/pam/pam_helpers.h" #include "lib/certmap/sss_certmap.h" #include "util/crypto/sss_crypto.h" #include "db/sysdb.h" -#define CERT_AUTH_DEFAULT_MATCHING_RULE "KRB5:<EKU>clientAuth" - struct cert_auth_info { char *cert; char *token_name; diff --git a/src/responder/ssh/ssh_cmd.c b/src/responder/ssh/ssh_cmd.c index e42e29bfda..a593c904f6 100644 --- a/src/responder/ssh/ssh_cmd.c +++ b/src/responder/ssh/ssh_cmd.c @@ -29,6 +29,7 @@ #include "responder/common/responder.h" #include "responder/common/cache_req/cache_req.h" #include "responder/ssh/ssh_private.h" +#include "responder/pam/pam_helpers.h" #include "lib/certmap/sss_certmap.h" struct ssh_cmd_ctx { @@ -159,6 +160,7 @@ static errno_t ssh_cmd_refresh_certmap_ctx(struct ssh_ctx *ssh_ctx, bool rule_added; bool all_rules = false; bool no_rules = false; + bool rules_present = false; ssh_ctx->cert_rules_error = false; @@ -195,6 +197,7 @@ static errno_t ssh_cmd_refresh_certmap_ctx(struct ssh_ctx *ssh_ctx, } for (c = 0; certmap_list[c] != NULL; c++) { + rules_present = true; if (!all_rules && !string_in_list(certmap_list[c]->name, ssh_ctx->cert_rules, true)) { @@ -227,12 +230,27 @@ static errno_t ssh_cmd_refresh_certmap_ctx(struct ssh_ctx *ssh_ctx, } if (!rule_added) { - DEBUG(SSSDBG_CONF_SETTINGS, - "No matching rule added, please check " - "ssh_use_certificate_matching_rules option values for typos .\n"); - - ret = EINVAL; - goto done; + if (!rules_present) { + DEBUG(SSSDBG_TRACE_FUNC, + "No rules available, trying to add default matching rule.\n"); + ret = sss_certmap_add_rule(sss_certmap_ctx, SSS_CERTMAP_MIN_PRIO, + CERT_AUTH_DEFAULT_MATCHING_RULE, + NULL, NULL); + if (ret != 0) { + DEBUG(SSSDBG_OP_FAILURE, + "Failed to add default matching rule [%d][%s].\n", + ret, sss_strerror(ret)); + goto done; + } + } else { + DEBUG(SSSDBG_CONF_SETTINGS, + "No matching rule added, please check " + "ssh_use_certificate_matching_rules option values for " + "typos.\n"); + + ret = EINVAL; + goto done; + } } ret = EOK; diff --git a/src/tests/cmocka/test_ssh_srv.c b/src/tests/cmocka/test_ssh_srv.c index fc43663a7f..a480134169 100644 --- a/src/tests/cmocka/test_ssh_srv.c +++ b/src/tests/cmocka/test_ssh_srv.c @@ -769,6 +769,62 @@ void test_ssh_user_pubkey_cert_with_all_rules(void **state) assert_int_equal(ret, EOK); } +void test_ssh_user_pubkey_cert_with_all_rules_but_no_rules_present(void **state) +{ + int ret; + struct sysdb_attrs *attrs; + /* Both rules are enabled, both certificates should be handled. */ + const char *rule_list[] = { "all_rules", NULL }; + + attrs = sysdb_new_attrs(ssh_test_ctx); + assert_non_null(attrs); + ret = sysdb_attrs_add_string(attrs, SYSDB_SSH_PUBKEY, TEST_SSH_PUBKEY); + assert_int_equal(ret, EOK); + ret = sysdb_attrs_add_base64_blob(attrs, SYSDB_USER_CERT, + SSSD_TEST_CERT_0001); + assert_int_equal(ret, EOK); + ret = sysdb_attrs_add_base64_blob(attrs, SYSDB_USER_CERT, + SSSD_TEST_CERT_0002); + assert_int_equal(ret, EOK); + + ret = sysdb_set_user_attr(ssh_test_ctx->tctx->dom, + ssh_test_ctx->ssh_user_fqdn, + attrs, + LDB_FLAG_MOD_ADD); + talloc_free(attrs); + assert_int_equal(ret, EOK); + + mock_input_user(ssh_test_ctx, ssh_test_ctx->ssh_user_fqdn); + will_return(__wrap_sss_packet_get_cmd, SSS_SSH_GET_USER_PUBKEYS); + will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); + will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); + will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); + will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); + + /* Enable certificate support */ + ssh_test_ctx->ssh_ctx->use_cert_keys = true; + ssh_test_ctx->ssh_ctx->rctx->domains->certmaps = NULL; + ssh_test_ctx->ssh_ctx->certmap_last_read = 0; + ssh_test_ctx->ssh_ctx->rctx->get_domains_last_call.tv_sec = 1; + ssh_test_ctx->ssh_ctx->cert_rules = discard_const(rule_list); +#ifdef HAVE_NSS + ssh_test_ctx->ssh_ctx->ca_db = discard_const("sql:" ABS_BUILD_DIR + "/src/tests/test_CA/p11_nssdb"); +#else + ssh_test_ctx->ssh_ctx->ca_db = discard_const(ABS_BUILD_DIR + "/src/tests/test_CA/SSSD_test_CA.pem"); +#endif + + set_cmd_cb(test_ssh_user_pubkey_cert_check); + ret = sss_cmd_execute(ssh_test_ctx->cctx, SSS_SSH_GET_USER_PUBKEYS, + ssh_test_ctx->ssh_cmds); + assert_int_equal(ret, EOK); + + /* Wait until the test finishes with EOK */ + ret = test_ev_loop(ssh_test_ctx->tctx); + assert_int_equal(ret, EOK); +} + void test_ssh_user_pubkey_cert_with_no_rules(void **state) { int ret; @@ -966,6 +1022,8 @@ int main(int argc, const char *argv[]) ssh_test_setup, ssh_test_teardown), cmocka_unit_test_setup_teardown(test_ssh_user_pubkey_cert_with_all_rules, ssh_test_setup, ssh_test_teardown), + cmocka_unit_test_setup_teardown(test_ssh_user_pubkey_cert_with_all_rules_but_no_rules_present, + ssh_test_setup, ssh_test_teardown), cmocka_unit_test_setup_teardown(test_ssh_user_pubkey_cert_with_no_rules, ssh_test_setup, ssh_test_teardown), cmocka_unit_test_setup_teardown(test_ssh_user_pubkey_cert_with_unknow_rule_name,
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected] 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/[email protected]
