date:20150611


On 06/11/2015 04:34 PM, David Kupka wrote:

Dne 11.6.2015 v 16:17 Martin Kosek napsal(a):

On 06/11/2015 03:55 PM, David Kupka wrote:

Dne 11.6.2015 v 14:12 thierry bordaz napsal(a):

On 06/10/2015 02:14 PM, David Kupka wrote:

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

Hello David,

The patch looks ok except it removes a permission to update 'uid' from
an active user. This permission is required to delete(preserve) an
active user.

 -# Active container
 -#
 -# Stage user administrators need write right on RDN when
 -# the active user is deleted (preserved)
 -'System: Write Active Users RDN by administrators': {
 -'ipapermlocation': DN(baseuser.active_container_dn,
 api.env.basedn),
 -'ipapermbindruletype': 'permission',
 -'ipapermtarget': DN('uid=*',
 baseuser.active_container_dn, api.env.basedn),
 -'ipapermtargetfilter': 
{'(objectclass=posixaccount)'},

 -'ipapermright': {'write'},
 -'ipapermdefaultattr': {'uid'},
 -'default_privileges': {'Stage User Administrators'},
 -},
 -#

I prepared a new patch (attached) with that permission and it makes
'user-del --preserve' happy.
Now I think the name would rather be something like: 'System: Preserve
an active user (user-del --preserve)'

I also added back this comment in two permissions 'Note: 
targetfilter is

the target parent container'.
This was to say that the targetfilter setting was intentional.
If you think it is not the right place, you may remove those comments.

Thanks
thierry



Hello Thierry,
Indeed, I accidentally removed these. Thank you for careful review.
Rebase is needed but it is due to change in VERSION and is useless 
to do it

before push as there are too much patches going to master right now.
Martin, are you (as a reporter) OK with the patch?



Not entirely. I still see some weird permission in stageuser.py:

 #
 # Active container
 #
 # Stage user administrators need write right on RDN when
 # the active user is deleted (preserved)
 'System: Write Active Users RDN by administrators': {
 'ipapermlocation': DN(baseuser.active_container_dn, 
api.env.basedn),

 'ipapermbindruletype': 'permission',
 'ipapermtarget': DN('uid=*', baseuser.active_container_dn,
api.env.basedn),
 'ipapermtargetfilter': {'(objectclass=posixaccount)'},
 'ipapermright': {'write'},
 'ipapermdefaultattr': {'uid'},
 'default_privileges': {'Stage User Administrators'},
 },

This was supposed to be ""System: Modify User RDN". When the name is 
also

fixed, I am fine.


Updated patch attached.



Hi David,

All the tests are ok. The patch is fine for me. ACK

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] [WIP] ipa-replica-manage del with managed topology

Attaching a wip patch for `ipa-replica-manage del` to work with managed 
topology.


There are two prerequisite patches, they add following commands. All 
commands has NO_CLI flag which means they are hidden in CLI.

- server-del
- serverservice-add, mod, del, show, find

serverservice is object name for server "services" in cn=masters. I 
don't like the "service" name much but it's already been used in general 
discussions.


The main patch introduces two distinct methods for deleting servers, one 
for managed topology another for the old method. They share some code.


There are some differences in behavior.

1. the original 'del' worked also with winsync agreements. I'm not sure 
why is that. Shouldn't 'disconnect' be used for winsync agreements? At 
least man page says that.


2. options --clean and --force aren't used in the new method. I don't 
think that they are required. They serve for deleting the server entry 
in cn=masters.  The new method is build around this deletion so that 
it's always done which also means the cleanup is done.


3. Clean RUV task is run after deleting server entry and related 
cleanup. I don't think it works well. From observing the changes, it 
looks like it's executed before topology plugin manages to delete the 
agreements. This task then doesn't want to end and it reports that it 
has not finished somewhere. It finishes successfully if dirsrv is 
restarted. Agreements are then removed as well and all is fine.


Ludwig, should the clean RUV step be done differently? E.g. somewhere 
else or after something finishes?

--
Petr Vobornik
From b3229d3feab3584e7e6e948ae6ea8e11a0a91244 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 11 Jun 2015 16:12:37 +0200
Subject: [PATCH 748/748] ipa-replica-manage: adjust del to work with managed
 topology

---
 install/tools/ipa-replica-manage | 205 +++
 1 file changed, 142 insertions(+), 63 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index a2b2c820d8e25a2587358e00dc4afc54b309d77b..865c74b4c9a94732a00c5046c26c6a688196dcc1 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -567,8 +567,144 @@ def enforce_host_existence(host, message=None):
 
 sys.exit(message)
 
+def ensure_last_services(api, hostname, masters, options):
+"""
+1. When deleting master, check if there will be at least one remaining
+   DNS and CA server.
+2. Pick CA renewal master
+
+Return this_services, other_services, ca_hostname
+"""
+
+this_services = []
+other_services = []
+ca_hostname = None
+
+for master in masters:
+services = api.Command['serverservice_find'](master, sizelimit=0).get('result')
+names = [s['cn'][0] for s in services]
+if master == hostname:
+this_services = names
+else:
+other_services.append(names)
+if ca_hostname is None and 'CA' in names:
+ca_hostname = master
+
+if 'CA' in this_services and not any(['CA' in o for o in other_services]):
+print "Deleting this server is not allowed as it would leave your installation without a CA."
+sys.exit(1)
+
+other_dns = True
+if 'DNS' in this_services and not any(['DNS' in o for o in other_services]):
+other_dns = False
+print "Deleting this server will leave your installation without a DNS."
+if not options.force and not ipautil.user_input("Continue to delete?", False):
+sys.exit("Deletion aborted")
+
+# test if replica is not DNSSEC master
+# allow to delete it if is last DNS server
+if 'DNS' in this_services and other_dns and not options.force:
+dnssec_masters = opendnssecinstance.get_dnssec_key_masters(api.Backend.ldap2)
+if hostname in dnssec_masters:
+print "Replica is active DNSSEC key master. Uninstall could break your DNS system."
+sys.exit("Deletion aborted")
+
+ca = cainstance.CAInstance(api.env.realm, certs.NSS_DIR)
+if ca.is_renewal_master(hostname):
+try:
+ca.set_renewal_master(options.host)
+except errors.NotFound:
+ca.set_renewal_master(ca_hostname)
+
+return this_services, other_services, ca_hostname
+
+
+def cleanup_server_dns_entries(realm, hostname, suffix, options):
+try:
+if bindinstance.dns_container_exists(options.host, suffix,
+ dm_password=options.dirman_passwd):
+bind = bindinstance.BindInstance()
+bind.remove_master_dns_records(hostname, realm, realm.lower())
+bind.remove_ipa_ca_dns_records(hostname, realm.lower())
+bind.remove_server_ns_records(hostname)
+
+keysyncd = dnskeysyncinstance.DNSKeySyncInstance()
+keysyncd.remove_replica_public_keys(hostname)
+except Exception, e:
+print "Failed to cleanup %s DNS entries: %s

Re: [Freeipa-devel] [PATCH 0244] DNSSEC: fix traceback in ipa-dnskeysyncd during shutdown phase

2015-06-11 Thread Petr Spacek

On 12.5.2015 14:51, Martin Basti wrote:
> https://fedorahosted.org/freeipa/ticket/4657
> 
> Patch attached.

ACK

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0256] DNS: add UnknonwRecord attribute to schema

2015-06-11 Thread Petr Spacek

On 22.5.2015 14:14, Martin Basti wrote:
> Patch attached.
> 
> Initial part of https://fedorahosted.org/freeipa/ticket/4939

ACK

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 445] install: Fix logging setup in server and replica install

2015-06-11 Thread Martin Basti


On 11/06/15 15:28, Jan Cholasta wrote:

Hi,

the attached patch reverts logging in ipa-server-install and 
ipa-replica-install to the old behavior.


Honza




ACK

--
Martin Basti

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0052] Stage User: Fix permissions naming and split them where, apropriate.

2015-06-11 Thread David Kupka


Dne 11.6.2015 v 16:17 Martin Kosek napsal(a):

On 06/11/2015 03:55 PM, David Kupka wrote:

Dne 11.6.2015 v 14:12 thierry bordaz napsal(a):

On 06/10/2015 02:14 PM, David Kupka wrote:

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

Hello David,

The patch looks ok except it removes a permission to update 'uid' from
an active user. This permission is required to delete(preserve) an
active user.

 -# Active container
 -#
 -# Stage user administrators need write right on RDN when
 -# the active user is deleted (preserved)
 -'System: Write Active Users RDN by administrators': {
 -'ipapermlocation': DN(baseuser.active_container_dn,
 api.env.basedn),
 -'ipapermbindruletype': 'permission',
 -'ipapermtarget': DN('uid=*',
 baseuser.active_container_dn, api.env.basedn),
 -'ipapermtargetfilter': {'(objectclass=posixaccount)'},
 -'ipapermright': {'write'},
 -'ipapermdefaultattr': {'uid'},
 -'default_privileges': {'Stage User Administrators'},
 -},
 -#

I prepared a new patch (attached) with that permission and it makes
'user-del --preserve' happy.
Now I think the name would rather be something like: 'System: Preserve
an active user (user-del --preserve)'

I also added back this comment in two permissions 'Note: targetfilter is
the target parent container'.
This was to say that the targetfilter setting was intentional.
If you think it is not the right place, you may remove those comments.

Thanks
thierry



Hello Thierry,
Indeed, I accidentally removed these. Thank you for careful review.
Rebase is needed but it is due to change in VERSION and is useless to do it
before push as there are too much patches going to master right now.
Martin, are you (as a reporter) OK with the patch?



Not entirely. I still see some weird permission in stageuser.py:

 #
 # Active container
 #
 # Stage user administrators need write right on RDN when
 # the active user is deleted (preserved)
 'System: Write Active Users RDN by administrators': {
 'ipapermlocation': DN(baseuser.active_container_dn, 
api.env.basedn),
 'ipapermbindruletype': 'permission',
 'ipapermtarget': DN('uid=*', baseuser.active_container_dn,
api.env.basedn),
 'ipapermtargetfilter': {'(objectclass=posixaccount)'},
 'ipapermright': {'write'},
 'ipapermdefaultattr': {'uid'},
 'default_privileges': {'Stage User Administrators'},
 },

This was supposed to be ""System: Modify User RDN". When the name is also
fixed, I am fine.


Updated patch attached.


--
David Kupka
From d4d7ee1c2c2e6ca88afa676d338cca4d80b8b379 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz 
Date: Thu, 11 Jun 2015 13:18:27 +0200
Subject: [PATCH] Stage User: Fix permissions naming and split them where
 apropriate.

---
 ACI.txt | 26 +++---
 VERSION |  4 +--
 ipalib/plugins/stageuser.py | 82 ++---
 3 files changed, 56 insertions(+), 56 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 60e9ebb10bc9b7266ff0d42a05d4d165d4ed2d55..08fc05ebc202a64b0e1584303c8dda5b5a1aa074 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -247,25 +247,27 @@ aci: (targetattr = "cn || createtimestamp || entryusn || ipaallowedtarget || mem
 dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
 aci: (targetfilter = "(objectclass=groupofprincipals)")(version 3.0;acl "permission:System: Remove Service Delegations";allow (delete) groupdn = "ldap:///cn=System: Remove Service Delegations,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
-aci: (targetattr = "*")(target = "ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=*)")(version 3.0;acl "permission:System: Add Stage Users by Provisioning and Administrators";allow (add) groupdn = "ldap:///cn=System: Add Stage Users by Provisioning and Administrators,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "*")(target = "ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=*)")(version 3.0;acl "permission:System: Add Stage User";allow (add) groupdn = "ldap:///cn=System: Add Stage User,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+dn: cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example
+aci: (targetattr = "*")(target = "ldap:///uid=*,cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=posixaccount)")(version 3.0;acl "permission:System: Modify Preserved Users";allow (write) groupdn = "ldap:///cn=System: Modify Preserved Users,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
-aci: (targetattr = "*")(targ

Re: [Freeipa-devel] with new cert profiles patches ipa-replica-prepare fails after update

2015-06-11 Thread Fraser Tweedale

On Thu, Jun 11, 2015 at 09:59:03AM +0200, Martin Babinsky wrote:
> On 06/04/2015 04:03 PM, Petr Vobornik wrote:
> >- ipa-replica-prepare works
> >- old IPA server was upgraded to today's master (with Cert profiles
> >patches)
> >- ipa-replica-prepare fails with:
> >
> >Log:
> >
> >ipa: DEBUG: approved_usage = SSL Server intended_usage = SSL Server
> >ipa: DEBUG: cert valid True for "CN=repl.example.com,O=EXAMPLE.COM"
> >ipa: DEBUG: handshake complete, peer = [beef::cafe]:8443
> >ipa: DEBUG: Protocol: TLS1.2
> >ipa: DEBUG: Cipher: TLS_RSA_WITH_AES_128_GCM_SHA256
> >ipa: DEBUG: request status 200
> >ipa: DEBUG: request reason_phrase u'OK'
> >ipa: DEBUG: request headers {'date': 'Thu, 04 Jun 2015 13:54:09 GMT',
> >'content-length': '148', 'content-type': 'application/xml', 'server':
> >'Apache-Coyote/1.1'}
> >ipa: DEBUG: request body ' >standalone="no"?>1Profile
> >caIPAserviceCert Not Found'
> >ipa.ipaserver.install.ipa_replica_prepare.ReplicaPrepare: DEBUG:   File
> >"/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in
> >execute
> > return_value = self.run()
> >   File
> >"/usr/lib/python2.7/site-packages/ipaserver/install/ipa_replica_prepare.py",
> >line 338, in run
> > self.copy_ds_certificate()
> >   File
> >"/usr/lib/python2.7/site-packages/ipaserver/install/ipa_replica_prepare.py",
> >line 383, in copy_ds_certificate
> > self.export_certdb("dscert", passwd_fname)
> >   File
> >"/usr/lib/python2.7/site-packages/ipaserver/install/ipa_replica_prepare.py",
> >line 595, in export_certdb
> > db.create_server_cert(nickname, hostname, ca_db)
> >   File "/usr/lib/python2.7/site-packages/ipaserver/install/certs.py",
> >line 337, in create_server_cert
> > cdb.issue_server_cert(self.certreq_fname, self.certder_fname)
> >   File "/usr/lib/python2.7/site-packages/ipaserver/install/certs.py",
> >line 419, in issue_server_cert
> > raise RuntimeError("Certificate issuance failed")
> >
> 
> Bump, I have also came across this issue (see log:
> http://pastebin.test.redhat.com/289434).
> 
> -- 
> Martin^3 Babinsky

Thanks for the reports.  I will try to reproduce and fix tomorr...
*looks at clock*... later on today, after the sun rises and I have
had some sleep :)

Cheers,
Fraser

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0052] Stage User: Fix permissions naming and split them where, apropriate.

2015-06-11 Thread Martin Kosek

On 06/11/2015 03:55 PM, David Kupka wrote:
> Dne 11.6.2015 v 14:12 thierry bordaz napsal(a):
>> On 06/10/2015 02:14 PM, David Kupka wrote:
>>> https://fedorahosted.org/freeipa/ticket/5057
>> Hello David,
>>
>> The patch looks ok except it removes a permission to update 'uid' from
>> an active user. This permission is required to delete(preserve) an
>> active user.
>>
>> -# Active container
>> -#
>> -# Stage user administrators need write right on RDN when
>> -# the active user is deleted (preserved)
>> -'System: Write Active Users RDN by administrators': {
>> -'ipapermlocation': DN(baseuser.active_container_dn,
>> api.env.basedn),
>> -'ipapermbindruletype': 'permission',
>> -'ipapermtarget': DN('uid=*',
>> baseuser.active_container_dn, api.env.basedn),
>> -'ipapermtargetfilter': {'(objectclass=posixaccount)'},
>> -'ipapermright': {'write'},
>> -'ipapermdefaultattr': {'uid'},
>> -'default_privileges': {'Stage User Administrators'},
>> -},
>> -#
>>
>> I prepared a new patch (attached) with that permission and it makes
>> 'user-del --preserve' happy.
>> Now I think the name would rather be something like: 'System: Preserve
>> an active user (user-del --preserve)'
>>
>> I also added back this comment in two permissions 'Note: targetfilter is
>> the target parent container'.
>> This was to say that the targetfilter setting was intentional.
>> If you think it is not the right place, you may remove those comments.
>>
>> Thanks
>> thierry
>>
> 
> Hello Thierry,
> Indeed, I accidentally removed these. Thank you for careful review.
> Rebase is needed but it is due to change in VERSION and is useless to do it
> before push as there are too much patches going to master right now.
> Martin, are you (as a reporter) OK with the patch?
> 

Not entirely. I still see some weird permission in stageuser.py:

#
# Active container
#
# Stage user administrators need write right on RDN when
# the active user is deleted (preserved)
'System: Write Active Users RDN by administrators': {
'ipapermlocation': DN(baseuser.active_container_dn, api.env.basedn),
'ipapermbindruletype': 'permission',
'ipapermtarget': DN('uid=*', baseuser.active_container_dn,
api.env.basedn),
'ipapermtargetfilter': {'(objectclass=posixaccount)'},
'ipapermright': {'write'},
'ipapermdefaultattr': {'uid'},
'default_privileges': {'Stage User Administrators'},
},

This was supposed to be ""System: Modify User RDN". When the name is also
fixed, I am fine.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 0265] Server Upgrade: Create NIS server configuration during upgrade in off mode

2015-06-11 Thread Martin Basti


Without this patch, upgrader shows the parent entry not found error.

NIS Server plugin is disabled by default, must be enabled by ipa-nis-manage

Patch attached.

--
Martin Basti

From fab2f8c471a300aba340eb7031606cbdceff96c8 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 11 Jun 2015 13:59:09 +0200
Subject: [PATCH] Server Upgrade: create default config for NIS Server plugin

Plugin is disabled by default.

This commit prevents false positive upgrade errors.
---
 install/updates/50-nis.update | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/install/updates/50-nis.update b/install/updates/50-nis.update
index f9aa19569a2d0a018b3e1e74f8c805b19f157746..149889ec7bdb38073eb6df88628792526cfe58e6 100644
--- a/install/updates/50-nis.update
+++ b/install/updates/50-nis.update
@@ -1,3 +1,21 @@
+# NIS Server plugin must be disabled by default
+# command 'ipa-nis-manage enable' enables NIS server
+dn: cn=NIS Server,cn=plugins,cn=config
+default:objectclass: top
+default:objectclass: nsSlapdPlugin
+default:objectclass: extensibleObject
+default:cn: NIS Server
+default:nsslapd-pluginpath: /usr/lib$LIBARCH/dirsrv/plugins/nisserver-plugin.so
+default:nsslapd-plugininitfunc: nis_plugin_init
+default:nsslapd-plugintype: object
+default:nsslapd-pluginbetxn: on
+default:nsslapd-pluginenabled: off
+default:nsslapd-pluginid: nis-server
+default:nsslapd-pluginversion: 0.10
+default:nsslapd-pluginvendor: redhat.com
+default:nsslapd-plugindescription: NIS Server Plugin
+default:nis-tcp-wrappers-name: nis-server
+
 # Correct syntax error that caused users to not appear
 dn: nis-domain=$DOMAIN+nis-map=netgroup, cn=NIS Server, cn=plugins, cn=config
 replace:nis-value-format: %merge(" ","%{memberNisNetgroup}","(%link(\"%ifeq(\\\"hostCategory\\\",\\\"all\\\",\\\"\\\",\\\"%collect(\\\"%{externalHost}\\\",\\\"%deref(\\\"memberHost\\\",\\\"fqdn\\\")\\\",\\\"%deref_r(\\\"member\\\",\\\"fqdn\\\")\\\",\\\"%deref_r(\\\"memberHost\\\",\\\"member\\\",\\\"fqdn\\\")\\\")\\\")\",\"-\",\",\",\"%ifeq(\\\"userCategory\\\",\\\"all\\\",\\\"\\\",\\\"%collect(\\\"%deref(\\\"memberUser\\\",\\\"uid\\\")\\\",\\\"%deref_r(\\\"member\\\",\\\"uid\\\")\\\",\\\"%deref_r(\\\"memberUser\\\",\\\"member\\\",\\\"uid\\\")\\\")\\\")\",\"-\"),%{nisDomainName:-})")::%merge(" ","%{memberNisNetgroup}","(%link(\"%ifeq(\\\"hostCategory\\\",\\\"all\\\",\\\"\\\",\\\"%collect(\\\"%{externalHost}\\\",\\\"%deref(\\\"memberHost\\\",\\\"fqdn\\\")\\\",\\\"%deref_r(\\\"member\\\",\\\"fqdn\\\")\\\",\\\"%deref_r(\\\"memberHost\\\",\\\"member\\\",\\\"fqdn\\\")\\\")\\\")\",\"-\",\",\",\"%ifeq(\\\"userCategory\\\",\\\"all\\\",\\\"\\\",\\\"%collect(\\\"%deref(\\\"memberUser\\\",\\\"uid\\\")\\\",\\\"%deref_r(\\\"member\\\",\\\"uid\\\")\\\",\\\"%deref_r(\\\"memberUser\\\",\\\"member\\\",\\\"uid\\\")\\\")\\\")\",\"-\"),%{nisDomainName:-})")
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] DNA range distribution to replicas by default

2015-06-11 Thread Simo Sorce

On Thu, 2015-06-11 at 12:38 +0200, Petr Spacek wrote:
> On 9.6.2015 15:06, Simo Sorce wrote:
> > On Tue, 2015-06-09 at 10:30 +0200, Petr Spacek wrote:
> >> Hello,
> >>
> >> I would like to discuss
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1211366
> >> "Error creating a user when jumping from an original server to replica".
> >>
> >> Currently the DNA ranges are distributed from master to other replicas on
> >> first attempt to get a number from particular range.
> >>
> >> This works well as long as the original master is reachable but fails
> >> miserably when the master is not reachable for any reason.
> >>
> >> It is apparently confusing to users [1][2] because it is counter-intuitive.
> >> They have created a replica to be sure that everything will work when the
> >> first server is down, right?
> >>
> >> Remediation is technically simple [3] (just assign a range to the new 
> >> replica)
> >> but it is confusing to the users, error-prone, and personally I feel that 
> >> this
> >> is an unnecessary obstacle.
> >>
> >> It seems to me that the original motivation for this behavior was that the
> >> masters were not able to request range back from other replicas when a 
> >> local
> >> range was depleted.
> >>
> >> This deficiency is tracked as
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1029640 and it is slated for 
> >> fix
> >> in 4.2.x time frame.
> >>
> >> Can we distribute ranges to the replicas during ipa-replica-install when we
> >> fix bug 1029640?
> > 
> > That was not the only reason, another reason is that you do not want to
> > distribute and fragment ranges to replicas that will never be used to
> > create users. What we should do perhaps, is to automatically give a
> > range to CA enabled masters so that at least those servers have a range.
> > If all your CAs are unavailable you have major issues anyway.
> > 
> > Though it is a bit bad to have magic behaviors, maybe we should have a
> > "main DNA range holder" role that can be assigned to arbitrary servers
> > (maybe the first replica gets it by default), and when done the server
> > acquire part of the range if it has none.
> 
> This concept sounds good to me!
> 
> I would only reverse the default, i.e. distribute ranges by default to all
> replicas and let admin to toggle a knob if he feels that his case really needs
> to limit range distribution.

By the time you *feel* that it may be too late.

> > Another option is that a replica can instantiate a whole new range if
> > all the range bearing servers are not around, but that also comes with
> > its own issues.
> > 
> > In general I wouldn't want to split by default, because in domains with
> > *many* replicas most of them are used for load balancing and will never
> > be used to create users, so the range would be wasted.
> 
> This should not be an issue when
> https://bugzilla.redhat.com/show_bug.cgi?id=1029640 is fixed because replicas
> will be able to request range back if the local chunk is depleted.
> 
> Is that correct?

To some degree, the main issue is when replicas get removed abruptly and
are not around to "give back" anything.
We would need to start working on a range-scavenging tool to reclaim
"lost" ranges if you go and automatically distribute ranges to every
replica that ever pops up.

Simo.

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

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation


On 06/11/2015 03:18 PM, Martin Basti wrote:

On 04/06/15 17:28, Petr Spacek wrote:

On 3.6.2015 17:14, Martin Basti wrote:

On 03/06/15 14:57, Petr Spacek wrote:

On 18.5.2015 13:48, Martin Basti wrote:

On 15/05/15 18:11, Petr Spacek wrote:

On 7.5.2015 18:12, Martin Basti wrote:

On 07/05/15 12:19, Petr Spacek wrote:

On 7.5.2015 08:59, David Kupka wrote:

On 05/06/2015 03:20 PM, Martin Basti wrote:

On 05/05/15 15:00, Martin Basti wrote:

On 30/04/15 15:37, David Kupka wrote:

On 04/24/2015 02:56 PM, Martin Basti wrote:

Patches attached.





Hi,
thanks for patches.

1. You changed message in DNSServerNotRespondingWarning
class but not
the test in ipatest/test_xmlrpc/test_dns_plugin.py

nitpick. Please spell 'edns' correctly. I've seen several
instances
of 'ends'.


Thank you,

updated patches attached:
* new error messages
* logging to debug log server output if exception was raised
* fixed test
* fixed spelling




Fixed tests (again)

Updated patches attached


The code looks good to me and tests are no longer broken. (I
would prefer
better fix of the tests but given that the priorities are
different now
it can
wait.)

Petr, can you please confirm that the patch set works for you?

Sorry, NACK:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: an internal error has occurred

# /var/log/httpd/error_log
ipa: ERROR: non-public: AssertionError:
Traceback (most recent call last):
  File
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line
350, in
wsgi_execute
result = self.Command[name](*args, **options)
  File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
443, in
__call__
ret = self.run(*args, **options)
  File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 760,
in run
return self.execute(*args, **options)
  File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
, in
execute
**options)
  File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
4405, in
_warning_if_forwarders_do_not_work
log=self.log)
  File "/usr/lib/python2.7/site-packages/ipalib/util.py",
line 715, in
validate_dnssec_zone_forwarder_step2
timeout=timeout)
  File "/usr/lib/python2.7/site-packages/ipalib/util.py",
line 610, in
_resolve_record
assert isinstance(nameserver_ip, basestring)
AssertionError
ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE:
dnsforwardzone_add(, idnsforwarders=(u'10.34.47.236',), all=False,
raw=False,
version=u'2.116'): AssertionError

This is constantly reproducible in my vm-090.abc. Let me know if
you
want to
take a look.


I'm attaching little response.patch which improves compatibility
with older
python-dns packages. This patch allows IPA to work while error
messages are
simply not as nice as they could be with latest python-dns :-)

check_fwd_msg.patch is a little nitpick, just to make sure everyone
understands the message.

BTW why some messages in check_forwarders() are printed using
'print' and
others using logger? I would prefer to use logger for everything
to make
sure
that logs contain all the information, including warnings.

Thank you for your time!


Thank you, fixed.

I  added missing except block after forwarders validation step2.

I confirm that this works but I just discovered another deficiency.

Setup:
- DNSSEC validation is enabled on IPA server
- forwarders uses fake TLD, e.g. 'test.'
- remote DNS server is responding, supports EDNS0 and so on

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The
DNS query
name does not exist: ptr.test..

Huh? Let's check named log:
forward zone 'ptr.test': loaded
validating ./SOA: got insecure response; parent indicates it
should be
secure

Sometimes I get SERVFAIL from IPA server, too.


Unfortunately this check was the main reason for writing this
patchset so we
need to improve it.

Maybe validate_dnssec_zone_forwarder_step2() could special-case
NXDOMAIN and
print the DNSSEC-validation-failed error, too? The problem is that
it could
trigger some false positives because NXDOMAIN may simply be caused
by a delay
somewhere.

Any ideas?

I add catch block for NXDOMAIN

By the way, this is also weird:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: DNS forward zone with name "ptr.test." already exists

Is it actually doing the check even if the forward zone exists
already? (This
is just nitpick, not a blocker!)


The first part is written by IPA client, it is not response from
server.
It is just written when user use --forwarder option.

Updated patch attached.

NACK, it does not work for me - it explodes when I try to add a
forward zone:

$ ipa dnsforwardzon

Re: [Freeipa-devel] [PATCH 0052] Stage User: Fix permissions naming and split them where, apropriate.

2015-06-11 Thread David Kupka


Dne 11.6.2015 v 14:12 thierry bordaz napsal(a):

On 06/10/2015 02:14 PM, David Kupka wrote:

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

Hello David,

The patch looks ok except it removes a permission to update 'uid' from
an active user. This permission is required to delete(preserve) an
active user.

-# Active container
-#
-# Stage user administrators need write right on RDN when
-# the active user is deleted (preserved)
-'System: Write Active Users RDN by administrators': {
-'ipapermlocation': DN(baseuser.active_container_dn,
api.env.basedn),
-'ipapermbindruletype': 'permission',
-'ipapermtarget': DN('uid=*',
baseuser.active_container_dn, api.env.basedn),
-'ipapermtargetfilter': {'(objectclass=posixaccount)'},
-'ipapermright': {'write'},
-'ipapermdefaultattr': {'uid'},
-'default_privileges': {'Stage User Administrators'},
-},
-#

I prepared a new patch (attached) with that permission and it makes
'user-del --preserve' happy.
Now I think the name would rather be something like: 'System: Preserve
an active user (user-del --preserve)'

I also added back this comment in two permissions 'Note: targetfilter is
the target parent container'.
This was to say that the targetfilter setting was intentional.
If you think it is not the right place, you may remove those comments.

Thanks
thierry



Hello Thierry,
Indeed, I accidentally removed these. Thank you for careful review.
Rebase is needed but it is due to change in VERSION and is useless to do 
it before push as there are too much patches going to master right now.

Martin, are you (as a reporter) OK with the patch?

--
David Kupka

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 445] install: Fix logging setup in server and replica install

2015-06-11 Thread Jan Cholasta


Hi,

the attached patch reverts logging in ipa-server-install and 
ipa-replica-install to the old behavior.


Honza

--
Jan Cholasta
>From c0c3e98be38484638bf670587775bbc9dfda2501 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 11 Jun 2015 13:04:31 +
Subject: [PATCH] install: Fix logging setup in server and replica install

https://fedorahosted.org/freeipa/ticket/4468
---
 ipapython/install/cli.py | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index b526ea7..1ba9a81 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -10,7 +10,7 @@ import collections
 import optparse
 import signal
 
-from ipapython import admintool
+from ipapython import admintool, ipa_log_manager
 from ipapython.ipautil import CheckedIPAddress, private_ccache
 
 from . import core, common
@@ -256,6 +256,21 @@ class ConfigureTool(admintool.AdminTool):
 
 index += 1
 
+def _setup_logging(self, log_file_mode='w', no_file=False):
+if no_file:
+log_file_name = None
+elif self.options.log_file:
+log_file_name = self.options.log_file
+else:
+log_file_name = self.log_file_name
+ipa_log_manager.standard_logging_setup(log_file_name,
+   debug=self.options.verbose)
+self.log = ipa_log_manager.log_mgr.get_logger(self)
+if log_file_name:
+self.log.debug('Logging to %s' % log_file_name)
+elif not no_file:
+self.log.debug('Not logging to a file')
+
 def run(self):
 kwargs = {}
 
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation

2015-06-11 Thread Martin Basti

On 04/06/15 17:28, Petr Spacek wrote:

On 3.6.2015 17:14, Martin Basti wrote:

On 03/06/15 14:57, Petr Spacek wrote:

On 18.5.2015 13:48, Martin Basti wrote:

On 15/05/15 18:11, Petr Spacek wrote:

On 7.5.2015 18:12, Martin Basti wrote:

On 07/05/15 12:19, Petr Spacek wrote:

On 7.5.2015 08:59, David Kupka wrote:

On 05/06/2015 03:20 PM, Martin Basti wrote:

On 05/05/15 15:00, Martin Basti wrote:

On 30/04/15 15:37, David Kupka wrote:

On 04/24/2015 02:56 PM, Martin Basti wrote:

Patches attached.

Hi,
thanks for patches.

1. You changed message in DNSServerNotRespondingWarning class but not
the test in ipatest/test_xmlrpc/test_dns_plugin.py

nitpick. Please spell 'edns' correctly. I've seen several instances
of 'ends'.

Thank you,

updated patches attached:
* new error messages
* logging to debug log server output if exception was raised
* fixed test
* fixed spelling

Fixed tests (again)

Updated patches attached

The code looks good to me and tests are no longer broken. (I would prefer
better fix of the tests but given that the priorities are different now
it can
wait.)

Petr, can you please confirm that the patch set works for you?

Sorry, NACK:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: an internal error has occurred

# /var/log/httpd/error_log
ipa: ERROR: non-public: AssertionError:
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line
350, in
wsgi_execute
result = self.Command[name](*args, **options)
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
443, in
__call__
ret = self.run(*args, **options)
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 760,
in run
return self.execute(*args, **options)
File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
, in
execute
**options)
File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
4405, in
_warning_if_forwarders_do_not_work
log=self.log)
File "/usr/lib/python2.7/site-packages/ipalib/util.py", line 715, in
validate_dnssec_zone_forwarder_step2
timeout=timeout)
File "/usr/lib/python2.7/site-packages/ipalib/util.py", line 610, in
_resolve_record
assert isinstance(nameserver_ip, basestring)
AssertionError
ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(, idnsforwarders=(u'10.34.47.236',), all=False, raw=False,
version=u'2.116'): AssertionError

This is constantly reproducible in my vm-090.abc. Let me know if you
want to
take a look.

I'm attaching little response.patch which improves compatibility with older
python-dns packages. This patch allows IPA to work while error messages are
simply not as nice as they could be with latest python-dns :-)

check_fwd_msg.patch is a little nitpick, just to make sure everyone
understands the message.

BTW why some messages in check_forwarders() are printed using 'print' and
others using logger? I would prefer to use logger for everything to make
sure
that logs contain all the information, including warnings.

Thank you for your time!

Thank you, fixed.

I added missing except block after forwarders validation step2.

I confirm that this works but I just discovered another deficiency.

Setup:
- DNSSEC validation is enabled on IPA server
- forwarders uses fake TLD, e.g. 'test.'
- remote DNS server is responding, supports EDNS0 and so on

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The DNS query
name does not exist: ptr.test..

Huh? Let's check named log:
forward zone 'ptr.test': loaded
validating ./SOA: got insecure response; parent indicates it should be
secure

Sometimes I get SERVFAIL from IPA server, too.

Unfortunately this check was the main reason for writing this patchset so we
need to improve it.

Maybe validate_dnssec_zone_forwarder_step2() could special-case NXDOMAIN and
print the DNSSEC-validation-failed error, too? The problem is that it could
trigger some false positives because NXDOMAIN may simply be caused by a delay
somewhere.

Any ideas?

I add catch block for NXDOMAIN

By the way, this is also weird:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: DNS forward zone with name "ptr.test." already exists

Is it actually doing the check even if the forward zone exists already? (This
is just nitpick, not a blocker!)

The first part is written by IPA client, it is not response from server.
It is just written when user use --forwarder option.

Updated patch attached.

NACK, it does not work for me - it explodes when I try to add a forward zone:

$ ipa dnsforwardzone-add ptr.test. --forwarder=192.0.2.1

ipa: E

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes


On 06/11/2015 01:41 PM, Petr Vobornik wrote:

On 06/11/2015 01:11 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:53 PM, Petr Vobornik wrote:

On 06/11/2015 12:35 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:19 PM, Petr Vobornik wrote:

On 06/11/2015 10:22 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to
other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



Looking at Ludwig's path 12, the patch completely forbids mod of
ipaReplTopoSegmentDirection?

that's what I thought we agreed on,


I thought, that we will only complain loudly on downgrade of connection.


so you would have to add a segment
in the opposite direction an they would be merged to both,
but maybe this is a bit strict.


This could work as well, but:

I just tried (without patch 12) to create:
1. A to B, left-right: success
2. B to A, right-left: "Server is unwilling to perform: Segment
already exists in topology or is self referential. Add rejected."

yes, B to  A, right-left is the same as A-B, left right


Sorry, you are right, I wrote it badly. I'm not sure if the servers are
broken from testing and previous bugs. Maybe I should reinstalled, but
I'm experiencing following weird behavior:

A-B segment, doesn't exist.

1. A to B, left-right: success
2. A to B, right-left: "Server is unwilling to perform: Segment already
exists in topology or is self referential. Add rejected."

If I try different direction (started with 4 segments):
1. A to B, right-left: success, 5 segments exist
2. A to B, left-right: success, 4 segments exist - the new ones are gone

Martin, can you reproduce it?

I'm currently working on something else, but will fetch some fresh VMs 
and try to reproduce this glitch ASAP.


I.e., the upgrade didn't happen.


I could allow for
ipaReplTopoSegmentDirection replace: both

So that upgrade from right-left and left-right to both is not
allowed?  If so then this patch needs to be updated.

depends a bit on what you prefer and what we can get in for alpha.


Depends what's better, I already have adjusted patch for ^^ so it's
not about the work.

so lets take the changes to your patch and we could still extend
functionality a bit for beta or later



OK, attaching rebased patch.



--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0052] Stage User: Fix permissions naming and split them where, apropriate.


On 06/10/2015 02:14 PM, David Kupka wrote:

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

Hello David,

The patch looks ok except it removes a permission to update 'uid' from 
an active user. This permission is required to delete(preserve) an 
active user.


   -# Active container
   -#
   -# Stage user administrators need write right on RDN when
   -# the active user is deleted (preserved)
   -'System: Write Active Users RDN by administrators': {
   -'ipapermlocation': DN(baseuser.active_container_dn,
   api.env.basedn),
   -'ipapermbindruletype': 'permission',
   -'ipapermtarget': DN('uid=*',
   baseuser.active_container_dn, api.env.basedn),
   -'ipapermtargetfilter': {'(objectclass=posixaccount)'},
   -'ipapermright': {'write'},
   -'ipapermdefaultattr': {'uid'},
   -'default_privileges': {'Stage User Administrators'},
   -},
   -#

I prepared a new patch (attached) with that permission and it makes 
'user-del --preserve' happy.
Now I think the name would rather be something like: 'System: Preserve 
an active user (user-del --preserve)'


I also added back this comment in two permissions 'Note: targetfilter is 
the target parent container'.

This was to say that the targetfilter setting was intentional.
If you think it is not the right place, you may remove those comments.

Thanks
thierry
From a35afd482cf08ed6ee721bf425041cae05c5e518 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz 
Date: Thu, 11 Jun 2015 13:18:27 +0200
Subject: [PATCH] Stage User: Fix permissions naming and split them where
 apropriate.

---
 ACI.txt | 26 ---
 VERSION |  4 +--
 ipalib/plugins/stageuser.py | 80 ++---
 3 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 59173ac1b593f15e079c7b1fce43ec9b0084ec91..338e5e174c575875ade1a35304c6f0e8dad9e39e 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -237,25 +237,27 @@ aci: (targetattr = "cn || createtimestamp || entryusn || ipaallowedtarget || mem
 dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
 aci: (targetfilter = "(objectclass=groupofprincipals)")(version 3.0;acl "permission:System: Remove Service Delegations";allow (delete) groupdn = "ldap:///cn=System: Remove Service Delegations,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
-aci: (targetattr = "*")(target = "ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=*)")(version 3.0;acl "permission:System: Add Stage Users by Provisioning and Administrators";allow (add) groupdn = "ldap:///cn=System: Add Stage Users by Provisioning and Administrators,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "*")(target = "ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=*)")(version 3.0;acl "permission:System: Add Stage User";allow (add) groupdn = "ldap:///cn=System: Add Stage User,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+dn: cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example
+aci: (targetattr = "*")(target = "ldap:///uid=*,cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=posixaccount)")(version 3.0;acl "permission:System: Modify Preserved Users";allow (write) groupdn = "ldap:///cn=System: Modify Preserved Users,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
-aci: (targetattr = "*")(target = "ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=*)")(version 3.0;acl "permission:System: Delete modify Stage Users by administrators";allow (delete,write) groupdn = "ldap:///cn=System: Delete modify Stage Users by administrators,cn=permissions,cn=pbac,dc=ipa,dc=example";)
-dn: dc=ipa,dc=example
-aci: (target_to = "ldap:///cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(target_from = "ldap:///cn=users,cn=accounts,dc=ipa,dc=example";)(targetfilter = "(objectclass=nsContainer)")(version 3.0;acl "permission:System: Preserve an active user to a delete Users";allow (moddn) groupdn = "ldap:///cn=System: Preserve an active user to a delete Users,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "*")(target = "ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=*)")(version 3.0;acl "permission:System: Modify Stage User";allow (write) groupdn = "ldap:///cn=System: Modify Stage User,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
-aci: (target_to = "ldap:///cn=users,cn=accounts,dc=ipa,dc=example";)(target_from = "ldap:///cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example")(targetfilter = "(objectclass=nsContainer)")(version 3.0;acl "permissio

Re: [Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes


On 06/11/2015 11:27 AM, Ludwig Krispenz wrote:

Thanks,
attached a new version with comments and trying to use more meaningful
function names






Ok. Thanks for the explanations. To help reading you may add a comment
saying the the first test is related to RA and the second to segments.

Other than that the fix is good for me. ACK




master:
* b3c2a4b810bfe31dc544648de8fe98dbb84ec320 make sure the agremment rdn 
match the rdn used in the segment
* 056518ab1af36fa4a8d7b4450616145aa0dbfd16 v2-reject modifications of 
endpoints and connectivity of a segment

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes



On 06/11/2015 01:41 PM, Petr Vobornik wrote:

On 06/11/2015 01:11 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:53 PM, Petr Vobornik wrote:

On 06/11/2015 12:35 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:19 PM, Petr Vobornik wrote:

On 06/11/2015 10:22 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to 
other

   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



Looking at Ludwig's path 12, the patch completely forbids mod of
ipaReplTopoSegmentDirection?

that's what I thought we agreed on,


I thought, that we will only complain loudly on downgrade of 
connection.



so you would have to add a segment
in the opposite direction an they would be merged to both,
but maybe this is a bit strict.


This could work as well, but:

I just tried (without patch 12) to create:
1. A to B, left-right: success
2. B to A, right-left: "Server is unwilling to perform: Segment
already exists in topology or is self referential. Add rejected."

yes, B to  A, right-left is the same as A-B, left right


Sorry, you are right, I wrote it badly. I'm not sure if the servers 
are broken from testing and previous bugs. Maybe I should reinstalled, 
but I'm experiencing following weird behavior:


A-B segment, doesn't exist.

1. A to B, left-right: success
2. A to B, right-left: "Server is unwilling to perform: Segment 
already exists in topology or is self referential. Add rejected."

that is probably a bug, will work on it


If I try different direction (started with 4 segments):
1. A to B, right-left: success, 5 segments exist
2. A to B, left-right: success, 4 segments exist - the new ones are gone

that seems weird, would need to reproduce to investigate.


Martin, can you reproduce it?



I.e., the upgrade didn't happen.


I could allow for
ipaReplTopoSegmentDirection replace: both

So that upgrade from right-left and left-right to both is not
allowed?  If so then this patch needs to be updated.

depends a bit on what you prefer and what we can get in for alpha.


Depends what's better, I already have adjusted patch for ^^ so it's
not about the work.

so lets take the changes to your patch and we could still extend
functionality a bit for beta or later



OK, attaching rebased patch.


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes


On 06/11/2015 01:11 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:53 PM, Petr Vobornik wrote:

On 06/11/2015 12:35 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:19 PM, Petr Vobornik wrote:

On 06/11/2015 10:22 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



Looking at Ludwig's path 12, the patch completely forbids mod of
ipaReplTopoSegmentDirection?

that's what I thought we agreed on,


I thought, that we will only complain loudly on downgrade of connection.


so you would have to add a segment
in the opposite direction an they would be merged to both,
but maybe this is a bit strict.


This could work as well, but:

I just tried (without patch 12) to create:
1. A to B, left-right: success
2. B to A, right-left: "Server is unwilling to perform: Segment
already exists in topology or is self referential. Add rejected."

yes, B to  A, right-left is the same as A-B, left right


Sorry, you are right, I wrote it badly. I'm not sure if the servers are 
broken from testing and previous bugs. Maybe I should reinstalled, but 
I'm experiencing following weird behavior:


A-B segment, doesn't exist.

1. A to B, left-right: success
2. A to B, right-left: "Server is unwilling to perform: Segment already 
exists in topology or is self referential. Add rejected."


If I try different direction (started with 4 segments):
1. A to B, right-left: success, 5 segments exist
2. A to B, left-right: success, 4 segments exist - the new ones are gone

Martin, can you reproduce it?



I.e., the upgrade didn't happen.


I could allow for
ipaReplTopoSegmentDirection replace: both

So that upgrade from right-left and left-right to both is not
allowed?  If so then this patch needs to be updated.

depends a bit on what you prefer and what we can get in for alpha.


Depends what's better, I already have adjusted patch for ^^ so it's
not about the work.

so lets take the changes to your patch and we could still extend
functionality a bit for beta or later



OK, attaching rebased patch.
--
Petr Vobornik
From 58284c506035906e3609bbf6f153936e49d2a21a Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Wed, 10 Jun 2015 14:44:09 +0200
Subject: [PATCH] topology: restrict direction changes

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
  direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302
---
 API.txt|  7 +++
 VERSION|  4 ++--
 install/ui/src/freeipa/topology.js | 11 ++-
 ipalib/plugins/topology.py |  3 ++-
 4 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/API.txt b/API.txt
index 15356f7ee6330de0822e7582580d2d4f48ad261d..853d26a59bb5bb1ebff698924a36a30b7757c398 100644
--- a/API.txt
+++ b/API.txt
@@ -4757,7 +4757,7 @@ arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primar
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 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: StrEnum('iparepltoposegmentdirection', attribute=True, cli_name='direction', default=u'both', multivalue=False, required=True, values=(u'both', u'left-right', u'right-left', u'none'))
+option: StrEnum('iparepltoposegmentdirection', attribute=True, cli_name='direction', default=u'both', multivalue=False, required=True, values=(u'both', u'left-right', u'right-left'))
 option: Str('iparepltoposegmentleftnode', attribute=True, cli_name='leftnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', required=True)
 option: Str('iparepltoposegmentrightnode', attribute=True, cli_name='rightnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', required=True)
 option: StrEnum('nsds5replicaenabled', attribute=True, cli_name='enabled', multivalue=False, required=False, values=(u'on', u'off'))
@@ -4786,7 +4786,7 @@ arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primar
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='name', maxlength=255, multivalue=False, primary_key=True, query=True, required=False)
-option: StrEnum('iparepltoposegmentdirection', attribute=True, autofill=False, cli_name='direction', default=u'both', multivalue=False, query=True, required=False, values=(u'both', u'left-right', u'right-left', u'none'))
+option: StrEn

Re: [Freeipa-devel] [PATCH] 870 disallow mod of topology segment nodes


On 06/11/2015 10:19 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

Mod of segment end will be disallowed in topology plugin.

Reasoning (by Ludwig):  if we want to properly allow mods to change
connectivity and endpoints, then we would need to check if the mod
disconnects the topology, delete existing agreements, check if the new
would be a duplicate and create new agmts. There could be some difficult
scenarios, like having
   A <--> B <--> C <--> D,
if you modify the segment B-C to A-D topology breaks and is then
reconnected.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



rebased and pushed to master: 5089dde2cdbe22cabdbf74f325711ea5dcc22490
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 02261] Revert 389 DS BuildRequires version


On 05/29/2015 10:43 AM, Lukas Slebodnik wrote:

On (29/05/15 10:33), Martin Basti wrote:

On 29/05/15 09:23, Lukas Slebodnik wrote:

On (12/05/15 21:03), Martin Basti wrote:

On 12/05/15 18:23, Martin Basti wrote:

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

To test this, the mkosek/freeipa-master copr repo with 389-ds-base 1.3.4.0
is needed.

All previous changes to uniqueness plugins were made just in master branch
so upgrade will not work correctly from master to newer master.

>From IPA 4.1 to master should work as expected.

Patch attached.




Updated patch attached.

--
Martin Basti


>From df2f521473a7e4f2438f675e4328ee59c8cf4617 Mon Sep 17 00:00:00 2001

From: Martin Basti 
Date: Tue, 12 May 2015 18:11:07 +0200
Subject: [PATCH] Server Upgrade: Fix uniqueness plugins

Due previous changes (in master branch only) the uniqueness plugins
became misconfigured.

After this patch:
* whole $SUFFIX will be checked by unique plugins
* just staged users are exluded from check

This reverts some changes in commit
52b7101c1148618d5c8e2ec25576cc7ad3e9b7bb

Since 389-ds-base 1.3.4.a1 new attribute 'uniqueness-exclude-subtrees'
can be used.

https://fedorahosted.org/freeipa/ticket/4921
---
freeipa.spec.in  |  6 +++---
install/share/unique-attributes.ldif | 12 ++--
install/updates/10-uniqueness.update | 20 ++--
3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
2bf14ef9e14f96b3100d45dd47d749b6bc3b4816..73736455655a100a2febef8e86db2c5a2f2419c9
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -34,7 +34,7 @@ Source0:freeipa-%{version}.tar.gz
BuildRoot:  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

%if ! %{ONLY_CLIENT}
-BuildRequires:  389-ds-base-devel >= 1.3.3.9
+BuildRequires:  389-ds-base-devel >= 1.3.4.a1

Please revert this change. FreeIPA can be built 1.3.3.9 as well.
and it block scanning pacakge with static analysers even on fedora-rawhide.

I managed to build pacakges with 389-ds-base 1.3.3.9.
It should be sufficient to have Requires for 1.3.4.a1.




BuildRequires:  svrcore-devel
BuildRequires:  policycoreutils >= 2.1.12-5
BuildRequires:  systemd-units
@@ -109,7 +109,7 @@ Group: System Environment/Base
Requires: %{name}-python = %{version}-%{release}
Requires: %{name}-client = %{version}-%{release}
Requires: %{name}-admintools = %{version}-%{release}
-Requires: 389-ds-base >= 1.3.3.9
+Requires: 389-ds-base >= 1.3.4.a1
Requires: openldap-clients > 2.4.35-4
Requires: nss >= 3.14.3-12.0
Requires: nss-tools >= 3.14.3-12.0
@@ -144,7 +144,7 @@ Requires: zip
Requires: policycoreutils >= 2.1.12-5
Requires: tar
Requires(pre): certmonger >= 0.76.8
-Requires(pre): 389-ds-base >= 1.3.3.9
+Requires(pre): 389-ds-base >= 1.3.4.a1
Requires: fontawesome-fonts
Requires: open-sans-fonts
Requires: openssl

LS

Patch attached.

--
Martin Basti




From 58e24b762c78f995d44ebf89e995df2360e9c055 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 29 May 2015 10:29:15 +0200
Subject: [PATCH] Revert 389-DS BuildRequires version to 1.3.3.9

---
freeipa.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
7dc576256865fb04b3f322b2094a5e3ece7776a5..d2943547bf4d967bac031b02eb74ab4693cb9872
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -34,7 +34,7 @@ Source0:freeipa-%{version}.tar.gz
BuildRoot:  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

%if ! %{ONLY_CLIENT}
-BuildRequires:  389-ds-base-devel >= 1.3.4.a1
+BuildRequires:  389-ds-base-devel >= 1.3.3.9
BuildRequires:  svrcore-devel
BuildRequires:  policycoreutils >= 2.1.12-5
BuildRequires:  systemd-units


Thank you.
rpms can be built on fedora-rawhide

ACK


Pushed to master: 6a92b32bf2dfe49e3f219beb6042e9fa71e18dcc




LS


--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0031] Update PKCS#11 mechanism constants for AES key wrapping to PKCS#11 v2.40


On 06/10/2015 03:53 PM, Martin Basti wrote:

On 08/06/15 16:18, Petr Spacek wrote:

Hello,

Update PKCS#11 mechanism constants for AES key wrapping to PKCS#11 v2.40.

SoftHSM 2.0.0rc1 was updates to these new constants to avoid collision
with
Blowfish mechanisms.


Older code *cannot* work SoftHSM 2.0.0rc1 and newer.

Symptoms include errors like this:

On DNSSEC key master:
ipa-ods-exporter: _ipap11helper.Error: Error at key wrapping: get buffer
length: 0x70

On DNSSEC replicas:
ipa-dnskeysyncd: subprocess.CalledProcessError: Command
''/usr/libexec/ipa/ipa-dnskeysync-replica'' returned non-zero exit
status 1


ACK



Pushed to master: 40680fd2a95ba0b00c81f5e22241b3a16d6eee54
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0051] Use 389-ds centralized scripts.


On 06/10/2015 03:51 PM, Martin Basti wrote:

On 09/06/15 16:43, David Kupka wrote:

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

ACK



Pushed to master: 4d05b5d18da84c1e9cc89e9d3c3432261863837a
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation


On 06/10/2015 03:25 PM, Martin Basti wrote:

On 04/06/15 17:28, Petr Spacek wrote:

On 3.6.2015 17:14, Martin Basti wrote:

On 03/06/15 14:57, Petr Spacek wrote:

On 18.5.2015 13:48, Martin Basti wrote:

On 15/05/15 18:11, Petr Spacek wrote:

On 7.5.2015 18:12, Martin Basti wrote:

On 07/05/15 12:19, Petr Spacek wrote:

On 7.5.2015 08:59, David Kupka wrote:

On 05/06/2015 03:20 PM, Martin Basti wrote:

On 05/05/15 15:00, Martin Basti wrote:

On 30/04/15 15:37, David Kupka wrote:

On 04/24/2015 02:56 PM, Martin Basti wrote:

Patches attached.





Hi,
thanks for patches.

1. You changed message in DNSServerNotRespondingWarning
class but not
the test in ipatest/test_xmlrpc/test_dns_plugin.py

nitpick. Please spell 'edns' correctly. I've seen several
instances
of 'ends'.


Thank you,

updated patches attached:
* new error messages
* logging to debug log server output if exception was raised
* fixed test
* fixed spelling




Fixed tests (again)

Updated patches attached


The code looks good to me and tests are no longer broken. (I
would prefer
better fix of the tests but given that the priorities are
different now
it can
wait.)

Petr, can you please confirm that the patch set works for you?

Sorry, NACK:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: an internal error has occurred

# /var/log/httpd/error_log
ipa: ERROR: non-public: AssertionError:
Traceback (most recent call last):
  File
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line
350, in
wsgi_execute
result = self.Command[name](*args, **options)
  File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
443, in
__call__
ret = self.run(*args, **options)
  File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 760,
in run
return self.execute(*args, **options)
  File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
, in
execute
**options)
  File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line
4405, in
_warning_if_forwarders_do_not_work
log=self.log)
  File "/usr/lib/python2.7/site-packages/ipalib/util.py",
line 715, in
validate_dnssec_zone_forwarder_step2
timeout=timeout)
  File "/usr/lib/python2.7/site-packages/ipalib/util.py",
line 610, in
_resolve_record
assert isinstance(nameserver_ip, basestring)
AssertionError
ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE:
dnsforwardzone_add(, idnsforwarders=(u'10.34.47.236',), all=False,
raw=False,
version=u'2.116'): AssertionError

This is constantly reproducible in my vm-090.abc. Let me know if
you
want to
take a look.


I'm attaching little response.patch which improves compatibility
with older
python-dns packages. This patch allows IPA to work while error
messages are
simply not as nice as they could be with latest python-dns :-)

check_fwd_msg.patch is a little nitpick, just to make sure everyone
understands the message.

BTW why some messages in check_forwarders() are printed using
'print' and
others using logger? I would prefer to use logger for everything
to make
sure
that logs contain all the information, including warnings.

Thank you for your time!


Thank you, fixed.

I  added missing except block after forwarders validation step2.

I confirm that this works but I just discovered another deficiency.

Setup:
- DNSSEC validation is enabled on IPA server
- forwarders uses fake TLD, e.g. 'test.'
- remote DNS server is responding, supports EDNS0 and so on

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The
DNS query
name does not exist: ptr.test..

Huh? Let's check named log:
forward zone 'ptr.test': loaded
validating ./SOA: got insecure response; parent indicates it
should be
secure

Sometimes I get SERVFAIL from IPA server, too.


Unfortunately this check was the main reason for writing this
patchset so we
need to improve it.

Maybe validate_dnssec_zone_forwarder_step2() could special-case
NXDOMAIN and
print the DNSSEC-validation-failed error, too? The problem is that
it could
trigger some false positives because NXDOMAIN may simply be caused
by a delay
somewhere.

Any ideas?

I add catch block for NXDOMAIN

By the way, this is also weird:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: DNS forward zone with name "ptr.test." already exists

Is it actually doing the check even if the forward zone exists
already? (This
is just nitpick, not a blocker!)


The first part is written by IPA client, it is not response from
server.
It is just written when user use --forwarder option.

Updated patch attached.

NACK, it does not work for me - it explodes when I try to add a
forward zone:

$ ipa dnsforwardzon

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes



On 06/11/2015 12:53 PM, Petr Vobornik wrote:

On 06/11/2015 12:35 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:19 PM, Petr Vobornik wrote:

On 06/11/2015 10:22 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



Looking at Ludwig's path 12, the patch completely forbids mod of
ipaReplTopoSegmentDirection?

that's what I thought we agreed on,


I thought, that we will only complain loudly on downgrade of connection.


so you would have to add a segment
in the opposite direction an they would be merged to both,
but maybe this is a bit strict.


This could work as well, but:

I just tried (without patch 12) to create:
1. A to B, left-right: success
2. B to A, right-left: "Server is unwilling to perform: Segment 
already exists in topology or is self referential. Add rejected."

yes, B to  A, right-left is the same as A-B, left right


I.e., the upgrade didn't happen.


I could allow for
ipaReplTopoSegmentDirection replace: both

So that upgrade from right-left and left-right to both is not
allowed?  If so then this patch needs to be updated.

depends a bit on what you prefer and what we can get in for alpha.


Depends what's better, I already have adjusted patch for ^^ so it's 
not about the work.
so lets take the changes to your patch and we could still extend 
functionality a bit for beta or later


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 868 rename topologysegment_refresh to topologysegment_reinitialize


On 06/11/2015 10:03 AM, Martin Babinsky wrote:

On 06/10/2015 02:27 PM, Petr Vobornik wrote:

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



ACK



Pushed to master: c9cbb1493a8c9e10020c7f2104a345cd43535259
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes


On 06/11/2015 12:35 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:19 PM, Petr Vobornik wrote:

On 06/11/2015 10:22 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



Looking at Ludwig's path 12, the patch completely forbids mod of
ipaReplTopoSegmentDirection?

that's what I thought we agreed on,


I thought, that we will only complain loudly on downgrade of connection.


so you would have to add a segment
in the opposite direction an they would be merged to both,
but maybe this is a bit strict.


This could work as well, but:

I just tried (without patch 12) to create:
1. A to B, left-right: success
2. B to A, right-left: "Server is unwilling to perform: Segment already 
exists in topology or is self referential. Add rejected."


I.e., the upgrade didn't happen.


I could allow for
ipaReplTopoSegmentDirection replace: both

So that upgrade from right-left and left-right to both is not
allowed?  If so then this patch needs to be updated.

depends a bit on what you prefer and what we can get in for alpha.


Depends what's better, I already have adjusted patch for ^^ so it's not 
about the work.

--
Petr Vobornik
From 75905481b892efef7d0e5ca92ceb88ff3ab073f3 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Wed, 10 Jun 2015 14:44:09 +0200
Subject: [PATCH] topology: restrict direction changes

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
  direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302
---
 API.txt|  7 +++
 VERSION|  5 ++---
 install/ui/src/freeipa/topology.js | 11 ++-
 ipalib/plugins/topology.py |  3 ++-
 4 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/API.txt b/API.txt
index 87029947aecaadcefb7fe09c7143db5a44ef2484..21845a75c1b9aecc597827a8c2af92314666b8d3 100644
--- a/API.txt
+++ b/API.txt
@@ -4573,7 +4573,7 @@ arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primar
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 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: StrEnum('iparepltoposegmentdirection', attribute=True, cli_name='direction', default=u'both', multivalue=False, required=True, values=(u'both', u'left-right', u'right-left', u'none'))
+option: StrEnum('iparepltoposegmentdirection', attribute=True, cli_name='direction', default=u'both', multivalue=False, required=True, values=(u'both', u'left-right', u'right-left'))
 option: Str('iparepltoposegmentleftnode', attribute=True, cli_name='leftnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', required=True)
 option: Str('iparepltoposegmentrightnode', attribute=True, cli_name='rightnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', required=True)
 option: StrEnum('nsds5replicaenabled', attribute=True, cli_name='enabled', multivalue=False, required=False, values=(u'on', u'off'))
@@ -4602,7 +4602,7 @@ arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primar
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='name', maxlength=255, multivalue=False, primary_key=True, query=True, required=False)
-option: StrEnum('iparepltoposegmentdirection', attribute=True, autofill=False, cli_name='direction', default=u'both', multivalue=False, query=True, required=False, values=(u'both', u'left-right', u'right-left', u'none'))
+option: StrEnum('iparepltoposegmentdirection', attribute=True, autofill=False, cli_name='direction', default=u'both', multivalue=False, query=True, required=False, values=(u'both', u'left-right', u'right-left'))
 option: Str('iparepltoposegmentleftnode', attribute=True, autofill=False, cli_name='leftnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', query=True, required=False)
 option: Str('iparepltoposegmentrightnode', attribute=True, autofill=False, cli_name='rightnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', query=True, required=False)
 option: StrEnum('nsds5replicaenabled', attribute=True, autofill=False, cli_name='enabled', multivalue=False, query=True, required=False, values=(u'on', u'off'))
@@ -4620,13 +4620,12 @@ output: ListOfEntries('result', (, ), Gettext('A list
 o

Re: [Freeipa-devel] [PATCHES 00012-0013 v7] Profiles and CA ACLs

2015-06-11 Thread Jan Cholasta


Dne 11.6.2015 v 07:16 Fraser Tweedale napsal(a):

On Wed, Jun 10, 2015 at 03:50:22PM +0200, Martin Basti wrote:

On 10/06/15 13:57, Martin Kosek wrote:

On 06/10/2015 01:50 PM, Jan Cholasta wrote:

Dne 10.6.2015 v 13:44 Martin Basti napsal(a):

On 10/06/15 06:40, Fraser Tweedale wrote:

On Tue, Jun 09, 2015 at 04:37:56PM +0200, Martin Basti wrote:

On 09/06/15 08:58, Fraser Tweedale wrote:

On Mon, Jun 08, 2015 at 08:49:06AM +0200, Martin Kosek wrote:

On 06/08/2015 03:31 AM, Fraser Tweedale wrote:

New patches attached.  Comments inline.

Thanks Fraser!

...

5)
Missing referint plugin configuration for attribute
'ipacaaclmembercertprofile'
Please add it into install/updates/25-referint.update (+ other
member
attributes if missing)


Added this.  There is a comment in 25-referint.update:

  # pres and eq indexes defined in 20-indices.update must be set
  # for all the attributes

Can you explain what is required here?  Is it just to add: I see
things for memberUser and memberHost in indices.ldif but nothing for
memberService.  Do I need to add to indices.ldif:

  dn: cn=memberProfile,cn=index,cn=userRoot,cn=ldbm
database,cn=plugins,cn=config
  changetype: add
  cn: memberProfile
  ObjectClass: top
  ObjectClass: nsIndex
  nsSystemIndex: false
  nsIndexType: eq
  nsIndexType: pres
  nsIndexType: sub

, and similarly for memberCa?  Sorry I do not know much about LDAP
indexing.

AFAIR, yes. BTW, where does the "sub" index come from? It is quite
an expensive
index to use and I now cannot think of memberProfile search where
you would
need a substring...

Thanks,
Martin

Updated patch attached, which adds the indices.  (Also rebased).

There is a commit that seems to indicate that substring index is
needed, so I have included substring indices in this patchset.
Copied Honza in case he wants to comment.

  commit a10521a1dcf69960d6ce0bf5657180b709c297c0
  Author: Jan Cholasta 
  Date:   Tue Jun 25 13:16:40 2013 +

  Add missing substring indices for attributes managed by the
referint plugin.

  The referint plugin does a substring search on these
attributes each time an
  entry is deleted, which causes a noticable slowdown for
large directories if
  the attributes are not indexed.

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

Cheers,
Fraser

ACK

Please send the upgrade patch ASAP :)

--
Martin Basti


Thank you for the ACK \o/

Since the patches have not been pushed, here is an updated patchset
which adds the upgrade behaviour.  There are no changes apart from
the additions to ipaserver/install/server/upgrade.py.

Cheers,
Fraser

ACK

NACK, the new OIDs are not registered.

BTW all new attribute names should have the "ipa" prefix. Also I would prefer
"CertProfile" instead of just "Profile" in certificate profile related names.
Please rename the attributes as follows:

 memberCa -> ipaMemberCa
 memberProfile -> ipaMemberCertProfile
 caCategory -> ipaCaCategory
 profileCategory -> ipaCertProfileCategory

Honza


+1. I see that other attributes from this feature use the ipa prefix already:

dn: cn=schema
attributeTypes: (2.16.840.1.113730.3.8.21.1.1 NAME 'ipaCertProfileStoreIssued'
DESC 'Store certificates issued using this profile' EQUALITY booleanMatch
SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4.2' )
objectClasses: (2.16.840.1.113730.3.8.21.2.1 NAME 'ipaCertProfile' SUP top
STRUCTURAL MUST ( cn $ description $ ipaCertProfileStoreIssued ) X-ORIGIN 'IPA
v4.2' )

Those OIDs should be BTW registered as well, if not already

OID registered.


Thanks!


Patches with updated names attached.
Can you Fraser check if I didn't break anything? :)


Everything LGTM.  Did some simple tessting.  There were conflicts;
rebased patches attached (no other changes).


Pushed to master: 947af1a037609fa42cbfd794301d5a5c4061c81b

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] DNA range distribution to replicas by default

2015-06-11 Thread Petr Spacek

On 9.6.2015 15:06, Simo Sorce wrote:
> On Tue, 2015-06-09 at 10:30 +0200, Petr Spacek wrote:
>> Hello,
>>
>> I would like to discuss
>> https://bugzilla.redhat.com/show_bug.cgi?id=1211366
>> "Error creating a user when jumping from an original server to replica".
>>
>> Currently the DNA ranges are distributed from master to other replicas on
>> first attempt to get a number from particular range.
>>
>> This works well as long as the original master is reachable but fails
>> miserably when the master is not reachable for any reason.
>>
>> It is apparently confusing to users [1][2] because it is counter-intuitive.
>> They have created a replica to be sure that everything will work when the
>> first server is down, right?
>>
>> Remediation is technically simple [3] (just assign a range to the new 
>> replica)
>> but it is confusing to the users, error-prone, and personally I feel that 
>> this
>> is an unnecessary obstacle.
>>
>> It seems to me that the original motivation for this behavior was that the
>> masters were not able to request range back from other replicas when a local
>> range was depleted.
>>
>> This deficiency is tracked as
>> https://bugzilla.redhat.com/show_bug.cgi?id=1029640 and it is slated for fix
>> in 4.2.x time frame.
>>
>> Can we distribute ranges to the replicas during ipa-replica-install when we
>> fix bug 1029640?
> 
> That was not the only reason, another reason is that you do not want to
> distribute and fragment ranges to replicas that will never be used to
> create users. What we should do perhaps, is to automatically give a
> range to CA enabled masters so that at least those servers have a range.
> If all your CAs are unavailable you have major issues anyway.
> 
> Though it is a bit bad to have magic behaviors, maybe we should have a
> "main DNA range holder" role that can be assigned to arbitrary servers
> (maybe the first replica gets it by default), and when done the server
> acquire part of the range if it has none.

This concept sounds good to me!

I would only reverse the default, i.e. distribute ranges by default to all
replicas and let admin to toggle a knob if he feels that his case really needs
to limit range distribution.

> Another option is that a replica can instantiate a whole new range if
> all the range bearing servers are not around, but that also comes with
> its own issues.
> 
> In general I wouldn't want to split by default, because in domains with
> *many* replicas most of them are used for load balancing and will never
> be used to create users, so the range would be wasted.

This should not be an issue when
https://bugzilla.redhat.com/show_bug.cgi?id=1029640 is fixed because replicas
will be able to request range back if the local chunk is depleted.

Is that correct?

Petr^2 Spacek

> Simo.
> 
>>
>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1211366#c0
>> [2] https://www.redhat.com/archives/freeipa-users/2015-May/msg00515.html
>> [3] http://blog-rcritten.rhcloud.com/?p=50

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes



On 06/11/2015 12:19 PM, Petr Vobornik wrote:

On 06/11/2015 10:22 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



Looking at Ludwig's path 12, the patch completely forbids mod of 
ipaReplTopoSegmentDirection? 
that's what I thought we agreed on, so you would have to add a segment 
in the opposite direction an they would be merged to both,
but maybe this is a bit strict. I could allow for 
ipaReplTopoSegmentDirection replace: both
So that upgrade from right-left and left-right to both is not 
allowed?  If so then this patch needs to be updated.

depends a bit on what you prefer and what we can get in for alpha.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 871 webui: make topology suffices UI readonly


On 06/11/2015 10:20 AM, Martin Babinsky wrote:

On 06/10/2015 03:25 PM, Petr Vobornik wrote:

On 06/10/2015 03:24 PM, Petr Vobornik wrote:

Admins should not modify topology suffices. They are created on
install/upgrade.

part of: https://fedorahosted.org/freeipa/ticket/4997


and with patch...




ACK



Pushed to master: ae56ca422d1897569717fa44a5d483b10e490f6a
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes


On 06/11/2015 10:22 AM, Martin Babinsky wrote:

On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK



Looking at Ludwig's path 12, the patch completely forbids mod of 
ipaReplTopoSegmentDirection? So that upgrade from right-left and 
left-right to both is not allowed?  If so then this patch needs to be 
updated.

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 864 add entries required by topology plugin on update


On 06/11/2015 10:01 AM, Martin Babinsky wrote:

On 06/04/2015 04:32 PM, Petr Vobornik wrote:

requires patch 863

These entries were not added on upgrade from old IPA servers and on
replica creation.

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



ACK



pushed to master:
https://fedorahosted.org/freeipa/changeset/99ce650b59dbf9da7dc95f1cade91fcfa55b8375

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 863 move replications managers group to, cn=sysaccounts, cn=etc, $SUFFIX


On 06/11/2015 10:00 AM, Martin Babinsky wrote:

On 06/08/2015 04:54 PM, Petr Vobornik wrote:

On 06/04/2015 04:32 PM, Petr Vobornik wrote:

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




missed one occurrence. Updated patch attached.



ACK



pushed to master:
https://fedorahosted.org/freeipa/changeset/7cf82cf9aac6cc5ecb8d575ce4f141ab2afa85a2

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes


Thanks,
attached a new version with comments and trying to use more meaningful 
function names

On 06/11/2015 10:49 AM, thierry bordaz wrote:

On 06/11/2015 10:40 AM, Ludwig Krispenz wrote:


On 06/11/2015 10:27 AM, thierry bordaz wrote:

On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:

Attached are two patches:
- reject direct modification of segment endpoints and connectivity
- better manage the rdn of a replication agreements represented by 
a segment




Hello Ludwig,

The patches looks good. Two questions:

  * Patch 0012
   if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
slapi_ch_free_string(&topo_agmt->rdn);
topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
}
What is the benefit of replacing a string by the same one ?


if strcasecmp is not 0, they are not the same

Shame on me !


  * Patch 0013
In ipa-topo-pre-mod.
if (ipa_topo_is_entry_managed(pb)){
if(ipa_topo_is_agmt_attr_restricted(pb)) {
errtxt = slapi_ch_smprintf("Entry and attributes are
managed by topology plugin."
   "No direct modifications
allowed.\n");
}
} else if (ipa_topo_check_connect_restrict(pb)) {
errtxt = slapi_ch_smprintf("Modification of connectivity
and segment nodes "
   " is not supported.\n");
}
If we have an external modify of replication agreement (managed
by topology plugin), then it will not call
'ipa_topo_check_connect_restrict'.
And the modify will not be reject if for example it updates
'ipaReplTopoSegmentDirection'.

this is not in an replication agreement. you cannot modify two 
entries in the same mod. The first if catches mos to a repl 
agreement, the second to a segment entry


Ok. Thanks for the explanations. To help reading you may add a comment 
saying the the first test is related to RA and the second to segments.


Other than that the fix is good for me. ACK



  * But I thought that this patch also wants to prevent external
update of some connectivity attribute of the managed entries.

Thanks
thierry







>From 8b131201fb47d4cc04acea8128042cd6cf2efbb0 Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz 
Date: Thu, 11 Jun 2015 11:22:19 +0200
Subject: [PATCH] v2-reject modifications of endpoints and connectivity of a
 segment

---
 daemons/ipa-slapi-plugins/topology/topology_pre.c | 69 ---
 1 file changed, 60 insertions(+), 9 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c b/daemons/ipa-slapi-plugins/topology/topology_pre.c
index 0a0cd65b592e2dc796a179e035598e5f641bb01e..cf935d8d2ad65b3c95f408d44511a8c0449c7288 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_pre.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c
@@ -60,7 +60,7 @@ int ipa_topo_is_entry_managed(Slapi_PBlock *pb)
 
 }
 int
-ipa_topo_is_modattr_restricted(Slapi_PBlock *pb)
+ipa_topo_is_agmt_attr_restricted(Slapi_PBlock *pb)
 {
 LDAPMod **mods;
 int i;
@@ -75,6 +75,24 @@ ipa_topo_is_modattr_restricted(Slapi_PBlock *pb)
 }
 return rc;
 }
+int
+ipa_topo_is_segm_attr_restricted(Slapi_PBlock *pb)
+{
+LDAPMod **mods;
+int i;
+int rc = 0;
+
+slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods);
+for (i = 0; (mods != NULL) && (mods[i] != NULL); i++) {
+if ((0 == strcasecmp(mods[i]->mod_type, "ipaReplTopoSegmentDirection")) ||
+(0 == strcasecmp(mods[i]->mod_type, "ipaReplTopoSegmentLeftNode")) ||
+(0 == strcasecmp(mods[i]->mod_type, "ipaReplTopoSegmentRightNode"))) {
+rc = 1;
+break;
+}
+}
+return rc;
+}
 
 /* connectivity check for topology
  * checks if the nodes of a segment would still be connected after
@@ -266,7 +284,7 @@ ipa_topo_connection_exists(struct node_fanout *fanout, char* from, char *to)
 }
 
 int
-ipa_topo_check_connect_reject(Slapi_PBlock *pb)
+ipa_topo_check_segment_is_valid(Slapi_PBlock *pb)
 {
 int rc = 0;
 Slapi_Entry *add_entry;
@@ -309,7 +327,29 @@ ipa_topo_check_connect_reject(Slapi_PBlock *pb)
 }
 
 int
-ipa_topo_check_disconnect_reject(Slapi_PBlock *pb)
+ipa_topo_check_segment_updates(Slapi_PBlock *pb)
+{
+int rc = 0;
+Slapi_Entry *mod_entry;
+char *pi;
+
+/* we have to check if the operation is triggered by the
+ * topology plugin itself - allow it
+ */
+slapi_pblock_get(pb, SLAPI_PLUGIN_IDENTITY,&pi);
+if (pi && 0 == strcasecmp(pi, ipa_topo_get_plugin_id())) {
+return 0;
+}
+slapi_pblock_get(pb,SLAPI_MODIFY_EXISTING_ENTRY,&mod_entry);
+if (TOPO_SEGMENT_ENTRY == ipa_topo_check_entry_type(mod_entry) &&
+(ipa_topo_is_segm_attr_restricted(pb))) {
+rc = 1;
+}
+return rc;
+}
+
+int
+ipa_topo_check_topology_disconnect(Slapi_PBlock *pb)
 {
 int rc = 1;
 Slapi_Entry *del_entry;
@@ -385,7 +425,7 @@ int ipa_topo_pre_add(Slapi_PBlock *pb)

Re: [Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes



On 06/11/2015 11:06 AM, Oleg Fayans wrote:

Hi Ludwig

Installed it and it did fix the problem with orphaned topology 
segments after replica removal. Thank you very much!
One more question: In which cases the segment removal should be 
allowed via `ipa topologysegment-del` command? Currently, when I try 
to remove a segment, that connects a master and a replica, by issuing 
this command on master, it refuses to do so:
Server is unwilling to perform: Removal of Segment disconnects 
topology.Deletion not allowed.
the deletion of A-B should only be allowed if there exists another path 
connecting A and B, eg A <-->C<-->D<-->B.


On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:

Attached are two patches:
- reject direct modification of segment endpoints and connectivity
- better manage the rdn of a replication agreements represented by a 
segment





--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.




-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes

2015-06-11 Thread Oleg Fayans


Hi Ludwig

Installed it and it did fix the problem with orphaned topology segments 
after replica removal. Thank you very much!
One more question: In which cases the segment removal should be allowed 
via `ipa topologysegment-del` command? Currently, when I try to remove a 
segment, that connects a master and a replica, by issuing this command 
on master, it refuses to do so:
Server is unwilling to perform: Removal of Segment disconnects 
topology.Deletion not allowed.


On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:

Attached are two patches:
- reject direct modification of segment endpoints and connectivity
- better manage the rdn of a replication agreements represented by a 
segment





--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes


On 06/11/2015 10:40 AM, Ludwig Krispenz wrote:


On 06/11/2015 10:27 AM, thierry bordaz wrote:

On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:

Attached are two patches:
- reject direct modification of segment endpoints and connectivity
- better manage the rdn of a replication agreements represented by a 
segment




Hello Ludwig,

The patches looks good. Two questions:

  * Patch 0012
   if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
slapi_ch_free_string(&topo_agmt->rdn);
topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
}
What is the benefit of replacing a string by the same one ?


if strcasecmp is not 0, they are not the same

Shame on me !


  * Patch 0013
In ipa-topo-pre-mod.
if (ipa_topo_is_entry_managed(pb)){
if(ipa_topo_is_agmt_attr_restricted(pb)) {
errtxt = slapi_ch_smprintf("Entry and attributes are
managed by topology plugin."
   "No direct modifications
allowed.\n");
}
} else if (ipa_topo_check_connect_restrict(pb)) {
errtxt = slapi_ch_smprintf("Modification of connectivity
and segment nodes "
   " is not supported.\n");
}
If we have an external modify of replication agreement (managed
by topology plugin), then it will not call
'ipa_topo_check_connect_restrict'.
And the modify will not be reject if for example it updates
'ipaReplTopoSegmentDirection'.

this is not in an replication agreement. you cannot modify two entries 
in the same mod. The first if catches mos to a repl agreement, the 
second to a segment entry


Ok. Thanks for the explanations. To help reading you may add a comment 
saying the the first test is related to RA and the second to segments.


Other than that the fix is good for me. ACK



  * But I thought that this patch also wants to prevent external
update of some connectivity attribute of the managed entries.

Thanks
thierry





-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes



On 06/11/2015 10:27 AM, thierry bordaz wrote:

On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:

Attached are two patches:
- reject direct modification of segment endpoints and connectivity
- better manage the rdn of a replication agreements represented by a 
segment




Hello Ludwig,

The patches looks good. Two questions:

  * Patch 0012
   if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
slapi_ch_free_string(&topo_agmt->rdn);
topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
}
What is the benefit of replacing a string by the same one ?


if strcasecmp is not 0, they are not the same


  * Patch 0013
In ipa-topo-pre-mod.
if (ipa_topo_is_entry_managed(pb)){
if(ipa_topo_is_agmt_attr_restricted(pb)) {
errtxt = slapi_ch_smprintf("Entry and attributes are
managed by topology plugin."
   "No direct modifications
allowed.\n");
}
} else if (ipa_topo_check_connect_restrict(pb)) {
errtxt = slapi_ch_smprintf("Modification of connectivity
and segment nodes "
   " is not supported.\n");
}
If we have an external modify of replication agreement (managed by
topology plugin), then it will not call
'ipa_topo_check_connect_restrict'.
And the modify will not be reject if for example it updates
'ipaReplTopoSegmentDirection'.

this is not in an replication agreement. you cannot modify two entries 
in the same mod. The first if catches mos to a repl agreement, the 
second to a segment entry


  * But I thought that this patch also wants to prevent external
update of some connectivity attribute of the managed entries.

Thanks
thierry



-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes


On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:

Attached are two patches:
- reject direct modification of segment endpoints and connectivity
- better manage the rdn of a replication agreements represented by a 
segment




Hello Ludwig,

The patches looks good. Two questions:

 * Patch 0012
   if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
   slapi_ch_free_string(&topo_agmt->rdn);
topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
}
   What is the benefit of replacing a string by the same one ?
 * Patch 0013
   In ipa-topo-pre-mod.
if (ipa_topo_is_entry_managed(pb)){
if(ipa_topo_is_agmt_attr_restricted(pb)) {
errtxt = slapi_ch_smprintf("Entry and attributes are
   managed by topology plugin."
   "No direct modifications
   allowed.\n");
}
} else if (ipa_topo_check_connect_restrict(pb)) {
errtxt = slapi_ch_smprintf("Modification of connectivity
   and segment nodes "
   " is not supported.\n");
}
   If we have an external modify of replication agreement (managed by
   topology plugin), then it will not call
   'ipa_topo_check_connect_restrict'.
   And the modify will not be reject if for example it updates
   'ipaReplTopoSegmentDirection'.
   But I thought that this patch also wants to prevent external update
   of some connectivity attribute of the managed entries.

Thanks
thierry

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 869 topology: restrict direction changes


On 06/10/2015 03:13 PM, Petr Vobornik wrote:

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 871 webui: make topology suffices UI readonly


On 06/10/2015 03:25 PM, Petr Vobornik wrote:

On 06/10/2015 03:24 PM, Petr Vobornik wrote:

Admins should not modify topology suffices. They are created on
install/upgrade.

part of: https://fedorahosted.org/freeipa/ticket/4997


and with patch...




ACK

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 870 disallow mod of topology segment nodes


On 06/10/2015 03:13 PM, Petr Vobornik wrote:

Mod of segment end will be disallowed in topology plugin.

Reasoning (by Ludwig):  if we want to properly allow mods to change
connectivity and endpoints, then we would need to check if the mod
disconnects the topology, delete existing agreements, check if the new
would be a duplicate and create new agmts. There could be some difficult
scenarios, like having
   A <--> B <--> C <--> D,
if you modify the segment B-C to A-D topology breaks and is then
reconnected.

part of: https://fedorahosted.org/freeipa/ticket/4302



ACK

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 868 rename topologysegment_refresh to topologysegment_reinitialize


On 06/10/2015 02:27 PM, Petr Vobornik wrote:

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



ACK

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 864 add entries required by topology plugin on update


On 06/04/2015 04:32 PM, Petr Vobornik wrote:

requires patch 863

These entries were not added on upgrade from old IPA servers and on
replica creation.

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



ACK

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 863 move replications managers group to, cn=sysaccounts, cn=etc, $SUFFIX


On 06/08/2015 04:54 PM, Petr Vobornik wrote:

On 06/04/2015 04:32 PM, Petr Vobornik wrote:

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




missed one occurrence. Updated patch attached.



ACK

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] with new cert profiles patches ipa-replica-prepare fails after update