[SSSD] [PATCHES] IFP: add FindByCertificate method for User objects
Pavel Březina
pbrezina at redhat.com
Tue Jun 16 08:44:35 UTC 2015
On 06/11/2015 01:08 PM, Sumit Bose wrote:
> On Thu, Jun 04, 2015 at 12:31:34PM +0200, Pavel Březina wrote:
>> On 06/04/2015 11:06 AM, Sumit Bose wrote:
>>> Hi,
>>>
>>> this patch-set aims to solve https://fedorahosted.org/sssd/ticket/2596.
>>> The first patch is unrelated but needed to make the changes in the
>>> second patch properly aligned.
>>>
>>> Patches 3,4 and 6 add some certificate related utilities while patch 5
>>> adds the backend changes and patch 7 the changes for InfoPipe.
>>>
>>> bye,
>>> Sumit
>
> Hi Pavel,
>
> thank you for the review.
>
>>
>>> +errno_t sysdb_search_object_by_cert(TALLOC_CTX *mem_ctx,
>>> + struct sss_domain_info *domain,
>>> + const char *cert,
>>> + const char **attrs,
>>> + struct ldb_result **res)
>>> +{
>>> + int ret;
>>> + char *user_filter;
>>> +
>>> + ret = sss_cert_derb64_to_ldap_filter(mem_ctx, cert, SYSDB_USER_CERT,
>>> + &user_filter);
>>> + if (ret != EOK) {
>>> + DEBUG(SSSDBG_OP_FAILURE, "sss_cert_derb64_to_ldap_filter failed.\n");
>>> + return ret;
>>> + }
>>> +
>> vv two spaces here
>
> fixed
>
>>> + ret = sysdb_search_object_by_str_attr(mem_ctx, domain,
>>> + SYSDB_USER_CERT_FILTER,
>>> + user_filter, attrs, res);
>>> + talloc_free(user_filter);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +errno_t sysdb_search_user_by_cert(TALLOC_CTX *mem_ctx,
>>> + struct sss_domain_info *domain,
>>> + const char *cert,
>>> + struct ldb_result **res)
>>> +{
>>> + const char *user_attrs[] = SYSDB_PW_ATTRS;
>>> +
>>> + return sysdb_search_object_by_cert(mem_ctx, domain, cert, user_attrs, res);
>> ^^ two spaces here
>
> fixed
>
>>> +}
>>
>> cache_req:
>>> search_str = state->input->type == CACHE_REQ_USER_BY_CERT ?
>>> state->input->orig_name : state->input->dom_objname;
>>
>> I think it will be better to create a new field in cache_req_input, say
>> 'cert', to not abuse fields that resemble object names. The code will be
>> cleaner and you can get rid of few parts like:
>
> I added cert.
>
>>
>>> if (state->input->orig_name != NULL && domain == NULL
>>> && (input->type == CACHE_REQ_USER_BY_NAME
>>> || input->type == CACHE_REQ_GROUP_BY_NAME
>>> || input->type == CACHE_REQ_INITGROUPS))
>
> new version attached.
>
> bye,
> Sumit
Hi,
I rebased it on top of current master (just trivial Makefile.am
conflicts) and fixed some indentation. Diff:
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 3aa7099..6d0aede 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -3720,8 +3720,8 @@ errno_t sysdb_search_object_by_cert(TALLOC_CTX
*mem_ctx,
}
ret = sysdb_search_object_by_str_attr(mem_ctx, domain,
- SYSDB_USER_CERT_FILTER,
- user_filter, attrs, res);
+ SYSDB_USER_CERT_FILTER,
+ user_filter, attrs, res);
talloc_free(user_filter);
return ret;
Ack to all patches but the last.
> @@ -82,6 +83,17 @@ cache_req_input_create(TALLOC_CTX *mem_ctx,
> goto fail;
> }
> break;
> + case CACHE_REQ_USER_BY_CERT:
> + if (name == NULL) {
> + DEBUG(SSSDBG_CRIT_FAILURE, "Bug: certificate cannot be NULL!\n");
> + goto fail;
> + }
Can you create a parameter 'cert' instead of abusing 'name' here?
> +
> + input->cert = talloc_strdup(input, name);
> + if (input->cert == NULL) {
> + goto fail;
> + }
> + break;
> + case CACHE_REQ_USER_BY_CERT:
> + fqn = talloc_asprintf(tmp_ctx, "CERT:%s@%s", input->cert, domain->name);
> + if (fqn == NULL) {
> + ret = ENOMEM;
> + goto done;
> + }
> + break;
The 'fqn' is used for debugging purpose. How large can string
'input->cert' get? I'm a little worried that it will just spam log files
with long string noone will ever read. Wouldn't it suffice to just print
few first/middle/last characters (or combined)?
> + search_str = (state->input->type == CACHE_REQ_USER_BY_CERT) ?
> + state->input->cert : state->input->dom_objname;
Since you have to break the line anyway, the following may read better,
what do you think?
search_str = state->input->dom_objname;
if (state->input->type == CERT) {
search_str = ...
}
> --- a/src/sbus/sssd_dbus_errors.h
> +++ b/src/sbus/sssd_dbus_errors.h
> @@ -25,5 +25,6 @@
>
> #define SBUS_ERROR_INTERNAL "org.freedesktop.sssd.Error.Internal"
> #define SBUS_ERROR_NOT_FOUND "org.freedesktop.sssd.Error.NotFound"
> +#define SBUS_ERROR_EINVAL "org.freedesktop.sssd.Error.InvalidArgument"
D-Bus error for EINVAL already exist: DBUS_ERROR_INVALID_ARGS. I think
we should stick to standard ones when applicable.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-adding-ldap_user_auth_type-where-missing.patch
Type: text/x-patch
Size: 3265 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150616/6d513138/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-LDAP-add-ldap_user_certificate-option.patch
Type: text/x-patch
Size: 8175 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150616/6d513138/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-certs-add-PEM-DER-conversion-utilities.patch
Type: text/x-patch
Size: 30772 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150616/6d513138/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-sysdb-add-sysdb_search_user_by_cert-and-sysdb_search.patch
Type: text/x-patch
Size: 8003 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150616/6d513138/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-LDAP-IPA-add-user-lookup-by-certificate.patch
Type: text/x-patch
Size: 8645 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150616/6d513138/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-ncache-add-calls-for-certificate-based-searches.patch
Type: text/x-patch
Size: 7297 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150616/6d513138/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-IFP-add-FindByCertificate-method-for-User-objects.patch
Type: text/x-patch
Size: 20423 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150616/6d513138/attachment-0006.bin>
More information about the sssd-devel
mailing list