Re: [Freeipa-devel] [PATCHES] 106-113 Access raw LDAP values directly from LDAPEntry

[Jan Cholasta]

On 9.10.2013 13:57, Petr Viktorin wrote:

On 09/26/2013 02:22 PM, Jan Cholasta wrote:

On 24.9.2013 15:35, Jan Cholasta wrote:

On 27.2.2013 16:31, Jan Cholasta wrote:

Hi,

these patches add the ability to access and manipulate raw attribute
values as they are returned from python-ldap to the LDAPEntry class.
This is useful for comparing entries, computing modlists for the modify
operation, deleting values that don't match the syntax of an attribute,
etc., because we don't need to care what syntax does a particular
attribute use, or what Python type is used for it in the framework (raw
values are always list of str). It should also make interaction with
non-389 DS LDAP servers easier in the framework.

(It might be too late for this kind of changes to get into 3.2 now, I'm
posting these patches mainly so that you are aware that they exist.)

Honza



This is now planned for 3.4:


I fixed some issues in these patches and refined the API. Updated
patches attached.

Also added a patch to use raw values when adding new entries and a patch
which refines LDAPEntry.single_value, so that it is consistent with the
rest of the changes introduced in the patches.

Patch 110 will probably be dropped in favor of Petr Viktorin's schema
update patches, but I included it anyway.

Incidentally, this also fixes
 and possibly also
.

Honza



Noticed a couple more issues and fixed them. Updated patches attached.

Honza


Thanks for the patches!


106. Make LDAPEntry a wrapper around dict rather than a dict subclass.

ipapython/ipaldap.py:847:
  warning: 1 line adds whitespace errors.

  if isinstance(_obj, LDAPEntry):
+data = dict(_obj._data)
  orig = _obj._orig

Is this necessary? `self.update(_obj)` is done later.


Probably not. But it's removed in patch 109.




  def __contains__(self, name):
-return self._names.has_key(self._attr_name(name))
+return self._names.has_key(name)

has_key() is deprecated for dict, it would make sense to prefer `name in
self._names` for CIDict too.


Sure, this line is from before CIDict got __contains__.



+def __eq__(self, other):
+if not isinstance(other, LDAPEntry):
+return NotImplemented
+if self._dn != other._dn:
+return False
+return super(LDAPEntry, self).__eq__(other)

I don't think equality checking makes sense on a LDAPEntry, where you
might have different capitalizations/variants of attribute names,
different _orig, or a different set of attributes loaded on the same
entry. It's not obvious which of those differences should make the
entries inequal.
I'd just base it on identity (`self is other`).


Right, I'm not sure why I even did it this way. But I remember seeing 
some code that did comparison of entries with ==...




  def __iter__(self):
  yield self._dn
  yield self

This makes the whole thing sort of hackish -- we need to reimplement
everything in MutableMapping that uses iter() internally :(
Hopefully we can get rid of it soon.


Yes, it's a shame MutableMapping uses iter() instead of iterkeys().


I'd welcome FIXME comments on whatever is reimplemented for this reason.


I thought the comment above __iter__ would be enough. Apparently I was 
wrong.





107. Introduce IPASimpleLDAPObject.decode method for decoding LDAP values.

Can you put in a docstring?


OK.





108. Always use lists for values in LDAPEntry internally.

@@ -698,6 +701,7 @@ class LDAPEntry(collections.MutableMapping):

  result._names = deepcopy(self._names)
  result._data = deepcopy(self._data)
+result._not_list = deepcopy(self._not_list)
  if self._orig is not self:
  result._orig = self._orig.clone()

It's better to use set() than deepcopy() for a set of strings.


Right.




109. Decode and encode attribute values in LDAPEntry on demand.

The syncing looks rather over-engineered to me.
Did you consider a custom MutableSequence for the values?
I think it would be much cleaner in the end than merging two sets of
changes together.


I'm not entirely happy about it either, but it works. I did consider a 
custom sequence type, but I didn't feel like it was the right time to 
this kind of change in this patchset. Unlike the (DN, dict) -> LDAPEntry 
transition, this change won't be backward compatible and there is a lot 
of isinstance(value, list) and entry[attr] = list() kind of things in 
the framework code.




I think it would also help (in the future?) to make the value lists more
set-like, since the order doesn't matter.


+1

Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Jan Cholasta]

On 12.9.2013 22:47, Nathaniel McCallum wrote:

On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:

patch attached


Update for ./makeapi attached.



Is ipaUserAuthType relevant only to Kerberos or to user authentication 
in general? For example, if "password" is removed from ipaUserAuthType 
of an user, will I be able to authenticate as that user with LDAP simple 
authentication?


Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH] 427 Remove --no-serial-autoincrement

[Jan Cholasta]

Hi,

On 9.10.2013 16:21, Martin Kosek wrote:

Deprecate this option and do not offer it in installation tools.
Without this option enabled, advanced DNS features like DNSSEC
would not work.

https://fedorahosted.org/freeipa/ticket/3962



Is there a reason not to remove serial_autoincrement from bindinstance? 
It is used only to set serial_autoincrement value in named.conf 
template, which can be hard-coded to "yes" in the template file.


Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH] 427 Remove --no-serial-autoincrement

[Martin Kosek]

On 10/10/2013 01:47 PM, Jan Cholasta wrote:
> Hi,
> 
> On 9.10.2013 16:21, Martin Kosek wrote:
>> Deprecate this option and do not offer it in installation tools.
>> Without this option enabled, advanced DNS features like DNSSEC
>> would not work.
>>
>> https://fedorahosted.org/freeipa/ticket/3962
>>
> 
> Is there a reason not to remove serial_autoincrement from bindinstance? It is
> used only to set serial_autoincrement value in named.conf template, which can
> be hard-coded to "yes" in the template file.
> 
> Honza
> 

Probably not, attaching updated patch.

Martin
From 6be380777466b83cedf3f7dfa01aa109d0b0ed08 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Wed, 9 Oct 2013 16:13:19 +0200
Subject: [PATCH] Remove --no-serial-autoincrement

Deprecate this option and do not offer it in installation tools.
Without this option enabled, advanced DNS features like DNSSEC
would not work.

https://fedorahosted.org/freeipa/ticket/3962
---
 install/share/bind.named.conf.template | 2 +-
 install/tools/ipa-dns-install  | 6 +-
 install/tools/ipa-server-install   | 4 
 install/tools/man/ipa-dns-install.1| 3 ---
 install/tools/man/ipa-server-install.1 | 3 ---
 ipaserver/install/bindinstance.py  | 7 +--
 6 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/install/share/bind.named.conf.template b/install/share/bind.named.conf.template
index 5727a1536accd135cb556d76dfccf965bc098e29..0984febb11633c171710a4d7f181f738e02fa637 100644
--- a/install/share/bind.named.conf.template
+++ b/install/share/bind.named.conf.template
@@ -45,5 +45,5 @@ dynamic-db "ipa" {
 	arg "auth_method sasl";
 	arg "sasl_mech GSSAPI";
 	arg "sasl_user DNS/$FQDN";
-	arg "serial_autoincrement $SERIAL_AUTOINCREMENT";
+	arg "serial_autoincrement yes";
 };
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 47acd8de6cb61e53db1d296ec38a0f7279f5f062..37a07f8e38d13260b659ad8b0e72014c6e36324e 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -52,9 +52,6 @@ def parse_options():
 parser.add_option("--zonemgr", action="callback", callback=bindinstance.zonemgr_callback,
   type="string",
   help="DNS zone manager e-mail address. Defaults to hostmaster@DOMAIN")
-parser.add_option("--no-serial-autoincrement", dest="serial_autoincrement",
-  default=True, action="store_false",
-  help="Do not enable SOA serial autoincrement")
 parser.add_option("-U", "--unattended", dest="unattended", action="store_true",
   default=False, help="unattended installation never prompts the user")
 
@@ -209,8 +206,7 @@ def main():
 print ""
 
 bind.setup(api.env.host, ip_address, api.env.realm, api.env.domain,
-   dns_forwarders, conf_ntp, reverse_zone, zonemgr=options.zonemgr,
-   serial_autoincrement=options.serial_autoincrement)
+   dns_forwarders, conf_ntp, reverse_zone, zonemgr=options.zonemgr)
 bind.create_instance()
 
 # Restart http instance to make sure that python-dns has the right resolver
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 8077bc207fd2cd48846f7c6fdb2bff14505dc10c..b871ef3f219d688fdad6b74f9b6502e1391c6bf6 100644
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -212,9 +212,6 @@ def parse_options():
   help="Do not use DNS for hostname lookup during installation")
 dns_group.add_option("--no-dns-sshfp", dest="create_sshfp", default=True, action="store_false",
   help="Do not automatically create DNS SSHFP records")
-dns_group.add_option("--no-serial-autoincrement", dest="serial_autoincrement",
-  default=True, action="store_false",
-  help="Do not enable SOA serial autoincrement")
 parser.add_option_group(dns_group)
 
 uninstall_group = OptionGroup(parser, "uninstall options")
@@ -1181,7 +1178,6 @@ def main():
 bind = bindinstance.BindInstance(fstore, dm_password)
 bind.setup(host_name, ip_address, realm_name, domain_name, dns_forwarders,
options.conf_ntp, reverse_zone, zonemgr=options.zonemgr,
-   serial_autoincrement=options.serial_autoincrement,
ca_configured=setup_ca)
 if options.setup_dns:
 api.Backend.ldap2.connect(bind_dn=DN(('cn', 'Directory Manager')), bind_pw=dm_password)
diff --git a/install/tools/man/ipa-dns-install.1 b/install/tools/man/ipa-dns-install.1
index 646601eecaded0d8490bb41945f4bd83ee81c087..bde30cadba9b8b461f5373b734c3edf2faf9d0af 100644
--- a/install/tools/man/ipa-dns-install.1
+++ b/install/tools/man/ipa-dns-install.1
@@ -49,9 +49,6 @@ Do not create new reverse DNS zone. If used on a replica and a reverse DNS zone
 \fB\-\-zonemgr\fR
 The e\-mail address of the DNS zone manager. Defaults to hostmaster@DOMAIN
 .TP
-\fB\-\-no\-serial\-autoincrement

Re: [Freeipa-devel] [PATCH] 427 Remove --no-serial-autoincrement

[Jan Cholasta]

On 10.10.2013 14:02, Martin Kosek wrote:

On 10/10/2013 01:47 PM, Jan Cholasta wrote:

Hi,

On 9.10.2013 16:21, Martin Kosek wrote:

Deprecate this option and do not offer it in installation tools.
Without this option enabled, advanced DNS features like DNSSEC
would not work.

https://fedorahosted.org/freeipa/ticket/3962



Is there a reason not to remove serial_autoincrement from bindinstance? It is
used only to set serial_autoincrement value in named.conf template, which can
be hard-coded to "yes" in the template file.

Honza



Probably not, attaching updated patch.

Martin



ACK.

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH] 428 PKI installation on replica failing due to missing proxy conf

[Jan Cholasta]

Hi,

On 9.10.2013 22:38, Martin Kosek wrote:

Proxy configuration was not detected correctly. Both
ipa-pki-proxy.conf and ipa.conf need to be in place and httpd
restarted to be able to check it's status.

https://fedorahosted.org/freeipa/ticket/3964



I had to install and reinstall PKI a lot before I found the real root
cause. Both
ipa-replica-install --setup-ca
and
ipa-ca-install
should be working now.



ACK.

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Nathaniel McCallum]

On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:
> On 12.9.2013 22:47, Nathaniel McCallum wrote:
> > On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:
> >> patch attached
> >
> > Update for ./makeapi attached.
> >
> 
> Is ipaUserAuthType relevant only to Kerberos or to user authentication 
> in general? For example, if "password" is removed from ipaUserAuthType 
> of an user, will I be able to authenticate as that user with LDAP simple 
> authentication?

If only "otp" is set, yes via password+otp.

If only "radius" is set, this behavior is currently undefined. We should
probably define it.

Nathaniel

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

[Freeipa-devel] [PATCH 0118] Do not create separate ranges for subdomains in case of POSIX trust

[Tomas Babej]

Hi,

This is a fix for a bug I found related to the subdomains code while 
working on the AD Continuous Integration testing.


--
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org

From 42d3932ad10e4c5ca60c24d2f4da0e9d6bc3b348 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Thu, 10 Oct 2013 15:11:16 +0200
Subject: [PATCH] trusts: Do not create ranges for subdomains in case of POSIX
 trust

For the AD trusts where the ID range for the root level domain is of
ipa-ad-trust-posix type, do not create a separate ranges for the
subdomains, since POSIX attributes provide global mapping.
---
 ipalib/plugins/trust.py | 50 ++---
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index f412c61850f1c110b991dff0429e8a6e38b8d374..f4e5d021d4b3893445b67a898f48c90aa1b4c51b 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -337,31 +337,56 @@ sides.
 result = self.execute_ad(full_join, *keys, **options)
 
 if not old_range:
-self.add_range(range_name, dom_sid, *keys, **options)
+# Store the created range type, since for POSIX trusts no
+# ranges for the subdomains should be added, POSIX attributes
+# provide a global mapping across all subdomains
+(created_range_type, _, _) = self.add_range(range_name, dom_sid,
+*keys, **options)
 
 trust_filter = "cn=%s" % result['value']
 ldap = self.obj.backend
 (trusts, truncated) = ldap.find_entries(
- base_dn = DN(api.env.container_trusts, api.env.basedn),
- filter = trust_filter)
-
+ base_dn=DN(api.env.container_trusts, api.env.basedn),
+ filter=trust_filter)
 
 result['result'] = entry_to_dict(trusts[0][1], **options)
-if options.get('trust_type') == u'ad':
-domains = fetch_domains_from_trust(self, self.trustinstance, result['result'], **options)
+
+# For AD trusts with algorithmic mapping, we need to add a separate
+# range for each subdomain.
+if (options.get('trust_type') == u'ad' and
+created_range_type != u'ipa-ad-trust-posix'):
+
+domains = fetch_domains_from_trust(self, self.trustinstance,
+   result['result'], **options)
 if domains and len(domains) > 0:
 for dom in domains:
 range_name = dom['cn'][0].upper() + '_id_range'
-range_type=options.get('range_type', u'ipa-ad-trust')
 dom_sid = dom['ipanttrusteddomainsid'][0]
+
+# Enforce the same range type as the range for the root
+# level domain.
+
+# This will skip the detection of the POSIX attributes if
+# they are not available, since it has been already
+# detected when creating the range for the root level domain
+passed_options = options
+passed_options.update(range_type=created_range_type)
+
+# Try to add the range for each subdomain
 try:
-self.add_range(range_name, dom_sid, range_type=range_type)
+self.add_range(range_name, dom_sid, *keys,
+   **passed_options)
 except errors.DuplicateEntry:
 pass
 
-result['result']['trusttype'] = [trust_type_string(result['result']['ipanttrusttype'][0])]
-result['result']['trustdirection'] = [trust_direction_string(result['result']['ipanttrustdirection'][0])]
-result['result']['truststatus'] = [trust_status_string(result['verified'])]
+# Format the output into human-readable values
+result['result']['trusttype'] = [trust_type_string(
+result['result']['ipanttrusttype'][0])]
+result['result']['trustdirection'] = [trust_direction_string(
+result['result']['ipanttrustdirection'][0])]
+result['result']['truststatus'] = [trust_status_string(
+result['verified'])]
+
 del result['verified']
 
 return result
@@ -620,6 +645,9 @@ sides.
iparangetype=range_type,
ipanttrusteddomainsid=dom_sid)
 
+# Return the values that were generated inside this function
+return range_type, range_size, base_id
+
 def execute_ad(self, full_join, *keys, **options):
 # Join domain using full credentials and with random trustdom
 # secret (will be generated by the join method)
-- 
1.8.3.1

___
Freeipa-devel maili

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Jan Cholasta]

On 10.10.2013 16:51, Nathaniel McCallum wrote:

On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:

On 12.9.2013 22:47, Nathaniel McCallum wrote:

On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:

patch attached


Update for ./makeapi attached.



Is ipaUserAuthType relevant only to Kerberos or to user authentication
in general? For example, if "password" is removed from ipaUserAuthType
of an user, will I be able to authenticate as that user with LDAP simple
authentication?


If only "otp" is set, yes via password+otp.

If only "radius" is set, this behavior is currently undefined. We should
probably define it.

Nathaniel



OK, thanks. I'm wondering if SSH public key authentication should be 
included in the list, it is one of the currently supported 
authentication methods after all.


I'm also wondering if HBAC could be extended to allow/deny access based 
on which authentication type was used. It might be useful for OTP token 
synchronization when password authentication is disabled (it could be 
used to allow password authentication *only* to the token 
synchronization service, if there's such a thing).


Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH] 426 Winsync re-initialize should not run memberOf fixup task

[Jan Cholasta]

Hi,

On 9.10.2013 13:31, Martin Kosek wrote:

Change re-initialize command to consider memberOf fixup task only
for non-winsync replication agreements. This patch also includes
few fixes for DsInstance to properly set realm and fqdn properties
needed when connecting to LDAP.

https://fedorahosted.org/freeipa/ticket/3854



-self.realm_name = realm_name
+self.realm_name = self.realm = realm_name

I think one realm property is enough. Can you please change dsinstance 
to use .realm instead of .realm_name?


Honza

--
Jan Cholasta

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

[Freeipa-devel] [PATCH] 429 Administrative password change does not respect password policy

[Martin Kosek]

When Directory Manager or a PassSync agent is changing a password,
it is not being expired, but standard expiration time should apply.
However, default expiration time was always applied (90 days)
even though administrator may have a custom policy for the user.

https://fedorahosted.org/freeipa/ticket/3968

-- 
Martin Kosek 
Supervisor, Software Engineering - Identity Management Team
Red Hat Inc.
From 312ae4d97e24965ed0928291f41e0b59973923a3 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Thu, 10 Oct 2013 18:00:53 +0200
Subject: [PATCH] Administrative password change does not respect password
 policy

When Directory Manager or a PassSync agent is changing a password,
it is not being expired, but standard expiration time should apply.
However, default expiration time was always applied (90 days)
even though administrator may have a custom policy for the user.

https://fedorahosted.org/freeipa/ticket/3968
---
 daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c | 44 
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index a54e91d87b5e700775b05dd2859c95abd739b626..f0339c47c5b242b436d77c4845c5914eae79814f 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
@@ -755,6 +755,7 @@ done:
 int ipapwd_CheckPolicy(struct ipapwd_data *data)
 {
 struct ipapwd_policy pol = {0};
+struct ipapwd_policy tmppol = {0};
 time_t acct_expiration;
 time_t pwd_expiration;
 time_t last_pwd_change;
@@ -765,11 +766,8 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data)
 pol.max_pwd_life = IPAPWD_DEFAULT_PWDLIFE;
 pol.min_pwd_length = IPAPWD_DEFAULT_MINLEN;
 
-if (data->changetype != IPA_CHANGETYPE_NORMAL) {
-/* We must skip policy checks (Admin change) but
- * force a password change on the next login.
- * But not if Directory Manager */
-if (data->changetype == IPA_CHANGETYPE_ADMIN) {
+switch(data->changetype) {
+case IPA_CHANGETYPE_ADMIN:
 /* The expiration date needs to be older than the current time
  * otherwise the KDC may not immediately register the password
  * as expired. The last password change needs to match the
@@ -777,16 +775,32 @@ int ipapwd_CheckPolicy(struct ipapwd_data *data)
  */
 data->timeNow -= 1;
 data->expireTime = data->timeNow;
-}
-
-/* do not load policies */
-} else {
-
-/* find the entry with the password policy */
-ret = ipapwd_getPolicy(data->dn, data->target, &pol);
-if (ret) {
-LOG_TRACE("No password policy, use defaults");
-}
+break;
+case IPA_CHANGETYPE_NORMAL:
+/* Find the entry with the password policy */
+ret = ipapwd_getPolicy(data->dn, data->target, &pol);
+if (ret) {
+LOG_TRACE("No password policy, use defaults");
+}
+break;
+case IPA_CHANGETYPE_DSMGR:
+/* PassSync agents and Directory Manager can administratively
+ * change the password without expiring it.
+ *
+ * Find password policy for the entry to properly set expiration.
+ * Do not store it in resulting policy to avoid aplying password
+ * quality checks on administratively set passwords
+ */
+ret = ipapwd_getPolicy(data->dn, data->target, &tmppol);
+if (ret) {
+LOG_TRACE("No password policy, use defaults");
+} else {
+pol.max_pwd_life = tmppol.max_pwd_life;
+}
+break;
+default:
+LOG_TRACE("Unknown password change type, use defaults");
+break;
 }
 
 tmpstr = slapi_entry_attr_get_charptr(data->target,
-- 
1.8.3.1

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

[Freeipa-devel] [PATCH] [DOC] Remove SELinux user paragraph replacement

[Jérôme Fenal]

Attached.
Replaced the dodgy sentence with Martin's one.

Regards,

J.
-- 
Jérôme Fenal


freeipa-jfenal-0002-Remove-SELinux-user.patch
Description: Binary data
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Dmitri Pal]

On 10/10/2013 10:51 AM, Nathaniel McCallum wrote:
> On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:
>> On 12.9.2013 22:47, Nathaniel McCallum wrote:
>>> On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:
 patch attached
>>> Update for ./makeapi attached.
>>>
>> Is ipaUserAuthType relevant only to Kerberos or to user authentication 
>> in general? For example, if "password" is removed from ipaUserAuthType 
>> of an user, will I be able to authenticate as that user with LDAP simple 
>> authentication?
> If only "otp" is set, yes via password+otp.
>
> If only "radius" is set, this behavior is currently undefined. We should
> probably define it.

If RADIUS is used you always rely on the external system to provide
authentication for this user.
Is this the definition you are looking for?

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


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/



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

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Dmitri Pal]

On 10/10/2013 11:30 AM, Jan Cholasta wrote:
> On 10.10.2013 16:51, Nathaniel McCallum wrote:
>> On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:
>>> On 12.9.2013 22:47, Nathaniel McCallum wrote:
 On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:
> patch attached

 Update for ./makeapi attached.

>>>
>>> Is ipaUserAuthType relevant only to Kerberos or to user authentication
>>> in general? For example, if "password" is removed from ipaUserAuthType
>>> of an user, will I be able to authenticate as that user with LDAP
>>> simple
>>> authentication?
>>
>> If only "otp" is set, yes via password+otp.
>>
>> If only "radius" is set, this behavior is currently undefined. We should
>> probably define it.
>>
>> Nathaniel
>>
>
> OK, thanks. I'm wondering if SSH public key authentication should be
> included in the list, it is one of the currently supported
> authentication methods after all.

You can't bind to LDAP or kinit with SSH keys so i do not think it is in
the same boat.
These methods are methods that IPA as a server would accept. I doubt
Kerberos will be extended to support authentication with SSH keys. I do
not think there is a use case for it.

>
> I'm also wondering if HBAC could be extended to allow/deny access
> based on which authentication type was used. It might be useful for
> OTP token synchronization when password authentication is disabled (it
> could be used to allow password authentication *only* to the token
> synchronization service, if there's such a thing).

The plan here is to really rely on the Kerberos ticket and data in it to
make the access control decisions. This is the spec that Anupam is
working on so that part is covered.


>
> Honza
>


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/



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

Re: [Freeipa-devel] [PATCH 0186-0191] Replace LDAP cache with RBTDB

[Petr Spacek]

On 8.10.2013 12:00, Tomas Hozza wrote:

On 10/02/2013 12:57 PM, Petr Spacek wrote:

On 13.9.2013 15:31, Petr Spacek wrote:

On 14.8.2013 16:42, Petr Spacek wrote:

On 14.8.2013 16:25, Petr Spacek wrote:

On 1.8.2013 15:57, Petr Spacek wrote:

Hello,

attached monster patches replace our internal cache/database with
RBTDB
implementation. See commit messages and comments inside.

This patch set provides very basic functionality (including DNS
support for
updates). Error handling definitely needs more love, but it should
be enough
for rapid DNSSEC prototyping.


Patch 186 v2: The code now applies incremental changes in LDAP to the
in-memory database. Commit message was modified to mention that
wildcards are
now supported.

Patch 187 v2: The code was re-worked and now it respects
serial_autoincrement
option.

Patch 188 v2: Minor comment clean-up and rebase on top of patch 187 v2.

Patch 189 v2: Call to deleterdataset() nested in substractrdataset()
was
deleted. This code was meant only for testing purposes.

These patch set is now ready for review. Please see commit messages!
Some
functionality is missing intentionally, but it will be fixed by
separate
patches.


It would be too easy!

Patch 186 v3: Commit message was extended with information that LDAP
MODRDN
operation is not supported at the moment.

Patch 187 v3: Missing file ldap_driver.h was added.


This extended patch set handles correctly object deletion from LDAP.

Patches 186-189 contain very minor changes, only moving code from one
place to
the other.

See commit messages for patches 190 and 191.

This should be testable. I would recommend to test the whole patch set at
once, most probably it doesn't make much sense to test patches
separately.


bind-dyndb-ldap-pspacek-0186-5-Use-RBTDB-instead-of-internal-LDAP-cache.patch
adds missing missing include (db.h) to zone_register.c.



ACK.

Patches 186-191 tested. Adding/removing/modifying records works fine.
Also PTR synchronization works. Zone transfer to slave and NOTIFY
tested when changes occurred on master.


Patch 191-2 fixed problem with zone removal and race condition during zone 
load. I would recommend you to test it with other patch I plan to send today :-)


--
Petr^2 Spacek
From b49ec1165419b29a1c14a6f5aacba0a7b28123b2 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Fri, 6 Sep 2013 14:55:52 +0200
Subject: [PATCH] Handle object deletion.

This is a bit tricky, because we receive only the DN of the
deleted object. ObjectClass is inferred from zone and forward
registers.

Signed-off-by: Petr Spacek 
---
 src/ldap_helper.c | 188 +-
 1 file changed, 144 insertions(+), 44 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index e54bfefb67342ff642979541b40941f602efecc7..0e287a1b942e0c47ffdb16a7d37cfd5f307a2248 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1389,6 +1389,8 @@ diff_ldap_rbtdb(isc_mem_t *mctx, dns_name_t *name, ldapdb_rdatalist_t *ldap_rdat
 		if (result != ISC_R_SUCCESS && result != ISC_R_NOMORE)
 			goto cleanup;
 	}
+	if (result == ISC_R_NOMORE)
+		result = ISC_R_SUCCESS;
 
 cleanup:
 	return result;
@@ -3403,8 +3405,6 @@ update_zone(isc_task_t *task, isc_event_t *event)
 	ldap_valuelist_t values;
 	isc_boolean_t zone_active = ISC_FALSE;
 
-	UNUSED(task);
-
 	mctx = pevent->mctx;
 	dns_name_init(&currname, NULL);
 	dns_name_init(&prevname, NULL);
@@ -3473,6 +3473,7 @@ cleanup:
 	ldap_entry_destroy(mctx, &entry);
 	isc_mem_detach(&mctx);
 	isc_event_free(&event);
+	isc_task_detach(&task);
 }
 
 static void
@@ -3484,8 +3485,6 @@ update_config(isc_task_t *task, isc_event_t *event)
 	ldap_entry_t *entry = pevent->entry;
 	isc_mem_t *mctx;
 
-	UNUSED(task);
-
 	mctx = pevent->mctx;
 
 	CHECK(manager_get_ldap_instance(pevent->dbname, &inst));
@@ -3502,6 +3501,7 @@ cleanup:
 	isc_mem_free(mctx, pevent->dn);
 	isc_mem_detach(&mctx);
 	isc_event_free(&event);
+	isc_task_detach(&task);
 }
 
 /**
@@ -3528,8 +3528,8 @@ update_record(isc_task_t *task, isc_event_t *event)
 	ldap_entry_t *entry = pevent->entry;
 	const char *fake_mname = NULL;
 
-	dns_db_t *rbtdb;
-	dns_db_t *ldapdb;
+	dns_db_t *rbtdb = NULL;
+	dns_db_t *ldapdb = NULL;
 	dns_diff_t diff;
 	dns_diff_t soa_diff;
 	dns_dbversion_t *version = NULL; /* version is shared between rbtdb and ldapdb */
@@ -3540,7 +3540,6 @@ update_record(isc_task_t *task, isc_event_t *event)
 	dns_diff_init(mctx, &diff);
 	dns_diff_init(mctx, &soa_diff);
 
-	UNUSED(task);
 #ifdef RBTDB_DEBUG
 	static unsigned int count = 0;
 #endif
@@ -3573,12 +3572,21 @@ update_restart:
 	rbtdb = NULL;
 	ldapdb = NULL;
 	CHECK(zr_get_zone_dbs(inst->zone_register, &name, &ldapdb, &rbtdb));
+	CHECK(dns_db_newversion(rbtdb, &version));
+
+	CHECK(dns_db_findnode(rbtdb, &name, ISC_TRUE, &node));
+	result = dns_db_allrdatasets(rbtdb, node, version, 0, &rbt_rds_iterator);
+	if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND)
+		goto cleanup;
+
 
 	/* This code is disabled because we don't have 

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Nathaniel McCallum]

On Thu, 2013-10-10 at 12:44 -0400, Dmitri Pal wrote:
> On 10/10/2013 10:51 AM, Nathaniel McCallum wrote:
> > On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:
> >> On 12.9.2013 22:47, Nathaniel McCallum wrote:
> >>> On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:
>  patch attached
> >>> Update for ./makeapi attached.
> >>>
> >> Is ipaUserAuthType relevant only to Kerberos or to user authentication 
> >> in general? For example, if "password" is removed from ipaUserAuthType 
> >> of an user, will I be able to authenticate as that user with LDAP simple 
> >> authentication?
> > If only "otp" is set, yes via password+otp.
> >
> > If only "radius" is set, this behavior is currently undefined. We should
> > probably define it.
> 
> If RADIUS is used you always rely on the external system to provide
> authentication for this user.
> Is this the definition you are looking for?

For Kerberos, yes. For LDAP, no. For LDAP, if "radius" is present,
single factor authentication should probably be permitted.

Nathaniel

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

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Dmitri Pal]

On 10/10/2013 03:13 PM, Nathaniel McCallum wrote:
> On Thu, 2013-10-10 at 12:44 -0400, Dmitri Pal wrote:
>> On 10/10/2013 10:51 AM, Nathaniel McCallum wrote:
>>> On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:
 On 12.9.2013 22:47, Nathaniel McCallum wrote:
> On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:
>> patch attached
> Update for ./makeapi attached.
>
 Is ipaUserAuthType relevant only to Kerberos or to user authentication 
 in general? For example, if "password" is removed from ipaUserAuthType 
 of an user, will I be able to authenticate as that user with LDAP simple 
 authentication?
>>> If only "otp" is set, yes via password+otp.
>>>
>>> If only "radius" is set, this behavior is currently undefined. We should
>>> probably define it.
>> If RADIUS is used you always rely on the external system to provide
>> authentication for this user.
>> Is this the definition you are looking for?
> For Kerberos, yes. For LDAP, no. For LDAP, if "radius" is present,
> single factor authentication should probably be permitted.
>
> Nathaniel
>
Why you think they should be inconsistent?
If you want to have this case covered I think we need a separate type
something like "kerberos_radius" which will work only in kerberos but
not in LDAP.
But IMO such mode will create a lot of confusion.
We can also do "kerberos_radius_ldap_pwd_and_radius" that would allow
radius for kerberos and allow local LDAP password or RADIUS for bind but
I do not see a reason for this case.
Can you explain?

-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/



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

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Nathaniel McCallum]

On Thu, 2013-10-10 at 15:53 -0400, Dmitri Pal wrote:
> On 10/10/2013 03:13 PM, Nathaniel McCallum wrote:
> > On Thu, 2013-10-10 at 12:44 -0400, Dmitri Pal wrote:
> >> On 10/10/2013 10:51 AM, Nathaniel McCallum wrote:
> >>> On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:
>  On 12.9.2013 22:47, Nathaniel McCallum wrote:
> > On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:
> >> patch attached
> > Update for ./makeapi attached.
> >
>  Is ipaUserAuthType relevant only to Kerberos or to user authentication 
>  in general? For example, if "password" is removed from ipaUserAuthType 
>  of an user, will I be able to authenticate as that user with LDAP simple 
>  authentication?
> >>> If only "otp" is set, yes via password+otp.
> >>>
> >>> If only "radius" is set, this behavior is currently undefined. We should
> >>> probably define it.
> >> If RADIUS is used you always rely on the external system to provide
> >> authentication for this user.
> >> Is this the definition you are looking for?
> > For Kerberos, yes. For LDAP, no. For LDAP, if "radius" is present,
> > single factor authentication should probably be permitted.
> >
> > Nathaniel
> >
> Why you think they should be inconsistent?
> If you want to have this case covered I think we need a separate type
> something like "kerberos_radius" which will work only in kerberos but
> not in LDAP.
> But IMO such mode will create a lot of confusion.
> We can also do "kerberos_radius_ldap_pwd_and_radius" that would allow
> radius for kerberos and allow local LDAP password or RADIUS for bind but
> I do not see a reason for this case.
> Can you explain?

We implemented RADIUS forwarding in the companion daemon. So the
implementation of the ipaUserAuthType for "radius" is occurring outside
389ds. Hence, when ipaUserAuthType says "radius" the companion daemon
will forward Kerberos OTP requests to RADIUS. 389ds binds, however, will
use standard password authentication in this case. There is no way
around this without merging RADIUS proxying into 389ds directly.

Nathaniel


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

Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Dmitri Pal]

On 10/10/2013 08:33 PM, Nathaniel McCallum wrote:
> On Thu, 2013-10-10 at 15:53 -0400, Dmitri Pal wrote:
>> On 10/10/2013 03:13 PM, Nathaniel McCallum wrote:
>>> On Thu, 2013-10-10 at 12:44 -0400, Dmitri Pal wrote:
 On 10/10/2013 10:51 AM, Nathaniel McCallum wrote:
> On Thu, 2013-10-10 at 10:04 +0200, Jan Cholasta wrote:
>> On 12.9.2013 22:47, Nathaniel McCallum wrote:
>>> On Thu, 2013-09-05 at 00:04 -0400, Nathaniel McCallum wrote:
 patch attached
>>> Update for ./makeapi attached.
>>>
>> Is ipaUserAuthType relevant only to Kerberos or to user authentication 
>> in general? For example, if "password" is removed from ipaUserAuthType 
>> of an user, will I be able to authenticate as that user with LDAP simple 
>> authentication?
> If only "otp" is set, yes via password+otp.
>
> If only "radius" is set, this behavior is currently undefined. We should
> probably define it.
 If RADIUS is used you always rely on the external system to provide
 authentication for this user.
 Is this the definition you are looking for?
>>> For Kerberos, yes. For LDAP, no. For LDAP, if "radius" is present,
>>> single factor authentication should probably be permitted.
>>>
>>> Nathaniel
>>>
>> Why you think they should be inconsistent?
>> If you want to have this case covered I think we need a separate type
>> something like "kerberos_radius" which will work only in kerberos but
>> not in LDAP.
>> But IMO such mode will create a lot of confusion.
>> We can also do "kerberos_radius_ldap_pwd_and_radius" that would allow
>> radius for kerberos and allow local LDAP password or RADIUS for bind but
>> I do not see a reason for this case.
>> Can you explain?
> We implemented RADIUS forwarding in the companion daemon. So the
> implementation of the ipaUserAuthType for "radius" is occurring outside
> 389ds. Hence, when ipaUserAuthType says "radius" the companion daemon
> will forward Kerberos OTP requests to RADIUS. 389ds binds, however, will
> use standard password authentication in this case. There is no way
> around this without merging RADIUS proxying into 389ds directly.


Ah right, I forget that only OTP is implemented in DS so it can be used
by bind and Kerberos.
Then we need to document expectation about "radius".

> Nathaniel
>
>


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/



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