On Fri, Jun 03, 2016 at 08:17:01PM +0200, Sumit Bose wrote:
> 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 <sbose(a)redhat.com>
> 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;
> + }
> +
[...]
> + if (el_res->num_values == 0) {
> + *_el_res = NULL;
> + } else {
> + *_el_res = talloc_steal(mem_ctx, el_res);
> + }
> +
> + return EOK;
You need to free tmp_ctx in this function.
ah. sorry, I added it to the caller and forgot to add it to the new
function. New version attached.
bye,
Sumit
> +}
> +
The rest of the patch LGTM.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org