Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

2013-11-27 Thread Nathaniel McCallum 
On Wed, 2013-11-27 at 12:28 +0100, Petr Viktorin wrote:
> >> ipatokenradiusserver is not validated. See validate_searchtimelimit in
> >> the config plugin for an example validator. You can use validate_ipaddr
> >> and validate_hostname from ipalib.util.
> >
> > Fixed.
> 
> Now the validation is too strict, a port is not accepted.

Fixed.

> Should non-FQDN hostnames be allowed?

I agree they should not. Fixed.

> >> ipatokenusermapattribute is also not validated. Not sure if it needs to be.
> >
> > I don't think validation is really possible outside of the permitted
> > characters for an LDAP attribute.
> 
> I think if "$%^&*" is allowed, we'll get a bug from QA soon enough.

Fixed.

> We generally output lists; this should also be a list with one element.

Fixed.

> Attaching updated tests.

A few of these tests are still failing for me, but it is not immediately
obvious why. They seem to be getting answers from previous queries. I'm
not sure if this is something wrong with my code or the tests. Can you
take a look at it?

Nathaniel
>From beb846295d68d2c5ce958bcf08790279761aaf58 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Mon, 11 Nov 2013 17:58:02 -0400
Subject: [PATCH] Add RADIUS proxy support to ipalib CLI

https://fedorahosted.org/freeipa/ticket/3368
---
 API.txt|  95 +++--
 VERSION|   2 +-
 install/share/70ipaotp.ldif|   2 +-
 install/updates/20-indices.update  |   7 ++
 install/updates/25-referint.update |   1 +
 install/updates/40-otp.update  |   5 ++
 ipalib/constants.py|   1 +
 ipalib/plugins/config.py   |   2 +-
 ipalib/plugins/radiusproxy.py  | 166 +
 ipalib/plugins/user.py |  65 +--
 10 files changed, 328 insertions(+), 18 deletions(-)
 create mode 100644 ipalib/plugins/radiusproxy.py

diff --git a/API.txt b/API.txt
index c29efad3382ff59e4753eff5354cba72bc1fe027..328fcf76ac6c8a8f5349e9f5631ec61bae7b3ea4 100644
--- a/API.txt
+++ b/API.txt
@@ -523,7 +523,7 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
 option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
 option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
 option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, required=False)
-option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password',))
+option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password', u'radius'))
 option: Str('ipauserobjectclasses', attribute=True, autofill=False, cli_name='userobjectclasses', csv=True, multivalue=True, required=False)
 option: IA5Str('ipausersearchfields', attribute=True, autofill=False, cli_name='usersearch', multivalue=False, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -2551,6 +2551,81 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: Output('value', , None)
+command: radiusproxy_add
+args: 1,11,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
+option: Str('addattr*', cli_name='addattr', exclude='webui')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
+option: Int('ipatokenradiusretries', attribute=True, cli_name='retries', maxvalue=10, minvalue=0, multivalue=False, required=False)
+option: Password('ipatokenradiussecret', attribute=True, cli_name='secret', confirm=True, multivalue=False, required=True)
+option: Str('ipatokenradiusserver', attribute=True, cli_name='server', multivalue=True, required=True)
+option: Int('ipatokenradiustimeout', attribute=True, cli_name='timeout', minvalue=1, multivalue=False, required=False)
+option: Str('ipatokenusermapattribute', attribute=True, cli_name='userattr', multivalue=False, required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Str('version?', exclude='webui')
+output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (, ), None)
+output: Output('value', , None)
+command: radiusproxy_del
+args: 1,2,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=True, primary_key=True, query=Tr

Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

2013-11-27 Thread Simo Sorce 
On Wed, 2013-11-27 at 15:12 -0500, Nathaniel McCallum wrote:
> On Wed, 2013-11-27 at 14:34 +, Simo Sorce wrote:
> > On Thu, 2013-11-21 at 15:54 -0500, Dmitri Pal wrote:
> > > On 11/21/2013 01:34 PM, Nathaniel McCallum wrote:
> > > >> The password can be retrieved with radiusproxy-show --all, because it 
> > > >> is 
> > > >> > not blocked by LDAP ACIs. Is that intended?
> > > > Yes. But I'm torn as to whether or not this is a good idea. Regular
> > > > users can't see radius proxy servers at all. Admins can see all
> > > > attributes.
> > > >
> > > > It is common in radius server deployments to have a text file readable
> > > > by root with the radius secret. The current LDAP policy replicates this
> > > > "expected" behavior. It may be wise to block all reads of the secret
> > > > though. I'm open to suggestions.
> > > >
> > > If it is readable by admin only I would leave it as is for now and
> > > address later when we redo ACIs.
> > 
> > Is this specific to the one and only admin account or does it extend to
> > any user in the admins group ?
> 
> All admins. See ipatokenRadiusConfiguration in
> install/share/default-aci.ldif. Read access is denied to everyone except
> admins. The entire class is hidden from normal users. See below.

Oh I see it now, sorry, my fault in reading that ACI.
I can't wait to finally straighten out our ACI system...

> > Looking at the current master it seem *any* user except anonymous can
> > read secrets ? Or is there a patch I am missing ?
> > I think this is too broad.
> 
> [root@freeipa ~]# kinit admin
> Password for ad...@example.com: 
> [root@freeipa ~]# ipa radiusproxy-find
> -
> 1 RADIUS proxy server matched
> -
>   RADIUS proxy server name: foo
>   Server: foo
> 
> Number of entries returned 1
> 
> 
> [root@freeipa ~]# kinit test
> Password for t...@example.com: 
> kinit: Password incorrect while getting initial credentials
> [root@freeipa ~]# kinit test
> Password for t...@example.com: 
> [root@freeipa ~]# ipa radiusproxy-find
> --
> 0 RADIUS proxy servers matched
> --
> 
> Number of entries returned 0
> 

Looks good, I would still prefer to not make the password readable by
default to admins, but I think we can further restrict this later once
we have the new ACI system in place, which should make it easier to
handle these cases.

Simo.

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

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

2013-11-27 Thread Nathaniel McCallum 
On Wed, 2013-11-27 at 14:34 +, Simo Sorce wrote:
> On Thu, 2013-11-21 at 15:54 -0500, Dmitri Pal wrote:
> > On 11/21/2013 01:34 PM, Nathaniel McCallum wrote:
> > >> The password can be retrieved with radiusproxy-show --all, because it is 
> > >> > not blocked by LDAP ACIs. Is that intended?
> > > Yes. But I'm torn as to whether or not this is a good idea. Regular
> > > users can't see radius proxy servers at all. Admins can see all
> > > attributes.
> > >
> > > It is common in radius server deployments to have a text file readable
> > > by root with the radius secret. The current LDAP policy replicates this
> > > "expected" behavior. It may be wise to block all reads of the secret
> > > though. I'm open to suggestions.
> > >
> > If it is readable by admin only I would leave it as is for now and
> > address later when we redo ACIs.
> 
> Is this specific to the one and only admin account or does it extend to
> any user in the admins group ?

All admins. See ipatokenRadiusConfiguration in
install/share/default-aci.ldif. Read access is denied to everyone except
admins. The entire class is hidden from normal users. See below.

> Looking at the current master it seem *any* user except anonymous can
> read secrets ? Or is there a patch I am missing ?
> I think this is too broad.

[root@freeipa ~]# kinit admin
Password for ad...@example.com: 
[root@freeipa ~]# ipa radiusproxy-find
-
1 RADIUS proxy server matched
-
  RADIUS proxy server name: foo
  Server: foo

Number of entries returned 1


[root@freeipa ~]# kinit test
Password for t...@example.com: 
kinit: Password incorrect while getting initial credentials
[root@freeipa ~]# kinit test
Password for t...@example.com: 
[root@freeipa ~]# ipa radiusproxy-find
--
0 RADIUS proxy servers matched
--

Number of entries returned 0




___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0086 Make Expression field required when adding automember condition

2013-11-27 Thread Petr Vobornik 

On 11/26/2013 03:58 PM, Ana Krivokapic wrote:

Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/4053.



ACK, pushed to master.
--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH 0207] Do not load invalid zones

2013-11-27 Thread Petr Spacek 

Hello,

Do not load invalid zones.

Without this patch, it was possible to load an invalid zone without
proper SOA or NS records because the fake SOA and NS records allowed
checks in dns_zone_load() to pass.

With this patch, no fake SOA or NS records are created and
dns_zone_load() is not called before end of the initial synchronization.

See the function ldapdb_associate() in ldap_driver.c and it's comments.

--
Petr^2 Spacek
From bd2f1f3d3c13d3efe5833146eb5bcb2bbf76b8d3 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Wed, 27 Nov 2013 16:25:30 +0100
Subject: [PATCH] Do not load invalid zones.

Without this patch, it was possible to load an invalid zone without
proper SOA or NS records because the fake SOA and NS records allowed
checks in dns_zone_load() to pass.

With this patch, no fake SOA or NS records are created and
dns_zone_load() is not called before end of the initial synchronization.

See the function ldapdb_associate() in ldap_driver.c and it's comments.

Signed-off-by: Petr Spacek 
---
 src/ldap_driver.c   | 124 
 src/ldap_driver.h   |  16 ++-
 src/ldap_helper.c   | 110 --
 src/ldap_helper.h   |   9 ++--
 src/syncrepl.c  |   2 +-
 src/types.h |   3 ++
 src/zone_register.c |  15 +--
 src/zone_register.h |   2 -
 8 files changed, 161 insertions(+), 120 deletions(-)

diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 11ade10a12edc89c4bd0e713788cc56b8afee83d..105c8a23dd42157f51eafdddb4f44f02335fb71e 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -66,7 +66,7 @@
 #define VALID_LDAPDB(ldapdb) \
 	((ldapdb) != NULL && (ldapdb)->common.impmagic == LDAPDB_MAGIC)
 
-typedef struct {
+struct ldapdb {
 	dns_db_t			common;
 	isc_refcount_t			refs;
 	ldap_instance_t			*ldap_inst;
@@ -92,7 +92,7 @@ typedef struct {
 	 * The purpose is to detect moment when the new version is closed.
 	 * That is the right time for unlocking newversion_lock. */
 	dns_dbversion_t			*newversion;
-} ldapdb_t;
+};
 
 dns_db_t * ATTR_NONNULLS
 ldapdb_get_rbtdb(dns_db_t *db) {
@@ -909,83 +909,57 @@ dns_ns_buildrdata(dns_name_t *origin, dns_name_t *ns_name,
   &ns, &rdatabuf));
 }
 
-/*
- * Create an SOA record for a newly-created zone
+/**
+ * Associate a pre-existing LDAP DB instance with a new DNS zone.
+ *
+ * @warning This is a hack.
+ *
+ * Normally, an empty database is created by dns_db_create() call during
+ * dns_zone_load().
+ *
+ * In our case, we need to create and populate databases on-the-fly
+ * as we process data from LDAP.
+ * We create an empty LDAP DB (which encapsulates internal RBT DB)
+ * for each zone when the zone is being added to zone_register.
+ *
+ * The database in zone register is modified on-the-fly and subsequent
+ * dns_db_create() call associates this populated database with the DNS zone.
+ *
+ * This allows us to call dns_zone_load() later when all the data are in place,
+ * so dns_zone_load() can be postponed until synchronization state sync_finish
+ * is reached.
+ *
+ * @param[in] argv [0] is database instance name
  */
-static isc_result_t ATTR_NONNULLS
-add_soa(isc_mem_t *mctx, dns_name_t *origin, dns_db_t *db) {
+isc_result_t
+ldapdb_associate(isc_mem_t *mctx, dns_name_t *name, dns_dbtype_t type,
+		 dns_rdataclass_t rdclass, unsigned int argc, char *argv[],
+		 void *driverarg, dns_db_t **dbp) {
+
 	isc_result_t result;
-	dns_rdata_t rdata_soa = DNS_RDATA_INIT;
-	dns_rdata_t rdata_ns = DNS_RDATA_INIT;
-	unsigned char buf_soa[DNS_SOA_BUFFERSIZE];
-	unsigned char buf_ns[DNS_SOA_BUFFERSIZE];
-	dns_fixedname_t ns_name;
-	dns_fixedname_t m_name;
-	dns_dbversion_t *ver = NULL;
-	dns_difftuple_t *tp_soa = NULL;
-	dns_difftuple_t *tp_ns = NULL;
-	dns_diff_t diff;
+	ldap_instance_t *ldap_inst = NULL;
+	zone_register_t *zr = NULL;
 
-	dns_diff_init(mctx, &diff);
-	result = dns_db_newversion(db, &ver);
-	if (result != ISC_R_SUCCESS) {
-		log_error_r("add_soa:dns_db_newversion");
-		goto failure;
-	}
+	UNUSED(driverarg); /* Currently we don't need any data */
 
-	/* Build SOA record */
-	dns_fixedname_init(&m_name);
-	dns_name_fromstring(dns_fixedname_name(&m_name), "pspacek.brq.redhat.com.", 0, mctx);
-	result = dns_soa_buildrdata(dns_fixedname_name(&m_name), dns_rootname, dns_rdataclass_in,
-0, 0, 0, 0, 0, buf_soa, &rdata_soa);
-	if (result != ISC_R_SUCCESS) {
-		log_error_r("add_soa:dns_soa_buildrdata");
-		goto failure;
-	}
+	REQUIRE(ISCAPI_MCTX_VALID(mctx));
+	REQUIRE(argc == LDAP_DB_ARGC);
+	REQUIRE(type == LDAP_DB_TYPE);
+	REQUIRE(rdclass == LDAP_DB_RDATACLASS);
+	REQUIRE(dbp != NULL && *dbp == NULL);
 
-	result = dns_difftuple_create(mctx, DNS_DIFFOP_ADD, origin, 3600,
-  &rdata_soa, &tp_soa);
-	if (result != ISC_R_SUCCESS) {
-			log_error_r("add_soa:dns_difftuple_create");
-			goto failure;
-	}
-	dns_diff_append(&diff, &tp_soa);
+	CHECK(manager_get_ldap_instance(argv[0], &ldap_inst));
+	zr = ldap_instance_getzr(ldap_inst);
+	if (zr == NU

Re: [Freeipa-devel] [PATCH 0133] ipa-cldap: Cut NetBIOS name after 15 characters

2013-11-27 Thread Simo Sorce 
On Wed, 2013-11-27 at 08:50 +0100, Tomas Babej wrote:
> 
Sorry to nitpick but ...

> diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
> b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
> index
> 7d29fe559be55607fcb6b83fa521372e5197b848..f2e74e2c5b6e0d04dd3dc0eb15f25593aa91da8e
>  100644
> --- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
> +++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
> @@ -161,9 +161,13 @@ static int ipa_cldap_encode_netlogon(char
> *fq_hostname, char *domain,
>  nlr->dns_domain = domain;
>  nlr->pdc_dns_name = fq_hostname;
>  nlr->domain_name = name;
> -pdc_name = talloc_asprintf(nlr, "%s", fq_hostname);
> +
> +/* copy the first 15 characters of the fully qualified hostname*/
> +pdc_name = talloc_asprintf(nlr, "%.*s", 15, fq_hostname);

Probably better to #define NETBIOS_NAME_MAX 15 somewhere above and then
use the macro here.

> +
>  for (p = pdc_name; *p; p++) {
> -if (*p == '.') {
> +/* Create the NetBIOS name from the first segment of the
> hostname */
> +if ((*p == '.') || (*p == '\0')) {

The second check is redundant, you'll never get there, the for loop will
bail earlier. I think you only need to add the comment here and not
touch the code as the asprintf above took care of properly terminating
the name at the 15 chars mark already.

>  *p = '\0';
>  break;
>  } 

Simo.

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

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 204-205 Spec file fixes

2013-11-27 Thread Jakub Hrozek 
On Wed, Nov 27, 2013 at 02:26:20PM +0100, Jan Cholasta wrote:
> Hi,
> 
> the attached patches fix .
> 
> Honza
> 
> -- 
> Jan Cholasta

> >From 27fe562102962416f3db17b1b30be978a8c201b3 Mon Sep 17 00:00:00 2001
> From: Jan Cholasta 
> Date: Wed, 27 Nov 2013 13:13:16 +
> Subject: [PATCH 1/2] Use hardening flags for ipa-optd.
> 
> https://fedorahosted.org/freeipa/ticket/4010
> ---
>  daemons/ipa-otpd/Makefile.am | 2 +-
>  freeipa.spec.in  | 4 
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/daemons/ipa-otpd/Makefile.am b/daemons/ipa-otpd/Makefile.am
> index ed99c3e..f0b7528 100644
> --- a/daemons/ipa-otpd/Makefile.am
> +++ b/daemons/ipa-otpd/Makefile.am
> @@ -1,5 +1,5 @@
>  AM_CFLAGS := $(CFLAGS) @LDAP_CFLAGS@ @LIBVERTO_CFLAGS@
> -AM_LDFLAGS := $(LDFLAGS) @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@
> +AM_LDFLAGS := $(LDFLAGS) @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@ -pie 
> -Wl,-z,relro -Wl,-z,now
>  
>  noinst_HEADERS = internal.h
>  libexec_PROGRAMS = ipa-otpd
> diff --git a/freeipa.spec.in b/freeipa.spec.in
> index 35b8714..8ee69fc 100644
> --- a/freeipa.spec.in
> +++ b/freeipa.spec.in
> @@ -5,6 +5,10 @@
>  %global POLICYCOREUTILSVER 2.1.12-5
>  %global gettext_domain ipa
>  
> +%if (0%{?fedora} > 15 || 0%{?rhel} >= 7)
> +%define _hardened_build 1
> +%endif
> +

I'm sorry, I removed Martin's e-mail by accident so I'll reply here. I
think defining the hardened build globally is fine, the only performance
impact is during startup and only small.

AFAIR, the C utilities in IPA are mostly daemons and you really want to
have full RELRO enabled there.

The only gotcha we found so far (well, Nalin did) was that SELinux was
not happy with full RELRO on some exotic architectures, like s390x

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

2013-11-27 Thread Simo Sorce 
On Thu, 2013-11-21 at 15:54 -0500, Dmitri Pal wrote:
> On 11/21/2013 01:34 PM, Nathaniel McCallum wrote:
> >> The password can be retrieved with radiusproxy-show --all, because it is 
> >> > not blocked by LDAP ACIs. Is that intended?
> > Yes. But I'm torn as to whether or not this is a good idea. Regular
> > users can't see radius proxy servers at all. Admins can see all
> > attributes.
> >
> > It is common in radius server deployments to have a text file readable
> > by root with the radius secret. The current LDAP policy replicates this
> > "expected" behavior. It may be wise to block all reads of the secret
> > though. I'm open to suggestions.
> >
> If it is readable by admin only I would leave it as is for now and
> address later when we redo ACIs.

Is this specific to the one and only admin account or does it extend to
any user in the admins group ?

Looking at the current master it seem *any* user except anonymous can
read secrets ? Or is there a patch I am missing ?
I think this is too broad.

Simo.

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

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 204-205 Spec file fixes

2013-11-27 Thread Martin Kosek 
On 11/27/2013 02:26 PM, Jan Cholasta wrote:
> Hi,
> 
> the attached patches fix .
> 
> Honza

Do we want to define

+%if (0%{?fedora} > 15 || 0%{?rhel} >= 7)
+%define _hardened_build 1
+%endif

globally? Wouldn't it trigger the hardening also for all our C utilities or
internal SLAPI plugins? Wouldn't it have performance implication for the SLAPI
plugins?

I am not sure, I would like to hear what the experts say.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCHES] 204-205 Spec file fixes

2013-11-27 Thread Jan Cholasta 

Hi,

the attached patches fix .

Honza

--
Jan Cholasta
>From 27fe562102962416f3db17b1b30be978a8c201b3 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 27 Nov 2013 13:13:16 +
Subject: [PATCH 1/2] Use hardening flags for ipa-optd.

https://fedorahosted.org/freeipa/ticket/4010
---
 daemons/ipa-otpd/Makefile.am | 2 +-
 freeipa.spec.in  | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-otpd/Makefile.am b/daemons/ipa-otpd/Makefile.am
index ed99c3e..f0b7528 100644
--- a/daemons/ipa-otpd/Makefile.am
+++ b/daemons/ipa-otpd/Makefile.am
@@ -1,5 +1,5 @@
 AM_CFLAGS := $(CFLAGS) @LDAP_CFLAGS@ @LIBVERTO_CFLAGS@
-AM_LDFLAGS := $(LDFLAGS) @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@
+AM_LDFLAGS := $(LDFLAGS) @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@ -pie -Wl,-z,relro -Wl,-z,now
 
 noinst_HEADERS = internal.h
 libexec_PROGRAMS = ipa-otpd
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 35b8714..8ee69fc 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -5,6 +5,10 @@
 %global POLICYCOREUTILSVER 2.1.12-5
 %global gettext_domain ipa
 
+%if (0%{?fedora} > 15 || 0%{?rhel} >= 7)
+%define _hardened_build 1
+%endif
+
 Name:   freeipa
 Version:__VERSION__
 Release:__RELEASE__%{?dist}
-- 
1.8.3.1

>From 046dc521aeb55c670596a2e689929dd71b7c4fa4 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 27 Nov 2013 13:20:22 +
Subject: [PATCH 2/2] Own /usr/share/ipa/ui/js/ in the spec file.

https://fedorahosted.org/freeipa/ticket/4010
---
 freeipa.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 8ee69fc..08c82f2 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -677,6 +677,7 @@ fi
 %{_usr}/share/ipa/ui/*.svg
 %{_usr}/share/ipa/ui/*.ttf
 %{_usr}/share/ipa/ui/*.woff
+%dir %{_usr}/share/ipa/ui/js
 %dir %{_usr}/share/ipa/ui/js/dojo
 %{_usr}/share/ipa/ui/js/dojo/dojo.js
 %dir %{_usr}/share/ipa/ui/js/libs
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 198-202 Refactor indirect membership processing

2013-11-27 Thread Petr Viktorin 

On 11/25/2013 03:27 PM, Jan Cholasta wrote:

On 8.11.2013 17:56, Petr Viktorin wrote:

Patch 198:

Also update ipaldap's find_entries docstring, it no longer uses IPA
defaults.


Done.




While you're touching this part of code, I had some other improvements
in mind -- you can consider them:

In find_entries,
 attrs_list = [a.lower() for a in attrs_list]
to make sure 'memberindirect' is case insensitive


Done.




In get_memberof, construct `indirect` as a set, for Ο(1) remove().

^ ignore that, it's nuked in 201 \o/


Changing MEMBERS_ALL et.al. from numbers to descriptive strings, for
easier debugging.

^ these can be removed entirely in 201


Removed.






Patch 199: Looks great


Patch 200:

objtype, res_list, red_id, res_ctrls = result
Minor typo --^


Fixed.




This construction won't work as you'd expect in Python 2:

try:
 (possibly raise interesting exception) (*)
except:
 try:
 (possibly raise exception to ignore) (**)
 except:
 pass
 raise  # (***)

The problem is that the exception in (**) overwrites the "current active
exception" raised in (*). In (***) the exception from the cleanup
will be
re-raised.
The solution is to store the wanted exception info, including the
traceback:
 exc_type, exc_value, exc_traceback = sys.exc_info()
and then re-raise explicitly:
 raise exc_type, exc_value, exc_traceback


Turns out LDAPError does not fly well with sys.exc_info() (all exception
data is lost, probably a python-ldap bug), so I used "except LDAPError,
e: ... raise e" instead.



Also, please log the ignored exception from cancelling the paged search.


Done.







Patch 201:
Great patch!
A nitpick, I'd rename _process_member{,of} to
_process_member{,of}indirect


Renamed.




Patch 202: Looks good
While we're on the subject: Each Plugin has an "api" attribute. It would
be nice if we started preferring `self.api` instead of the global
singleton wherever possible, even though they're currently always the
same.


+1, fixed.

Not fixed, but it's okay for now.


Updated patches attached.

Honza



Thanks! ACK, pushed to master: 4050e553c30d8c8d93c7045f871f8c1cef65aa71

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

2013-11-27 Thread Petr Viktorin 

Sorry for the late review!

On 11/21/2013 07:34 PM, Nathaniel McCallum wrote:

On Fri, 2013-11-15 at 12:34 +0100, Petr Viktorin wrote:



The password can be retrieved with radiusproxy-show --all, because it is
not blocked by LDAP ACIs. Is that intended?


Yes. But I'm torn as to whether or not this is a good idea. Regular
users can't see radius proxy servers at all. Admins can see all
attributes.

It is common in radius server deployments to have a text file readable
by root with the radius secret. The current LDAP policy replicates this
"expected" behavior. It may be wise to block all reads of the secret
though. I'm open to suggestions.


I'm fine either way, just making sure it gets some thought.
Let's see what Simo has to say on this.


ipatokenradiusserver is not validated. See validate_searchtimelimit in
the config plugin for an example validator. You can use validate_ipaddr
and validate_hostname from ipalib.util.


Fixed.


Now the validation is too strict, a port is not accepted.

Should non-FQDN hostnames be allowed?


ipatokenusermapattribute is also not validated. Not sure if it needs to be.


I don't think validation is really possible outside of the permitted
characters for an LDAP attribute.


I think if "$%^&*" is allowed, we'll get a bug from QA soon enough.


The --secret CLI option does nothing (it doesn't take a value, and the
secret is prompted for whether or not the option is given). It should be
flagged no_option. (Or file an RFE for better Password semantics)


Fixed.


For the user commands, --radius takes the name on input, but a DN is
output. Is that expected?


Fixed.


We generally output lists; this should also be a list with one element.

Attaching updated tests.

--
Petr³

From a8145b2531222604e7883b298e00929727319a5a Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 15 Nov 2013 12:26:54 +0100
Subject: [PATCH] Add tests for the radiusproxy plugin

---
 ipatests/test_xmlrpc/objectclasses.py   |   5 +
 ipatests/test_xmlrpc/test_radiusproxy_plugin.py | 384 
 2 files changed, 389 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_radiusproxy_plugin.py

diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 75ac3eb174aaa50eecfda875024b62dbdab238a5..089ee69a38b37f11de539e60b9ecacdec7a7de0b 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -161,3 +161,8 @@
 u'nsContainer',
 u'domainRelatedObject',
 ]
+
+radiusproxy = [
+u'ipatokenradiusconfiguration',
+u'top',
+]
diff --git a/ipatests/test_xmlrpc/test_radiusproxy_plugin.py b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
new file mode 100644
index ..4a10b393c8a1500c1bdf354f72b962cda6ab984a
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
@@ -0,0 +1,384 @@
+# Authors:
+#   Petr Viktorin 
+#
+# Copyright (C) 2013  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+
+from ipalib import errors, api, _
+from ipapython.dn import DN
+from ipatests.test_xmlrpc.xmlrpc_test import Declarative
+from ipatests.test_xmlrpc.test_user_plugin import get_user_result
+from ipatests.test_xmlrpc import objectclasses
+
+radius1 = u'testradius'
+radius1_fqdn = u'testradius.test'
+radius1_dn = DN(('cn=testradius'), ('cn=radiusproxy'), api.env.basedn)
+user1 = u'tuser1'
+password1 = u'very*secure123'
+
+
+class test_raduisproxy(Declarative):
+
+cleanup_commands = [
+('radiusproxy_del', [radius1], {}),
+('user_del', [user1], {}),
+]
+
+tests = [
+
+dict(
+desc='Try to retrieve non-existent %r' % radius1,
+command=('radiusproxy_show', [radius1], {}),
+expected=errors.NotFound(
+reason=u'%s: RADIUS proxy server not found' % radius1),
+),
+
+
+dict(
+desc='Try to update non-existent %r' % radius1,
+command=('radiusproxy_mod', [radius1], {}),
+expected=errors.NotFound(
+reason=_('%s: RADIUS proxy server not found') % radius1),
+),
+
+
+dict(
+desc='Try to delete non-existent %r' % radius1,
+command=('radiusproxy_del', [radius1], {}),
+expected=errors.NotFound(
+reason=_('%s: RADIU

[Freeipa-devel] [PATCH] 0128 subdomains: Use AD admin credentials when trust is being established

2013-11-27 Thread Alexander Bokovoy 

Hi!

Attached patch should solve an issue when fetching subdomains fails
shortly after trust has been established due to MS-PAC caching effects
on KDC. We have already made an alternative path to use when AD admin
credentials are available but failed to actually use them here.

Details in the patch.

https://fedorahosted.org/freeipa/ticket/4046
--
/ Alexander Bokovoy
>From d5cddafe5ca11c54ab2d06a12efddbd80b3da5c7 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Wed, 27 Nov 2013 12:17:43 +0200
Subject: [PATCH 2/2] subdomains: Use AD admin credentials when trust is being
 established

When AD administrator credentials passed, they stored in realm_passwd,
not realm_password in the options.

Additionally, force Samba auth module to use NTLMSSP in case we have
credentials because at the point when trust is established, KDC is not
yet ready to issue tickets to a service in the other realm due to
MS-PAC information caching effects. The logic is a bit fuzzy because
credentials code makes decisions on what to use based on the smb.conf
parameters and Python bindings to set parameters to smb.conf make it so
that auth module believes these parameters were overidden by the user
through the command line and ignore some of options. We have to do calls
in the right order to forse NTLMSSP use instead of Kerberos.

Fixes https://fedorahosted.org/freeipa/ticket/4046
---
 ipalib/plugins/trust.py | 2 +-
 ipaserver/dcerpc.py | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 5ba0905..5861d96 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -1231,7 +1231,7 @@ api.register(trustdomain_del)
 def fetch_domains_from_trust(self, trustinstance, trust_entry, **options):
 trust_name = trust_entry['cn'][0]
 creds = None
-password = options.get('realm_password', None)
+password = options.get('realm_passwd', None)
 if password:
 creds = u"%s%%%s" % (options.get('realm_admin'), password)
 domains = ipaserver.dcerpc.fetch_domains(self.api, 
trustinstance.local_flatname, trust_name, creds=creds)
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 0dde347..985360b 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -964,7 +964,6 @@ def fetch_domains(api, mydomain, trustdomain, creds=None):
 NETR_TRUST_ATTRIBUTE_TREAT_AS_EXTERNAL  = 0x0040)
 
 def communicate(td):
-td.creds.guess(td.parm)
 netrc = net.Net(creds=td.creds, lp=td.parm)
 try:
 result = netrc.finddc(domain=trustdomain, 
flags=nbt.NBT_SERVER_LDAP | nbt.NBT_SERVER_DS)
@@ -988,10 +987,13 @@ def fetch_domains(api, mydomain, trustdomain, creds=None):
 td.creds.set_kerberos_state(credentials.MUST_USE_KERBEROS)
 if ccache_name:
 with installutils.private_ccache(path=ccache_name):
+td.creds.guess(td.parm)
 domains = communicate(td)
 else:
 td.creds.set_kerberos_state(credentials.DONT_USE_KERBEROS)
+td.creds.guess(td.parm)
 td.creds.parse_string(creds)
+td.creds.set_workstation(api.env.host)
 domains = communicate(td)
 
 if domains is None:
-- 
1.8.4.2

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

2013-11-27 Thread Petr Viktorin 

On 11/21/2013 09:54 PM, Dmitri Pal wrote:

On 11/21/2013 01:34 PM, Nathaniel McCallum wrote:

The password can be retrieved with radiusproxy-show --all, because it is

not blocked by LDAP ACIs. Is that intended?

Yes. But I'm torn as to whether or not this is a good idea. Regular
users can't see radius proxy servers at all. Admins can see all
attributes.

It is common in radius server deployments to have a text file readable
by root with the radius secret. The current LDAP policy replicates this
"expected" behavior. It may be wise to block all reads of the secret
though. I'm open to suggestions.


If it is readable by admin only I would leave it as is for now and
address later when we redo ACIs.


CCing Simo since this is ACI-related


--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel