Hi, currently the code which generates ssh key from the public keys in the user certificates fails if one certificate cannot be validated and terminates the whole request. It is of course valid that the user entry might contain certificates which SSSD cannot validate and since we just won't generate a ssh-key in this case SSSD should just skip those entires and return ssh-keys for every valid certificate.
You can test the patch even without a real certificate by e.g. adding a ssh-key to an IPA user object. Then 'sss_ssh_authorizedkeys username' should return this key. If you now add some random data the the userCertificate object of the same user, call 'sss_cache -E' and call 'sss_ssh_authorizedkeys username' again, you get nothing because the random data cannot be validated and hence the whole request is aborted. With the attached patch sss_ssh_authorizedkeys should return the ssh-key again. bye, Sumit
From 540c69184a128bb840c7f41cabfb0cfe62f344a7 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Thu, 2 Jun 2016 18:22:03 +0200 Subject: [PATCH] ssh: skip invalid certificates Current an invalid certificate cause the whole ssh key lookup request to abort. Since it is possible that e.g. the LDAP user entry contains certificates where the client does not have the needed CA certificates for validation we should just ignore invalid certificates. Resolves https://fedorahosted.org/sssd/ticket/2977 --- src/responder/ssh/sshsrv_cmd.c | 171 ++++++++++++++++++++++++++++++----------- 1 file changed, 126 insertions(+), 45 deletions(-) diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c index 5954cec1ba3a688ae2d14f2304d722d91c26d592..51922d59a8150ceb7cd96e728c1c482b373639a8 100644 --- a/src/responder/ssh/sshsrv_cmd.c +++ b/src/responder/ssh/sshsrv_cmd.c @@ -781,9 +781,94 @@ ssh_cmd_parse_request(struct ssh_cmd_ctx *cmd_ctx) return EOK; } +static errno_t get_valid_certs_keys(TALLOC_CTX *mem_ctx, + struct ssh_cmd_ctx *cmd_ctx, + struct ldb_message_element *el_cert, + struct ssh_ctx *ssh_ctx, + struct ldb_message_element **_el_res) +{ + TALLOC_CTX *tmp_ctx; + uint8_t *key; + size_t key_len; + char *cert_verification_opts; + struct cert_verify_opts *cert_verify_opts; + int ret; + struct ldb_message_element *el_res; + struct cli_ctx *cctx = cmd_ctx->cctx; + size_t d; + + if (el_cert == NULL) { + DEBUG(SSSDBG_TRACE_ALL, "Mssing element, nothing to do.\n"); + return EOK; + } + + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); + return ENOMEM; + } + + ret = confdb_get_string(cctx->rctx->cdb, tmp_ctx, + CONFDB_MONITOR_CONF_ENTRY, + CONFDB_MONITOR_CERT_VERIFICATION, NULL, + &cert_verification_opts); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to read p11_child_timeout from confdb: [%d] %s\n", + ret, sss_strerror(ret)); + return ret; + } + + if (cert_verification_opts != NULL) { + ret = parse_cert_verify_opts(tmp_ctx, cert_verification_opts, + &cert_verify_opts); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Failed to parse verifiy option.\n"); + return ret; + } + } + + el_res = talloc_zero(tmp_ctx, struct ldb_message_element); + if (el_res == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n"); + return ENOMEM; + } + + el_res->values = talloc_array(el_res, struct ldb_val, el_cert->num_values); + if (el_res->values == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n"); + return ENOMEM; + } + + for (d = 0; d < el_cert->num_values; d++) { + ret = cert_to_ssh_key(tmp_ctx, ssh_ctx->ca_db, + el_cert->values[d].data, + el_cert->values[d].length, + cert_verify_opts, &key, &key_len); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "cert_to_ssh_key failed, ignoring.\n"); + continue; + } + + el_res->values[el_res->num_values].data = + talloc_steal(el_res->values, key); + el_res->values[el_res->num_values].length = key_len; + el_res->num_values++; + } + + if (el_res->num_values == 0) { + *_el_res = NULL; + } else { + *_el_res = talloc_steal(mem_ctx, el_res); + } + + return EOK; +} + static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, struct ldb_message_element *el, - bool cert_data, + bool skip_base64_decode, struct ssh_ctx *ssh_ctx, size_t fqname_len, const char *fqname, @@ -797,8 +882,6 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, int ret; size_t d; TALLOC_CTX *tmp_ctx; - char *cert_verification_opts; - struct cert_verify_opts *cert_verify_opts; if (el == NULL) { DEBUG(SSSDBG_TRACE_ALL, "Mssing element, nothing to do.\n"); @@ -812,36 +895,9 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, } for (d = 0; d < el->num_values; d++) { - if (cert_data) { - - ret = confdb_get_string(cctx->rctx->cdb, tmp_ctx, - CONFDB_MONITOR_CONF_ENTRY, - CONFDB_MONITOR_CERT_VERIFICATION, NULL, - &cert_verification_opts); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Failed to read p11_child_timeout from confdb: [%d] %s\n", - ret, sss_strerror(ret)); - return ret; - } - - if (cert_verification_opts != NULL) { - ret = parse_cert_verify_opts(tmp_ctx, cert_verification_opts, - &cert_verify_opts); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Failed to parse verifiy option.\n"); - return ret; - } - } - - ret = cert_to_ssh_key(tmp_ctx, ssh_ctx->ca_db, - el->values[d].data, el->values[d].length, - cert_verify_opts, &key, &key_len); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, "cert_to_ssh_key failed.\n"); - return ret; - } + if (skip_base64_decode) { + key = el->values[d].data; + key_len = el->values[d].length; } else { key = sss_base64_decode(tmp_ctx, (const char *) el->values[d].data, &key_len); @@ -888,12 +944,14 @@ ssh_cmd_build_reply(struct ssh_cmd_ctx *cmd_ctx) struct ldb_message_element *el_override = NULL; struct ldb_message_element *el_orig = NULL; struct ldb_message_element *el_user_cert = NULL; + struct ldb_message_element *el_user_cert_keys = NULL; uint32_t count = 0; const char *name; char *fqname; uint32_t fqname_len; struct ssh_ctx *ssh_ctx = talloc_get_type(cctx->rctx->pvt_ctx, struct ssh_ctx); + TALLOC_CTX *tmp_ctx; ret = sss_packet_new(cctx->creq, 0, sss_packet_get_cmd(cctx->creq->in), @@ -902,6 +960,12 @@ ssh_cmd_build_reply(struct ssh_cmd_ctx *cmd_ctx) return ret; } + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); + return ENOMEM; + } + el = ldb_msg_find_element(cmd_ctx->result, SYSDB_SSH_PUBKEY); if (el) { count = el->num_values; @@ -923,13 +987,21 @@ ssh_cmd_build_reply(struct ssh_cmd_ctx *cmd_ctx) el_user_cert = ldb_msg_find_element(cmd_ctx->result, SYSDB_USER_CERT); if (el_user_cert) { - /* TODO check if cert is valid */ - count += el_user_cert->num_values; + ret = get_valid_certs_keys(cmd_ctx, cmd_ctx, el_user_cert, ssh_ctx, + &el_user_cert_keys); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "get_valid_certs_keys failed.\n"); + goto done; + } + + if (el_user_cert_keys) { + count += el_user_cert_keys->num_values; + } } ret = sss_packet_grow(cctx->creq->out, 2*sizeof(uint32_t)); if (ret != EOK) { - return ret; + goto done; } sss_packet_get_body(cctx->creq->out, &body, &body_len); @@ -937,7 +1009,8 @@ ssh_cmd_build_reply(struct ssh_cmd_ctx *cmd_ctx) SAFEALIGN_SET_UINT32(body+c, 0, &c); if (count == 0) { - return EOK; + ret = EOK; + goto done; } name = ldb_msg_find_attr_as_string(cmd_ctx->result, SYSDB_NAME, NULL); @@ -945,13 +1018,15 @@ ssh_cmd_build_reply(struct ssh_cmd_ctx *cmd_ctx) DEBUG(SSSDBG_OP_FAILURE, "Got unnamed result for [%s@%s]\n", cmd_ctx->name, cmd_ctx->domain->name); - return ENOENT; + ret = ENOENT; + goto done; } fqname = talloc_asprintf(cmd_ctx, "%s@%s", name, cmd_ctx->domain->name); if (!fqname) { - return ENOMEM; + ret = ENOMEM; + goto done; } fqname_len = strlen(fqname)+1; @@ -960,31 +1035,37 @@ ssh_cmd_build_reply(struct ssh_cmd_ctx *cmd_ctx) fqname_len, fqname, &c); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "decode_and_add_base64_data failed.\n"); - return ret; + goto done; } ret = decode_and_add_base64_data(cmd_ctx, el_orig, false, ssh_ctx, fqname_len, fqname, &c); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "decode_and_add_base64_data failed.\n"); - return ret; + goto done; } ret = decode_and_add_base64_data(cmd_ctx, el_override, false, ssh_ctx, fqname_len, fqname, &c); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "decode_and_add_base64_data failed.\n"); - return ret; + goto done; } - ret = decode_and_add_base64_data(cmd_ctx, el_user_cert, true, ssh_ctx, + ret = decode_and_add_base64_data(cmd_ctx, el_user_cert_keys, true, ssh_ctx, fqname_len, fqname, &c); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "decode_and_add_base64_data failed.\n"); - return ret; + goto done; } - return EOK; + ret = EOK; + +done: + + talloc_free(tmp_ctx); + + return ret; } static errno_t -- 2.1.0
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
