On Wed, Feb 24, 2016 at 10:31:31AM +0100, Pavel Březina wrote:
On 02/23/2016 02:09 PM, Sumit Bose wrote:
>On Tue, Feb 23, 2016 at 01:24:30PM +0100, Pavel Březina wrote:
>>https://fedorahosted.org/sssd/ticket/2934
>
>> From 94ae3c5231dc7f1cd9f9d172d13a11a8afcacd16 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
>>Date: Tue, 23 Feb 2016 11:02:42 +0100
>>Subject: [PATCH] remove user certificate if not found on the server
>>
>>If the user is not found by cert lookup when the user is already
>>cached, two things may happen:
>>1) cert was removed from the user object
>>2) user was removed
>>
>>Instead of issuing another cert lookup we will just remove cert
>>attribute from the cache not touching the expiration timestamp so
>>the user may be updated later when needed.
>>
>>Resolves:
>>https://fedorahosted.org/sssd/ticket/2934
>>---
>> src/db/sysdb.h | 3 ++-
>> src/db/sysdb_ops.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
>> src/providers/ldap/ldap_id.c | 10 ++++++++++
>> 3 files changed, 59 insertions(+), 1 deletion(-)
>>
>>diff --git a/src/db/sysdb.h b/src/db/sysdb.h
>>index
2e797fd7fa39163c2ab6a10e51228e0f1af3f9e3..d074d4d661554a798151caee831cc672a927712f 100644
>>--- a/src/db/sysdb.h
>>+++ b/src/db/sysdb.h
>>@@ -1154,7 +1154,8 @@ errno_t sysdb_search_user_by_cert(TALLOC_CTX *mem_ctx,
>> const char *cert,
>> struct ldb_result **res);
>>
>>-
>>+errno_t sysdb_remove_cert(struct sss_domain_info *domain,
>>+ const char *cert);
>>
>> /* === Functions related to GPOs === */
>>
>>diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
>>index
ab0d59ca6db620dfbf7e74a93745df242b6fc3a3..aa688f42f9a6f7f0f86e1171df0a5f0346a59ea5 100644
>>--- a/src/db/sysdb_ops.c
>>+++ b/src/db/sysdb_ops.c
>>@@ -3764,6 +3764,53 @@ errno_t sysdb_search_user_by_cert(TALLOC_CTX *mem_ctx,
>> return sysdb_search_object_by_cert(mem_ctx, domain, cert, user_attrs,
res);
>> }
>>
>>+static errno_t sysdb_remove_user_cert(struct sss_domain_info *domain,
>>+ const char *name)
>>+{
>>+ struct ldb_message_element el = { 0, SYSDB_USER_CERT, 0, NULL };
>>+ struct sysdb_attrs attrs = { 1, &el };
>>+
>>+ DEBUG(SSSDBG_TRACE_FUNC, "Removing certificate from user %s@%s",
>>+ name, domain->name);
>>+
>>+ return sysdb_set_user_attr(domain, name, &attrs, SYSDB_MOD_DEL);
>
>I would recommend to use sysdb_set_entry_attr() because you already have
>the dn as res->msgs[0]->dn in sysdb_remove_cert. With this I think you
>can move everything into sysdb_remove_cert() as well without making to
>more complex or longer.
>
>bye,
>Sumit
Good idea, see attached patch.
Thank you, the patch is working as expected and passes CI as well.
Nevertheless see comment below.
From 04acbe4139a560049b66c15ce8526c16ef668d54 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Tue, 23 Feb 2016 11:02:42 +0100
Subject: [PATCH] remove user certificate if not found on the server
If the user is not found by cert lookup when the user is already
cached, two things may happen:
1) cert was removed from the user object
2) user was removed
Instead of issuing another cert lookup we will just remove cert
attribute from the cache not touching the expiration timestamp so
the user may be updated later when needed.
Resolves:
https://fedorahosted.org/sssd/ticket/2934
---
src/db/sysdb.h | 3 ++-
src/db/sysdb_ops.c | 31 +++++++++++++++++++++++++++++++
src/providers/ldap/ldap_id.c | 10 ++++++++++
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 2e797fd7fa39163c2ab6a10e51228e0f1af3f9e3..d074d4d661554a798151caee831cc672a927712f
100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1154,7 +1154,8 @@ errno_t sysdb_search_user_by_cert(TALLOC_CTX *mem_ctx,
const char *cert,
struct ldb_result **res);
-
+errno_t sysdb_remove_cert(struct sss_domain_info *domain,
+ const char *cert);
/* === Functions related to GPOs === */
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index ab0d59ca6db620dfbf7e74a93745df242b6fc3a3..573f3de001ffabd6e6aa17d8e808bf9363531608
100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -3764,6 +3764,37 @@ errno_t sysdb_search_user_by_cert(TALLOC_CTX *mem_ctx,
return sysdb_search_object_by_cert(mem_ctx, domain, cert, user_attrs, res);
}
+errno_t sysdb_remove_cert(struct sss_domain_info *domain,
+ const char *cert)
+{
+ struct ldb_message_element el = { 0, SYSDB_USER_CERT, 0, NULL };
+ struct sysdb_attrs del_attrs = { 1, &el };
+ const char *attrs[] = {SYSDB_NAME, NULL};
+ struct ldb_result *res = NULL;
+ errno_t ret;
+
+ ret = sysdb_search_object_by_cert(NULL, domain, cert, attrs, &res);
+ if (ret == ENOENT || res == NULL || res->count == 0) {
+ ret = EOK;
+ goto done;
+ } else if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to lookup object by cert "
+ "[%d]: %s\n", ret, sss_strerror(ret));
+ goto done;
+ } else if (res->count > 1) {
I think this case might actually happen and is valid. If the certificate
is moved from one user to another one on the server and the new news is
looked up the certificate will be in the user entry of the old and new
owner because we do not force uniqueness here. Authentication won't work
because we cannot find a unique user but nonetheless the case might
happen.
To solve this I would suggest to remove the certificate from all entries
and reset SYSDB_CACHE_EXPIRE as well. This way the certificate is remove
from the old owner and will be read again for the new owner.
Additionally I think SYSDB_CACHE_EXPIRE should be reset always. Since
it is possible to assign multiple certificates to a user the user should
be looked up again in case only a single certificate is removed on the
server side.
bye,
Sumit
+ DEBUG(SSSDBG_MINOR_FAILURE, "More then one result
found!");
+ ret = ERR_INTERNAL;
+ goto done;
+ }
+
+ ret = sysdb_set_entry_attr(domain->sysdb, res->msgs[0]->dn,
+ &del_attrs, SYSDB_MOD_DEL);
+
+done:
+ talloc_free(res);
+ return ret;
+}
+
errno_t sysdb_get_sids_of_members(TALLOC_CTX *mem_ctx,
struct sss_domain_info *dom,
const char *group_name,
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index b7cef4e13d70f738428cdbf70b661de92a87b4a5..8923e7e0c3fdae4614d6bfef2665de7854b62e8e
100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -529,6 +529,16 @@ static void users_get_done(struct tevent_req *subreq)
*/
break;
+ case BE_FILTER_CERT:
+ ret = sysdb_remove_cert(state->domain, state->name);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to remove user
certificate"
+ "[%d]: %s\n", ret, sss_strerror(ret));
+ tevent_req_error(req, ret);
+ return;
+ }
+ break;
+
default:
tevent_req_error(req, EINVAL);
return;
--
2.1.0
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org