[SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary

2015-08-10 Thread Jakub Hrozek
Hi,

the attached patches fix #2742. The first one makes sure we can print
the certificate (or any binary attribute, really) safely. We only need
to make sure to escape the attribute values before saving them to sysdb,
because then ldb guarantees terminating them.

The second just switches the attribute value. I tested using this howto:
http://www.freeipa.org/page/V4/User_Certificates#How_to_Test

You'll also want to use a recent enough IPA version, one that fixes:
https://fedorahosted.org/freeipa/ticket/5173

Then, on the client, call:
dbus-send --print-reply \
  --system \
  --dest=org.freedesktop.sssd.infopipe \
  /org/freedesktop/sssd/infopipe/Users \
  org.freedesktop.sssd.infopipe.Users.FindByCertificate \
  string:"$( openssl x509 < cert.pem )"

The result will be an object path.
>From ac7c546415dad4ae449f1708d8efc53169ab0c9c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Mon, 10 Aug 2015 12:40:30 +0200
Subject: [PATCH 1/2] LDAP: use ldb_binary_encode when printing attribute
 values

---
 src/providers/ldap/sdap_utils.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/providers/ldap/sdap_utils.c b/src/providers/ldap/sdap_utils.c
index 
f5ce8ee54f60a6c4c4cdbd5e50b20d973c175e83..9da46ea70bf80e7f4d12fdfc7d1c97e99de8d000
 100644
--- a/src/providers/ldap/sdap_utils.c
+++ b/src/providers/ldap/sdap_utils.c
@@ -35,6 +35,7 @@ sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs,
 const char *objname = name ?: "object";
 const char *desc = attr_desc ?: attr_name;
 unsigned int num_values, i;
+char *printable;
 
 ret = sysdb_attrs_get_el(ldap_attrs, attr_name, &el);
 if (ret) {
@@ -50,8 +51,16 @@ sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs,
 } else {
 num_values = multivalued ? el->num_values : 1;
 for (i = 0; i < num_values; i++) {
+printable = ldb_binary_encode(ldap_attrs, el->values[i]);
+if (printable == NULL) {
+DEBUG(SSSDBG_MINOR_FAILURE, "ldb_binary_encode failed..\n");
+continue;
+}
+
 DEBUG(SSSDBG_TRACE_INTERNAL, "Adding %s [%s] to attributes "
-  "of [%s].\n", desc, el->values[i].data, objname);
+  "of [%s].\n", desc, printable, objname);
+
+talloc_zfree(printable);
 
 ret = sysdb_attrs_add_mem(attrs, attr_name, el->values[i].data,
   el->values[i].length);
-- 
2.4.3

>From cf2ef9a8f0f880b0d722d0740952c7bfc3dd1502 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Mon, 10 Aug 2015 12:40:39 +0200
Subject: [PATCH 2/2] IPA: Change the default of ldap_user_certificate to
 userCertificate;binary

This is safe from ldb point of view, because ldb gurantees the data is
NULL-terminated. We must be careful before we save the data, though.

Resolves:
https://fedorahosted.org/sssd/ticket/2742
---
 src/man/sssd-ldap.5.xml  | 2 +-
 src/providers/ipa/ipa_opts.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
index 
978fdbe773cddce2d2fc5f78109bf7316b00b0a3..123ac3fac3cb1feaef67ba44be65f98cd0ab8043
 100644
--- a/src/man/sssd-ldap.5.xml
+++ b/src/man/sssd-ldap.5.xml
@@ -821,7 +821,7 @@
 certificate of the user.
 
 
-Default: no set in the general case, 
userCertificate
+Default: no set in the general case, 
userCertificate;binary
 for IPA
 
 
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h
index 
9576228d1bf3424c8867bda058b59c3ca6b2216b..f6c40dddbb58cd8af1079a351137422083e26cfe
 100644
--- a/src/providers/ipa/ipa_opts.h
+++ b/src/providers/ipa/ipa_opts.h
@@ -204,7 +204,7 @@ struct sdap_attr_map ipa_user_map[] = {
 { "ldap_user_nds_login_allowed_time_map", "loginAllowedTimeMap", 
SYSDB_NDS_LOGIN_ALLOWED_TIME_MAP, NULL },
 { "ldap_user_ssh_public_key", "ipaSshPubKey", SYSDB_SSH_PUBKEY, NULL },
 { "ldap_user_auth_type", "ipaUserAuthType", SYSDB_AUTH_TYPE, NULL },
-{ "ldap_user_certificate", "userCertificate", SYSDB_USER_CERT, NULL },
+{ "ldap_user_certificate", "userCertificate;binary", SYSDB_USER_CERT, NULL 
},
 SDAP_ATTR_MAP_TERMINATOR
 };
 
-- 
2.4.3

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary

2015-08-12 Thread Jakub Hrozek
On Mon, Aug 10, 2015 at 12:59:24PM +0200, Jakub Hrozek wrote:
> Hi,
> 
> the attached patches fix #2742. The first one makes sure we can print
> the certificate (or any binary attribute, really) safely. We only need
> to make sure to escape the attribute values before saving them to sysdb,
> because then ldb guarantees terminating them.
> 
> The second just switches the attribute value. I tested using this howto:
> http://www.freeipa.org/page/V4/User_Certificates#How_to_Test
> 
> You'll also want to use a recent enough IPA version, one that fixes:
> https://fedorahosted.org/freeipa/ticket/5173
> 
> Then, on the client, call:
> dbus-send --print-reply \
>   --system \
>   --dest=org.freedesktop.sssd.infopipe \
>   /org/freedesktop/sssd/infopipe/Users \
>   org.freedesktop.sssd.infopipe.Users.FindByCertificate \
>   string:"$( openssl x509 < cert.pem )"
> 
> The result will be an object path.

Jan, any chance you can test these patches if I build you a package?
(Sorry, Sumit is on vacation)
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary

2015-08-13 Thread Pavel Březina

On 08/10/2015 12:59 PM, Jakub Hrozek wrote:

Hi,

the attached patches fix #2742. The first one makes sure we can print
the certificate (or any binary attribute, really) safely. We only need
to make sure to escape the attribute values before saving them to sysdb,
because then ldb guarantees terminating them.

The second just switches the attribute value. I tested using this howto:
 http://www.freeipa.org/page/V4/User_Certificates#How_to_Test

You'll also want to use a recent enough IPA version, one that fixes:
 https://fedorahosted.org/freeipa/ticket/5173

Then, on the client, call:
 dbus-send --print-reply \
   --system \
   --dest=org.freedesktop.sssd.infopipe \
   /org/freedesktop/sssd/infopipe/Users \
   org.freedesktop.sssd.infopipe.Users.FindByCertificate \
   string:"$( openssl x509 < cert.pem )"

The result will be an object path.


Ack.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary

2015-08-14 Thread Jakub Hrozek
On Thu, Aug 13, 2015 at 12:43:35PM +0200, Pavel Březina wrote:
> On 08/10/2015 12:59 PM, Jakub Hrozek wrote:
> >Hi,
> >
> >the attached patches fix #2742. The first one makes sure we can print
> >the certificate (or any binary attribute, really) safely. We only need
> >to make sure to escape the attribute values before saving them to sysdb,
> >because then ldb guarantees terminating them.
> >
> >The second just switches the attribute value. I tested using this howto:
> > http://www.freeipa.org/page/V4/User_Certificates#How_to_Test
> >
> >You'll also want to use a recent enough IPA version, one that fixes:
> > https://fedorahosted.org/freeipa/ticket/5173
> >
> >Then, on the client, call:
> > dbus-send --print-reply \
> >   --system \
> >   --dest=org.freedesktop.sssd.infopipe \
> >   /org/freedesktop/sssd/infopipe/Users \
> >   org.freedesktop.sssd.infopipe.Users.FindByCertificate \
> >   string:"$( openssl x509 < cert.pem )"
> >
> >The result will be an object path.
> 
> Ack.

Thanks for the patience during the tmate.io review :-)

Pushed to master:
32445affe3612428eddde043cdc672a01c189714
619e21ed9c7a71e35e53f38867b53ed974f1d36a
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary

2015-08-21 Thread Jan Cholasta

Hi,

On 10.8.2015 12:59, Jakub Hrozek wrote:

Hi,

the attached patches fix #2742. The first one makes sure we can print
the certificate (or any binary attribute, really) safely. We only need
to make sure to escape the attribute values before saving them to sysdb,
because then ldb guarantees terminating them.

The second just switches the attribute value. I tested using this howto:
 http://www.freeipa.org/page/V4/User_Certificates#How_to_Test

You'll also want to use a recent enough IPA version, one that fixes:
 https://fedorahosted.org/freeipa/ticket/5173

Then, on the client, call:
 dbus-send --print-reply \
   --system \
   --dest=org.freedesktop.sssd.infopipe \
   /org/freedesktop/sssd/infopipe/Users \
   org.freedesktop.sssd.infopipe.Users.FindByCertificate \
   string:"$( openssl x509 < cert.pem )"

The result will be an object path.


LGTM, but I would think userCertificate;binary should be the default 
everywhere, i.e. generic LDAP, as that is the correct attribute name 
according to RFC 4523. IMHO when someone uses the standard name in 
generic LDAP, they should not be forced to change SSSD configuration 
because of it.


Honza

--
Jan Cholasta
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary

2015-08-21 Thread Simo Sorce
On Fri, 2015-08-21 at 09:50 +0200, Jan Cholasta wrote:
> Hi,
> 
> On 10.8.2015 12:59, Jakub Hrozek wrote:
> > Hi,
> >
> > the attached patches fix #2742. The first one makes sure we can print
> > the certificate (or any binary attribute, really) safely. We only need
> > to make sure to escape the attribute values before saving them to sysdb,
> > because then ldb guarantees terminating them.
> >
> > The second just switches the attribute value. I tested using this howto:
> >  http://www.freeipa.org/page/V4/User_Certificates#How_to_Test
> >
> > You'll also want to use a recent enough IPA version, one that fixes:
> >  https://fedorahosted.org/freeipa/ticket/5173
> >
> > Then, on the client, call:
> >  dbus-send --print-reply \
> >--system \
> >--dest=org.freedesktop.sssd.infopipe \
> >/org/freedesktop/sssd/infopipe/Users \
> >org.freedesktop.sssd.infopipe.Users.FindByCertificate \
> >string:"$( openssl x509 < cert.pem )"
> >
> > The result will be an object path.
> 
> LGTM, but I would think userCertificate;binary should be the default 
> everywhere, i.e. generic LDAP, as that is the correct attribute name 
> according to RFC 4523. IMHO when someone uses the standard name in 
> generic LDAP, they should not be forced to change SSSD configuration 
> because of it.

+1

Simo

-- 
Simo Sorce * Red Hat, Inc * New York

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel