Re: [Freeipa-devel] [PATCH] Password vault
Thanks for the review. New patch attached to be applied on top of all
previous patches. Please see comments below.
On 3/6/2015 3:53 PM, Jan Cholasta wrote:
Patch 353:
1) Please follow PEP8 in new code.
The pep8 tool reports these errors in existing files:
./ipalib/constants.py:98:80: E501 line too long (84 79 characters)
./ipalib/plugins/baseldap.py:1527:80: E501 line too long (81 79
characters)
./ipalib/plugins/user.py:915:80: E501 line too long (80 79 characters)
as well as many errors in the files this patch adds.
For some reason pylint keeps crashing during build so I cannot run it
for all files. I'm fixing the errors that I can see. If you see other
errors please let me know while I'm still trying to figure out the problem.
Is there an existing ticket for fixing PEP8 errors? Let's use that for
fixing the errors in the existing code.
2) Pylint reports the following error:
ipatests/test_xmlrpc/test_vault_plugin.py:153:
[E0102(function-redefined), test_vault] class already defined line 27)
Fixed.
3) The container_vault config option should be renamed to
container_vaultcontainer, as it is used in the vaultcontainer plugin,
not the vault plugin.
It was named container_vault because it defines the DN for of the
subtree that contains all vault-related entries. I moved the base_dn
variable from vaultcontainer object to the vault object for clarity.
4) The vault object should be child of the vaultcontainer object.
Not only is this correct from the object model perspective, but it would
also make all the container_id hacks go away.
It's a bit difficult because it will affect how the container vault
ID's are represented on the CLI.
In the design the container ID would be a single value like this:
$ ipa vault-add /services/server.example.com/HTTP
And if the vault ID is relative (without initial slash), it will be
appended to the user's private container (i.e. /users/username/):
$ ipa vault-add PrivateVault
The implementation is not complete yet. Currently it accepts this format:
$ ipa vault-add vault name [--container container ID]
and I'm still planning to add this:
$ ipa vault-add vault ID
If the vault must be a child of vaultcontainer, and the vaultcontainer
must be a child of a vaultcontainer, does it mean the vault ID would
have to be split into separate arguments like this?
$ ipa vaultcontainer-add services server.example.com HTTP
If that's the case we'd lose the ability to specify a relative vault ID.
5) When specifying param flags, use set literals.
This is especially wrong, because it's not a tuple, but a string in
parentheses:
+flags=('virtual_attribute'),
Fixed.
6) The `container` param of vault should actually be an option in
vault_* commands.
Also it should be renamed to `container_id`, for consistency with
vaultcontainer.
Fixed. It was actually made to be consistent with the 'parent' attribute
in the vaultcontainer class. Now the 'parent' has been renamed to
'parent_id' as well.
7) The `vault_id` param of vault should have no_option in flags, since
it is output-only.
Fixed.
8) Don't translate docstrings where not necessary:
+def get_dn(self, *keys, **options):
+__doc__ = _(
+Generates vault DN from vault ID.
+)
Only plugin modules and classes should have translated docstrings.
Fixed.
9) This looks wrong in vault.get_dn() and vaultcontainer.get_dn():
+name = None
+if keys:
+name = keys[0]
Primary key of the object should always be set, so the if statement
should not be there.
Fixed.
Also, primary key of any given object is always last in keys, so use
keys[-1] instead of keys[0].
Fixed.
10) Use self.api instead of api to access the API in plugins.
Fixed.
11) No clever optimizations like this please:
+# vault DN cannot be the container base DN
+if len(dn) == len(api.Object.vaultcontainer.base_dn):
+raise ValueError('Invalid vault DN: %s' % dn)
Compare the DNs by value instead.
Actually the DN values have already been compared in the code right
above it:
# make sure the DN is a vault DN
if not dn.endswith(self.api.Object.vaultcontainer.base_dn):
raise ValueError('Invalid vault DN: %s' % dn)
This code confirms that the incoming vault DN is within the vault
subtree. After that, the DN length comparison above is just to make sure
the incoming vault DN is not the root of the vault subtree itself. It
doesn't need to compare the values again.
12) vault.split_id() is not used anywhere.
Removed.
13) Bytes is not base64-encoded data:
+Bytes('data?',
+cli_name='data',
+doc=_('Base-64 encoded binary data to archive'),
+),
It is base64-encoded in the CLI, but on the API level it is not. The doc
should say just Binary data to archive.
Fixed.
14) Use File instead of Str for input files:
+Str('in?',
+cli_name='in',
+
Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)
On 10.3.2015 17:35, Simo Sorce wrote: On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote: On 10.3.2015 15:53, Simo Sorce wrote: On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote: Hello, I would like to discuss Generic support for unknown DNS RR types (RFC 3597 [0]). Here is the proposal: LDAP schema === - 1 new attribute: ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) The attribute should be added to existing idnsRecord object class as MAY. This new attribute should contain data encoded according to RFC 3597 section 5 [5]: The RDATA section of an RR of unknown type is represented as a sequence of white space separated words as follows: The special token \# (a backslash immediately followed by a hash sign), which identifies the RDATA as having the generic encoding defined herein rather than a traditional type-specific encoding. An unsigned decimal integer specifying the RDATA length in octets. Zero or more words of hexadecimal data encoding the actual RDATA field, each containing an even number of hexadecimal digits. If the RDATA is of zero length, the text representation contains only the \# token and the single zero representing the length. Examples from RFC: a.example. CLASS32 TYPE731 \# 6 abcd ( ef 01 23 45 ) b.example. HS TYPE62347 \# 0 e.example. IN A \# 4 0A01 e.example. CLASS1 TYPE1 10.0.0.2 Open questions about LDAP format Should we include \# constant? We know that the attribute contains record in RFC 3597 syntax so it is not strictly necessary. I think it would be better to follow RFC 3597 format. It allows blind copypasting from other tools, including direct calls to python-dns. It also eases writing conversion tools between DNS and LDAP format because they do not need to change record values. Another question is if we should explicitly include length of data represented in hexadecimal notation as a decimal number. I'm very strongly inclined to let it there because it is very good sanity check and again, it allows us to re-use existing tools including parsers. I will ask Uninett.no for standardization after we sort this out (they own the OID arc we use for DNS records). Attribute usage === Every DNS RR type has assigned a number [1] which is used on wire. RR types which are unknown to the server cannot be named by their mnemonic/type name because server would not be able to do name-number conversion and to generate DNS wire format. As a result, we have to encode the RR type number somehow. Let's use attribute sub-types. E.g. a record with type 65280 and hex value 0A01 will be represented as: GenericRecord;TYPE65280: \# 4 0A01 CLI === $ ipa dnsrecord-add zone.example owner \ --generic-type=65280 --generic-data='\# 4 0A01' $ ipa dnsrecord-show zone.example owner Record name: owner TYPE65280 Record: \# 4 0A01 ACK? :-) Almost. We should refrain from using subtypes when not necessary, and in this case it is not necessary. Use: GenericRecord: 65280 \# 4 0A01 I was considering that too but I can see two main drawbacks: 1) It does not work very well with DS ACI (targetattrfilter, anyone?). Adding generic write access to GenericRecord == ability to add TLSA records too, which you may not want. IMHO it is perfectly reasonable to limit write access to certain types (e.g. to one from private range). 2) We would need a separate substring index for emulating filters like (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence index which will be handy one day when we decide to handle upgrades like GenericRecord;TYPE256-UriRecord. Another (less important) annoyance is that conversion tools would have to mangle record data instead of just converting attribute name-record type. I can be convinced that subtypes are not necessary but I do not see clear advantage of avoiding them. What is the problem with subtypes? Poor support by most clients, so it is generally discouraged. Hmm, it does not sound like a thing we should care in this case. DNS tree is not meant for direct consumption by LDAP clients (compare with cn=compat). IMHO the only two clients we should care are FreeIPA framework and bind-dyndb-ldap so I do not see this as a problem, really. If someone wants to access DNS tree by hand - sure, use a standard compliant client! Working ACI and LDAP filters sounds like good price for supporting only standards compliant clients. AFAIK OpenLDAP works well and I suspect that ApacheDS will work too because Eclipse has nice support for sub-types built-in. If I can draw some conclusions from that, sub-types are not a
Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)
On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote: On 10.3.2015 17:35, Simo Sorce wrote: On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote: On 10.3.2015 15:53, Simo Sorce wrote: On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote: Hello, I would like to discuss Generic support for unknown DNS RR types (RFC 3597 [0]). Here is the proposal: LDAP schema === - 1 new attribute: ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) The attribute should be added to existing idnsRecord object class as MAY. This new attribute should contain data encoded according to RFC 3597 section 5 [5]: The RDATA section of an RR of unknown type is represented as a sequence of white space separated words as follows: The special token \# (a backslash immediately followed by a hash sign), which identifies the RDATA as having the generic encoding defined herein rather than a traditional type-specific encoding. An unsigned decimal integer specifying the RDATA length in octets. Zero or more words of hexadecimal data encoding the actual RDATA field, each containing an even number of hexadecimal digits. If the RDATA is of zero length, the text representation contains only the \# token and the single zero representing the length. Examples from RFC: a.example. CLASS32 TYPE731 \# 6 abcd ( ef 01 23 45 ) b.example. HS TYPE62347 \# 0 e.example. IN A \# 4 0A01 e.example. CLASS1 TYPE1 10.0.0.2 Open questions about LDAP format Should we include \# constant? We know that the attribute contains record in RFC 3597 syntax so it is not strictly necessary. I think it would be better to follow RFC 3597 format. It allows blind copypasting from other tools, including direct calls to python-dns. It also eases writing conversion tools between DNS and LDAP format because they do not need to change record values. Another question is if we should explicitly include length of data represented in hexadecimal notation as a decimal number. I'm very strongly inclined to let it there because it is very good sanity check and again, it allows us to re-use existing tools including parsers. I will ask Uninett.no for standardization after we sort this out (they own the OID arc we use for DNS records). Attribute usage === Every DNS RR type has assigned a number [1] which is used on wire. RR types which are unknown to the server cannot be named by their mnemonic/type name because server would not be able to do name-number conversion and to generate DNS wire format. As a result, we have to encode the RR type number somehow. Let's use attribute sub-types. E.g. a record with type 65280 and hex value 0A01 will be represented as: GenericRecord;TYPE65280: \# 4 0A01 CLI === $ ipa dnsrecord-add zone.example owner \ --generic-type=65280 --generic-data='\# 4 0A01' $ ipa dnsrecord-show zone.example owner Record name: owner TYPE65280 Record: \# 4 0A01 ACK? :-) Almost. We should refrain from using subtypes when not necessary, and in this case it is not necessary. Use: GenericRecord: 65280 \# 4 0A01 I was considering that too but I can see two main drawbacks: 1) It does not work very well with DS ACI (targetattrfilter, anyone?). Adding generic write access to GenericRecord == ability to add TLSA records too, which you may not want. IMHO it is perfectly reasonable to limit write access to certain types (e.g. to one from private range). 2) We would need a separate substring index for emulating filters like (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence index which will be handy one day when we decide to handle upgrades like GenericRecord;TYPE256-UriRecord. Another (less important) annoyance is that conversion tools would have to mangle record data instead of just converting attribute name-record type. I can be convinced that subtypes are not necessary but I do not see clear advantage of avoiding them. What is the problem with subtypes? Poor support by most clients, so it is generally discouraged. Hmm, it does not sound like a thing we should care in this case. DNS tree is not meant for direct consumption by LDAP clients (compare with cn=compat). IMHO the only two clients we should care are FreeIPA framework and bind-dyndb-ldap so I do not see this as a problem, really. If someone wants to access DNS tree by hand - sure, use a standard compliant client! Working ACI and LDAP filters sounds like good price for supporting only standards
Re: [Freeipa-devel] [PATCHES] SPEC: Require python2 version of sssd bindings
On (06/03/15 15:05), Lukas Slebodnik wrote: On (05/03/15 16:20), Petr Vobornik wrote: On 03/05/2015 11:23 AM, Lukas Slebodnik wrote: On (05/03/15 08:54), Petr Vobornik wrote: On 02/27/2015 09:50 PM, Lukas Slebodnik wrote: ehlo, Please review attached patches and fix freeipa in fedora 22 ASAP. I think the most critical is 1st patch sh$ git grep SSSDConfig | grep import install/tools/ipa-upgradeconfig:import SSSDConfig ipa-client/ipa-install/ipa-client-automount:import SSSDConfig ipa-client/ipa-install/ipa-client-install:import SSSDConfig BTW package python-sssdconfig is provides since sssd-1.10.0alpha1 (2013-04-02) but it was not explicitely required. The latest python3 changes in sssd (fedora 22) is just a result of negligent packaging of freeipa. LS Fedora 22 was amended. Patch 1: ACK Patch 2: ACK Patch3: the package name is libsss_nss_idmap-python not python-libsss_nss_idmap which already is required in adtrust package In sssd upstream we decided to rename package libsss_nss_idmap-python to python-libsss_nss_idmap according to new rpm python guidelines. The python3 version has alredy correct name. We will rename package in downstream with next major release (1.13). Of course it we will add Provides: libsss_nss_idmap-python. We can push 3rd patch later or I can update 3rd patch. What do you prefer? Than you very much for review. LS Patch 3 should be updated to not forget the remaining change in ipa-python package. It then should be updated downstream and master when 1.13 is released in Fedora, or in master sooner if SSSD 1.13 becomes the minimal version required by master. Fixed. BTW Why ther is a pylint comment for some sssd modules I did not kave any pylint problems after removing comment. ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401 ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401 And why are these modules optional (try except) ipalib/plugins/trust.py-31-try: ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401 ipalib/plugins/trust.py-33-_murmur_installed = True ipalib/plugins/trust.py-34-except Exception, e: ipalib/plugins/trust.py-35-_murmur_installed = False ipalib/plugins/trust.py-36- ipalib/plugins/trust.py-37-try: ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401 ipalib/plugins/trust.py-39-_nss_idmap_installed = True ipalib/plugins/trust.py-40-except Exception, e: ipalib/plugins/trust.py-41-_nss_idmap_installed = False LS From 41523fd6ab9ea95644cac1a6cd20386a620a1df5 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lsleb...@redhat.com Date: Fri, 27 Feb 2015 20:40:06 +0100 Subject: [PATCH 1/3] SPEC: Explicitly requires python-sssdconfig bump LS -- 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] Rename IPAv3_AD_trust_setup?
On 03/10/2015 09:06 AM, Alexander Bokovoy wrote: On Tue, 10 Mar 2015, Martin Kosek wrote: Hi, I just saw someone refer to [1] with respect to FreeIPA 4.x. Would it make sense to just rename the page from [1] to [2] (with keeping redirect of course)? This would move the page from Howto/ name space which we use for community HOWTO articles and move it to standard default name space. We would also not confuse people with the v3 version, since it applies to all our FreeIPA versions, including v4. [1] http://www.freeipa.org/page/Howto/IPAv3_AD_trust_setup [2] http://www.freeipa.org/page/Active_Directory_Trust_setup I'm fine if there is a redirect. Ok, good. I renamed the page and fixed the links in our wiki. This is the new link: http://www.freeipa.org/page/Active_Directory_trust_setup I actually changed all the links to point to this page, some were still pointing to the old, obsolete HOWTO. This one was moved to Obsolete name space, so that we do not clutter the main name space: http://www.freeipa.org/page/Obsolete:IPAv3_testing_AD_trust HTH. If you see other issues, please tell me (or fix it right away). Martin -- 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 306-316] Automated migration tool from Winsync
Hi, Dne 10.3.2015 v 16:35 Tomas Babej napsal(a): On 03/09/2015 12:26 PM, Tomas Babej wrote: Hi, this couple of patches provides a initial implementation of the winsync migration tool: https://fedorahosted.org/freeipa/ticket/4524 Some parts could use some polishing, but this is a sound foundation. Tomas Attaching one more patch to the bundle. This one should make the winsync tool readily available after install. Tomas Nitpicks: The winsync_migrate module should be in ipaserver.install. Also I don't see why it has to be a package when there is just one short file in it. By convention, the AdminTool subclass should be named WinsyncMigrate, or the tool should be named ipa-migrate-winsync. Honza -- 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] Generic support for unknown DNS RR types (RFC 3597)
On 11.3.2015 15:45, Martin Kosek wrote: On 03/11/2015 03:38 PM, Petr Spacek wrote: On 11.3.2015 15:28, Martin Kosek wrote: On 03/11/2015 12:43 PM, Petr Spacek wrote: On 11.3.2015 11:34, Jan Cholasta wrote: Dne 11.3.2015 v 11:12 Petr Spacek napsal(a): On 10.3.2015 20:04, Simo Sorce wrote: On Tue, 2015-03-10 at 19:24 +0100, Petr Spacek wrote: On 10.3.2015 18:36, Simo Sorce wrote: On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote: On 10.3.2015 17:35, Simo Sorce wrote: On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote: On 10.3.2015 15:53, Simo Sorce wrote: On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote: Hello, I would like to discuss Generic support for unknown DNS RR types (RFC 3597 [0]). Here is the proposal: LDAP schema === - 1 new attribute: ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) The attribute should be added to existing idnsRecord object class as MAY. This new attribute should contain data encoded according to RFC 3597 section 5 [5]: The RDATA section of an RR of unknown type is represented as a sequence of white space separated words as follows: The special token \# (a backslash immediately followed by a hash sign), which identifies the RDATA as having the generic encoding defined herein rather than a traditional type-specific encoding. An unsigned decimal integer specifying the RDATA length in octets. Zero or more words of hexadecimal data encoding the actual RDATA field, each containing an even number of hexadecimal digits. If the RDATA is of zero length, the text representation contains only the \# token and the single zero representing the length. Examples from RFC: a.example. CLASS32 TYPE731 \# 6 abcd ( ef 01 23 45 ) b.example. HS TYPE62347 \# 0 e.example. IN A \# 4 0A01 e.example. CLASS1 TYPE1 10.0.0.2 Open questions about LDAP format Should we include \# constant? We know that the attribute contains record in RFC 3597 syntax so it is not strictly necessary. I think it would be better to follow RFC 3597 format. It allows blind copypasting from other tools, including direct calls to python-dns. It also eases writing conversion tools between DNS and LDAP format because they do not need to change record values. Another question is if we should explicitly include length of data represented in hexadecimal notation as a decimal number. I'm very strongly inclined to let it there because it is very good sanity check and again, it allows us to re-use existing tools including parsers. I will ask Uninett.no for standardization after we sort this out (they own the OID arc we use for DNS records). Attribute usage === Every DNS RR type has assigned a number [1] which is used on wire. RR types which are unknown to the server cannot be named by their mnemonic/type name because server would not be able to do name-number conversion and to generate DNS wire format. As a result, we have to encode the RR type number somehow. Let's use attribute sub-types. E.g. a record with type 65280 and hex value 0A01 will be represented as: GenericRecord;TYPE65280: \# 4 0A01 CLI === $ ipa dnsrecord-add zone.example owner \ --generic-type=65280 --generic-data='\# 4 0A01' $ ipa dnsrecord-show zone.example owner Record name: owner TYPE65280 Record: \# 4 0A01 ACK? :-) Almost. We should refrain from using subtypes when not necessary, and in this case it is not necessary. Use: GenericRecord: 65280 \# 4 0A01 I was considering that too but I can see two main drawbacks: 1) It does not work very well with DS ACI (targetattrfilter, anyone?). Adding generic write access to GenericRecord == ability to add TLSA records too, which you may not want. IMHO it is perfectly reasonable to limit write access to certain types (e.g. to one from private range). 2) We would need a separate substring index for emulating filters like (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence index which will be handy one day when we decide to handle upgrades like GenericRecord;TYPE256-UriRecord. Another (less important) annoyance is that conversion tools would have to mangle record data instead of just converting attribute name-record type. I can be convinced that subtypes are not necessary but I do not see clear advantage of avoiding them. What is the problem with subtypes? Poor support by most clients, so it is generally discouraged. Hmm, it does not sound like a thing we should care in this case. DNS tree is not meant for direct consumption by LDAP clients (compare
Re: [Freeipa-devel] [PATCHES] SPEC: Require python2 version of sssd bindings
On 03/06/2015 03:13 PM, Alexander Bokovoy wrote: On Fri, 06 Mar 2015, Lukas Slebodnik wrote: On (05/03/15 16:20), Petr Vobornik wrote: On 03/05/2015 11:23 AM, Lukas Slebodnik wrote: On (05/03/15 08:54), Petr Vobornik wrote: On 02/27/2015 09:50 PM, Lukas Slebodnik wrote: ehlo, Please review attached patches and fix freeipa in fedora 22 ASAP. I think the most critical is 1st patch sh$ git grep SSSDConfig | grep import install/tools/ipa-upgradeconfig:import SSSDConfig ipa-client/ipa-install/ipa-client-automount:import SSSDConfig ipa-client/ipa-install/ipa-client-install:import SSSDConfig BTW package python-sssdconfig is provides since sssd-1.10.0alpha1 (2013-04-02) but it was not explicitely required. The latest python3 changes in sssd (fedora 22) is just a result of negligent packaging of freeipa. LS Fedora 22 was amended. Patch 1: ACK Patch 2: ACK Patch3: the package name is libsss_nss_idmap-python not python-libsss_nss_idmap which already is required in adtrust package In sssd upstream we decided to rename package libsss_nss_idmap-python to python-libsss_nss_idmap according to new rpm python guidelines. The python3 version has alredy correct name. We will rename package in downstream with next major release (1.13). Of course it we will add Provides: libsss_nss_idmap-python. We can push 3rd patch later or I can update 3rd patch. What do you prefer? Than you very much for review. LS Patch 3 should be updated to not forget the remaining change in ipa-python package. It then should be updated downstream and master when 1.13 is released in Fedora, or in master sooner if SSSD 1.13 becomes the minimal version required by master. Fixed. BTW Why ther is a pylint comment for some sssd modules I did not kave any pylint problems after removing comment. ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401 ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401 And why are these modules optional (try except) Because they are needed to properly load in the case trust subpackages are not installed, to generate proper messages to users who will try these commands, like 'ipa trust-add' while the infrastructure is not in place. pylint is dumb for such cases. Alexander, the point was not to require python_nss_idmap and python-sss-murmur on ipa clients? If so python-sss-murmur should be required only by trust-ad package and not python package (patch2). And patch 3 (adding libsss_nss_idmap-python to python package) should not be used. -- 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 0044] Man pages: ipa-replica-prepare can only be created on first master
On 03/12/2015 02:37 PM, Gabe Alford wrote: Hello, Fix for https://fedorahosted.org/freeipa/ticket/4944. Since there seems to be plenty of time, I added it to the freeipa-4-1 branch. Thanks Gabe! I would still suggest against moving the tickets to milestones yourself, all new tickets should still undergo the weekly triage so that all core developers see it and we can decide the target milestone. With this one, it would likely indeed end in 4.1.x, especially given you contributed a patch, but still... For the patch itself, I still think the wording is not as should be: - following line is not entirely trie, you can install can create replica also on servers installed with ipa-replica-install :-) +A replica can be created on any IPA master server installed with ipa\-server\-install. - Following line may also use some rewording: However if you want to create a replica as a redundant CA with an existing replica or master, ipa\-replica\-prepare should be run on a replica or master that contains the CA. Maybe we should add subsection to DESCRIPTION section, with following lines: - A replica should only be installed on the same or higher version of IPA on the remote system. - A replica with PKI can only be installed from replica file prepared on a master with PKI Makes sense? Martin -- 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 0044] Man pages: ipa-replica-prepare can only be created on first master
Hello, Fix for https://fedorahosted.org/freeipa/ticket/4944. Since there seems to be plenty of time, I added it to the freeipa-4-1 branch. Thanks, Gabe From 0887f4f4595e62ce4d24f1b031418e47da7586fb Mon Sep 17 00:00:00 2001 From: Gabe redhatri...@gmail.com Date: Thu, 12 Mar 2015 07:26:34 -0600 Subject: [PATCH] ipa-replica-prepare can only be created on the first master - https://fedorahosted.org/freeipa/ticket/4944 --- install/tools/man/ipa-replica-prepare.1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/tools/man/ipa-replica-prepare.1 b/install/tools/man/ipa-replica-prepare.1 index 1879d2ee88fc78fb755a702a2b2fe9a93e153b45..8d97c27b36b54d5ce95bd85f0d9adb4022a6ecfb 100644 --- a/install/tools/man/ipa-replica-prepare.1 +++ b/install/tools/man/ipa-replica-prepare.1 @@ -24,7 +24,7 @@ ipa\-replica\-prepare [\fIOPTION\fR]... hostname .SH DESCRIPTION Generates a replica file that may be used with ipa\-replica\-install to create a replica of an IPA server. -A replica can only be created on an IPA server installed with ipa\-server\-install (the first server). +A replica can be created on any IPA master server installed with ipa\-server\-install. However if you want to create a replica as a redundant CA with an existing replica or master, ipa\-replica\-prepare should be run on a replica or master that contains the CA. You must provide the fully\-qualified hostname of the machine you want to install the replica on and a host\-specific replica_file will be created. It is host\-specific because SSL server certificates are generated as part of the process and they are specific to a particular hostname. -- 1.8.3.1 -- 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 0208] Respect --test option in upgrade plugins
On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. -- 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
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 12.3.2015 17:05, Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) BTW 'proper' fix would be to implement transactions in DS, run all updates in one huge transaction and do roll-back at the end. That would allow us to actually test updates which can depend on each other etc. -- 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] [PATCHES 0204-0207] Server upgrade: Make LDAP data upgrade deterministic
On 03/06/2015 04:50 PM, Martin Basti wrote: The patchset ensure, the upgrade order will respect ordering of entries in *.update files. Required for: https://fedorahosted.org/freeipa/ticket/4904 Patch 205 also fixes https://fedorahosted.org/freeipa/ticket/3560 Required patch mbasti-0203 Patches attached. Changes in code looks good and the upgrade process still works, ACK. -- 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
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. rob -- 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 0204-0207] Server upgrade: Make LDAP data upgrade deterministic
Martin Basti wrote: The patchset ensure, the upgrade order will respect ordering of entries in *.update files. Required for: https://fedorahosted.org/freeipa/ticket/4904 Patch 205 also fixes https://fedorahosted.org/freeipa/ticket/3560 Required patch mbasti-0203 Patches attached. Just reading the patches, untested. I think ordered should default to True in the update() method of ldapupdater to keep in spirit with the design. Otherwise LGTM that it implements what was designed. rob -- 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 0203] Remove unused PRE_SCHEMA upgrade
David Kupka wrote: On 03/06/2015 04:52 PM, Martin Basti wrote: This upgrade step is not used anymore. Required by: https://fedorahosted.org/freeipa/ticket/4904 Patch attached. Looks and works good to me, ACK. Is this going away because one can simply create an update file that exists alphabetically before the schema update? If so then ACK. rob -- 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 0203] Remove unused PRE_SCHEMA upgrade
On 12/03/15 16:22, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 04:52 PM, Martin Basti wrote: This upgrade step is not used anymore. Required by: https://fedorahosted.org/freeipa/ticket/4904 Patch attached. Looks and works good to me, ACK. Is this going away because one can simply create an update file that exists alphabetically before the schema update? If so then ACK. rob No this never works, and will not work without changes in DS, I was discussing this with DS guys. If you add new replica to schema, the schema has to be there before data replication. Martin -- 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 0203] Remove unused PRE_SCHEMA upgrade
Martin Basti wrote: On 12/03/15 16:22, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 04:52 PM, Martin Basti wrote: This upgrade step is not used anymore. Required by: https://fedorahosted.org/freeipa/ticket/4904 Patch attached. Looks and works good to me, ACK. Is this going away because one can simply create an update file that exists alphabetically before the schema update? If so then ACK. rob No this never works, and will not work without changes in DS, I was discussing this with DS guys. If you add new replica to schema, the schema has to be there before data replication. Martin That's a rather narrow case though. You could make changes that only affect existing schema, or something in cn=config. rob -- 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 0018-0019] ipa-dns-install: Use LDAPI for all DS connections
On 03/12/2015 03:59 PM, Martin Babinsky wrote: On 03/11/2015 03:13 PM, Martin Basti wrote: On 11/03/15 13:00, Martin Babinsky wrote: These patches solve https://fedorahosted.org/freeipa/ticket/4933. They are to be applied to master branch. I will rebase them for ipa-4-1 after the review. Thank you for the patches. I have a few comments: IPA-4-1 Replace simple bind with LDAPI is too big change for 4-1, we should start TLS if possible to avoid MINSSF0 error. The LDAPI patches should go only into IPA master branch. You can do something like this: --- a/ipaserver/install/service.py +++ b/ipaserver/install/service.py @@ -107,6 +107,10 @@ class Service(object): if not self.realm: raise errors.NotFound(reason=realm is missing for %s % (self)) conn = ipaldap.IPAdmin(ldapi=self.ldapi, realm=self.realm) +elif self.dm_password is not None: +conn = ipaldap.IPAdmin(self.fqdn, port=389, + cacert=paths.IPA_CA_CRT, + start_tls=True) else: conn = ipaldap.IPAdmin(self.fqdn, port=389) PATCH 0018: 1) please add there more chatty commit message about using LDAPI 2) I do not like much idea of adding 'realm' kwarg into __init__ method of OpenDNSSECInstance IIUC, it is because get_masters() method, which requires realm to use LDAPI. You can just add ods.realm=realm, before call get_master() in ipa-dns-install if options.dnssec_master: +ods.realm=api.env.realm dnssec_masters = ods.get_masters() (Honza will change it anyway during refactoring) PATCH 0019: 1) commit message deserves to be more chatty, can you explain there why you removed kerberos cache? Martin^2 Attaching updated patches. Patch 0018 should go to both 4.1 and master branches. Patch 0019 should go only to master. One more update. Patch 0018 is for both 4.1 and master. Patch 0019 is for master only. -- Martin^3 Babinsky From b8fa778811cdde75da7faa5a2bc37a20655db372 Mon Sep 17 00:00:00 2001 From: Martin Babinsky mbabi...@redhat.com Date: Thu, 12 Mar 2015 16:14:22 +0100 Subject: [PATCH] ipa-dns-install: use STARTTLS to connect to DS BindInstance et al. now use STARTTLS to set up secure connection to DS during ipa-dns-install. This fixes https://fedorahosted.org/freeipa/ticket/4933 --- install/tools/ipa-dns-install| 12 ipaserver/install/bindinstance.py| 10 ++ ipaserver/install/dnskeysyncinstance.py | 7 --- ipaserver/install/odsexporterinstance.py | 5 +++-- ipaserver/install/opendnssecinstance.py | 5 +++-- ipaserver/install/service.py | 10 -- 6 files changed, 32 insertions(+), 17 deletions(-) diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install index 11f79f0f9be226aaee8c95deb31e7e21f8a18dbb..2b6ad02abee9428870b0a554f5b4088e77b0e852 100755 --- a/install/tools/ipa-dns-install +++ b/install/tools/ipa-dns-install @@ -151,7 +151,7 @@ def main(): confirm=False, validate=False) if dm_password is None: sys.exit(Directory Manager password required) -bind = bindinstance.BindInstance(fstore, dm_password) +bind = bindinstance.BindInstance(fstore, dm_password, start_tls=True) # try the connection try: @@ -160,7 +160,8 @@ def main(): except errors.ACIError: sys.exit(Password is not valid!) -ods = opendnssecinstance.OpenDNSSECInstance(fstore, dm_password) +ods = opendnssecinstance.OpenDNSSECInstance(fstore, dm_password, +start_tls=True) if options.dnssec_master: dnssec_masters = ods.get_masters() # we can reinstall current server if it is dnssec master @@ -214,10 +215,13 @@ def main(): bind.create_instance() # on dnssec master this must be installed last -dnskeysyncd = dnskeysyncinstance.DNSKeySyncInstance(fstore, dm_password) +dnskeysyncd = dnskeysyncinstance.DNSKeySyncInstance(fstore, dm_password, +start_tls=True) dnskeysyncd.create_instance(api.env.host, api.env.realm) if options.dnssec_master: -ods_exporter = odsexporterinstance.ODSExporterInstance(fstore, dm_password) +ods_exporter = odsexporterinstance.ODSExporterInstance(fstore, + dm_password, + start_tls=True) ods_exporter.create_instance(api.env.host, api.env.realm) ods.create_instance(api.env.host, api.env.realm) diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index 4e630e8ddfed524d021d19016f48615fc8c0ab9d..a6839f5882d8994b99deee459ecb0160bb47cfef 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@
Re: [Freeipa-devel] [PATCH 0203] Remove unused PRE_SCHEMA upgrade
On 12/03/15 17:08, Rob Crittenden wrote: Martin Basti wrote: On 12/03/15 16:22, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 04:52 PM, Martin Basti wrote: This upgrade step is not used anymore. Required by: https://fedorahosted.org/freeipa/ticket/4904 Patch attached. Looks and works good to me, ACK. Is this going away because one can simply create an update file that exists alphabetically before the schema update? If so then ACK. rob No this never works, and will not work without changes in DS, I was discussing this with DS guys. If you add new replica to schema, the schema has to be there before data replication. Martin That's a rather narrow case though. You could make changes that only affect existing schema, or something in cn=config. rob Let summarize this: * It is unused code * we have schema update to modify schema (is there any extra requirement to modify schema before schema update? I though the schema update replace old schema with new) * it is not usable on new replicas (why to modify up to date schema?, why to modify new configuration?) * we can not use this to update data * only way how we can us this is to change non-replicating data, on current server. However, might there be really need to update cn=config before schema update? Martin -- 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 0203] Remove unused PRE_SCHEMA upgrade
On 03/06/2015 04:52 PM, Martin Basti wrote: This upgrade step is not used anymore. Required by: https://fedorahosted.org/freeipa/ticket/4904 Patch attached. Looks and works good to me, ACK. -- 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
Re: [Freeipa-devel] [PATCH] Password vault
On 3/11/2015 9:12 PM, Endi Sukma Dewata wrote:
Thanks for the review. New patch attached to be applied on top of all
previous patches. Please see comments below.
New patch #362-1 attached replacing #362. It fixed some issues in
handle_not_found().
--
Endi S. Dewata
From 6741138b647427cd3448ff72369dfc6fa21354aa Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Mon, 9 Mar 2015 12:09:20 -0400
Subject: [PATCH] Vault improvements.
The vault plugins have been modified to clean up the code, to fix
some issues, and to improve error handling. The LDAPCreate and
LDAPSearch classes have been refactored to allow subclasses to
provide custom error handling. The test scripts have been updated
accordingly.
https://fedorahosted.org/freeipa/ticket/3872
---
API.txt| 50 ++--
ipalib/plugins/baseldap.py | 35 +--
ipalib/plugins/user.py | 6 +-
ipalib/plugins/vault.py| 273 ++---
ipalib/plugins/vaultcontainer.py | 226 +
ipalib/plugins/vaultsecret.py | 108
ipatests/test_xmlrpc/test_vault_plugin.py | 2 +-
ipatests/test_xmlrpc/test_vaultcontainer_plugin.py | 24 +-
ipatests/test_xmlrpc/test_vaultsecret_plugin.py| 2 +-
9 files changed, 371 insertions(+), 355 deletions(-)
diff --git a/API.txt b/API.txt
index
ffbffa78cde372d5c7027b758be58bf07caebbc6..3a741755ab3e15e0175599a16a090b04d46d6be8
100644
--- a/API.txt
+++ b/API.txt
@@ -4518,7 +4518,7 @@ args: 1,20,3
arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255,
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False,
exclude='webui')
-option: Str('container', attribute=False, cli_name='container',
multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('container_id?', cli_name='container_id')
option: Bytes('data?', cli_name='data')
option: Str('description', attribute=True, cli_name='desc', multivalue=False,
required=False)
option: Str('escrow_public_key_file?', cli_name='escrow_public_key_file')
@@ -4543,7 +4543,7 @@ command: vault_add_member
args: 1,7,3
arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255,
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True,
required=True)
option: Flag('all', autofill=True, cli_name='all', default=False,
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
option: Flag('no_members', autofill=True, default=False, exclude='webui')
option: Flag('raw', autofill=True, cli_name='raw', default=False,
exclude='webui')
@@ -4556,7 +4556,7 @@ command: vault_add_owner
args: 1,7,3
arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255,
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True,
required=True)
option: Flag('all', autofill=True, cli_name='all', default=False,
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
option: Flag('no_members', autofill=True, default=False, exclude='webui')
option: Flag('raw', autofill=True, cli_name='raw', default=False,
exclude='webui')
@@ -4569,7 +4569,7 @@ command: vault_archive
args: 1,15,3
arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255,
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True,
required=True)
option: Flag('all', autofill=True, cli_name='all', default=False,
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
option: Bytes('data?', cli_name='data')
option: Bytes('encryption_key?', cli_name='encryption_key')
option: Str('in?', cli_name='in')
@@ -4589,7 +4589,7 @@ output: PrimaryKey('value', None, None)
command: vault_del
args: 1,3,3
arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255,
multivalue=True, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True,
required=True)
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
option: Flag('continue', autofill=True, cli_name='continue', default=False)
option: Str('version?', exclude='webui')
output: Output('result', type 'dict', None)
@@ -4600,7 +4600,7 @@ args: 1,15,4
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='vault_name',
maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True,
Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes
On 03/04/2015 11:25 AM, Nathan Kinder wrote: On 03/04/2015 10:58 AM, Martin Basti wrote: On 04/03/15 19:56, Nathan Kinder wrote: On 03/04/2015 10:41 AM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/28/2015 01:13 PM, Nathan Kinder wrote: On 02/28/2015 01:07 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 01:18 PM, Nathan Kinder wrote: On 02/27/2015 01:08 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 12:20 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/26/2015 12:55 AM, Martin Kosek wrote: On 02/26/2015 03:28 AM, Nathan Kinder wrote: Hi, The two attached patches address some issues that affect ipa-client-install when syncing time from the NTP server. Now that we use ntpd to perform the time sync, the client install can end up hanging forever when the server is not reachable (firewall issues, etc.). These patches address the issues in two different ways: 1 - Don't attempt to sync time when --no-ntp is specified. 2 - Implement a timeout capability that is used when we run ntpd to perform the time sync to prevent indefinite hanging. The one potentially contentious issue is that this introduces a new dependency on python-subprocess32 to allow us to have timeout support when using Python 2.x. This is packaged for Fedora, but I don't see it on RHEL or CentOS currently. It would need to be packaged there. https://fedorahosted.org/freeipa/ticket/4842 Thanks, -NGK Thanks for Patches. For the second patch, I would really prefer to avoid new dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could use some workaround instead, as in: http://stackoverflow.com/questions/3733270/python-subprocess-timeout I don't like having to add an additional dependency either, but the alternative seems more risky. Utilizing the subprocess32 module (which is really just a backport of the normal subprocess module from Python 3.x) is not invasive for our code in ipautil.run(). Adding some sort of a thread that has to kill the spawned subprocess seems more risky (see the discussion about a race condition in the stackoverflow thread above). That said, I'm sure the thread/poll method can be made to work if you and others feel strongly that this is a better approach than adding a new dependency. Why not use /usr/bin/timeout from coreutils? That sounds like a perfectly good idea. I wasn't aware of it's existence (or it's possible that I forgot about it). Thanks for the suggestion! I'll test out a reworked version of the patch. Do you think that there is value in leaving the timeout capability centrally in ipautil.run()? We only need it for the call to 'ntpd' right now, but there might be a need for using a timeout for other commands in the future. The alternative is to just modify synconce_ntp() to use /usr/bin/timeout and leave ipautil.run() alone. I think it would require a lot of research. One of the programs spawned by this is pkicreate which could take quite some time, and spawning a clone in particular. It is definitely an interesting idea but I think it is safest for now to limit it to just NTP for now. What I meant was that we would have an optional keyword timeout parameter to ipautil.run() that defaults to None, just like my subprocess32 approach. If a timeout is not passed in, we would use subprocess.Popen() to run the specified command just like we do today. We would only actually pass the timeout parameter to ipautil.run() in synconce_ntp() for now, so no other commands would have a timeout in effect. The capability would be available for other commands this way though. Let me propose a patch with this implementation, and if you don't like it, we can leave ipautil.run() alone and restrict the changes to synconce_ntp(). An updated patch 0002 is attached that uses the approach mentioned above. Looks good. Not to nitpick to death but... Can you add timeout to ipaplatform/base/paths.py as BIN_TIMEOUT = /usr/bin/timeout and reference that instead? It's for portability. Sure. I was wondering if we should do something around a full path. And a question. I'm impatient. Should there be a notice that it will timeout after n seconds somewhere so people like me don't ^C after 2 seconds? Or is that just overkill and I need to learn patience? Probably both. :) There's always going to be someone out there who will do ctrl-C, so I think printing out a notice is a good idea. I'll add this. Stylistically, should we prefer p.returncode is 15 or p.returncode == 15? After some reading, it seems that '==' should be used. Small integers work with 'is', but '==' is the consistent way that equality of integers should be checked. I'll modify this. Another updated patch 0002 is attached that addresses Rob's review comments. Thanks, -NGK LGTM. Does someone else have time to test this? I also don't know if there is a policy on
Re: [Freeipa-devel] [PATCHES 0018-0019] ipa-dns-install: Use LDAPI for all DS connections
On 03/11/2015 03:13 PM, Martin Basti wrote: On 11/03/15 13:00, Martin Babinsky wrote: These patches solve https://fedorahosted.org/freeipa/ticket/4933. They are to be applied to master branch. I will rebase them for ipa-4-1 after the review. Thank you for the patches. I have a few comments: IPA-4-1 Replace simple bind with LDAPI is too big change for 4-1, we should start TLS if possible to avoid MINSSF0 error. The LDAPI patches should go only into IPA master branch. You can do something like this: --- a/ipaserver/install/service.py +++ b/ipaserver/install/service.py @@ -107,6 +107,10 @@ class Service(object): if not self.realm: raise errors.NotFound(reason=realm is missing for %s % (self)) conn = ipaldap.IPAdmin(ldapi=self.ldapi, realm=self.realm) +elif self.dm_password is not None: +conn = ipaldap.IPAdmin(self.fqdn, port=389, + cacert=paths.IPA_CA_CRT, + start_tls=True) else: conn = ipaldap.IPAdmin(self.fqdn, port=389) PATCH 0018: 1) please add there more chatty commit message about using LDAPI 2) I do not like much idea of adding 'realm' kwarg into __init__ method of OpenDNSSECInstance IIUC, it is because get_masters() method, which requires realm to use LDAPI. You can just add ods.realm=realm, before call get_master() in ipa-dns-install if options.dnssec_master: +ods.realm=api.env.realm dnssec_masters = ods.get_masters() (Honza will change it anyway during refactoring) PATCH 0019: 1) commit message deserves to be more chatty, can you explain there why you removed kerberos cache? Martin^2 Attaching updated patches. Patch 0018 should go to both 4.1 and master branches. Patch 0019 should go only to master. -- Martin^3 Babinsky From aea965b42ad52b7504dd06b8e62861e1a7be4da1 Mon Sep 17 00:00:00 2001 From: Martin Babinsky mbabi...@redhat.com Date: Wed, 11 Mar 2015 15:37:08 +0100 Subject: [PATCH] ipa-dns-install: use STARTTLS to connect to DS BindInstance et al. now use STARTTLS to set up secure connection to DS during ipa-dns-install. This fixes https://fedorahosted.org/freeipa/ticket/4933 --- install/tools/ipa-dns-install| 12 ipaserver/install/bindinstance.py| 12 ++-- ipaserver/install/dnskeysyncinstance.py | 14 +++--- ipaserver/install/odsexporterinstance.py | 15 +++ ipaserver/install/opendnssecinstance.py | 15 +++ ipaserver/install/service.py | 10 -- 6 files changed, 43 insertions(+), 35 deletions(-) diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install index 11f79f0f9be226aaee8c95deb31e7e21f8a18dbb..2b6ad02abee9428870b0a554f5b4088e77b0e852 100755 --- a/install/tools/ipa-dns-install +++ b/install/tools/ipa-dns-install @@ -151,7 +151,7 @@ def main(): confirm=False, validate=False) if dm_password is None: sys.exit(Directory Manager password required) -bind = bindinstance.BindInstance(fstore, dm_password) +bind = bindinstance.BindInstance(fstore, dm_password, start_tls=True) # try the connection try: @@ -160,7 +160,8 @@ def main(): except errors.ACIError: sys.exit(Password is not valid!) -ods = opendnssecinstance.OpenDNSSECInstance(fstore, dm_password) +ods = opendnssecinstance.OpenDNSSECInstance(fstore, dm_password, +start_tls=True) if options.dnssec_master: dnssec_masters = ods.get_masters() # we can reinstall current server if it is dnssec master @@ -214,10 +215,13 @@ def main(): bind.create_instance() # on dnssec master this must be installed last -dnskeysyncd = dnskeysyncinstance.DNSKeySyncInstance(fstore, dm_password) +dnskeysyncd = dnskeysyncinstance.DNSKeySyncInstance(fstore, dm_password, +start_tls=True) dnskeysyncd.create_instance(api.env.host, api.env.realm) if options.dnssec_master: -ods_exporter = odsexporterinstance.ODSExporterInstance(fstore, dm_password) +ods_exporter = odsexporterinstance.ODSExporterInstance(fstore, + dm_password, + start_tls=True) ods_exporter.create_instance(api.env.host, api.env.realm) ods.create_instance(api.env.host, api.env.realm) diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index 4e630e8ddfed524d021d19016f48615fc8c0ab9d..ca73b43f6da2f0cf3ad8423b24b2ce19062d0df2 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -533,13 +533,13 @@ class DnsBackup(object): class BindInstance(service.Service): -def __init__(self,
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. rob -- 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 0208] Respect --test option in upgrade plugins
On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) -- 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] [PATCHES] SPEC: Require python2 version of sssd bindings
On Thu, 12 Mar 2015, Petr Vobornik wrote:
On 03/06/2015 03:13 PM, Alexander Bokovoy wrote:
On Fri, 06 Mar 2015, Lukas Slebodnik wrote:
On (05/03/15 16:20), Petr Vobornik wrote:
On 03/05/2015 11:23 AM, Lukas Slebodnik wrote:
On (05/03/15 08:54), Petr Vobornik wrote:
On 02/27/2015 09:50 PM, Lukas Slebodnik wrote:
ehlo,
Please review attached patches and fix freeipa in fedora 22 ASAP.
I think the most critical is 1st patch
sh$ git grep SSSDConfig | grep import
install/tools/ipa-upgradeconfig:import SSSDConfig
ipa-client/ipa-install/ipa-client-automount:import SSSDConfig
ipa-client/ipa-install/ipa-client-install:import SSSDConfig
BTW package python-sssdconfig is provides since sssd-1.10.0alpha1
(2013-04-02)
but it was not explicitely required.
The latest python3 changes in sssd (fedora 22) is just a result of
negligent
packaging of freeipa.
LS
Fedora 22 was amended.
Patch 1: ACK
Patch 2: ACK
Patch3:
the package name is libsss_nss_idmap-python not
python-libsss_nss_idmap
which already is required in adtrust package
In sssd upstream we decided to rename package
libsss_nss_idmap-python to
python-libsss_nss_idmap according to new rpm python guidelines.
The python3 version has alredy correct name.
We will rename package in downstream with next major release (1.13).
Of course it we will add Provides: libsss_nss_idmap-python.
We can push 3rd patch later or I can update 3rd patch.
What do you prefer?
Than you very much for review.
LS
Patch 3 should be updated to not forget the remaining change in
ipa-python
package.
It then should be updated downstream and master when 1.13 is released in
Fedora, or in master sooner if SSSD 1.13 becomes the minimal version
required
by master.
Fixed.
BTW Why ther is a pylint comment for some sssd modules
I did not kave any pylint problems after removing comment.
ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401
ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint:
disable=F0401
And why are these modules optional (try except)
Because they are needed to properly load in the case trust subpackages
are not installed, to generate proper messages to users who will try
these commands, like 'ipa trust-add' while the infrastructure is not in
place.
pylint is dumb for such cases.
Alexander, the point was not to require python_nss_idmap and
python-sss-murmur on ipa clients?
Pylint is not used on ipa clients. The import statements do protection
against failed import and that's what we use on the client side.
If so python-sss-murmur should be required only by trust-ad package
and not python package (patch2). And patch 3 (adding
libsss_nss_idmap-python to python package) should not be used.
We already have dependencies in trust-ad subpackage:
%package server-trust-ad
Summary: Virtual package to install packages required for Active Directory
trusts
Group: System Environment/Base
Requires: %{name}-server = %version-%release
Requires: m2crypto
Requires: samba-python
Requires: samba = %{samba_version}
Requires: samba-winbind
Requires: libsss_idmap
Requires: libsss_nss_idmap-python
However, we don't ship the original plugins in this package because
otherwise you wouldn't be able to use 'ipa trust*' from any machine
other than those where trust-ad subpackage is installed. That's why we
use import statements and catch the import exceptions.
--
/ Alexander Bokovoy
--
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] SPEC: Require python2 version of sssd bindings
On Thu, 12 Mar 2015, Alexander Bokovoy wrote:
On Thu, 12 Mar 2015, Petr Vobornik wrote:
On 03/06/2015 03:13 PM, Alexander Bokovoy wrote:
On Fri, 06 Mar 2015, Lukas Slebodnik wrote:
On (05/03/15 16:20), Petr Vobornik wrote:
On 03/05/2015 11:23 AM, Lukas Slebodnik wrote:
On (05/03/15 08:54), Petr Vobornik wrote:
On 02/27/2015 09:50 PM, Lukas Slebodnik wrote:
ehlo,
Please review attached patches and fix freeipa in fedora 22 ASAP.
I think the most critical is 1st patch
sh$ git grep SSSDConfig | grep import
install/tools/ipa-upgradeconfig:import SSSDConfig
ipa-client/ipa-install/ipa-client-automount:import SSSDConfig
ipa-client/ipa-install/ipa-client-install:import SSSDConfig
BTW package python-sssdconfig is provides since sssd-1.10.0alpha1
(2013-04-02)
but it was not explicitely required.
The latest python3 changes in sssd (fedora 22) is just a result of
negligent
packaging of freeipa.
LS
Fedora 22 was amended.
Patch 1: ACK
Patch 2: ACK
Patch3:
the package name is libsss_nss_idmap-python not
python-libsss_nss_idmap
which already is required in adtrust package
In sssd upstream we decided to rename package
libsss_nss_idmap-python to
python-libsss_nss_idmap according to new rpm python guidelines.
The python3 version has alredy correct name.
We will rename package in downstream with next major release (1.13).
Of course it we will add Provides: libsss_nss_idmap-python.
We can push 3rd patch later or I can update 3rd patch.
What do you prefer?
Than you very much for review.
LS
Patch 3 should be updated to not forget the remaining change in
ipa-python
package.
It then should be updated downstream and master when 1.13 is released in
Fedora, or in master sooner if SSSD 1.13 becomes the minimal version
required
by master.
Fixed.
BTW Why ther is a pylint comment for some sssd modules
I did not kave any pylint problems after removing comment.
ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401
ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint:
disable=F0401
And why are these modules optional (try except)
Because they are needed to properly load in the case trust subpackages
are not installed, to generate proper messages to users who will try
these commands, like 'ipa trust-add' while the infrastructure is not in
place.
pylint is dumb for such cases.
Alexander, the point was not to require python_nss_idmap and
python-sss-murmur on ipa clients?
Pylint is not used on ipa clients. The import statements do protection
against failed import and that's what we use on the client side.
If so python-sss-murmur should be required only by trust-ad package
and not python package (patch2). And patch 3 (adding
libsss_nss_idmap-python to python package) should not be used.
We already have dependencies in trust-ad subpackage:
%package server-trust-ad
Summary: Virtual package to install packages required for Active Directory
trusts
Group: System Environment/Base
Requires: %{name}-server = %version-%release
Requires: m2crypto
Requires: samba-python
Requires: samba = %{samba_version}
Requires: samba-winbind
Requires: libsss_idmap
Requires: libsss_nss_idmap-python
However, we don't ship the original plugins in this package because
otherwise you wouldn't be able to use 'ipa trust*' from any machine
other than those where trust-ad subpackage is installed. That's why we
use import statements and catch the import exceptions.
Sent too early.
... and python-sss-murmur shoudl be required by trust-ad subpackage,
yes.
--
/ Alexander Bokovoy
--
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] Generic support for unknown DNS RR types (RFC 3597)
On 11.3.2015 17:02, Martin Kosek wrote: On 03/11/2015 04:55 PM, Petr Spacek wrote: On 11.3.2015 15:45, Martin Kosek wrote: On 03/11/2015 03:38 PM, Petr Spacek wrote: On 11.3.2015 15:28, Martin Kosek wrote: On 03/11/2015 12:43 PM, Petr Spacek wrote: On 11.3.2015 11:34, Jan Cholasta wrote: Dne 11.3.2015 v 11:12 Petr Spacek napsal(a): On 10.3.2015 20:04, Simo Sorce wrote: On Tue, 2015-03-10 at 19:24 +0100, Petr Spacek wrote: On 10.3.2015 18:36, Simo Sorce wrote: On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote: On 10.3.2015 17:35, Simo Sorce wrote: On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote: On 10.3.2015 15:53, Simo Sorce wrote: On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote: Hello, I would like to discuss Generic support for unknown DNS RR types (RFC 3597 [0]). Here is the proposal: LDAP schema === - 1 new attribute: ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) The attribute should be added to existing idnsRecord object class as MAY. This new attribute should contain data encoded according to RFC 3597 section 5 [5]: The RDATA section of an RR of unknown type is represented as a sequence of white space separated words as follows: The special token \# (a backslash immediately followed by a hash sign), which identifies the RDATA as having the generic encoding defined herein rather than a traditional type-specific encoding. An unsigned decimal integer specifying the RDATA length in octets. Zero or more words of hexadecimal data encoding the actual RDATA field, each containing an even number of hexadecimal digits. If the RDATA is of zero length, the text representation contains only the \# token and the single zero representing the length. Examples from RFC: a.example. CLASS32 TYPE731 \# 6 abcd ( ef 01 23 45 ) b.example. HS TYPE62347 \# 0 e.example. IN A \# 4 0A01 e.example. CLASS1 TYPE1 10.0.0.2 Open questions about LDAP format Should we include \# constant? We know that the attribute contains record in RFC 3597 syntax so it is not strictly necessary. I think it would be better to follow RFC 3597 format. It allows blind copypasting from other tools, including direct calls to python-dns. It also eases writing conversion tools between DNS and LDAP format because they do not need to change record values. Another question is if we should explicitly include length of data represented in hexadecimal notation as a decimal number. I'm very strongly inclined to let it there because it is very good sanity check and again, it allows us to re-use existing tools including parsers. I will ask Uninett.no for standardization after we sort this out (they own the OID arc we use for DNS records). Attribute usage === Every DNS RR type has assigned a number [1] which is used on wire. RR types which are unknown to the server cannot be named by their mnemonic/type name because server would not be able to do name-number conversion and to generate DNS wire format. As a result, we have to encode the RR type number somehow. Let's use attribute sub-types. E.g. a record with type 65280 and hex value 0A01 will be represented as: GenericRecord;TYPE65280: \# 4 0A01 CLI === $ ipa dnsrecord-add zone.example owner \ --generic-type=65280 --generic-data='\# 4 0A01' $ ipa dnsrecord-show zone.example owner Record name: owner TYPE65280 Record: \# 4 0A01 ACK? :-) Almost. We should refrain from using subtypes when not necessary, and in this case it is not necessary. Use: GenericRecord: 65280 \# 4 0A01 I was considering that too but I can see two main drawbacks: 1) It does not work very well with DS ACI (targetattrfilter, anyone?). Adding generic write access to GenericRecord == ability to add TLSA records too, which you may not want. IMHO it is perfectly reasonable to limit write access to certain types (e.g. to one from private range). 2) We would need a separate substring index for emulating filters like (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence index which will be handy one day when we decide to handle upgrades like GenericRecord;TYPE256-UriRecord. Another (less important) annoyance is that conversion tools would have to mangle record data instead of just converting attribute name-record type. I can be convinced that subtypes are not necessary but I do not see clear advantage of avoiding them. What is the problem with subtypes? Poor support by most clients, so it is generally discouraged. Hmm, it does not sound like a thing we
Re: [Freeipa-devel] Purpose of default user group
On 10.3.2015 16:01, Jakub Hrozek wrote: On Tue, Mar 10, 2015 at 03:52:44PM +0100, Martin Kosek wrote: On 03/10/2015 03:27 PM, Rob Crittenden wrote: Petr Vobornik wrote: Hi, I would like to ask what is a purpose of a default user group - by default ipausers? Default group is also a required field in ipa config. To be able to apply some (undefined) group policy to all users. I'm not aware that it has ever been used for this. I would also interested in the use cases, especially given all the pain we have with ipausers and large user bases. Especially that for current policies (SUDO, HBAC, SELinux user policy), we always have other means to specify all users. yes, but those means usually specify both AD and IPA users, right? I always thought ipausers is a handy shortcut for selecting IPA users only and not AD users. I always thought that ipausers is an equivalent of domain users in AD world (compare with Trusted domain users). In my admin life I considered domain users to be useful alias for real authenticated user accounts (compare with Everyone = even unauthenticated access, Authenticated users = includes machine accounts too.) Moreover, getting rid of ipausers does not help with 'big groups problem' in any way. E.g. at university you are almost inevitably going to have groups like 'students' which will contain more than 90 % of users anyway. -- 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] Purpose of default user group
Alexander Bokovoy wrote: On Tue, 10 Mar 2015, Petr Spacek wrote: On 10.3.2015 16:01, Jakub Hrozek wrote: On Tue, Mar 10, 2015 at 03:52:44PM +0100, Martin Kosek wrote: On 03/10/2015 03:27 PM, Rob Crittenden wrote: Petr Vobornik wrote: Hi, I would like to ask what is a purpose of a default user group - by default ipausers? Default group is also a required field in ipa config. To be able to apply some (undefined) group policy to all users. I'm not aware that it has ever been used for this. I would also interested in the use cases, especially given all the pain we have with ipausers and large user bases. Especially that for current policies (SUDO, HBAC, SELinux user policy), we always have other means to specify all users. yes, but those means usually specify both AD and IPA users, right? I always thought ipausers is a handy shortcut for selecting IPA users only and not AD users. I always thought that ipausers is an equivalent of domain users in AD world (compare with Trusted domain users). In my admin life I considered domain users to be useful alias for real authenticated user accounts (compare with Everyone = even unauthenticated access, Authenticated users = includes machine accounts too.) Moreover, getting rid of ipausers does not help with 'big groups problem' in any way. E.g. at university you are almost inevitably going to have groups like 'students' which will contain more than 90 % of users anyway. For what use we need this distinction in IPA itself? - ACI (permissions) have separate notion to describe anonymous/any authenticated dichotomy - HBAC has 'all' category for users which in HBAC context means all authenticated users Where else we would need ipausers other than default POSIX group which we are not using it for? Petr's point is that deleting ipausers is a short-term solution that ignores the underlying problem. But yeah, ipausers is a solution looking for a problem AFAIK. It was a future-proofing move because if we ever decided we needed on, slurping in all the users at once and adding to some common group would be time-consuming. rob -- 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] Purpose of default user group
On 03/10/2015 05:08 PM, Rob Crittenden wrote: Alexander Bokovoy wrote: On Tue, 10 Mar 2015, Petr Spacek wrote: On 10.3.2015 16:01, Jakub Hrozek wrote: On Tue, Mar 10, 2015 at 03:52:44PM +0100, Martin Kosek wrote: On 03/10/2015 03:27 PM, Rob Crittenden wrote: Petr Vobornik wrote: Hi, I would like to ask what is a purpose of a default user group - by default ipausers? Default group is also a required field in ipa config. To be able to apply some (undefined) group policy to all users. I'm not aware that it has ever been used for this. I would also interested in the use cases, especially given all the pain we have with ipausers and large user bases. Especially that for current policies (SUDO, HBAC, SELinux user policy), we always have other means to specify all users. yes, but those means usually specify both AD and IPA users, right? I always thought ipausers is a handy shortcut for selecting IPA users only and not AD users. I always thought that ipausers is an equivalent of domain users in AD world (compare with Trusted domain users). In my admin life I considered domain users to be useful alias for real authenticated user accounts (compare with Everyone = even unauthenticated access, Authenticated users = includes machine accounts too.) Moreover, getting rid of ipausers does not help with 'big groups problem' in any way. E.g. at university you are almost inevitably going to have groups like 'students' which will contain more than 90 % of users anyway. For what use we need this distinction in IPA itself? - ACI (permissions) have separate notion to describe anonymous/any authenticated dichotomy - HBAC has 'all' category for users which in HBAC context means all authenticated users Where else we would need ipausers other than default POSIX group which we are not using it for? Petr's point is that deleting ipausers is a short-term solution that ignores the underlying problem. But yeah, ipausers is a solution looking for a problem AFAIK. It was a future-proofing move because if we ever decided we needed on, slurping in all the users at once and adding to some common group would be time-consuming. I wonder if it would help if these special groups do not have explicit members defined, but are more descriptive. Something like DS Dynamic Groups [1]. If we could define - ipausers are all users in this container having this objectclass and DS and SSSD would take care of the rest. I am not sure if it would help with performance, it would be easier at least for managing the membership. I am also not sure how would we create the group for AD users. [1] https://fedorahosted.org/389/ticket/128 -- 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] Purpose of default user group
On Tue, 2015-03-10 at 16:01 +0100, Jakub Hrozek wrote: On Tue, Mar 10, 2015 at 03:52:44PM +0100, Martin Kosek wrote: On 03/10/2015 03:27 PM, Rob Crittenden wrote: Petr Vobornik wrote: Hi, I would like to ask what is a purpose of a default user group - by default ipausers? Default group is also a required field in ipa config. To be able to apply some (undefined) group policy to all users. I'm not aware that it has ever been used for this. I would also interested in the use cases, especially given all the pain we have with ipausers and large user bases. Especially that for current policies (SUDO, HBAC, SELinux user policy), we always have other means to specify all users. yes, but those means usually specify both AD and IPA users, right? I always thought ipausers is a handy shortcut for selecting IPA users only and not AD users. We should probably turn ipausers into a fully virtual group that is added to the user's Authorization data in the KDC (MS-PAC or in future PAD). This way it will be possible to reference it in sssd but will not create issues with memberships in the server. But we need the PAD first, I guess. (we could do something with authentication indicators too, but that would be a hack). 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
