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 <[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;
> +    }
> +

[...]

> +    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.
> +}
> +

The rest of the patch LGTM.
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to