Re: [Freeipa-devel] [PATCH] 355 Avoid internal error when user is not Trust admin
On 02/19/2013 10:19 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/24/2013 12:01 PM, Martin Kosek wrote:
When user tries to perform any action requiring communication with
trusted domain, IPA server tries to retrieve a trust secret on his
behalf to be able to establish the connection. This happens for
example during group-add-member command when external user is
being resolved in the AD.
When user is not member of Trust admins group, the retrieval crashes
and reports internal error. Catch this exception and rather report
properly formatted ACIError.
I hit this error after updating to the latest FreeIPA version with the AD
CVE
fixed.
Martin
I filed a ticket to not loose this fix and patch. Attaching an updated patch
with ticket URL in description.
Martin
The patch fixes the problem but the error is untranslated:
member group: AD\Domain Admins: Insufficient access:
Gettext('communication
with trusted domains is allowed for Trusts administrator group members only',
domain='ipa', localedir=None)
rob
I think this is just because this string is not in our ipa.pot file yet (will
be when we do Transifex refresh).
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0035] Use default.conf as flag of IPA client being installed
Hi,
When installing / uninstalling IPA client, the checks that
determine whether IPA client is installed now take the existence
of /etc/ipa/default.conf into consideration.
The client will not uninstall unless either something is backed
up or /etc/ipa/default.conf file does exist.
The client will not install if something is backed up or
default.conf file does exist (unless it's installation on master).
https://fedorahosted.org/freeipa/ticket/3331
Tomas
From 6a81800dedab33881a4c3573efa80cac50c84d40 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 19 Feb 2013 17:59:50 +0100
Subject: [PATCH] Use default.conf as flag of IPA client being installed
When installing / uninstalling IPA client, the checks that
determine whether IPA client is installed now take the existence
of /etc/ipa/default.conf into consideration.
The client will not uninstall unless either something is backed
up or /etc/ipa/default.conf file does exist.
The client will not install if something is backed up or
default.conf file does exist (unless it's installation on master).
https://fedorahosted.org/freeipa/ticket/3331
---
ipa-client/ipa-install/ipa-client-install | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 2d32e28ece72c1058152787058c83ae5b06df64c..bad79ac3e383c6cb049be1522835e52105daefee 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -283,7 +283,9 @@ def delete_ipa_domain():
def uninstall(options, env):
-if not fstore.has_files():
+# Consider IPA not installed if nothing is backed up and default.conf file
+# does not exist.
+if not fstore.has_files() and not os.path.exists('/etc/ipa/default.conf'):
root_logger.error(IPA client is not configured on this system.)
return CLIENT_NOT_CONFIGURED
@@ -2326,7 +2328,11 @@ def main():
if options.uninstall:
return uninstall(options, env)
-if fstore.has_files():
+# Consider client installed if something is backed up or default.conf does
+# exist. Only exception is server installation, as it creates default.conf
+# of its own.
+if fstore.has_files() or \
+ (not options.on_master and os.path.exists('/etc/ipa/default.conf')):
root_logger.error(IPA client is already configured on this system.)
root_logger.info(
If you want to reinstall the IPA client, uninstall it first +
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0024] Make options checks in idrange-add/mod consistent
On 12/11/2012 12:34 PM, Tomas Babej wrote: Hi, **NOTE**: This is to be applied on top of my PATCH 0021 Option checks in idrange-add/mod have been made consistent. Both now enforce the following checks: - dom_sid and secondary_rid_base cannot be used together - rid_base must be used together if dom_rid is set - secondary_rid_base and rid_base must be used together if dom_rid is not set cat Unit test for third check has been added. http://fedorahosted.org/freeipa/ticket/3170 Tomas I think this needs a rebase. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation
On 02/18/2013 01:08 PM, Martin Kosek wrote:
On 02/18/2013 12:47 PM, Sumit Bose wrote:
On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:
On 15.2.2013 15:22, Ana Krivokapic wrote:
Hello,
The .isalpha() check in validate_domain_name() was too strict,
causing some commands like ipa dnsrecord-add to fail.
https://fedorahosted.org/freeipa/ticket/3385
I would add --force option rather than removing whole check, if it's
possible.
Would it be possible to mention RFC in the error message? Something
like _('top level domain label must be alphabetic (RFC 1123 section
2.1)')
?
IMHO it is handy, because it educates users.
The problem is that this check is always done on the last component of
the domain_name even if it is just a sub-domain of the FreeIPA domain,
where e.g. numbers are valid characters.
At the beginning of validate_domain_name() a trailing '.' is stripped
away. iirc the trailing '.' is an indication for a complete, fully
qualified name. Would it work if the presence of the trailing '.' is
saved and the check is only done if there was a '.'?
bye,
Sumit
Sure. Though I am now not 100% sure that some IPA functions do not use this
validator with a fqdn hostname without trailing dot. If not, I am for fixing
this function as Sumit and Petr suggested.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
After some thought, I decided to change the approach.
As pointed out by Sumit, the problem was that the validate_domain_name()
function did not distinguish between fqdn and non-fqdn domains
(subdomains of the IPA domain). The trailing dot is not a clear
indication either, because some IPA functions use this validator with an
fqdn without the trailing dot.
To fix this, I introduced an additional parameter to this function - a
flag which indicates whether the domain name is an fqdn or not. The is
.isalpha() check is then performed only in the case of an fqdn.
I also improved the error message to mention the relevant RFC, as
suggested by Petr.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
From 9859fa0a4a3e48833c48e22c4be518f28f623a60 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Wed, 20 Feb 2013 04:25:32 -0500
Subject: [PATCH] Improve domain name validation
Check for alphabetic only characters is now performed only in the
case of fully qualified domain names. This fixes the bug that caused
some ipa commands (e.g. ipa dnsrecord-add) to fail due to the presence
of numbers in the domain name.
https://fedorahosted.org/freeipa/ticket/3385
---
ipalib/util.py | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/ipalib/util.py b/ipalib/util.py
index 9ed27c84ec8d189507410ba141d8968fc38f674c..5a5af69eac68adc51ee5d55a8c570275f64c6aee 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -235,7 +235,7 @@ def validate_dns_label(dns_label, allow_underscore=False):
'DNS label may not start or end with -') \
% dict(underscore=underscore_err_msg))
-def validate_domain_name(domain_name, allow_underscore=False):
+def validate_domain_name(domain_name, allow_underscore=False, fqdn=True):
if domain_name.endswith('.'):
domain_name = domain_name[:-1]
@@ -244,9 +244,9 @@ def validate_domain_name(domain_name, allow_underscore=False):
# apply DNS name validator to every name part
map(lambda label:validate_dns_label(label,allow_underscore), domain_name)
-if not domain_name[-1].isalpha():
+if fqdn and not domain_name[-1].isalpha():
# see RFC 1123
-raise ValueError(_('top level domain label must be alphabetic'))
+raise ValueError(_('top level domain label must be alphabetic (RFC 1123 section 2.1)'))
def validate_zonemgr(zonemgr):
See RFC 1033, 1035
@@ -308,9 +308,9 @@ def validate_hostname(hostname, check_fqdn=True, allow_underscore=False):
if '.' not in hostname:
if check_fqdn:
raise ValueError(_('not fully qualified'))
-validate_dns_label(hostname,allow_underscore)
+validate_dns_label(hostname, allow_underscore)
else:
-validate_domain_name(hostname,allow_underscore)
+validate_domain_name(hostname, allow_underscore, check_fqdn)
def normalize_sshpubkey(value):
return SSHPublicKey(value).openssh()
--
1.8.1.2
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 12/21/2012 12:15 PM, Tomas Babej wrote:
Hi,
Sending updated and rebased versions of patches 0024 and 0025.
Tomas
Sending rebased version, these got quite rotten.
Tomas
From f21b135d546678544ccf05efd587b46bba88e07a Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Fri, 21 Dec 2012 05:34:37 -0500
Subject: [PATCH] Make options checks in idrange-add/mod consistent
Both now enforce the following checks:
- dom_sid and secondary_rid_base cannot be used together
- rid_base must be used together if dom_rid is set
- secondary_rid_base and rid_base must be used together
if dom_rid is not set
Unit test for third check has been added.
http://fedorahosted.org/freeipa/ticket/3170
---
ipalib/plugins/idrange.py | 56 +-
tests/test_xmlrpc/test_range_plugin.py | 46 +++-
2 files changed, 87 insertions(+), 15 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 445473394223f3f7c69d1e32be71d7c6944e6252..97b9d2489b51cf1fc62f49e0d3faf9674851d68a 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -402,11 +402,13 @@ class idrange_add(LDAPCreate):
entry_attrs['objectclass'].append('ipatrustedaddomainrange')
else:
+ # secondary base rid must be set if and only if base rid is set
if is_set('ipasecondarybaserid') != is_set('ipabaserid'):
raise errors.ValidationError(name='ID Range setup',
error=_('Options secondary-rid-base and rid-base must '
'be used together'))
+# and they must not overlap
if is_set('ipabaserid') and is_set('ipasecondarybaserid'):
if self.obj.are_rid_ranges_overlapping(
entry_attrs['ipabaserid'],
@@ -483,7 +485,14 @@ class idrange_mod(LDAPUpdate):
assert isinstance(dn, DN)
attrs_list.append('objectclass')
+try:
+(old_dn, old_attrs) = ldap.get_entry(dn, ['*'])
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))
# This needs to stay in options since there is no
# ipanttrusteddomainname attribute in LDAP
@@ -496,6 +505,8 @@ class idrange_mod(LDAPUpdate):
sid = self.obj.get_trusted_domain_sid_from_name(
options['ipanttrusteddomainname'])
+# we translate the name into sid so further validation can rely
+# on ipanttrusteddomainsid attribute only
if sid is not None:
entry_attrs['ipanttrusteddomainsid'] = sid
else:
@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
'not be found. Please specify the SID directly '
'using dom-sid option.'))
-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base cannot '
+'be used together'))
-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))
+
+if is_set('ipanttrusteddomainsid'):
+# Validate SID as the one of trusted domains
+# perform this check only if the attribute was changed
+self.obj.validate_trusted_domain_sid(
+entry_attrs['ipanttrusteddomainsid'])
+else:
+# secondary base rid must be set if and only if base rid is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base must '
+'be used together'))
# ensure that primary and secondary rid ranges do not
Re: [Freeipa-devel] [PATCH 0027] Add checks for SELinux in install scripts
On Tue 19 Feb 2013 08:37:26 PM CET, Rob Crittenden wrote: Tomas Babej wrote: On 02/04/2013 04:21 PM, Rob Crittenden wrote: Tomas Babej wrote: On 01/30/2013 05:12 PM, Tomas Babej wrote: Hi, The checks make sure that SELinux is: - installed and enabled (on server install) - installed and enabled OR not installed (on client install) Please note that client installs with SELinux not installed are allowed since freeipa-client package has no dependency on SELinux. (any objections to this approach?) The (unsupported) option --allow-no-selinux has been added. It can used to bypass the checks. Parts of platform-dependant code were refactored to use newly added is_selinux_enabled() function. https://fedorahosted.org/freeipa/ticket/3359 Tomas I forgot to edit the man pages. Thanks Rob! Updated patch attached. Tomas After a bit of off-line discussion I don't think we're quite ready yet to require SELinux by default on client installations (even with a flag to work around it). The feeling is this would be disruptive to existing automation. Can you still do the check but not enforce it, simply display a big warning if SELinux is disabled? rob Sure, here is the updated patch. I edited the commit message, RFE description and man pages according to the new behaviour. Tomas The patch looks good, I'm just wondering about one thing. The default value for is_selinux_enabled() is True in ipapython/services.py.in. So this means that any non-Red Hat/non-Fedora system, by default, is going to assume that SELinux is enabled. My hesitation has to when we call check_selinux_status(). It may incorrectly error out. I suspect that the user would have to work around this using --allow-selinux-disabled but this wouldn't make a lot of sense since they actually do have SELinux disabled. Yes, you're right. And the error message would not even be helpful since it would tell the user to install policycoreutils package. This would be the case both with server and client installs when selinux would not be installed at all. What do you think? rob Well we have 2 options as I see it: 1.) We can either return None as default, and add checks to check_selinux_status, restore_context and install scripts that would ensure that we behave properly when is_selinux_enabled() is not implemented. 2.) We can remove the default value, since it would cause forementioned crash and add comment that this function needs to be implemented properly in every platform file. I'm probably for option 2, there's no need to clutter the code with checks that compensate for improper platform file implementations. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 355 Avoid internal error when user is not Trust admin
On 02/20/2013 09:15 AM, Martin Kosek wrote:
On 02/19/2013 10:19 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/24/2013 12:01 PM, Martin Kosek wrote:
When user tries to perform any action requiring communication with
trusted domain, IPA server tries to retrieve a trust secret on his
behalf to be able to establish the connection. This happens for
example during group-add-member command when external user is
being resolved in the AD.
When user is not member of Trust admins group, the retrieval crashes
and reports internal error. Catch this exception and rather report
properly formatted ACIError.
I hit this error after updating to the latest FreeIPA version with the AD CVE
fixed.
Martin
I filed a ticket to not loose this fix and patch. Attaching an updated patch
with ticket URL in description.
Martin
The patch fixes the problem but the error is untranslated:
member group: AD\Domain Admins: Insufficient access: Gettext('communication
with trusted domains is allowed for Trusts administrator group members only',
domain='ipa', localedir=None)
rob
I think this is just because this string is not in our ipa.pot file yet (will
be when we do Transifex refresh).
Martin
I don't have AD so I can't investigate, but this problem is usually due
to the error being converted to string instead of using the strerror
attribute.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0031-0032] Improve HBAC rule handling in selinuxusermap-add/mod/find
On 02/19/2013 10:33 PM, Rob Crittenden wrote:
Tomas Babej wrote:
On 02/06/2013 07:57 PM, Rob Crittenden wrote:
Tomas Babej wrote:
Hi,
this pair of patches improves HBAC rule handling in selinuxusermap
commands.
Patch 0031 deals with:
https://fedorahosted.org/freeipa/ticket/3349
Patch 0032 takes care of:
https://fedorahosted.org/freeipa/ticket/3348
and is to be applied on top of Patch 0031.
See commit messages for detailed info.
Tomas
ACK for patch 0032.
For patch 0031 we can't change the data type of an existing attribute.
It will break backwards compatibility. Can you test with an older
client to see if it cares (it may not care about the name of the
type). If older clients will work then this is probably ok.
I gather that seealso detected as a DN attribute and converted into a
DN class and this is blowing up the Str validator?
Yes, that was exactly the case.
rob
I added a workaround for older client versions, tested it with
freeipa-client/admintools 2.2, works as expeceted.
However, this only should be issue if there is older admintools package
on the client than on the server.
Outline is such as follows: I added a new flag for DNParam seelalso
attribute, called 'allow_malformed' that allows any string to be passed
to DNParam. Its value gets wrapped in 'malformed=yes,value=value'.
This allows to parse out the string in selinuxusermap-add/mod
pre_callback out of the DN and search for the rule with such name so
that it's DN gets in LDAP instead.
Updated patch attached.
Tomas
I like where you're going with this, just a couple of comments:
1. Should we come up with a more universal name for allow_malformed?
Is this something that we should allow at a higher level? I was
thinking allow_raw, or allow_non_dn, or something like that.
To me, allow_non_dn sounds is just as specific as allow_malformed,
they both refer to DN specifically.
I'd go with allow_raw, if need for such pattern may eventually arise for
other parameter classes than DNParam.
What do you mean by higher level, turning this hack into a feature
Param class? I don't see how this would work, each parameter
class that implements its own type validation as DNParam needs
to override _convert_scalar(). And in every such class we would need
to wrap our raw value so that it is represented in the type of this
parameter,
as we do with DN(('malformed','yes'),('value',value)) now.
Maybe we could skip type validation in _convert_scalar default
implementation or catch the error raised somehow, and let the type be
invalid, but I'm not aware of the consenquences. I would need to
investigate.
Wouldn't it cause failure deeper in the framework?
Or did you by higher level mean simply picking a more general name for the
flag so it can be reused in other parameter classes with the same name?
2. I think that if a bad dn is passed in as a Str the conversion into
a DN won't be handled:
+if 'allow_malformed' in self.flags:
+dn = DN(('malformed','yes'),('value',value))
Should this be wrapped in a try/except to raise a ConversionError if
it fails?
Yes, thanks for that catch.
rob
Tomas
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0022, 0115-0116 Make Sudo commands case-sensitive
On 12/17/2012 04:08 PM, Petr Viktorin wrote: https://fedorahosted.org/freeipa/ticket/2482 The first two patches are rebased from what I sent back in March; the third fixes ACIs using targetfilter. I finally got to your patches. Generally, everything worked like charm, I have just few minor comments: 0022: - patch needs a rebase - patch description is confusing, we are talking about RDN sudocmd and not CN 0115: I would optimize the LDAP calls a little: 1) Use sudorule base DN as a base for the LDAP search 2) Do not call LDAP search twice, but just once and then collect the result. Now you use 2 LDAP searches with following filters: ((objectClass=ipasudorule)(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)) ((objectClass=ipasudorule)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)) We can do just one LDAP search with this filter: ((objectClass=ipasudorule)(|(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))) 0116: - patch description needs amending: s/CN/SUDOCMD/ Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On Wed, 20 Feb 2013, Tomas Babej wrote:
On 12/21/2012 12:15 PM, Tomas Babej wrote:
Hi,
Sending updated and rebased versions of patches 0024 and 0025.
Tomas
Sending rebased version, these got quite rotten.
Thanks for updating them.
@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
'not be found. Please specify the SID directly '
'using dom-sid option.'))
-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base cannot '
+'be used together'))
Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it here too.
-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))
Same here.
+# secondary base rid must be set if and only if base rid is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base must '
+'be used together'))
Same here.
+dict(
+desc='Try to modify ID range %r so it has only primary rid range
set' % (testrange8),
+command=('idrange_mod', [testrange8],
+ dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options secondary_rid_base and
rid_base must be used together'),
+),
And synchronize error message here too.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add delegation info to MS-PAC
On 02/07/2013 10:42 PM, Simo Sorce wrote: This information is not strictly required but is part of the MS-PAC specification and I had some time to kill on the plane on my last trip back. I tested it briefly with cross-realm trusts and it seem to work fine. Neither IPA nor AD2012 complained when looking at PACs, do far. Simo. Can we make a ticket for this one, so that it does not get lost in the swarm of freeipa-devel patches? (and so that we can triage it properly) Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:
On Wed, 20 Feb 2013, Tomas Babej wrote:
On 12/21/2012 12:15 PM, Tomas Babej wrote:
Hi,
Sending updated and rebased versions of patches 0024 and 0025.
Tomas
Sending rebased version, these got quite rotten.
Thanks for updating them.
@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
'not be found. Please specify the SID
directly '
'using dom-sid option.'))
-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base
cannot '
+'be used together'))
Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it
here too.
-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))
Same here.
+# secondary base rid must be set if and only if base rid
is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base
must '
+'be used together'))
Same here.
+dict(
+desc='Try to modify ID range %r so it has only primary
rid range set' % (testrange8),
+command=('idrange_mod', [testrange8],
+ dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options
secondary_rid_base and rid_base must be used together'),
+),
And synchronize error message here too.
Thanks!
Sending the updated patch 0024.
Tomas
From d36c9aa5fb7dd044e212de4cd17adbc044d14777 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Fri, 21 Dec 2012 05:34:37 -0500
Subject: [PATCH] Make options checks in idrange-add/mod consistent
Both now enforce the following checks:
- dom_sid and secondary_rid_base cannot be used together
- rid_base must be used together if dom_rid is set
- secondary_rid_base and rid_base must be used together
if dom_rid is not set
Unit test for third check has been added.
http://fedorahosted.org/freeipa/ticket/3170
---
ipalib/plugins/idrange.py | 56 +-
tests/test_xmlrpc/test_range_plugin.py | 46 +++-
2 files changed, 87 insertions(+), 15 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 445473394223f3f7c69d1e32be71d7c6944e6252..6a4c7a5b5de7971abf374572cd3bdadfaae93651 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -402,11 +402,13 @@ class idrange_add(LDAPCreate):
entry_attrs['objectclass'].append('ipatrustedaddomainrange')
else:
+ # secondary base rid must be set if and only if base rid is set
if is_set('ipasecondarybaserid') != is_set('ipabaserid'):
raise errors.ValidationError(name='ID Range setup',
error=_('Options secondary-rid-base and rid-base must '
'be used together'))
+# and they must not overlap
if is_set('ipabaserid') and is_set('ipasecondarybaserid'):
if self.obj.are_rid_ranges_overlapping(
entry_attrs['ipabaserid'],
@@ -483,7 +485,14 @@ class idrange_mod(LDAPUpdate):
assert isinstance(dn, DN)
attrs_list.append('objectclass')
+try:
+(old_dn, old_attrs) = ldap.get_entry(dn, ['*'])
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))
# This needs to stay in options since there is no
# ipanttrusteddomainname attribute in LDAP
@@ -496,6
Re: [Freeipa-devel] [PATCH] Add delegation info to MS-PAC
On Wed, 2013-02-20 at 15:12 +0100, Martin Kosek wrote: On 02/07/2013 10:42 PM, Simo Sorce wrote: This information is not strictly required but is part of the MS-PAC specification and I had some time to kill on the plane on my last trip back. I tested it briefly with cross-realm trusts and it seem to work fine. Neither IPA nor AD2012 complained when looking at PACs, do far. Simo. Can we make a ticket for this one, so that it does not get lost in the swarm of freeipa-devel patches? (and so that we can triage it properly) https://fedorahosted.org/freeipa/ticket/3442 Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Backup and Restore design
Simo Sorce wrote: On Tue, 2013-02-19 at 22:43 -0500, Rob Crittenden wrote: I've looked into some basic backup and restore procedures for IPA. My findings are here: http://freeipa.org/page/V3/Backup_and_Restore Great summary! For the catastrofic failure scenario, should we mention how to put back a full failed and restored machine online ? I am thinking the restored server may be behind (even if only by a few entries) in the replication, so the CSNs in the other replicas will not match. I guess we should mention a full resync may be needed ? Or is there a way to bring back CSNs on replicas ? Good questions. It depends on how long the machine was down and how many changes have happened. It is possible that one would want to do a full re-init. I'll add that to the design. In the 'Returning to a good state' case, can we consider some split brain approach, were we sever replication and rebuild one server at a time ? Perhaps using a firewall, but then you run the risk of each of those servers accepting changes during the rebuild and you could end up with a lot of collisions, which sort of goes against the point of restoring to a known good state. The changelog is the key here. I'll have to ponder this one a bit, I'm a bit conflicted on the right approach. Maybe we can think of a way to 'mark' all server as 'bad' so that on restore the replication agreements do not need to be changed but changes from 'bad' servers will not be accepted ? I guess this crosses also a request by someone to be able to 'pause' replication, would use the same mechanism I guess. AFAIK there is an option to pause replication now (at least in 1.3). What you can't do is drop the changelog AFAIK. That is the real problem. If you want to restore to a known state you need to drop all the changelog entries since that time. I'll check with the 389-ds team to see if that is possible. Since we know the time of the backup, we might be able to drop newer entries than that. Full system backup: in the first part it is said the process is offline, but in the 'LDAP' section you say ldapi is used, but that would mean DS is running ? Also are we sure we can get all data we need via ldapi ? Are we going to miss any operational attribute ? The full backup is offline because it is just using tar. This is sort of a brute-force backup, copying bits from A to B. The data backup is online and creates a task in 389-ds to write the data and changelog to a file. It should write everything completely. We don't do an ldapsearch. I chose to not back up in ldif because this would back up just the data and not the changelog. The other advantage is that the db2bak format includes the indexes and ldif restore would require a rebuild. For restore are we sure we can reload data w/o alterations ? What about plugins ? will we have to disable all plugins during a restore ? Yes, it should be fine. I'm hoping that the version will help us with this, to prevent someone from restoring an ancient backup on a new system, for example (or the reverse). For the open questions. Size of backup: I think we should make it easy to configure the system to use a custom directory to dump the backups. This way admins can make sure it is on a different partition/disk or even over NFS and that the backup will not fill up the disk on which DS is running. That's a good idea. I'll have to think about where we would configure that. Perhaps as an optional argument to the backup command. We should definitely allow to encrypt backup files, a gpg public key would be sufficient. Ok. I wasn't sure if there would be any corruption concerns. For replica cases, maybe we can create a command that dumps the changelog from a good replica and then allow us to reply all changes that are missing from the backup to bring the server up to the last minute ? This would happen when we went online anyway though, at least for the entries currently in the changelog. I guess this would have the advantage of doing it in bulk and not over a (potentially) slow link. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0022, 0115-0116 Make Sudo commands case-sensitive
On 02/20/2013 12:46 PM, Martin Kosek wrote:
On 12/17/2012 04:08 PM, Petr Viktorin wrote:
https://fedorahosted.org/freeipa/ticket/2482
The first two patches are rebased from what I sent back in March; the third
fixes ACIs using targetfilter.
I finally got to your patches. Generally, everything worked like charm, I have
just few minor comments:
0022:
- patch needs a rebase
- patch description is confusing, we are talking about RDN sudocmd and not
CN
0115:
I would optimize the LDAP calls a little:
1) Use sudorule base DN as a base for the LDAP search
2) Do not call LDAP search twice, but just once and then collect the result.
Now you use 2 LDAP searches with following filters:
((objectClass=ipasudorule)(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))
((objectClass=ipasudorule)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))
We can do just one LDAP search with this filter:
((objectClass=ipasudorule)(|(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)))
0116:
- patch description needs amending: s/CN/SUDOCMD/
Martin
Thanks. Attaching updated patches.
--
Petr³
From 87485f70aca0932cfb53a63c91842f762602e9b2 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 8 Mar 2012 07:55:00 -0500
Subject: [PATCH] Use ipauniqueid for the RDN of sudo commands
Since sudo commands are case-sensitive, we can't use 'sudocmd'
as the RDN.
Tests for case-sensitive behavior included
https://fedorahosted.org/freeipa/ticket/2482
---
ipalib/plugins/sudocmd.py |1 +
tests/test_xmlrpc/test_sudocmd_plugin.py | 82 +---
tests/test_xmlrpc/test_sudocmdgroup_plugin.py | 85 +
tests/test_xmlrpc/xmlrpc_test.py | 13 +++-
4 files changed, 153 insertions(+), 28 deletions(-)
diff --git a/ipalib/plugins/sudocmd.py b/ipalib/plugins/sudocmd.py
index 42068edea3c51804be9ee5919934462afbee578f..f27a58cadd6e6abc16611621387f26125737bf78 100644
--- a/ipalib/plugins/sudocmd.py
+++ b/ipalib/plugins/sudocmd.py
@@ -62,6 +62,7 @@ class sudocmd(LDAPObject):
'memberof': ['sudocmdgroup'],
}
uuid_attribute = 'ipauniqueid'
+rdn_attribute = 'ipauniqueid'
label = _('Sudo Commands')
label_singular = _('Sudo Command')
diff --git a/tests/test_xmlrpc/test_sudocmd_plugin.py b/tests/test_xmlrpc/test_sudocmd_plugin.py
index 75b6bbccbb47cc03f4408b791c687f3880bb934a..0ea8e10b8144f1b16527f2786be24de75eb6 100644
--- a/tests/test_xmlrpc/test_sudocmd_plugin.py
+++ b/tests/test_xmlrpc/test_sudocmd_plugin.py
@@ -21,18 +21,20 @@
Test the `ipalib/plugins/sudocmd.py` module.
-from ipalib import api, errors
-from tests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid
+from ipalib import errors
+from tests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_sudocmddn,
+fuzzy_uuid)
from tests.test_xmlrpc import objectclasses
-from ipapython.dn import DN
sudocmd1 = u'/usr/bin/sudotestcmd1'
+sudocmd1_camelcase = u'/usr/bin/sudoTestCmd1'
class test_sudocmd(Declarative):
cleanup_commands = [
('sudocmd_del', [sudocmd1], {}),
+('sudocmd_del', [sudocmd1_camelcase], {}),
]
tests = [
@@ -72,16 +74,35 @@ class test_sudocmd(Declarative):
value=sudocmd1,
summary=u'Added Sudo Command %s' % sudocmd1,
result=dict(
-dn=DN(('sudocmd',sudocmd1),('cn','sudocmds'),('cn','sudo'),
- api.env.basedn),
+dn=fuzzy_sudocmddn,
sudocmd=[sudocmd1],
description=[u'Test sudo command 1'],
objectclass=objectclasses.sudocmd,
ipauniqueid=[fuzzy_uuid],
),
),
),
+dict(
+desc='Create %r' % sudocmd1_camelcase,
+command=('sudocmd_add', [sudocmd1_camelcase],
+dict(
+description=u'Test sudo command 2',
+),
+),
+expected=dict(
+value=sudocmd1_camelcase,
+summary=u'Added Sudo Command %s' % sudocmd1_camelcase,
+result=dict(
+dn=fuzzy_sudocmddn,
+sudocmd=[sudocmd1_camelcase],
+description=[u'Test sudo command 2'],
+objectclass=objectclasses.sudocmd,
+ipauniqueid=[fuzzy_uuid],
+),
+),
+),
+
dict(
desc='Try to create duplicate %r' % sudocmd1,
@@ -94,16 +115,26 @@ class test_sudocmd(Declarative):
u'name %s already exists' % sudocmd1),
),
+dict(
+desc='Try to create duplicate %r' % sudocmd1_camelcase,
+command=('sudocmd_add',
Re: [Freeipa-devel] [PATCHES] 0104-0106 Provide means of displaying warning and informational messages on clients
On 02/19/2013 12:15 PM, Petr Viktorin wrote:
On 02/13/2013 11:18 AM, Petr Viktorin wrote:
On 01/29/2013 05:06 PM, Petr Viktorin wrote:
On 01/04/2013 07:20 PM, Petr Viktorin wrote:
On 12/14/2012 09:04 AM, Jan Cholasta wrote:
On 13.12.2012 18:09, Petr Viktorin wrote:
On 12/13/2012 04:43 PM, Martin Kosek wrote:
On 12/13/2012 10:59 AM, Petr Viktorin wrote:
It's time to give this to another set of eyes :)
Design document: http://freeipa.org/page/V3/Messages
Ticket: https://fedorahosted.org/freeipa/ticket/2732
More info is in commit messages.
Because of https://fedorahosted.org/freeipa/ticket/3294, I needed to
change the
design document: when the client doesn't send the API version, it is
assumed
it's at a version before capabilities were introduced (i.e. 2.47).
The client still gets a warning if the version is missing. Except
for
those
commands where IPA didn't send a version -- ping, cert-show, etc. --
the
warning wouldn't pass validation on old clients. (I'm assuming that
our client
is so far the only one that validates so strictly.)
I did a basic test of this patch and also quickly read through the
patches and
besides nitpicks (like unused inspect module in
tests/test_ipalib/test_messages.py in patch 0105) I did not find any
obvious
errors in the Python code.
Noted, will fix in future versions of the patch.
However, this patch breaks WebUI badly, I did not even get to a
log in
screen.
Cooperation with Petr Vobornik will be needed. In my case, I got
blank
screen
and Javascript error:
TypeError: IPA.messages.dialogs is undefined
https://vm-037.idm.lab.bos.redhat.com/ipa/ui/ipa.js
Line 1460
I assume this is related to the Internal Error that was returned in
the JSON call
{
error: null,
id: null,
principal: ad...@idm.lab.bos.redhat.com,
result: {
count: 5,
results: [
{
error: an internal error has occurred,
error_code: 903,
error_name: InternalError
},
{
...
This can be reproduced with:
# curl -v -H Content-Type:application/json -H
referer:https://`hostname`/ipa; -H Accept:applicaton/json
--negotiate -u :
--cacert /etc/ipa/ca.crt -d
'{method:i18n_messages,params:[[],{}],id:0}' -X POST
https://`hostname`/ipa/json
Good catch! The i18n_messages plugin already defines a messages
output. When I renamed this from warnings to messages I forgot to
check for clashes.
Since i18n_messages is an internal command only used by the Web UI, we
can rename its output to texts without breaking compatibility.
I'm attaching a preliminary fix (for both backend and UI), but
hopefully
it won't be necessary, see below.
I am also not sure I like the requirement of a specific version
option
to be
always passed. I would prefer that missing version option would mean
I use the
most recent version of API instead - it would make the custom
JSONRPC/XMLRPC
calls easier to use.
But since the version option was not being sent for some commands, we
may not
have a choice anyway if we do not want to break old clients in
case we
add some
capabilities to these commands.
I see three other options, all worse:
- Do not use capabilities for the affected commands, meaning no new
functionality can be added to them (and by extension, no new
functionality common to all commands can be added).
- Treat a missing version as the current version
- Break backwards compatibility
And one possibly better (thanks to Petr¹ and Martin for opening my
eyes
off-list!):
- Deprecate XML-RPC. All XML-RPC requests would be pinned to current
version (2.47). Capabilities/messages would only apply to JSON-RPC.
This would also allow us to solve the above name-clashing problem
elegantly. Here is a reminder of what a JSON response looks like:
{
error: null,
id: 0,
principal: ad...@idm.lab.bos.redhat.com,
result: {
summary: IPA server version 3.1.0GIT2e4bd02. API version
2.47
},
version: 3.1.0GIT2e4bd02
}
A XML-RPC response only contains the result part of that.
So with JSON, we can put the messages in the top level, which is much
better design.
Custom info in the top level seems to be a violation of the JSON-RPC
spec. I'd rather not do more of those, so I'm withdrawing this idea.
XML-RPC sucks in other ways too. We already have a workaround for its
inability to attach extra info to errors (commit
88262a75ffe7a25640333dcc4da2100830cae821, Add instructions support to
PublicError).
I've opened a RFC here: https://fedorahosted.org/freeipa/ticket/3299.
+1, XML-RPC sucks. This should have been done a long time ago.
Honza
Here are new patches.
XML-RPC requests with missing version are assumed to be old (the version
before capabilities are introduced, 2.47). This takes care of backcompat
with clients with bug 3294.
JSON-RPC requests with missing version are assumed to be
Re: [Freeipa-devel] Backup and Restore design
On 02/20/2013 08:38 AM, Rob Crittenden wrote: Simo Sorce wrote: On Tue, 2013-02-19 at 22:43 -0500, Rob Crittenden wrote: I've looked into some basic backup and restore procedures for IPA. My findings are here: http://freeipa.org/page/V3/Backup_and_Restore Great summary! For the catastrofic failure scenario, should we mention how to put back a full failed and restored machine online ? I am thinking the restored server may be behind (even if only by a few entries) in the replication, so the CSNs in the other replicas will not match. I guess we should mention a full resync may be needed ? Or is there a way to bring back CSNs on replicas ? Good questions. It depends on how long the machine was down and how many changes have happened. It is possible that one would want to do a full re-init. I'll add that to the design. The replication protocol will detect if a replica is too out of date to bring up to date with an incremental update, and requires a re-init. In the 'Returning to a good state' case, can we consider some split brain approach, were we sever replication and rebuild one server at a time ? Perhaps using a firewall, but then you run the risk of each of those servers accepting changes during the rebuild and you could end up with a lot of collisions, which sort of goes against the point of restoring to a known good state. The changelog is the key here. I'll have to ponder this one a bit, I'm a bit conflicted on the right approach. Maybe we can think of a way to 'mark' all server as 'bad' so that on restore the replication agreements do not need to be changed but changes from 'bad' servers will not be accepted ? I guess this crosses also a request by someone to be able to 'pause' replication, would use the same mechanism I guess. AFAIK there is an option to pause replication now (at least in 1.3). What you can't do is drop the changelog AFAIK. That is the real problem. If you want to restore to a known state you need to drop all the changelog entries since that time. I'll check with the 389-ds team to see if that is possible. Since we know the time of the backup, we might be able to drop newer entries than that. Not sure what you mean - what exactly do you want to do with the changelog? Full system backup: in the first part it is said the process is offline, but in the 'LDAP' section you say ldapi is used, but that would mean DS is running ? Also are we sure we can get all data we need via ldapi ? Are we going to miss any operational attribute ? The full backup is offline because it is just using tar. This is sort of a brute-force backup, copying bits from A to B. The data backup is online and creates a task in 389-ds to write the data and changelog to a file. It should write everything completely. We don't do an ldapsearch. I chose to not back up in ldif because this would back up just the data and not the changelog. The other advantage is that the db2bak format includes the indexes and ldif restore would require a rebuild. It is good to have a long term backup in LDIF format - no matter what happens to the database, if you have an LDIF backup, you can always recreate your data. So it's good to have both - db2bak format for shorter term/frequent backups, and LDIF for longer term/infrequent backups. For restore are we sure we can reload data w/o alterations ? What about plugins ? will we have to disable all plugins during a restore ? Yes, it should be fine. I'm hoping that the version will help us with this, to prevent someone from restoring an ancient backup on a new system, for example (or the reverse). For the open questions. Size of backup: I think we should make it easy to configure the system to use a custom directory to dump the backups. This way admins can make sure it is on a different partition/disk or even over NFS and that the backup will not fill up the disk on which DS is running. That's a good idea. I'll have to think about where we would configure that. Perhaps as an optional argument to the backup command. You'll have to figure out a way around selinux, or add some sort of selinux magic that allows db2bak to write there. We should definitely allow to encrypt backup files, a gpg public key would be sufficient. Ok. I wasn't sure if there would be any corruption concerns. For replica cases, maybe we can create a command that dumps the changelog from a good replica and then allow us to reply all changes that are missing from the backup to bring the server up to the last minute ? This would happen when we went online anyway though, at least for the entries currently in the changelog. I guess this would have the advantage of doing it in bulk and not over a (potentially) slow link. That should happen automatically with the replication protocol - it will attempt to bring older replicas up-to-date or, if they are too far out of date, will complain that they need a re-init. rob
Re: [Freeipa-devel] [PATCHES] 0177-0179 Add missing dict methods to CIDict
On 02/19/2013 01:51 PM, Jan Cholasta wrote:
Hi,
On 5.2.2013 18:02, Petr Viktorin wrote:
CIDict, our case-insensitive dictionary, inherits from dict but did not
reimplement the full dict interface. Calling the missing methods
silently invoked case-sensitive behavior. Our code seems to avoid that,
but it's a bit of a minefield for new development.
Patch 119 adds the missing dict methods (except
view{items,keys,values}(), which now raise errors), and adds tests.
Can you please also add the (obj, **kwargs) and (**kwargs) variants of
__init__ and update?
Added, thanks for the catch.
Patches 117-118 modernize the testsuite a bit (these have been sitting
in my queue for a while, I think now is a good time to submit them):
The first one moves some old tests from the main code tree to tests/.
(The adtrust_install test wasn't run before, this move makes nose notice
it).
The second converts CIDict's unittest-based suite to nose.
Honza
--
Petr³
From da16447f327302227db6d0e663ccdb14066856e9 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 5 Feb 2013 10:24:46 -0500
Subject: [PATCH] Add missing dict methods to CIDict
Make the CIDict interface match standard dict (except view* methods).
Add __contains__, __iter__, clear.
Add keyword and iterable support for __init__, update.
Also add values() and itervalues(). Previously the dict versions were
used; the new ones guarantee that the order matches keys().
Mark view* methods as not implemented.
Document that CIDict.copy() returns a plain dict.
Test the above additions, and fromkeys() which worked but wasn't tested.
---
ipapython/ipautil.py | 66 +-
tests/test_ipapython/test_ipautil.py | 64
2 files changed, 113 insertions(+), 17 deletions(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index c0ac3a1f76397e79672f1b99c2d46463d8e0af3e..81e85dd25dd73628569c285ad7c67cbd16daf065 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -439,10 +439,13 @@ class CIDict(dict):
If you extend UserDict, isinstance(foo, dict) returns false.
-def __init__(self, default=None):
+def __init__(self, default=None, **kwargs):
super(CIDict, self).__init__()
-self._keys = {}
-self.update(default or {})
+self._keys = {} # mapping of lowercased keys to proper case
+if default:
+self.update(default)
+if kwargs:
+self.update(kwargs)
def __getitem__(self, key):
return super(CIDict, self).__getitem__(key.lower())
@@ -455,11 +458,22 @@ class CIDict(dict):
def __delitem__(self, key):
lower_key = key.lower()
del self._keys[lower_key]
-return super(CIDict, self).__delitem__(key.lower())
+return super(CIDict, self).__delitem__(lower_key)
-def update(self, dict):
-for key in dict.keys():
-self[key] = dict[key]
+def update(self, new=None, **kwargs):
+if new:
+try:
+keys = new.keys
+except AttributeError:
+self.update(dict(new))
+else:
+for key in keys():
+self[key] = new[key]
+for key, value in kwargs.iteritems():
+self[key] = value
+
+def __contains__(self, key):
+return super(CIDict, self).__contains__(key.lower())
def has_key(self, key):
return super(CIDict, self).has_key(key.lower())
@@ -470,26 +484,31 @@ class CIDict(dict):
except KeyError:
return failobj
+def __iter__(self):
+return self._keys.itervalues()
+
def keys(self):
-return self._keys.values()
+return list(self.iterkeys())
def items(self):
-result = []
-for k in self._keys.values():
-result.append((k, self[k]))
-return result
+return list(self.iteritems())
+
+def values(self):
+return list(self.itervalues())
def copy(self):
+Returns a copy of this CIDict as an ordinary dict
copy = {}
-for k in self._keys.values():
-copy[k] = self[k]
-return copy
+return dict(self.items())
def iteritems(self):
-return self.copy().iteritems()
+return ((k, self[k]) for k in self._keys.itervalues())
def iterkeys(self):
-return self.copy().iterkeys()
+return self._keys.itervalues()
+
+def itervalues(self):
+return (v for k, v in self.iteritems())
def setdefault(self, key, value=None):
try:
@@ -515,6 +534,19 @@ class CIDict(dict):
return (key, value)
+def clear(self):
+self._keys.clear()
+return super(CIDict, self).clear()
+
+def viewitems(self):
+raise NotImplementedError('CIDict.viewitems is not implemented')
+
+def viewkeys(self):
+raise
Re: [Freeipa-devel] [PATCHES] 0022, 0115-0116 Make Sudo commands case-sensitive
On 02/20/2013 05:00 PM, Petr Viktorin wrote: On 02/20/2013 12:46 PM, Martin Kosek wrote: On 12/17/2012 04:08 PM, Petr Viktorin wrote: https://fedorahosted.org/freeipa/ticket/2482 The first two patches are rebased from what I sent back in March; the third fixes ACIs using targetfilter. I finally got to your patches. Generally, everything worked like charm, I have just few minor comments: 0022: - patch needs a rebase - patch description is confusing, we are talking about RDN sudocmd and not CN 0115: I would optimize the LDAP calls a little: 1) Use sudorule base DN as a base for the LDAP search 2) Do not call LDAP search twice, but just once and then collect the result. Now you use 2 LDAP searches with following filters: ((objectClass=ipasudorule)(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)) ((objectClass=ipasudorule)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)) We can do just one LDAP search with this filter: ((objectClass=ipasudorule)(|(memberallowcmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test)(memberdenycmd=sudocmd=/usr/bin/less,cn=sudocmds,cn=sudo,dc=linux,dc=ad,dc=test))) 0116: - patch description needs amending: s/CN/SUDOCMD/ Martin Thanks. Attaching updated patches. ACK. Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Backup and Restore design
Rich Megginson wrote: On 02/20/2013 08:38 AM, Rob Crittenden wrote: Simo Sorce wrote: On Tue, 2013-02-19 at 22:43 -0500, Rob Crittenden wrote: I've looked into some basic backup and restore procedures for IPA. My findings are here: http://freeipa.org/page/V3/Backup_and_Restore Great summary! For the catastrofic failure scenario, should we mention how to put back a full failed and restored machine online ? I am thinking the restored server may be behind (even if only by a few entries) in the replication, so the CSNs in the other replicas will not match. I guess we should mention a full resync may be needed ? Or is there a way to bring back CSNs on replicas ? Good questions. It depends on how long the machine was down and how many changes have happened. It is possible that one would want to do a full re-init. I'll add that to the design. The replication protocol will detect if a replica is too out of date to bring up to date with an incremental update, and requires a re-init. Ok, I'll update the design with this, thanks. In the 'Returning to a good state' case, can we consider some split brain approach, were we sever replication and rebuild one server at a time ? Perhaps using a firewall, but then you run the risk of each of those servers accepting changes during the rebuild and you could end up with a lot of collisions, which sort of goes against the point of restoring to a known good state. The changelog is the key here. I'll have to ponder this one a bit, I'm a bit conflicted on the right approach. Maybe we can think of a way to 'mark' all server as 'bad' so that on restore the replication agreements do not need to be changed but changes from 'bad' servers will not be accepted ? I guess this crosses also a request by someone to be able to 'pause' replication, would use the same mechanism I guess. AFAIK there is an option to pause replication now (at least in 1.3). What you can't do is drop the changelog AFAIK. That is the real problem. If you want to restore to a known state you need to drop all the changelog entries since that time. I'll check with the 389-ds team to see if that is possible. Since we know the time of the backup, we might be able to drop newer entries than that. Not sure what you mean - what exactly do you want to do with the changelog? As an example, if we have 2 IPA masters and we restore the data on one of them, as soon as it comes back upon the other is going to push the changelog onto it (as it should) so they are in sync again. So the question is, how do we restore several masters at the same time without apply changings from the changelog? What I was going to ask is, can we delete all changelog entries from Time X until now? That would prevent the sync issues, but it would retain the part of the changelog we care about. Full system backup: in the first part it is said the process is offline, but in the 'LDAP' section you say ldapi is used, but that would mean DS is running ? Also are we sure we can get all data we need via ldapi ? Are we going to miss any operational attribute ? The full backup is offline because it is just using tar. This is sort of a brute-force backup, copying bits from A to B. The data backup is online and creates a task in 389-ds to write the data and changelog to a file. It should write everything completely. We don't do an ldapsearch. I chose to not back up in ldif because this would back up just the data and not the changelog. The other advantage is that the db2bak format includes the indexes and ldif restore would require a rebuild. It is good to have a long term backup in LDIF format - no matter what happens to the database, if you have an LDIF backup, you can always recreate your data. So it's good to have both - db2bak format for shorter term/frequent backups, and LDIF for longer term/infrequent backups. Ok, so maybe when we do a full backup first we snapshot the database to ldif via db2ldif.pl, then back up the whole shebang? For restore are we sure we can reload data w/o alterations ? What about plugins ? will we have to disable all plugins during a restore ? Yes, it should be fine. I'm hoping that the version will help us with this, to prevent someone from restoring an ancient backup on a new system, for example (or the reverse). For the open questions. Size of backup: I think we should make it easy to configure the system to use a custom directory to dump the backups. This way admins can make sure it is on a different partition/disk or even over NFS and that the backup will not fill up the disk on which DS is running. That's a good idea. I'll have to think about where we would configure that. Perhaps as an optional argument to the backup command. You'll have to figure out a way around selinux, or add some sort of selinux magic that allows db2bak to write there. We should definitely allow to encrypt backup files, a gpg public key would be sufficient. Ok. I wasn't sure if
Re: [Freeipa-devel] Backup and Restore design
On 02/20/2013 09:44 AM, Rob Crittenden wrote: Rich Megginson wrote: On 02/20/2013 08:38 AM, Rob Crittenden wrote: Simo Sorce wrote: On Tue, 2013-02-19 at 22:43 -0500, Rob Crittenden wrote: I've looked into some basic backup and restore procedures for IPA. My findings are here: http://freeipa.org/page/V3/Backup_and_Restore Great summary! For the catastrofic failure scenario, should we mention how to put back a full failed and restored machine online ? I am thinking the restored server may be behind (even if only by a few entries) in the replication, so the CSNs in the other replicas will not match. I guess we should mention a full resync may be needed ? Or is there a way to bring back CSNs on replicas ? Good questions. It depends on how long the machine was down and how many changes have happened. It is possible that one would want to do a full re-init. I'll add that to the design. The replication protocol will detect if a replica is too out of date to bring up to date with an incremental update, and requires a re-init. Ok, I'll update the design with this, thanks. In the 'Returning to a good state' case, can we consider some split brain approach, were we sever replication and rebuild one server at a time ? Perhaps using a firewall, but then you run the risk of each of those servers accepting changes during the rebuild and you could end up with a lot of collisions, which sort of goes against the point of restoring to a known good state. The changelog is the key here. I'll have to ponder this one a bit, I'm a bit conflicted on the right approach. Maybe we can think of a way to 'mark' all server as 'bad' so that on restore the replication agreements do not need to be changed but changes from 'bad' servers will not be accepted ? I guess this crosses also a request by someone to be able to 'pause' replication, would use the same mechanism I guess. AFAIK there is an option to pause replication now (at least in 1.3). What you can't do is drop the changelog AFAIK. That is the real problem. If you want to restore to a known state you need to drop all the changelog entries since that time. I'll check with the 389-ds team to see if that is possible. Since we know the time of the backup, we might be able to drop newer entries than that. Not sure what you mean - what exactly do you want to do with the changelog? As an example, if we have 2 IPA masters and we restore the data on one of them, as soon as it comes back upon the other is going to push the changelog onto it (as it should) so they are in sync again. So the question is, how do we restore several masters at the same time without apply changings from the changelog? What I was going to ask is, can we delete all changelog entries from Time X until now? That would prevent the sync issues, but it would retain the part of the changelog we care about. Is this the problem you are trying to solve? You have a situation where some bogus data was introduced into your system, and that bogus data has now been replicated everywhere. You want to rollback the state of everything to before the bogus data was introduced. Let's assume you want to delete the bogus data and everything that happened after that. The first step is to pick a server to restore, and restore that server from a backup. The first problem is that this server will need to reject any replicated updates, but still allow regular client updates, after the restore process is complete (the db is in read-only mode during the restore). The only way to do this now would be to first disable all replication agreements on all other replicas going to this server, which would be quite painful. Alternately - during the restore process, change the replica generation of the restored server - other servers would see the different replica generation and would refuse to send updates (and report lots of replication errors). Once the first server is restored, you would just use the online or offline replica init procedure. Full system backup: in the first part it is said the process is offline, but in the 'LDAP' section you say ldapi is used, but that would mean DS is running ? Also are we sure we can get all data we need via ldapi ? Are we going to miss any operational attribute ? The full backup is offline because it is just using tar. This is sort of a brute-force backup, copying bits from A to B. The data backup is online and creates a task in 389-ds to write the data and changelog to a file. It should write everything completely. We don't do an ldapsearch. I chose to not back up in ldif because this would back up just the data and not the changelog. The other advantage is that the db2bak format includes the indexes and ldif restore would require a rebuild. It is good to have a long term backup in LDIF format - no matter what happens to the database, if you have an LDIF backup, you can always recreate your data. So it's good to have
Re: [Freeipa-devel] [PATCH] 355 Avoid internal error when user is not Trust admin
On 02/20/2013 12:30 PM, Petr Viktorin wrote:
On 02/20/2013 09:15 AM, Martin Kosek wrote:
On 02/19/2013 10:19 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/24/2013 12:01 PM, Martin Kosek wrote:
When user tries to perform any action requiring communication with
trusted domain, IPA server tries to retrieve a trust secret on his
behalf to be able to establish the connection. This happens for
example during group-add-member command when external user is
being resolved in the AD.
When user is not member of Trust admins group, the retrieval crashes
and reports internal error. Catch this exception and rather report
properly formatted ACIError.
I hit this error after updating to the latest FreeIPA version with the AD
CVE
fixed.
Martin
I filed a ticket to not loose this fix and patch. Attaching an updated
patch
with ticket URL in description.
Martin
The patch fixes the problem but the error is untranslated:
member group: AD\Domain Admins: Insufficient access:
Gettext('communication
with trusted domains is allowed for Trusts administrator group members
only',
domain='ipa', localedir=None)
rob
I think this is just because this string is not in our ipa.pot file yet (will
be when we do Transifex refresh).
Martin
I don't have AD so I can't investigate, but this problem is usually due to the
error being converted to string instead of using the strerror attribute.
You are right, attaching a patch which fixes it for group-add-member. But just
with using a quick grep, I see we do not use strerror on a lot of other places,
we may want to open a ticket to fix that too.
Martin
From 0662aedeefec4e8dff621ad7d0f1ead881a559ca Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 24 Jan 2013 11:51:58 +0100
Subject: [PATCH] Avoid internal error when user is not Trust admin
When user tries to perform any action requiring communication with
trusted domain, IPA server tries to retrieve a trust secret on his
behalf to be able to establish the connection. This happens for
example during group-add-member command when external user is
being resolved in the AD.
When user is not member of Trust admins group, the retrieval crashes
and reports internal error. Catch this exception and rather report
properly formatted ACIError. Also make sure that this exception is
properly processed in group-add-member post callback.
https://fedorahosted.org/freeipa/ticket/3390
---
ipalib/plugins/group.py | 2 +-
ipaserver/dcerpc.py | 27 +++
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index 4994dacb3218e03e1f92b7c16bf355c8ffa4d6f9..06e80931a0d77beb93b08cdf2637e3c750c1bafa 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -387,7 +387,7 @@ class group_add_member(LDAPAddMember):
try:
actual_sid = domain_validator.get_trusted_domain_object_sid(sid)
except errors.PublicError, e:
-failed_sids.append((sid, unicode(e)))
+failed_sids.append((sid, e.strerror))
else:
sids.append(actual_sid)
restore = []
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index b471bccee414281e26eaaf404b59fb3268d37112..140e26f77f6dd405e30fc13422869d9667da6ba0 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -156,10 +156,29 @@ class DomainValidator(object):
self.ATTR_TRUST_AUTHOUT])
result = dict()
-for entry in entries:
-result[entry[1][self.ATTR_TRUST_PARTNER][0]] = (entry[1][self.ATTR_FLATNAME][0].lower(),
-security.dom_sid(entry[1][self.ATTR_TRUSTED_SID][0]),
-entry[1][self.ATTR_TRUST_AUTHOUT][0])
+for dn, entry in entries:
+try:
+trust_partner = entry[self.ATTR_TRUST_PARTNER][0]
+flatname_normalized = entry[self.ATTR_FLATNAME][0].lower()
+trusted_sid = entry[self.ATTR_TRUSTED_SID][0]
+except KeyError, e:
+# Some piece of trusted domain info in LDAP is missing
+# Skip the domain, but leave log entry for investigation
+api.log.warn(Trusted domain '%s' entry misses an attribute: %s,
+dn, e)
+continue
+trust_authout = entry.get(self.ATTR_TRUST_AUTHOUT, [None])[0]
+
+# We were able to read all Trusted domain attributes but the secret
+# User is not member of trust admins group
+if trust_authout is None:
+raise errors.ACIError(
+
Re: [Freeipa-devel] [PATCH] 355 Avoid internal error when user is not Trust admin
Martin Kosek wrote:
On 02/20/2013 12:30 PM, Petr Viktorin wrote:
On 02/20/2013 09:15 AM, Martin Kosek wrote:
On 02/19/2013 10:19 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/24/2013 12:01 PM, Martin Kosek wrote:
When user tries to perform any action requiring communication with
trusted domain, IPA server tries to retrieve a trust secret on his
behalf to be able to establish the connection. This happens for
example during group-add-member command when external user is
being resolved in the AD.
When user is not member of Trust admins group, the retrieval crashes
and reports internal error. Catch this exception and rather report
properly formatted ACIError.
I hit this error after updating to the latest FreeIPA version with the AD CVE
fixed.
Martin
I filed a ticket to not loose this fix and patch. Attaching an updated patch
with ticket URL in description.
Martin
The patch fixes the problem but the error is untranslated:
member group: AD\Domain Admins: Insufficient access:
Gettext('communication
with trusted domains is allowed for Trusts administrator group members only',
domain='ipa', localedir=None)
rob
I think this is just because this string is not in our ipa.pot file yet (will
be when we do Transifex refresh).
Martin
I don't have AD so I can't investigate, but this problem is usually due to the
error being converted to string instead of using the strerror attribute.
You are right, attaching a patch which fixes it for group-add-member. But just
with using a quick grep, I see we do not use strerror on a lot of other places,
we may want to open a ticket to fix that too.
Martin
ACK, pushed to master and ipa-3-1
I think another ticket for your grep findings would be a good idea.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] NSS 3.14.3 Release, this fixes the certutil bug encountered during install
NSS 3.14.3 was just released by the NSS team. This is critical for us because it fixes https://bugzilla.mozilla.org/show_bug.cgi?id=840714 which was causing install failures as reported on this list. I expect a new RPM will follow shortly. John Original Message Subject: [ANNOUNCE] NSS 3.14.3 Release Date: Wed, 20 Feb 2013 12:07:38 -0800 From: Ryan Sleevi ryan-mozdevtechcry...@sleevi.com Reply-To: mozilla's crypto code discussion list dev-tech-cry...@lists.mozilla.org To: dev-tech-cry...@lists.mozilla.org The NSS Development Team is pleased to announce the release of NSS 3.14.3. The official release notes are available at https://developer.mozilla.org/en-US/docs/NSS/NSS_3.14.3_release_notes , and are reproduced at the end of this message. This release includes mitigations for recently discussed Lucky Thirteen attack (CVE-2013-1620). However, please note the limitations of the mitigations discussed in the release notes below. Introduction: Network Security Services (NSS) 3.14.3 is a patch release for NSS 3.14. The bug fixes in NSS 3.14.3 are described in the Bugs Fixed section below. Distribution Information * The CVS tag is NSS_3_14_3_RTM. NSS 3.14.3 requires NSPR 4.9.5 or newer. * NSS 3.14.3 source distributions are also available on ftp.mozilla.org for secure HTTPS download: - Source tarballs: https://ftp.mozilla.org/pub/mozilla.org/security/nss/releases/NSS_3_14_3_RTM/src/ New in NSS 3.14.3 * No new major functionality is introduced in this release. This release is a patch release to address CVE-2013-1620. New Functions * in pk11pub.h - PK11_SignWithSymKey - Similar to PK11_Sign, performs a signing operation in a single operation. However, unlike PK11_Sign, which uses a SECKEYPrivateKey, PK11_SignWithSymKey performs the signature using a symmetric key, such as commonly used for generating MACs. New Types * CK_NSS_MAC_CONSTANT_TIME_PARAMS - Parameters for use with CKM_NSS_HMAC_CONSTANT_TIME and CKM_NSS_SSL3_MAC_CONSTANT_TIME. New PKCS #11 Mechanisms * CKM_NSS_HMAC_CONSTANT_TIME - Constant-time HMAC operation for use when verifying a padded, MAC-then-encrypted block of data. CKM_NSS_SSL3_MAC_CONSTANT_TIME - Constant-time MAC operation for use when verifying a padded, MAC-then-encrypted block of data using the SSLv3 MAC. Notable Changes in NSS 3.14.3 * CVE-2013-1620 Recent research by Nadhem AlFardan and Kenny Patterson has highlighted a weakness in the handling of CBC padding as used in SSL, TLS, and DTLS that allows an attacker to exploit timing differences in MAC processing. The details of their research and the attack can be found at http://www.isg.rhul.ac.uk/tls/, and has been referred to as Lucky Thirteen. NSS 3.14.3 includes changes to the softoken and ssl libraries to address and mitigate these attacks, contributed by Adam Langley of Google. This attack is mitigated when using NSS 3.14.3 with an NSS Cryptographic Module (softoken) version 3.14.3 or later. However, this attack is only partially mitigated if NSS 3.14.3 is used with the current FIPS validated NSS Cryptographic Module, version 3.12.9.1. * Bug 840714 - certutil -a was not correctly producing ASCII output as requested. * Bug 837799 - NSS 3.14.2 broke compilation with older versions of sqlite that lacked the SQLITE_FCNTL_TEMPFILENAME file control. NSS 3.14.3 now properly compiles when used with older versions of sqlite. Acknowledgements * The NSS development team would like to thank Nadhem AlFardan and Kenny Patterson (Royal Holloway, University of London) for responsibly disclosing the issue by providing advance copies of their research. In addition, thanks to Adam Langley (Google) for the development of a mitigation for the issues raised in the paper, along with Emilia Kasper and Bodo Möller (Google) for assisting in the review and improvements to the initial patches. Bugs fixed in NSS 3.14.3 * https://bugzilla.mozilla.org/buglist.cgi?list_id=5689256;resolution=FIXED;classification=Components;query_format=advanced;target_milestone=3.14.3;product=NSS Compatibility * NSS 3.14.3 shared libraries are backward compatible with all older NSS 3.x shared libraries. A program linked with older NSS 3.x shared libraries will work with NSS 3.14.3 shared libraries without recompiling or relinking. Furthermore, applications that restrict their use of NSS APIs to the functions listed in NSS Public Functions will remain compatible with future versions of the NSS shared libraries. Feedback * Bugs discovered should be reported by filing a bug report with bugzilla.mozilla.org (product NSS). -- dev-tech-crypto mailing list dev-tech-cry...@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-tech-crypto ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
