[Freeipa-devel] Mirroring from pagure to github is broken

[Martin Basti]

https://pagure.io/fedora-infrastructure/issue/5946

Mirroring is broken, please make sure you pushed commits to both pagure 
and github.



Martin^2

--
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] Announcing FreeIPA 4.3.3

[Martin Basti]

Release date: 2017-03-23

The FreeIPA team would like to announce FreeIPA 4.3.3 release!

It can be downloaded from http://www.freeipa.org/page/Downloads.

Please note that this is the last upstream release of FreeIPA 4.3.x branch.

This announcement is also available at 
<http://www.freeipa.org/page/Releases/4.3.3>.



== Highlights in 4.3.3 ==
=== Enhancements ===
=== Known Issues ===
=== Bug fixes ===
FreeIPA 4.3.3 is a stabilization release for the features delivered as a
part of 4.3.0. There are more than 20 bug-fixes which details can be seen in
the list of resolved tickets below.

== Upgrading ==
Upgrade instructions are available on [[Upgrade]] page.

== Feedback ==
Please provide comments, bugs and other feedback via the freeipa-users 
mailing

list (http://www.redhat.com/mailman/listinfo/freeipa-users) or #freeipa
channel on Freenode.

== Resolved tickets ==
* 6774 FreeIPA client <= 4.4 fail to parse 4.5 cookies
* 6561 CVE-2016-7030 freeipa: ipa: DoS attack against kerberized 
services by abusing password policy
* 6560 CVE-2016-9575 freeipa: ipa: Insufficient permission check in 
certprofile-mod

* 6485 Document make_delete_command method in UserTracker
* 6378 Tests: Fix failing sudo test
* 6317 backport #6213 Incorrect test for 
DNSForwardPolicyConflictWithEmptyZone warning in test_xmlrpc/test_dns_plugin
* 6316 backport #6199 Received ACIError instead of DuplicatedError in 
stageuser_tests

* 6311 Fix or remove the  `LDAPUpdate.update_from_dict` method
* 6287 Refer to nodes in TestWrongClientDomain replica promotion tests 
as replicas
* 6284 Tests: avoid skipping tests because of missing files when running 
as outoftree

* 6278 Use OAEP padding with custodia (to avoid CVE-2016-6298)
* 6262 Fix integration sudo tests setup and checks
* 6254 kinit_admin raises an exception if server uninstallation is 
called from test teardown with server not installed

* 6244 build: add python-libsss_nss_idmap and python-sss to BuildRequires
* 6205 The ipa-server-upgrade command failed when named-pkcs11 does not 
happen to run during dnf upgrade

* 6177 ca-less test are broken - invalid usage of ipautil.run
* 6167 Incorrect domainlevel info in tests
* 6166 Subsequent external CA installation fails
* 6147 Failing automember tests due to manager output normalization
* 6134 Command "ipa-replica-prepare" not allowed to create line 
replication topology
* 6120 ipa-adtrust-install: when running with --netbios-name="", the 
NetBIOS name is changed without notification

* 6076 Mulitple domain Active Directory Trust conflict
* 6056 custodia.conf and server.keys file is world-readable.
* 6016 ipa-ca-install on replica tries to connect to master:8443
* 5696 Add conflicts with bind-chroot to spec.
== Detailed changelog since 4.3.2 ==
=== Alexander Bokovoy (5) ===
* ipa-kdb: search for password policies globally
* ipa-kdb: simplify trusted domain parent search
* trust: make sure ID range is created for the child domain even if it 
exists

* trust: automatically resolve DNS trust conflicts for triangle trusts
* ipaserver/dcerpc: reformat to make the code closer to pep8

=== Christian Heimes (3) ===
* Use RSA-OAEP instead of RSA PKCS#1 v1.5
* Secure permissions of Custodia server.keys
* RedHatCAService should wait for local Dogtag instance

=== David Kupka (1) ===
* password policy: Add explicit default password policy for hosts and 
services


=== Fraser Tweedale (2) ===
* certprofile-mod: correctly authorise config update
* cert-revoke: fix permission check bypass (CVE-2016-5404)

=== Ganna Kaihorodova (1) ===
* Fix for integration tests replication layouts

=== Jan Cholasta (2) ===
* Revert "spec: add conflict with bind-chroot to freeipa-server-dns"
* install: fix external CA cert validation

=== Lenka Doudova (7) ===
* Document make_delete_command method in UserTracker
* Tests: Fix integration sudo test
* Tests: Fix integration sudo tests setup and checks
* Tests: Avoid skipping tests due to missing files
* Raise error when running ipa-adtrust-install with empty netbios--name
* Tests: Fix failing automember tests
* Tests: Remove DNS configuration from trust tests

=== Martin Babinsky (1) ===
* add python-libsss_nss_idmap and python-sss to BuildRequires

=== Martin Basti (5) ===
* Become IPA 4.3.3
* Update Contributors.txt
* Raise DuplicatedEnrty error when user exists in delete_container
* Catch DNS exceptions during emptyzones named.conf upgrade
* Start named during configuration upgrade.

=== Oleg Fayans (3) ===
* Changed addressing to the client hosts to be replicas
* Disabled raiseonerr in kinit call during topology level check
* Fixed incorrect domainlevel determination in tests

=== Peter Lacko (1) ===
* Test URIs in certificate.

=== Petr Spacek (3) ===
* Tests: fix test_forward_zones in test_xmlrpc/test_dns_plugin
* DNS server upgrade: do not fail when DNS server did not respond
* Fix ipa-replica-prepare's error message about missing local CA instance

=== Petr Vobornik (1

[Freeipa-devel] Announcing FreeIPA 4.4.4

[Martin Basti]

Release date: 2017-03-23

The FreeIPA team would like to announce FreeIPA 4.4.4 release!

It can be downloaded from http://www.freeipa.org/page/Downloads. Builds for
Fedora 24 will be available in the official COPR repository 
<https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-4-4/>.


This announcement is also available 
at<http://www.freeipa.org/page/Releases/4.4.4>.



== Highlights in 4.4.4 ==
=== Enhancements ===
=== Known Issues ===
=== Bug fixes ===
FreeIPA 4.4.4 is a stabilization release for the features delivered as a
part of 4.4.0.

== Upgrading ==
Upgrade instructions are available on [[Upgrade]] page.

== Feedback ==
Please provide comments, bugs and other feedback via the freeipa-users 
mailing

list (http://www.redhat.com/mailman/listinfo/freeipa-users) or #freeipa
channel on Freenode.

== Resolved tickets ==
* 6776 krb5 1.15 broke DAL principal free
* 6738 Ipa-kra-install fails with weird output when backspace is used 
during typing Directory Manager password
* 6713 ipa: Insufficient permission check for ca-del, ca-disable and 
ca-enable commands (CVE-2017-2590)

* 6647 batch param compatibility is incorrect
* 6608 IPA server installation should check if IPv6 stack is enabled
* 6600 Legacy client tests doesn't have tree domain role.
* 6588 replication race condition prevents IPA to install
* 6575 ipa-replica-install fails on requesting DS cert when master is 
not configured with IPv6
* 6070 ipa-replica-install fails to install when resolv.conf incomplete 
entries

== Detailed changelog since 4.4.3 ==
=== Alexander Bokovoy (1) ===
* ipa-kdb: support KDB DAL version 6.1

=== David Kupka (1) ===
* ipapython.ipautil.nolog_replace: Do not replace empty value

=== Florence Blanc-Renaud (1) ===
* Do not configure PKI ajp redirection to use "::1"

=== Fraser Tweedale (2) ===
* ca: correctly authorise ca-del, ca-enable and ca-disable
* Set up DS TLS on replica in CA-less topology

=== Ganna Kaihorodova (1) ===
* Tests: Add tree root domain role in legacy client tests

=== Jan Cholasta (1) ===
* compat: fix `Any` params in `batch` and `dnsrecord`

=== Martin Basti (7) ===
* Become IPA 4.4.4
* Update Contributors.txt
* FreeIPA 4.4.4 translations
* Bump python-dns to improve processing of non-complete resolv.conf
* Use proper logging for error messages
* Wait until HTTPS principal entry is replicated to replica
* wait_for_entry: use only DN as parameter

=== Stanislav Laznicka (2) ===
* Add debug log in case cookie retrieval went wrong
* Fix cookie with Max-Age processing

=== Tomas Krizek (1) ===
* server install: require IPv6 stack to be enabled

=== Thorsten Scherf (1) ===
* added ssl verification using IPA trust anchor

-- 
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] [DRAFT] release notes FreeIPA 4.4.4

[Martin Basti]

Please check the draft of the release notes for FreeIPA 4.4.4 release: 
http://www.freeipa.org/page/Releases/4.4.4


Martin^2

--
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] [DRAFT] release notes FreeIPA 4.3.3

[Martin Basti]

Please check the draft of the release notes for FreeIPA 4.3.3 release: 
http://www.freeipa.org/page/Releases/4.3.3


Martin^2

--
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] Announcing FreeIPA 4.5.0

[Martin Basti]

ers to the files used by Travis CI
* Travis CI: use specific Python version during build
* introduce install step to .travis.yml and cache pip installs
* split out lint to a separate Travis job
* Travis: offload test execution to a separate script
* Travis CI: a separate script to run test tasks
* Put the commands informing and displaying build logs on single line
* travis: mark FreeIPA as python project
* Bump up ipa-docker-test-runner version
* Add a basic test suite for `kadmin.local` interface
* Make `kadmin` family of functions return the result of ipautil.run
* gracefully handle setting replica bind dn group on old masters
* add missing attribute to ipaca replica during CA topology update
* Revert "upgrade: add replica bind DN group check interval to CA
topology config"
* bindinstance: use data in named.conf to determine configuration status
* Use ipa-docker-test-runner to run tests in Travis CI
* Configuration file for ipa-docker-test-runner
* Add 'env_confdir' to constants
* Fix pep-8 transgressions in ipalib/misc.py
* Make `env` and `plugins` commands local again
* Revert "Add 'ipa localenv' subcommand"
* Enhance __repr__ method of Principal
* replication: ensure bind DN group check interval is set on replica config
* upgrade: add replica bind DN group check interval to CA topology config
* Improve the robustness FreeIPA's i18n module and its tests
* Use common procedure to setup initial replication in both domain levels
* ensure that the initial sync using GSSAPI works agains old masters
* replication: refactor the code setting principals as replica bind DNs
* replication: augment setup_promote_replication method
* Turn replication manager group into ReplicationManager class member
* Fix the naming of ipa-dnskeysyncd service principal
* installutils: remove 'install_service_keytab' function
* domain-level agnostic keytab retrieval in httpinstance
* installers: restart DS after KDC is configured
* dsinstance: use keytab retrieval method from parent class
* use DM credentials to retrieve service keytab only in DLO
* Service: common method for service keytab requests
* Turn Kerberos-related properties to Service class members
* Make service user name a class member of Service
* service installers: clean up the inheritance
* fix incorrect invocation of ipa-getkeytab during DL0 host enrollment
* do partial host enrollment in domain level 0 replica install
* Separate function to purge IPA host principals from keytab
* certs: do not re-create NSS database when requesting service cert
* initialize empty /etc/http/alias during server/replica install
* CertDB: add API for non-destructive initialization from PKCS#12 bundle
* test_ipagetkeytab: use system-wide IPA CA cert location in tests
* Extend keytab retrieval test suite to cover new options
* Modernize ipa-getkeytab test suite
* extend ipa-getkeytab to support other LDAP bind methods
* ipa-getkeytab: expose CA cert path as option
* server-del: fix incorrect check for one IPA master
* Revert "Fix install scripts debugging"
* do not use keys() method when iterating through dictionaries
* remove trailing newlines form python modules
* mod_nss: use more robust quoting of NSSNickname directive
* Move character escaping function to ipautil
* Make Continuous installer continuous only during execution phase
* use separate exception handlers for executors and validators
* ipa passwd: use correct normalizer for user principals
* trust-fetch-domains: contact forest DCs when fetching trust domain info
* netgroup: avoid extraneous LDAP search when retrieving primary key from DN
* advise: Use `name` instead of `__name__` to get plugin names
* Use Travis-CI for basic sanity checks
* ldapupdate: Use proper inheritance in BadSyntax exception
* raise ValidationError when deprecated param is passed to command
* Always fetch forest info from root DCs when establishing one-way trust
* factor out `populate_remote_domain` method into module-level function
* Always fetch forest info from root DCs when establishing two-way trust

=== Martin Basti (134) ===
* Become IPA 4.5.0
* Update 4.5 translations
* Add copy-schema-to-ca for RHEL6 to contrib/
* Remove copy-schema-to-ca.py from master branch
* pylint: bump dependency to version >= 1.6
* backup: backup anonymous keytab
* tests: use --setup-kra in tests
* KRA: add --setup-kra to ipa-server-install
* man: add missing --setup-adtrust option to manpage
* ipactl restart: log httplib failues as debug
* Tests: search for disabled users
* Test: DNS nsupdate from dns-update-system-records
* DNS: dns-update-system-record can create nsupdate file
* py3: ipa_generate_password: do not compare None and Int
* py3: change_admin_password: use textual mode
* py3: create DNS zonefile: use textual mode
* py3: upgradeinstance: use bytes literals with LDIF operations
* py3: upgradeinstance: decode data before storing them as backup...
* py3: upgradeinstance: open dse.ldif in textual mode
* custodia: kem.set_keys: 

Re: [Freeipa-devel] [DRAFT] Release notes FreeIPA 4.5.0

[Martin Basti]

On 15.03.2017 00:49, Fraser Tweedale wrote:
> On Tue, Mar 14, 2017 at 01:51:19PM +0100, Martin Basti wrote:
>> Hello,
>>
>> DRAFT for FreeIPA 4.5.0 release notes is ready
>> http://www.freeipa.org/page/Releases/4.5.0
>>
>> Please update/let me know what is missing, what is extra.
>>
>>
>> Martin^2
>>
> I think we should add https://pagure.io/freeipa/issue/2614 to the
> `Enhancements' section.  There is no design page for it but it was a
> big effort and it gives the deployer complete control over the IPA
> CA subject DN (previously this was very restricted).
>
> Thanks,
> Fraser

Can you suggest what to write to release notes (preferably in copy paste
form)

thank you :)



signature.asc
Description: OpenPGP digital signature
-- 
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] [DRAFT] Release notes FreeIPA 4.5.0

[Martin Basti]

On 14.03.2017 14:50, Jakub Hrozek wrote:
> On Tue, Mar 14, 2017 at 01:51:19PM +0100, Martin Basti wrote:
>> Hello,
>>
>> DRAFT for FreeIPA 4.5.0 release notes is ready
>> http://www.freeipa.org/page/Releases/4.5.0
>>
>> Please update/let me know what is missing, what is extra.
> Please update this paragraph:
> 
> AD User Short Names
>
> Support for AD users short names has been added. Short
> names can be enabled from CLI by setting ipa config-mod
> --domain-resolution-order="domain.test:ad.domain1.test:ad.domain2.test"
> or from WebUI under Configuration tab. No manual configuration on SSSD
> side is required.
> 
>
> With a note that this feature is not supported by SSSD yet and the work
> is tracked with https://pagure.io/SSSD/sssd/issue/3210
>
I updated that section. Shouldn't we remove it completely from release
notes because it will not work until new SSSD is released?



signature.asc
Description: OpenPGP digital signature
-- 
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] [DRAFT] Release notes FreeIPA 4.5.0

[Martin Basti]

On 14.03.2017 15:06, Alexander Bokovoy wrote:
> On ti, 14 maalis 2017, Luc de Louw wrote:
>> My 3 cents...
>>
>> "Please note that FIPS 140-2 support may not work on some platforms"
>>
>> -> Does is work in Fedora? Should be worth mention it so people are
>> more encouraged to test it in Fedora before its getting to RHEL 7.4
> I think we should actually add an explicit statement for trust to AD not
> currently supporting FIPS 140-2 mode.
>
I will add it to known issues



signature.asc
Description: OpenPGP digital signature
-- 
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] [DRAFT] Release notes FreeIPA 4.5.0

[Martin Basti]

On 14.03.2017 14:56, Luc de Louw wrote:
> My 3 cents...
>
> "Please note that FIPS 140-2 support may not work on some platforms"
>
> -> Does is work in Fedora? Should be worth mention it so people are
> more encouraged to test it in Fedora before its getting to RHEL 7.4
>
> Thanks,
>
> Luc

We cannot guarantee that FIPS mode will work with fedora, any package
update may break it.

>
>
>
> On 03/14/2017 02:50 PM, Jakub Hrozek wrote:
>> On Tue, Mar 14, 2017 at 01:51:19PM +0100, Martin Basti wrote:
>>> Hello,
>>>
>>> DRAFT for FreeIPA 4.5.0 release notes is ready
>>> http://www.freeipa.org/page/Releases/4.5.0
>>>
>>> Please update/let me know what is missing, what is extra.
>>
>> Please update this paragraph:
>> 
>> AD User Short Names
>>
>> Support for AD users short names has been added. Short
>> names can be enabled from CLI by setting ipa config-mod
>> --domain-resolution-order="domain.test:ad.domain1.test:ad.domain2.test"
>> or from WebUI under Configuration tab. No manual configuration on SSSD
>> side is required.
>> 
>>
>> With a note that this feature is not supported by SSSD yet and the work
>> is tracked with https://pagure.io/SSSD/sssd/issue/3210
>>
>




signature.asc
Description: OpenPGP digital signature
-- 
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] [DRAFT] Release notes FreeIPA 4.5.0

[Martin Basti]

On 14.03.2017 15:08, Florence Blanc-Renaud wrote:
> On 03/14/2017 01:51 PM, Martin Basti wrote:
>> Hello,
>>
>> DRAFT for FreeIPA 4.5.0 release notes is ready
>> http://www.freeipa.org/page/Releases/4.5.0
>>
>> Please update/let me know what is missing, what is extra.
>>
>>
>> Martin^2
>>
>>
>>
>>
> Hi Martin,
>
> thank you for the release notes. Could you update the section about
> Certificate Identity Mapping?
> '''
> Certificate Identity Mapping
>
> Support for multiple certificates on Smart cards has been added. User
> can choose which certificate is used to authenticate. This allows to
> define multiple certificates per user.
> The same certificate can be used by different accounts, and the
> mapping between a certificate and an account can be done through
> binary match of the whole certificate or a match on custom certificate
> attributes (such as Subject + Issuer).
> '''
>
> I also noted a typo:
> '''
> Bug fixes
> Contains all bugfixes and enhacements
> '''
> should be enhancements.
>
> Thank you,
> Flo
>
>

Thank you, updated



signature.asc
Description: OpenPGP digital signature
-- 
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] [DRAFT] Release notes FreeIPA 4.5.0

[Martin Basti]

Hello,

DRAFT for FreeIPA 4.5.0 release notes is ready
http://www.freeipa.org/page/Releases/4.5.0

Please update/let me know what is missing, what is extra.


Martin^2




signature.asc
Description: OpenPGP digital signature
-- 
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] Please review: V4/AD user short names design draft

[Martin Basti]

On 07.03.2017 15:41, Martin Babinsky wrote:
> On Tue, Mar 07, 2017 at 04:34:42PM +0200, Alexander Bokovoy wrote:
>> On ti, 07 maalis 2017, Simo Sorce wrote:
>>> On Tue, 2017-03-07 at 09:38 +0100, Martin Babinsky wrote:
 On 03/06/2017 01:48 PM, Simo Sorce wrote:
> On Mon, 2017-03-06 at 07:47 +0100, Martin Babinsky wrote:
>> On 03/02/2017 02:54 PM, Simo Sorce wrote:
>>> On Thu, 2017-03-02 at 08:10 +0100, Martin Babinsky wrote:
 In this case it would probably be a good idea to think about "forward
 compatibility" and define a new AUX objectclass bringing in
 'ipaDomainResolutionOrder' instead of extending two separate
 objectclasses. In this way we may the just extend whathever object we
 desire to carry the override in an easy and clean way.
>>> I agree.
>>> Simo.
>>>
>> Now the most difficult question remains... How to name this objectclass.
>> I personally am out of ideas but will try my best to come up with
>> something meaningful.
> Try to describe what the option ultimately does with as few words as
> possible.
>
> Simo.
>
>
 I was thinking about this and since we are performing name qualification
 (short-name -> fully-qualified name incl. domain/realm part), I would
 like to propose the following naming schema:

 objectlasses: ( OID_TBD NAME ipaNameQualificationData Desc 'data used
 for short name qualification data' SUP top AUXILIARY MAY
 (ipaNameQualificationDomainList) X-ORIGIN 'IPA 4.5' )

 attributeTypes: ( OID_TBD NAME 'ipaNameQualificationDomainList' DESC
 'List of domains used to qualify user short name' EQUALITY
 caseIgnoreIA5Match SINGLE-VALUE SYNTAX 1.3.6.1.4.1.1466.115.121.1.26
 X-ORIGIN 'IPA v4.5' )

 Let me know if you are ok with this or am I overengineering the names?

 I would like to solve this quickly so that I can finish the design and
 start implementation.
>>> I was thinking that we can use acronyms here to make it less of a
>>> mouthful and also more easily recognizable:
>>> My idea is:
>>> - ipaNameQualificationData -> ipaFQDNPolicies
>>> - ipaNameQualificationDomainList -> ipaFQDNCheckOrder
>> Sounds good to me.
>> -- 
>> / Alexander Bokovoy
> I am not sure about the relation of this to any policy, but I guess that is
> just nitpicking.
>
> I will wait awhile for others to object and then update design.
>
I agree to not use "policy" in the name
Martin^2



signature.asc
Description: OpenPGP digital signature
-- 
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] Please review: V4/AD user short names design draft

[Martin Basti]

On 01.03.2017 17:04, Simo Sorce wrote:

On Wed, 2017-03-01 at 16:47 +0100, Martin Babinsky wrote:

On 03/01/2017 04:32 PM, Simo Sorce wrote:

On Wed, 2017-03-01 at 16:17 +0100, Martin Babinsky wrote:

On 03/01/2017 03:42 PM, Simo Sorce wrote:

On Tue, 2017-02-28 at 13:29 +0100, Martin Babinsky wrote:

Hello list,

I have put together a draft of design page describing server-side
implementation of user short name -> fully-qualified name resolution.[1]

In the end I have taken the liberty to change a few aspects of the
design we have agreed on before and I will be grad if we can discuss
them further.

Me and Honza have discussed the object that should hold the domain
resolution order and given the fact that IPA domain can also be a part
of this list, we have decided that this information is no longer bound
to trust configuration and should be a part of the global config instead.

Also we have purposefully cut down the API only to a raw manipulation of
the attribute using an option of `ipa config-mod`. The reasons for this
are twofold:

* the developer resources are quite scarce and it may be good to
follow YAGNI[2] principle to implement the dumbest API now and not to
invest into more high-level interface unless there is a demand for it

* we can imagine that the manipulation of the domain resolution order
is a rare operation (ideally only once all trusts are established), so I
am not convinced that it is worth investing into designing higher-level API

I propose we first develop the "dumber" parts first to unblock the SSSD
part. If we have spare cycle afterwards then we can design and implement
more bells-and-whistles afterwards.

[1] https://www.freeipa.org/page/V4/AD_User_Short_Names
[2] https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it

Thank you Martin,
this is a good initial proposal.

I have a few issues with this design:
- It conflates the idea of ordering with the idea of shortening user
names

I fail to see where the conflation takes place. The ordered list is
stored on the server. The client then uses it to expand short names. I
guess I am just missing something.

The attribute is called ipaNTDomainResolutionOrder, nothing in that
attribute says anything about making names become short names.
If it were ipaNTShortNameDomainResolutionOrder then it would be
specific, as it is it seem just to refer to the order in which domain
are resolved, but that is somethign we want in order to determine which
domains SSSD is going to make use short names too, not just the order in
which domains are resolved.
I hope this makes it clearer.


- It allows only for one setting for all the machines, no way to treat
different groups of machines differently


Yes it was discussed that the setting will be global. I would implement
local overrides only when there is a demand for the feature given
development time is short.

Demand is immediate, and it is obvious IMO.


Such demand was not made clear during previous discussions and was not
mentioned by SSSD guys either AFAIK.

I guess this is why we do reviews :-)


The first one is probably just a matter of using a more specific name
for the new attribute, or, perhaps not use a new attribute at all but
just use ipaConfigString with an agreed syntax like:
ipaConfigString: Domains Use Short Name List: aaa bbb ccc ddd

The side effect of using ipaConfigString is that we can set this on
older servers too, so people do not have to upgrade their servers to use
this. Old servers will not have any validation, but that is ok, sssd
must be prepared to receive a bad list and deal with it appropriately
anyway.


No more 'ipaConfigString' attribute values, please. Me and everyone else
fixing e.g. replication issues can relate to the pain of doing CRUD
operations involving them.

ipaConfigString was devised explicitly so that configuration options
could be added without replication issues because the string can be
accepted by any server version.
So what replication issues are there ?
What has CRUD to do with it ?


Well consider client doing a) retrieve ipaDomainResolutionOrder and
split it by delimiter, or b) retrieve values of ipaConfigString, iterate
until you find one that starts with "Domains Use Short Name list:",
strip off the rest of the value and split it by delimiter.

I do not see any problem with this.

I disagree,

ipaConfigString evokes that this is IPA configuration, but AFAIK the 
SSSD is the consumer of data and it is unrelated to configuration of IPA 
server. If you plan to extend usage of 'ipaDomainResolutionOrder' to 
more entries than one, then is better to have separate attribute that 
allows better LDAP searches (debugging, support).
Why SSSD instead of downloading the exact attribute content should do a 
parsing of messy values that can be inside ipaConfigString?


Why we suddenly plan to support older servers with a new feature? In 
past access to new features required to upgrade freeipa, why we should 
increase complexity of code and ldap 

Re: [Freeipa-devel] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

[Martin Basti]

On 28.02.2017 12:38, Lukas Slebodnik wrote:

On (28/02/17 12:17), Martin Basti wrote:


On 28.02.2017 12:03, Petr Vobornik wrote:

On 02/28/2017 12:00 PM, Petr Vobornik wrote:

On 02/27/2017 12:46 PM, Petr Vobornik wrote:

Hello list,

today and tomorrow a migration of FreeIPA issue tracker[1] and git repo
will take place.

It is due to FedoraHosted sunset [2]. Both will be migrated to
pagure.io
[3].

During this migration it won't be possible to add new tickets and
comments to Trac or Pagure.

[1] https://fedorahosted.org/freeipa/
[2]
https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
[3] https://pagure.io/

Thank you for understanding,

Issue tracker and git repo were migrated. They can be used now.

https://pagure.io/freeipa

Additional steps will follow
- redirection of old URLs to new
- sync with github


Also we need to setup rights for the repo.

I've created group 'freeipa'. My proposal is to add all people who had
git commit rights to the group. Set the group to have 'commit' right on
'freeipa' pagure project.

Former admins can be added as admins to the project directly.

Martin2 is working on setting up sync with Git Hub:
- https://pagure.io/fedora-infrastructure/issue/5844


and

https://pagure.io/fedora-infrastructure/issue/5845

Please do NOT push to old repository, for users of ipatool change your
repositories to pagure and would be good to postpone pushing until mirroring
to github is enabled.


The best is to asg on fedora-infrastructure to chown the git repo on
fedorahosted, so no one can push changes there.

LS


Petr1 has a reason why it cannot be done, something with copr IIRC

--
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] Migration of FreeIPA issue tracker - Trac and git repo to pagure.io

[Martin Basti]

On 28.02.2017 12:03, Petr Vobornik wrote:

On 02/28/2017 12:00 PM, Petr Vobornik wrote:

On 02/27/2017 12:46 PM, Petr Vobornik wrote:

Hello list,

today and tomorrow a migration of FreeIPA issue tracker[1] and git repo
will take place.

It is due to FedoraHosted sunset [2]. Both will be migrated to 
pagure.io

[3].

During this migration it won't be possible to add new tickets and
comments to Trac or Pagure.

[1] https://fedorahosted.org/freeipa/
[2]
https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
[3] https://pagure.io/

Thank you for understanding,


Issue tracker and git repo were migrated. They can be used now.

https://pagure.io/freeipa

Additional steps will follow
- redirection of old URLs to new
- sync with github



Also we need to setup rights for the repo.

I've created group 'freeipa'. My proposal is to add all people who had 
git commit rights to the group. Set the group to have 'commit' right 
on 'freeipa' pagure project.


Former admins can be added as admins to the project directly.

Martin2 is working on setting up sync with Git Hub:
- https://pagure.io/fedora-infrastructure/issue/5844



and

https://pagure.io/fedora-infrastructure/issue/5845

Please do NOT push to old repository, for users of ipatool change your 
repositories to pagure and would be good to postpone pushing until 
mirroring to github is enabled.


Martin^2

--
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] MD5 certificate fingerprints removal

[Martin Basti]

On 24.02.2017 08:46, Tomas Krizek wrote:

On 02/24/2017 08:34 AM, Standa Laznicka wrote:

On 02/24/2017 08:29 AM, Jan Cholasta wrote:

On 23.2.2017 19:06, Martin Basti wrote:


On 23.02.2017 15:09, Tomas Krizek wrote:

On 02/22/2017 01:44 PM, Fraser Tweedale wrote:

On Wed, Feb 22, 2017 at 01:41:22PM +0100, Tomas Krizek wrote:

On 02/22/2017 12:28 AM, Fraser Tweedale wrote:

On Tue, Feb 21, 2017 at 05:23:07PM +0100, Standa Laznicka wrote:

On 02/21/2017 04:24 PM, Tomas Krizek wrote:

On 02/21/2017 03:23 PM, Rob Crittenden wrote:

Standa Laznicka wrote:

Hello,

Since we're trying to make FreeIPA work in FIPS we got to
the point
where we need to do something with MD5 fingerprints in the
cert plugin.
Eventually we came to a realization that it'd be best to get
rid of them
as a whole. These are counted by the framework and are not
stored
anywhere. Note that alongside with these fingerprints SHA1
fingerprints
are also counted and those are there to stay.

The question for this ML is, then - is it OK to remove these
or would
you rather have them replaced with SHA-256 alongside the
SHA-1? MD5 is a
grandpa and I think it should go.

I based the values displayed on what certutil displayed at
the time (7
years ago). I don't know that anyone uses these fingerprints.
The
OpenSSL equivalent doesn't include them by default.

You may be able to deprecate fingerprints altogether.

rob

I think it's useful to display the certificate's fingerprint.
I'm in
favor of removing md5 and adding sha256 instead.


Rob, thank you for sharing the information of where the cert
fingerprints
are originated! `certutil` shipped with nss-3.27.0-1.3
currently displays
SHA-256 and SHA1 fingerprints for certificates so I propose
going that way
too.


IMO we should remove MD5 and SHA-1, and add SHA-256. But we should
also make no API stability guarantee w.r.t. the fingerprint
attributes, i.e. to allow us to move to newer digests in future
(and
remove broken/no-longer-secure ones).  We should advise that if a
customer has a hard requirement on a particular digest that they
should compute it themselves from the certificate.

Cheers,
Fraser

What is the motivation to remove SHA-1? Are there any attacks
besides
theoretical ones on SHA-1?

Do other libraries already deprecate SHA-1?


Come to think of it, I was thinking about SHA-1 signatures (which
are completely forbidden in the public PKI nowadays).  But for
fingerprints it is not so bad (for now).

Thanks,
Fraser

Actually, there's been a practical SHA1 attack just published [1].
Computational complexity was
9,223,372,036,854,775,808 SHA1 computations, which takes about 110
years
on a single GPU.

Therefore, I'm in favor to deprecate SHA1 as well and provide only
SHA256.

[1] - https://shattered.io/




I think we should wait with removal SHA1, don't remove it prematurely.
As MD5 is deprecated for very long time, SHA1 is not and we are not
using it for any cryptographic operation nor certificates. It is just
informational fingerprint.

+1


+1, I don't favour the
http://new2.fjcdn.com/gifs/Everybody_d72014_61779.gif approach.


People will most likely be using the software even years after its
upstream release, so I think its best to address these issues sooner
rather than later.

SHA256 fingerprints should be added even if we decide to keep SHA1 for now.


+1 for adding SHA256

--
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] MD5 certificate fingerprints removal

[Martin Basti]

On 23.02.2017 15:09, Tomas Krizek wrote:

On 02/22/2017 01:44 PM, Fraser Tweedale wrote:

On Wed, Feb 22, 2017 at 01:41:22PM +0100, Tomas Krizek wrote:

On 02/22/2017 12:28 AM, Fraser Tweedale wrote:

On Tue, Feb 21, 2017 at 05:23:07PM +0100, Standa Laznicka wrote:

On 02/21/2017 04:24 PM, Tomas Krizek wrote:

On 02/21/2017 03:23 PM, Rob Crittenden wrote:

Standa Laznicka wrote:

Hello,

Since we're trying to make FreeIPA work in FIPS we got to the point
where we need to do something with MD5 fingerprints in the cert plugin.
Eventually we came to a realization that it'd be best to get rid of them
as a whole. These are counted by the framework and are not stored
anywhere. Note that alongside with these fingerprints SHA1 fingerprints
are also counted and those are there to stay.

The question for this ML is, then - is it OK to remove these or would
you rather have them replaced with SHA-256 alongside the SHA-1? MD5 is a
grandpa and I think it should go.

I based the values displayed on what certutil displayed at the time (7
years ago). I don't know that anyone uses these fingerprints. The
OpenSSL equivalent doesn't include them by default.

You may be able to deprecate fingerprints altogether.

rob

I think it's useful to display the certificate's fingerprint. I'm in
favor of removing md5 and adding sha256 instead.


Rob, thank you for sharing the information of where the cert fingerprints
are originated! `certutil` shipped with nss-3.27.0-1.3 currently displays
SHA-256 and SHA1 fingerprints for certificates so I propose going that way
too.


IMO we should remove MD5 and SHA-1, and add SHA-256.  But we should
also make no API stability guarantee w.r.t. the fingerprint
attributes, i.e. to allow us to move to newer digests in future (and
remove broken/no-longer-secure ones).  We should advise that if a
customer has a hard requirement on a particular digest that they
should compute it themselves from the certificate.

Cheers,
Fraser

What is the motivation to remove SHA-1? Are there any attacks besides
theoretical ones on SHA-1?

Do other libraries already deprecate SHA-1?


Come to think of it, I was thinking about SHA-1 signatures (which
are completely forbidden in the public PKI nowadays).  But for
fingerprints it is not so bad (for now).

Thanks,
Fraser

Actually, there's been a practical SHA1 attack just published [1].
Computational complexity was
9,223,372,036,854,775,808 SHA1 computations, which takes about 110 years
on a single GPU.

Therefore, I'm in favor to deprecate SHA1 as well and provide only SHA256.

[1] - https://shattered.io/





I think we should wait with removal SHA1, don't remove it prematurely. 
As MD5 is deprecated for very long time, SHA1 is not and we are not 
using it for any cryptographic operation nor certificates. It is just 
informational fingerprint.
-- 
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] Release: script for updating contributors

[Martin Basti]

On 23.02.2017 12:31, Martin Kosek wrote:

Hi all,

Based on my recent Contributors.txt update and on Martin Basti's request in the
pull request:
https://github.com/freeipa/freeipa/pull/493#issuecomment-281938080

I added my (hacky) script for updating the file in the freeipa-tools repo and
updated our Release page:

http://www.freeipa.org/page/Release#Update_Contributors.txt

HTH!



+1, Thank you!

--
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] [INFO] Freeipa/freeipa-master copr repo required for FreeIPA from master branch

[Martin Basti]

Hello,

from now you need freeipa/freeipa-master copr repo to run IPA built from 
master branch (at least on F25/F24) due bind and bind-dyndb-ldap packages.


Sorry for inconvenience.

Martin^2

--
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] [design] add nsupdate output format to dns-update-system-records

[Martin Basti]

On 30.01.2017 07:27, Jan Cholasta wrote:

On 27.1.2017 22:45, Petr Vobornik wrote:

On 01/27/2017 01:37 PM, Martin Basti wrote:

Hello all,

I'm working on adding support of nsupdate format to output of `ipa
dns-update-system-records` command, that will allow to copy 
output

to nsupdate utility and update IPA DNS records on external server. I
propose following:

1) new option --nsupdate-format

This option will replace standard output with nsupdate format output.
Feel free to propose better name.


2) client or server side plugin?

I'd like to implement this behavior only at client side. But this will
not work in cases when only server api or RPC calls are used. Do you
think that we should support this uses case (in server side plugin)?


Martin




The main ideal was to allow piping into nsupdate. i don't remember if
server side returns in human readable form or in structured way. If in
structured, then implementation in client side plugin would make better
sense.

+1 to Alexander on '--out nsupdate' option


+1 to both --out and client-side implementation.



I see agreement here so PR should land soon :)

Martin^2

--
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] [design] add nsupdate output format to dns-update-system-records

[Martin Basti]

Hello all,

I'm working on adding support of nsupdate format to output of `ipa 
dns-update-system-records` command, that will allow to copy output 
to nsupdate utility and update IPA DNS records on external server. I 
propose following:


1) new option --nsupdate-format

This option will replace standard output with nsupdate format output. 
Feel free to propose better name.



2) client or server side plugin?

I'd like to implement this behavior only at client side. But this will 
not work in cases when only server api or RPC calls are used. Do you 
think that we should support this uses case (in server side plugin)?



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] Stageuser API

[Martin Basti]

On 17.01.2017 12:38, Christian Heimes wrote:

On 2017-01-16 15:52, David Kupka wrote:

Hello everyone!

I've noticed that our API for stageuser is missing some commands that
user has (stageuser-{add,remove}-{principal,cert}). I was wondering if
there is reason for it but after asking some fellows developers it seems
that there's none.

I understand the stageuser area as a place where user entry can be
created and amended during the hiring process in organization, example:

1. HR creates the entry with just basic informations (givenname,
surname, manager)
2. IT assigns basic account information (uid, gid)
3. based on to-be-employee manager's request IT adds additional group
membership (memberOf)
4. based on to-be-employee request IT adds login alias (krbPrincipalName)
5. Security Officer adds certificate from Smart Card assigned to the
to-be-employee
6. HR adds extra information to the account (address, marital status, ...)
7. Facilities update work place related information (seat number, phone
number, ...)
8. At the first day IT activates the user account.

Considering this work flow I think it might be useful to have the same
API for stageuser as for the user.

Does the example work flow make sense?
Should we provide the same set of commands for user and stageuser?

I see one potential issue in your proposal. A stage user does not
reserve its user name. The unique index on uid excludes the staging user
and deleted user branch. Therefore it is possible to create a user with
the same login name as a staging user.

We either have to ensure that this name clash does not cause any trouble
with certificates or we have to enforce uniqueness of uid across the
whole tree. For FreeIPA it's probably fine because we compare certs
bytes. Third party applications parse the cert subject instead and use
the subject to identify a user.

Christian





AFAIK the non-uniques of stageuser and user names causes more pain than 
gain, this is not the first case when we have an issue with that. Maybe 
we should reevaluate this behavior and enforce uid uniqueness with 
stageusers too.


Note: we explicitly updated uniqueness plugin to allow conflicting names 
but I don't remember the reason from top of my head.


Martin^2
-- 
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] using Reviewer field on Github instead of Trac

[Martin Basti]

On 09.12.2016 09:05, Petr Spacek wrote:

Dear FreeIPA developers,

I just noticed that Github PRs now have Reviewers field.

Can we replace reviewed-by field in Trac with Reviewers field on Github? It is
easier to set myself as Reviewer on Github as it does not force me to edit
ticket. Assuming the Github workflow works, the ipatool could pick up this
value automatically and add it to commit messages as needed...

Given that Trac is going away is sounds like good move to me :-)



I tried it yesterday, but reviewers are not shown in list of PRs 
https://github.com/freeipa/freeipa/pulls


So you have to still assign yourself (as assignee) to PR to show others 
that PR has reviewer.


Martin^2

--
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] [freeipa PR#314][edited] RFC: privilege separation for ipa framework code

[Martin Basti]

On 08.12.2016 22:47, Simo Sorce wrote:

On Thu, 2016-12-08 at 21:46 +0100, simo5 wrote:

URL: https://github.com/freeipa/freeipa/pull/314
Author: simo5
  Title: #314: RFC: privilege separation for ipa framework code
Action: edited

  Changed field: body
Original value:
"""
As part of the External Authentication work this PR implements the privilege 
separation portion of the design available here: 
https://www.freeipa.org/page/V4/External_Authentication and implements tickets: 
https://fedorahosted.org/freeipa/ticket/5959 and 
https://fedorahosted.org/freeipa/ticket/4189

The update process from an old server has not been implemented yet, so this is 
just an RFC request at this stage. Please look at the code and let me know if 
you notice any major issue with it so we can correct mistakes early.

This PR depends on improvements and fixes to two dependencies: mod_auth_gssapi 
and gssproxy, which are not released/accepted upstream yet (all PRs filed, and 
will be available soon).
In order to allow trying the code, I made two copr repos with the necessary 
changes available here:
- https://copr.fedorainfracloud.org/coprs/simo/mod_auth_gssapi/
- https://copr.fedorainfracloud.org/coprs/simo/gssproxy/

I tested a new install and both gssapi as well as password authentication work 
(via command line and web browser). I have not tested OTP authentication yet.

There are 2 fundamental changes in this code:
- the session handling code has been dropped in favor of deferring session 
handling to mod_auth_gssapi, simplifying the code greatly. As part of this 
change we stop using memcached.
- the framework configuration is changed to work as a different user from the 
Apache framework and depends on gssproxy in order to be able to access 
necessary credentials. (Apache itself is also using gssproxy and does not have 
direct access to the HTTP keytab.)
   This required two changes in the form-based authentication workflow:
   * The armor cache is obtained via anonymous pkinit as we do not have access 
anymore to the HTTP keytab. This means this PR depends on #62 (until it is 
accepted commits from that PR are in this PR)
   * The actual authentication is done via a loopback HTTP request to apache 
after we obtain a TGT, this is done in order to obtain a session cookie from 
mod_auth_gssapi as well as to be able to immediately discard the TGT and just 
keep the HTTP ticket instead.

@jcholast @pvoborni Please provide comments on the framework changes.
@rcritten @abbra do you have ideas on how to deal with dropping a service 
(memcached) on upgrade ?
"""

--
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

There seem to be a bug in the mailing list posting script when someone
edits a PR description, I see the original text here but not the new
text!

Simo.


It is expected,

 Changed field: body
Original value:

I just haven't had time to implement sending a new values (because of format, 
of github messages it is not so simple) I may try to finish github notification 
RFEs today

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] NTP in FreeIPA

[Martin Basti]

On 24.11.2016 20:31, Gabe Alford wrote:
On Thu, Nov 24, 2016 at 9:14 AM, Martin Basti <mba...@redhat.com 
<mailto:mba...@redhat.com>> wrote:




On 24.11.2016 16:11, Gabe Alford wrote:

On Thu, Nov 24, 2016 at 1:29 AM, Martin Basti <mba...@redhat.com
<mailto:mba...@redhat.com>> wrote:



On 24.11.2016 07:06, David Kupka wrote:

On 22/11/16 23:15, Gabe Alford wrote:

I would say that it is worth keeping in FreeIPA. I
know myself and some
customers use its functionality by having the clients
sync to the IPA
servers and have the servers sync to the NTP source.
This way if the NTP
source ever gets disrupted for long periods of time
(which has happened in
my environment) the client time drifts with the
authentication source. This
is the way that AD often works and is configured.


Hello Gabe,
I agree that it's common practice to synchronize all
nodes in network with single source in order to have the
same time and save bandwidth. Also I understand that it's
comfortable to let FreeIPA installer take care of it.
But I don't think FreeIPA should do it IMO this is job
for Ansible or similar tool. Also the problem is that in
some situations FreeIPA installer makes it worse.

Example:

1. Install FreeIPA server (ipa1.example.org
<http://ipa1.example.org>)
2. Install FreeIPA client on all nodes in network
3. Install replica (ipa2.example.org
<http://ipa2.example.org>) of FreeIPA server to increase
redundancy


Why not have NTP look at a _srv_records?


Do ntpclients support this natively?  I just found some ugly hacks
for chrony, i.e extra service that is dynamically changing config
file.
But yes this may be way too, but dirty.


You are right. It is an ugly. I wonder if we can push to make it not 
so ugly so that _srv_ is used for both Chrony and NTP which IMO makes 
those two products better. If not and the desire is truly to get rid 
of chrony/ntp configuration on the client side, what about adding 
Chrony and NTP configuration to ipa-advise?


And I realized that this may be applicable only if IPA is installed with 
integrated DNS, when IPA automatically updates system services DNS 
records. With external DNS we will bother admins to create SRV records, 
so it is the same as creating DHCP configuration.


we can add it to ipa-advise.

Martin^2


Now all the clients have ipa1.example.org
<http://ipa1.example.org> as the only server in
/etc/ntp.conf. If the first FreeIPA server becomes
unreachable all clients will be able to contact KDC on
the other server thanks to DNS autodiscovery in libkrb5
but will be unable to synchronize time.


This can be resolved by DHCP configured NTP. When NTP server
changed, you just change DHCPd config and hosts conf will be
synced.
We may keep NTP on IPA server side configured, but I'm voting
for removing it from clients and document+endorse people to
use DHCP (anyway distros have always enabled some time
synchronization so it should naturally work without even in
small deployments)


If NTP is still configured on the IPA server, this may be less of
an issue. Not everyone has/is/will be using ansible. Also in
secure environments, DHCP
is not allowed/used at all.

Also NTP is somehow incompatible with containers, usually
containers have time synchronized from host, and by default
IPA client container don't do NTP configuration.


Isn't that what the --no-ntp option in the client is for anyway?


Let deprecate it in 4.5

Martin^2




On Tue, Nov 22, 2016 at 7:05 AM, Jan Cholasta
<jchol...@redhat.com <mailto:jchol...@redhat.com>> wrote:

On 22.11.2016 13:06, Petr Spacek wrote:

On 22.11.2016 12:15, David Kupka wrote:

Hello everyone!

Is it worth to keep configuring NTP in
FreeIPA?

In usual environment there're no special
requirements for time
synchronization
and the distribution default (be it ntpd,
chrony or anything else) will
just
work. Any tampering with the
configuration can't make it any better.

In environment with spe

Re: [Freeipa-devel] NTP in FreeIPA

[Martin Basti]

On 24.11.2016 16:11, Gabe Alford wrote:
On Thu, Nov 24, 2016 at 1:29 AM, Martin Basti <mba...@redhat.com 
<mailto:mba...@redhat.com>> wrote:




On 24.11.2016 07:06, David Kupka wrote:

On 22/11/16 23:15, Gabe Alford wrote:

I would say that it is worth keeping in FreeIPA. I know
myself and some
customers use its functionality by having the clients sync
to the IPA
servers and have the servers sync to the NTP source. This
way if the NTP
source ever gets disrupted for long periods of time (which
has happened in
my environment) the client time drifts with the
authentication source. This
is the way that AD often works and is configured.


Hello Gabe,
I agree that it's common practice to synchronize all nodes in
network with single source in order to have the same time and
save bandwidth. Also I understand that it's comfortable to let
FreeIPA installer take care of it.
But I don't think FreeIPA should do it IMO this is job for
Ansible or similar tool. Also the problem is that in some
situations FreeIPA installer makes it worse.

Example:

1. Install FreeIPA server (ipa1.example.org
<http://ipa1.example.org>)
2. Install FreeIPA client on all nodes in network
3. Install replica (ipa2.example.org
<http://ipa2.example.org>) of FreeIPA server to increase
redundancy


Why not have NTP look at a _srv_records?


Do ntpclients support this natively?  I just found some ugly hacks for 
chrony, i.e extra service that is dynamically changing config file.

But yes this may be way too, but dirty.



Now all the clients have ipa1.example.org
<http://ipa1.example.org> as the only server in /etc/ntp.conf.
If the first FreeIPA server becomes unreachable all clients
will be able to contact KDC on the other server thanks to DNS
autodiscovery in libkrb5 but will be unable to synchronize time.


This can be resolved by DHCP configured NTP. When NTP server
changed, you just change DHCPd config and hosts conf will be synced.
We may keep NTP on IPA server side configured, but I'm voting for
removing it from clients and document+endorse people to use DHCP
(anyway distros have always enabled some time synchronization so
it should naturally work without even in small deployments)


If NTP is still configured on the IPA server, this may be less of an 
issue. Not everyone has/is/will be using ansible. Also in secure 
environments, DHCP

is not allowed/used at all.

Also NTP is somehow incompatible with containers, usually
containers have time synchronized from host, and by default IPA
client container don't do NTP configuration.


Isn't that what the --no-ntp option in the client is for anyway?


Let deprecate it in 4.5

Martin^2




On Tue, Nov 22, 2016 at 7:05 AM, Jan Cholasta
<jchol...@redhat.com <mailto:jchol...@redhat.com>> wrote:

On 22.11.2016 13:06, Petr Spacek wrote:

On 22.11.2016 12:15, David Kupka wrote:

Hello everyone!

Is it worth to keep configuring NTP in FreeIPA?

In usual environment there're no special
requirements for time
synchronization
and the distribution default (be it ntpd,
chrony or anything else) will
just
work. Any tampering with the configuration
can't make it any better.

In environment with special requirements
(network disconnected from
public
internet, nodes disconnected from topology for
longer time, ...) time
synchronization must be taken care of
accordingly by system
administrator and
FreeIPA simply can't help here.

Also there are problems and weird behavior
with the current FreeIPA
installers:

* ipa-client-install replaces all servers in
/etc/ntp.conf with the ones
specified by user or resolved from DNS. If
none were provided nor
resolved the
FreeIPA server specified/resolved during
installation it used. This
leads in
just single server in the configuration and no
   

Re: [Freeipa-devel] NTP in FreeIPA

[Martin Basti]

On 24.11.2016 07:06, David Kupka wrote:

On 22/11/16 23:15, Gabe Alford wrote:

I would say that it is worth keeping in FreeIPA. I know myself and some
customers use its functionality by having the clients sync to the IPA
servers and have the servers sync to the NTP source. This way if the NTP
source ever gets disrupted for long periods of time (which has 
happened in
my environment) the client time drifts with the authentication 
source. This

is the way that AD often works and is configured.


Hello Gabe,
I agree that it's common practice to synchronize all nodes in network 
with single source in order to have the same time and save bandwidth. 
Also I understand that it's comfortable to let FreeIPA installer take 
care of it.
But I don't think FreeIPA should do it IMO this is job for Ansible or 
similar tool. Also the problem is that in some situations FreeIPA 
installer makes it worse.


Example:

1. Install FreeIPA server (ipa1.example.org)
2. Install FreeIPA client on all nodes in network
3. Install replica (ipa2.example.org) of FreeIPA server to increase 
redundancy


Now all the clients have ipa1.example.org as the only server in 
/etc/ntp.conf. If the first FreeIPA server becomes unreachable all 
clients will be able to contact KDC on the other server thanks to DNS 
autodiscovery in libkrb5 but will be unable to synchronize time.




This can be resolved by DHCP configured NTP. When NTP server changed, 
you just change DHCPd config and hosts conf will be synced.
We may keep NTP on IPA server side configured, but I'm voting for 
removing it from clients and document+endorse people to use DHCP (anyway 
distros have always enabled some time synchronization so it should 
naturally work without even in small deployments)


Also NTP is somehow incompatible with containers, usually containers 
have time synchronized from host, and by default IPA client container 
don't do NTP configuration.


Let deprecate it in 4.5

Martin^2




On Tue, Nov 22, 2016 at 7:05 AM, Jan Cholasta  
wrote:



On 22.11.2016 13:06, Petr Spacek wrote:


On 22.11.2016 12:15, David Kupka wrote:


Hello everyone!

Is it worth to keep configuring NTP in FreeIPA?

In usual environment there're no special requirements for time
synchronization
and the distribution default (be it ntpd, chrony or anything else) 
will

just
work. Any tampering with the configuration can't make it any better.

In environment with special requirements (network disconnected from
public
internet, nodes disconnected from topology for longer time, ...) time
synchronization must be taken care of accordingly by system
administrator and
FreeIPA simply can't help here.

Also there are problems and weird behavior with the current FreeIPA
installers:

* ipa-client-install replaces all servers in /etc/ntp.conf with 
the ones

specified by user or resolved from DNS. If none were provided nor
resolved the
FreeIPA server specified/resolved during installation it used. This
leads in
just single server in the configuration and no time 
synchronization when

this
server is down/decommissioned.

* ipa-client-install replaces the NTP configuration. If there was any
parts
previously edited by system administrator it's lost.

* ipa-server-install adds {0-4}.$PLATFORM.pool.ntp.org to 
/etc/ntp.conf.

What's the point in doing that? These servers're already in the
configuration
file installed with ntp package.

I have NTP-related WIP patches that solve some of the issues but in
general I
would prefer to remove the whole thing together with documenting 
"Please

make
sure that time on all FreeIPA servers and clients is synchronized. On
most
distributions this was already done during system installation."

Can we mark NTP options deprecated in 4.5 and remove them and stop
touching
any time syncing service in 4.6?



Considering that default config is just fine for normal cases, and 
given

how
poorly integrated it is into FreeIPA, I agree with David. FreeIPA 
should

get
out of configuration management business.



+1

--
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










--
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] Removing ipa.pot file from git tree

[Martin Basti]

Hello list,

we plan to remove ipa.pot file from git tree, as this is file can be 
generated from code during build time, and it is required only for 
pushing sources to Zanata. Does anybody remember reason why this file 
was added into git tree?


Note: Translated strings (*.po files) will remain in git tree.


If nobody is against it will be removed from git today, as it creates 
only huge diffs all the time without any extra value.



Martin^2

--
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] Design document: Integration Improvements

[Martin Basti]

On 11.11.2016 15:25, Christian Heimes wrote:

Hello,

I have released the first version of a new design document. It describes
how I'm going to improve integration of FreeIPA's client libraries
(ipalib, ipapython, ipaclient, ipaplatform) for third party developers.

http://www.freeipa.org/page/V4/Integration_Improvements

Regards,
Christian





Hello, I have a few questions:

1) dynamic platform files

Currently all RHEL/fedora-derived platforms work with the same 
rhel/fedora packages. How do you want to achieve this with dynamic 
platform files, do you want to keep mappings between platforms and 
platform file? What about distributions that have in /etc/release just mess?


2) if I understand correctly, you want to separate client installer code 
and client CLI code. In past we had freeipa-admintools but it was 
removed because it was really tightly bounded to installed client. Do 
you want to revive it and make it independent?


3) why instead of environ variable we cannot have specified paths with 
priority where IPA config can be located?

For example:
1) ./.ipa.conf
2) ~/.ipa.conf
3) /etc/ipa/default.conf  <-- as last resort
-- 
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] [Test][Patch-0047] Added a test for Ticket N 5964

[Martin Basti]

On 10.11.2016 10:06, Oleg Fayans wrote:



On 11/10/2016 09:43 AM, Martin Basti wrote:




ACK


On the other hand, make it a conditional one. The link in the comment
does not work. Please fix that.


--
Milan Kubik





--
Milan Kubik


After offline discussion and some clarification, the comment is 
right. ACK


--
Milan Kubik


Because patches are scattered over this thread, am I right that those
versions should be pushed?

freeipa-ofayans-0047.7-Automated-clean-ruv-subcommand-tests.patch
freeipa-ofayans-0048.4-Automated-ipa-replica-manage-del-tests.patch


Precisely!



Martin^2



Pushed to:
master: dc58f8f2a17adc642ae6f32fe9c9eb05d993c9d0
ipa-4-4: ddfa173488aa903b3e028f7e6328dbb4dcc21695

--
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] [Test][Patch-0047] Added a test for Ticket N 5964

[Martin Basti]

ACK

On the other hand, make it a conditional one. The link in the comment 
does not work. Please fix that.


--
Milan Kubik





--
Milan Kubik



After offline discussion and some clarification, the comment is right. ACK

--
Milan Kubik


Because patches are scattered over this thread, am I right that those 
versions should be pushed?


freeipa-ofayans-0047.7-Automated-clean-ruv-subcommand-tests.patch
freeipa-ofayans-0048.4-Automated-ipa-replica-manage-del-tests.patch

Martin^2
-- 
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] [Test][Patch-0047] Added a test for Ticket N 5964

[Martin Basti]

almost ACK, but the ticket in commit message is closed as invalid. So 
I'm quite puzzled now what to do.



On 03.11.2016 13:28, Oleg Fayans wrote:

ping for review

On 10/19/2016 04:54 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. Fixed both issues.

$ ipa-run-tests test_integration/test_topology.py -k TestCASpecificRUVs
WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
Permission denied: 'lextab.py'
WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
denied: 'yacctab.py'
 


test session starts
= 



platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder-0.5, multihost-1.0
collected 5 items

test_integration/test_topology.py ..

 


2 passed in 2444.84 seconds
= 





On 10/17/2016 07:05 PM, Martin Basti wrote:

1)

you don't need to disable/enable dirsrv, just stop/start. Please remove
disable/enable parts


2)




traceback





self = 

def test_delete_ruvs(self):
"""
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
Test_Plan#Test_case:_clean-ruv_subcommand
"""
replica = self.replicas[0]
master = self.master
res1 = master.run_command(['ipa-replica-manage', 'list-ruv',
'-p',
master.config.dirman_password])

assert(res1.stdout_text.count(replica.hostname) == 2 and

   "Certificate Server Replica Update Vectors" in res1), (
"CA-specific RUVs are not displayed")
E   TypeError: argument of type 'SSHCommand' is not iterable

test_integration/test_topology.py:215: TypeError



entering PDB






/usr/lib/python2.7/site-packages/ipatests/test_integration/test_topology.py(215)test_delete_ruvs() 





-> assert(res1.stdout_text.count(replica.hostname) == 2 and



On 14.10.2016 11:36, Oleg Fayans wrote:

Right you are! I am sorry.

On 10/13/2016 06:10 PM, Martin Basti wrote:

I think that you forgot to squash commits. Patch 47 doesn't apply


On 13.10.2016 14:01, Oleg Fayans wrote:

Hi Martin,

Thanks for the review.
With disabling directory server it works as well, thanks for the 
hint.

Also I moved the cleanup logic to the test itself for the sake of
simplicity. Patch-0048 was not changed

On 10/12/2016 02:35 PM, Martin Basti wrote:

1)

Can you just turn off dirsrv on replica instead of doing iptables
magic?


2) NACK

No more eval() ever in code, use 'getattr', 'get' or whatever in 
the

object that can be used.

+evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:

Hi Martin,

After extensive discussion with Ludwig, I finally got the clue on
how
does this feature work. When we uninstall the replica, the master
cleans the replication agreements with this replica and
automatically
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the
replica,
the replica's RUVs get recreated on master (replication 
works!). So,

the only way to test the clean-ruv subcommand is to turn off the
replica, or block the traffic on it so it gets inaccessible to
updates
from master.
The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended_to_handle_CA-specific_RUVs 







[2]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand 







On 08/05/2016 06:36 PM, Martin Basti wrote:



On 03.08.2016 14:45, Oleg Fayans wrote:

Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:



On 08.07.2016 15:41, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:



On 27.06.2016 13:53, Oleg Fayans wrote:

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get
reviewed
before
4.4 release? They cover a good part of the Managed Topology
4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:

One more test was added to the patch-0048

On 06/17/2016 09:43 AM, Oleg Fayans wrote:

Fixed a bug in the previous patch, automated 2 more
testcases
from
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan 










On 06/16/2016 04:46 PM, Oleg Fayans wrote:









IIUC, this will turn off the machine completely, how is 
cleanup

done
then.  AFAIK our tests cannot turn on machine again and run
cleanup, so
you will not be abl

Re: [Freeipa-devel] [Test][Patch-0049, 0050] Certs in ID overrides test

[Martin Basti]

LGTM


On 03.11.2016 09:42, Oleg Fayans wrote:

One more ping for review

On 10/27/2016 02:21 PM, Oleg Fayans wrote:

ping for review

On 10/25/2016 10:24 AM, Oleg Fayans wrote:

Integration part of the tests is ready. 2 tests:

1. Adds a cert to idoverride of a windows user
2. sssd part - looks up user by his certificate using dbus-sssd

Second and third dbus call are executed as a string insted of as array
of strings because it just does not work otherwise. Some quote escaping
gets screwed probably, but the system returns "Error
org.freedesktop.DBus.Error.UnknownInterface: Unknown interface" if the
command is executed using the standard array-based approach

The run looks like this:

bash-4.3$ ipa-run-tests test_integration/test_idviews.py --pdb
WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
Permission denied: 'lextab.py'
WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
denied: 'yacctab.py'
 test session starts

platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder-0.5, multihost-1.0
collected 2 items

test_integration/test_idviews.py ..

 2 passed in 948.44 seconds
=


On 10/21/2016 10:54 AM, Oleg Fayans wrote:

Added one more test, resolved the pep8 issues

On 10/19/2016 12:32 PM, Oleg Fayans wrote:

Hi Martin,

As you suggested, I've extended the
test_xmlrpc/test_add_remove_cert_cmd.py to contain basic tests for
certs
in idoverrides.
The integration part still needs some polishing in the part 
related to

user lookup by cert

On 10/14/2016 03:57 PM, Martin Babinsky wrote:

On 10/14/2016 03:48 PM, Oleg Fayans wrote:

So, did I understand correctly, that there would be 2 patches: one
containing test for basic idoverrides functionality without
AD-integration, and the second one - with AD-integration and an 
sssd

check, correct?
I guess, the
freeipa-ofayans-0050.1-Automated-test-for-certs-in-idoverrides-feature.patch 







might be a good candidate for the first one, I only have to change
the
filename to test_idviews.py, right?



Oleg, we already have XMLRPC tests for idoverrides:

ipatests/test_xmlrpc/test_idviews_plugin.py

Is there any particular reason why not to extend them with add
cert/remove cert operations?

Even better, you can extend
`ipatests/test_xmlrpc/test_add_remove_cert_cmd.py` suite by doing 
the

same set of tests on idoverrideuser objects.

Or am I missing something?


On 09/15/2016 10:32 AM, Martin Basti wrote:



On 15.09.2016 10:10, Oleg Fayans wrote:

Hi Martin,

The file was renamed. Did I understand correctly that for now we
are
leaving the test as is and are planning to extend it later?


I would like to have there SSSD check involved, please use what
Summit
recommends. No new test cases.

And this can be done by separate patch, I want to have API/CLI
certificate override tests for non-AD idview (extending current
tests I
posted in this thread)

Martin^2


On 09/15/2016 09:49 AM, Martin Basti wrote:



On 14.09.2016 18:53, Sumit Bose wrote:

On Wed, Sep 14, 2016 at 06:03:37PM +0200, Martin Basti wrote:


On 14.09.2016 17:53, Alexander Bokovoy wrote:

On Wed, 14 Sep 2016, Martin Basti wrote:


On 14.09.2016 17:41, Alexander Bokovoy wrote:

On Wed, 14 Sep 2016, Martin Basti wrote:

1)
I still don't see the reason why AD trust is needed. 
Default

trust ID view is added just by ipa-adtrust-install, adding
trust is not needed for current implementation. You don't
need AD for this, IDviews is generic feature not just for
AD. Is that user configured on AD side?

You cannot add non-AD user to 'default trust view', so you
will
not be
able to set up certificates to ID override which does not
exist.

For non-'default trust view' you can add both IPA and AD
users,
so using
some other view and then assign certificate for a ID
override in
that
one.

Ok then, but anyway I would like to see API/CLI tests for 
this

feature with proper output validation.


How can be this tested with SSSD?

You need to log into the system with a certificate...

Is this possible from test? We are logged remotely as root, is
there any
cmdline util which allows us to test certificate against AD
user?


You can use 'sss_ssh_authorizedkeys aduser@ad.domain' which
should
return the ssh key derived from the public key in the
certificate.
This
should work for certificate stored in AD as well as for
overrides.

You can also you the DBus lookup by certificate as described in
https://fedorahosted.org/sssd/wiki/DesignDocs/LookupUsersByCertificate 







.

HTH

bye,
Sumit


Thank you Alexander and Summit for hints.

Oleg I realized we don't have any other idviews integration 
tests


So I propose to rename test file you are adding to
test_idviews.py. We
can add more

[Freeipa-devel] Github: rebases and commits order

[Martin Basti]

FYI: when you change order of commits using `git rebase` github doesn't 
respect this and shows commits in UI based on author date ordering.


https://github.com/isaacs/github/issues/386


It is just for your information, we started using bigger amount of 
commits and surprise, surprise UI shows commits in PR almost random 
order due rebases (git show works fine locally).


ipatool has been already fixed to push in correct order (please pull the 
latest version)



Martin^2

--
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] 0102-0104: webui: Add support for setting custom table pagination size

[Martin Basti]

On 11.08.2016 16:18, Pavel Vomacka wrote:

Hello,

please review attached patches.

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




bump for review
-- 
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] webui: 0084, 0101: refactoring rpc module

[Martin Basti]

On 09.08.2016 13:29, Pavel Vomacka wrote:

Hello,

please review attached patches.

The rpc module is now separated from display layer
and changing activity text while loading metadata.

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




bump for review
-- 
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] 956 replicainstall: log ACI and LDAP errors in promotion check

[Martin Basti]

On 30.03.2016 10:06, Martin Basti wrote:



On 24.03.2016 15:27, Petr Vobornik wrote:

to enable debugging of such errors.

E.g.: https://fedorahosted.org/freeipa/ticket/5741



Can we log the whole traceback to get exact place where error happened?

Martin^2



bump
-- 
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] restrict setkeytab operation

[Martin Basti]

On 31.08.2016 14:36, Martin Basti wrote:




On 26.07.2016 13:38, Simo Sorce wrote:

On Mon, 2016-07-25 at 11:26 -0400, Simo Sorce wrote:

On Mon, 2016-07-25 at 11:10 -0400, Rob Crittenden wrote:

Simo Sorce wrote:

On Mon, 2016-07-25 at 10:55 -0400, Rob Crittenden wrote:

Simo Sorce wrote:

As described in #232 start restricting the use of the setkeytab
operation to just the computers objects.

I haven't tested this with older RHEL/CentOS machines that actully use
the setkeytab operation as I do not have such an old VM handy right now.

Meanwhile I'd like to know if ppl agree with this approach.

What about services?

Do we automatically acquire keytab for services in the old clients ?

Are you thinking about scripted ipa-getkytab callouts ?

You are limiting access to host keytabs, what about service keytabs?
Should they be or are they now similarly restricted?

Installers for something like Foreman may try to generate a service
keytab in its installer, probably using admin credentials. I am planning
to do the same in Openstack.

Ok I'll amend the patch to allow service keytabs to still use the
setkeytab control still, and restrict only users.
However note that the idea of using this method is that admin can change
this default on their own, so they can restrict more or less if they
want, to that end I need to remember how to set a default that we do not
override in the update file.

Simo.


Amended patch to allow services too.
Only users are excluded.

Simo.





bump for review



bump for review
-- 
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] Limiting pull request notification sizes

[Martin Basti]

On 19.10.2016 16:38, Petr Vobornik wrote:

Looking at: [Freeipa-devel] [freeipa PR#171][synchronized] Build system
cleanup phase 2

I see that the attached freeipa-pr-171.patch has 7.8 MB. With couple
forced pushed to the private branch it creates quite big traffic a takes
space.

Is it possible to limit sizes of these attachments to let's say 50KB?

I.e., I would be interested in the small patches but let's read the
large ones on GitHub.


It is possible, you can:

*) open issue against freeipa/freeipa-tools
or
*) sent PR against freeipa/freeipa-tools

PS: github will not show you the big patches :D you have to use git show

Martin^2

--
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] [help]

[Martin Basti]

Comments inline


On 19.10.2016 10:30, 郑磊 wrote:







--
祝：
工作顺利！生活愉快！
--
长沙研发中心 郑磊
Phone：18684703229
Email：zheng...@kylinos.cn
Company：天津麒麟信息技术有限公司
Address：湖南长沙市开福区三一大道工美大厦十四楼
-- Original --
*From: * "Martin Basti"<mba...@redhat.com>;
*Date: * Wed, Oct 19, 2016 04:03 PM
*To: * "郑磊"<zheng...@kylinos.cn>; 
"freeipa-devel"<freeipa-devel@redhat.com>;

*Subject: * Re: [Freeipa-devel] [help]



On 19.10.2016 03:35, 郑磊 wrote:

Hello,
In the process of using freeipa, we found a lot of content without 
Chinese translation. In order to we can better use the platform,



we made a Chinese translation.
Sorry but I don't see any updates in zanata 
https://fedora.zanata.org/iteration/view/freeipa/master/languages/zh-CN?dswid=-4750


If you do a custom/manual translations this will not scale and you 
will do the same every release



I have applied to join Chinese translation organization in zanata 
https://fedora.zanata.org/language/view/zh-CN?dswid=2727, but there is 
no reply. And I have tried to translate in zanata 
https://fedora.zanata.org/iteration/view/freeipa/master/languages/zh-CN?dswid=-4750, 
But there seems to be no permission to modify. Whether I need to put 
the translation file zh_CN.po to create pull request against 
freeipa/freeipa on github?


No please don't, we always generate .po files from zanata before 
releases, so all changes are overwritten


Ok, let me know how the adding you to zanata continue, if nobody will 
answer for longer time, I can put you just to freeIPA project as translator.






I think the translation work for freeipa internationalization 
promotion also can have certain help. In addition, in use process, we 
found that when test the corresponding function of operation in the 
Web interface is not a straightforward logging which needs to query 
in the background, and it may not be intuitive and convenient. In 
order to we can intuitively record what we have done the operation in 
the Web interface, we do the log module plug-ins.




webUI is just layer on top of our API calls.


I know, What I mean is that no matter we are in API calls mode to 
manipulate freeipa or Web UI way, we all can intuitive see clearly 
under the log module. The log module function is to record any of our 
operation.



I think that they can be many caveats (access control, replication, 
etc.) maybe would be good to see your code (you can send PR) how is you 
current implementation.


Martin




--
祝：
工作顺利！生活愉快！
--
长沙研发中心 郑磊
Phone：18684703229
Email：zheng...@kylinos.cn
Company：天津麒麟信息技术有限公司
Address：湖南长沙市开福区三一大道工美大厦十四楼
------ Original --
*From: * "Martin Basti"<mba...@redhat.com>;
*Date: * Tue, Oct 18, 2016 06:50 PM
*To: * "郑磊"<zheng...@kylinos.cn>; 
"freeipa-devel"<freeipa-devel@redhat.com>;

*Subject: * Re: [Freeipa-devel] [help]



On 18.10.2016 03:28, 郑磊 wrote:

Hello everyone,
I'm using freeipa, and having a test and research with the 
function of freeipa. At the same time, I have carried on the Chinese 
translation to the web interface, also added own log module in web 
interface, referring to the following screenshots. However, for 
these changes I don't know how to interact with the community. Is 
there a administrator to review these changes? Who should I send 
mail to? Please help me. Thank you very much!






Hello,

at first you can write here what is your goal, what are your use 
cases and how do you plan to achieve that.


Then, if we agree on solution, you can send code as pull request to 
our github repository https://github.com/freeipa/freeipa



Martin^2




-- 
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] [help]

[Martin Basti]

On 19.10.2016 03:35, 郑磊 wrote:

Hello,
In the process of using freeipa, we found a lot of content without 
Chinese translation. In order to we can better use the platform,



we made a Chinese translation.
Sorry but I don't see any updates in zanata 
https://fedora.zanata.org/iteration/view/freeipa/master/languages/zh-CN?dswid=-4750


If you do a custom/manual translations this will not scale and you will 
do the same every release


I think the translation work for freeipa internationalization 
promotion also can have certain help. In addition, in use process, we 
found that when test the corresponding function of operation in the 
Web interface is not a straightforward logging which needs to query in 
the background, and it may not be intuitive and convenient. In order 
to we can intuitively record what we have done the operation in the 
Web interface, we do the log module plug-ins.




webUI is just layer on top of our API calls.






--
祝：
工作顺利！生活愉快！
--
长沙研发中心 郑磊
Phone：18684703229
Email：zheng...@kylinos.cn
Company：天津麒麟信息技术有限公司
Address：湖南长沙市开福区三一大道工美大厦十四楼
-- Original --
*From: * "Martin Basti"<mba...@redhat.com>;
*Date: * Tue, Oct 18, 2016 06:50 PM
*To: * "郑磊"<zheng...@kylinos.cn>; 
"freeipa-devel"<freeipa-devel@redhat.com>;

*Subject: * Re: [Freeipa-devel] [help]



On 18.10.2016 03:28, 郑磊 wrote:

Hello everyone,
I'm using freeipa, and having a test and research with the 
function of freeipa. At the same time, I have carried on the Chinese 
translation to the web interface, also added own log module in web 
interface, referring to the following screenshots. However, for these 
changes I don't know how to interact with the community. Is there a 
administrator to review these changes? Who should I send mail to? 
Please help me. Thank you very much!






Hello,

at first you can write here what is your goal, what are your use cases 
and how do you plan to achieve that.


Then, if we agree on solution, you can send code as pull request to 
our github repository https://github.com/freeipa/freeipa



Martin^2


-- 
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] [help]

[Martin Basti]

On 18.10.2016 03:28, 郑磊 wrote:

Hello everyone,
I'm using freeipa, and having a test and research with the 
function of freeipa. At the same time, I have carried on the Chinese 
translation to the web interface, also added own log module in web 
interface, referring to the following screenshots. However, for these 
changes I don't know how to interact with the community. Is there a 
administrator to review these changes? Who should I send mail to? 
Please help me. Thank you very much!






Hello,

at first you can write here what is your goal, what are your use cases 
and how do you plan to achieve that.


Then, if we agree on solution, you can send code as pull request to our 
github repository https://github.com/freeipa/freeipa



Martin^2
-- 
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] [Test][Patch-0047] Added a test for Ticket N 5964

[Martin Basti]

1)

you don't need to disable/enable dirsrv, just stop/start. Please remove 
disable/enable parts



2)

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> traceback 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

self = object at 0x7f6a502eec90>


def test_delete_ruvs(self):
"""
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
Test_Plan#Test_case:_clean-ruv_subcommand
"""
replica = self.replicas[0]
master = self.master
res1 = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
  master.config.dirman_password])
>   assert(res1.stdout_text.count(replica.hostname) == 2 and
   "Certificate Server Replica Update Vectors" in res1), (
"CA-specific RUVs are not displayed")
E   TypeError: argument of type 'SSHCommand' is not iterable

test_integration/test_topology.py:215: TypeError
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> entering PDB 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> 
/usr/lib/python2.7/site-packages/ipatests/test_integration/test_topology.py(215)test_delete_ruvs()


-> assert(res1.stdout_text.count(replica.hostname) == 2 and



On 14.10.2016 11:36, Oleg Fayans wrote:

Right you are! I am sorry.

On 10/13/2016 06:10 PM, Martin Basti wrote:

I think that you forgot to squash commits. Patch 47 doesn't apply


On 13.10.2016 14:01, Oleg Fayans wrote:

Hi Martin,

Thanks for the review.
With disabling directory server it works as well, thanks for the hint.
Also I moved the cleanup logic to the test itself for the sake of
simplicity. Patch-0048 was not changed

On 10/12/2016 02:35 PM, Martin Basti wrote:

1)

Can you just turn off dirsrv on replica instead of doing iptables 
magic?



2) NACK

No more eval() ever in code, use 'getattr', 'get' or whatever in the
object that can be used.

+evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:

Hi Martin,

After extensive discussion with Ludwig, I finally got the clue on how
does this feature work. When we uninstall the replica, the master
cleans the replication agreements with this replica and automatically
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the 
replica,

the replica's RUVs get recreated on master (replication works!). So,
the only way to test the clean-ruv subcommand is to turn off the
replica, or block the traffic on it so it gets inaccessible to 
updates

from master.
The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended_to_handle_CA-specific_RUVs 





[2]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand 





On 08/05/2016 06:36 PM, Martin Basti wrote:



On 03.08.2016 14:45, Oleg Fayans wrote:

Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:



On 08.07.2016 15:41, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:



On 27.06.2016 13:53, Oleg Fayans wrote:

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get reviewed
before
4.4 release? They cover a good part of the Managed Topology 4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:

One more test was added to the patch-0048

On 06/

Re: [Freeipa-devel] Broken IPA installation caused by new python-dns package

[Martin Basti]

On 13.10.2016 19:49, Petr Vobornik wrote:

On 10/12/2016 11:11 AM, Petr Spacek wrote:

On 10.10.2016 10:28, Martin Basti wrote:

https://bodhi.fedoraproject.org/updates/FEDORA-2016-1857421df6


Please set karma accordingly


Traceback:

...

2016-10-10T04:44:05Z DEBUG The ipa-replica-install command failed, exception:
TypeError: 'unicode' does not have the buffer interface
2016-10-10T04:44:05Z ERROR 'unicode' does not have the buffer interface


I'll investigate if IPA using it wrong or there is new error introduced in
pyhton-dns

For archaeologists:
Fix
https://github.com/freeipa/freeipa/pull/150
was merged.


We've pushed PR 150 to 4.4 and master. 4.4.2 release fixes f25 and f26
but F24 has 4.3 branch.

Is it correct to assume that 4.3 is also affected?

If so, then we need either to backport the patch to 4.3 and fix Fedora
directly or completely block the python-dns update on f24.


4.3 shouldn't be affected, because the code that has been failing is 
only in 4.4+ in DNS Locations related feature


Martin^2

--
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] [Test][Patch-0047] Added a test for Ticket N 5964

[Martin Basti]

I think that you forgot to squash commits. Patch 47 doesn't apply


On 13.10.2016 14:01, Oleg Fayans wrote:

Hi Martin,

Thanks for the review.
With disabling directory server it works as well, thanks for the hint.
Also I moved the cleanup logic to the test itself for the sake of 
simplicity. Patch-0048 was not changed


On 10/12/2016 02:35 PM, Martin Basti wrote:

1)

Can you just turn off dirsrv on replica instead of doing iptables magic?


2) NACK

No more eval() ever in code, use 'getattr', 'get' or whatever in the
object that can be used.

+evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:

Hi Martin,

After extensive discussion with Ludwig, I finally got the clue on how
does this feature work. When we uninstall the replica, the master
cleans the replication agreements with this replica and automatically
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the replica,
the replica's RUVs get recreated on master (replication works!). So,
the only way to test the clean-ruv subcommand is to turn off the
replica, or block the traffic on it so it gets inaccessible to updates
from master.
The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended_to_handle_CA-specific_RUVs 




[2]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand 




On 08/05/2016 06:36 PM, Martin Basti wrote:



On 03.08.2016 14:45, Oleg Fayans wrote:

Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:



On 08.07.2016 15:41, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:



On 27.06.2016 13:53, Oleg Fayans wrote:

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get reviewed
before
4.4 release? They cover a good part of the Managed Topology 4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:

One more test was added to the patch-0048

On 06/17/2016 09:43 AM, Oleg Fayans wrote:

Fixed a bug in the previous patch, automated 2 more testcases
from
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan 







On 06/16/2016 04:46 PM, Oleg Fayans wrote:









IIUC, this will turn off the machine completely, how is cleanup 
done

then.  AFAIK our tests cannot turn on machine again and run
cleanup, so
you will not be able to run more tests on the same topology 
without

manual cleanup and manual start.

+replica = self.replicas[0]
+replica.run_command(['poweroff'])

IMO would be better to just call 'ipactl stop' instead of 
'poweroff'


Agreed! Fixed.



Martin^2






*Automated ipa-replica-manage del tests*

1)
+replica.run_command(['ipactl', 'stop'])
+time.sleep(3)

Why do you need sleep here?


Removed, it was left from the old "poweroff" approach




2)
+ruvid_re = re.compile(".*%s:389: (\d+).*" % 
replica.hostname)

+replica_ruvs = ruvid_re.findall(result.stdout_text)
+master.run_command(['ipa-replica-manage', 'clean-ruv', 'f',
+'-p', master.config.dirman_password,
+replica_ruvs[0]])

Because you are using re.findall(), without any match you will 
receive

IndexError here replica_ruvs[0]. IMO it deserves assert before


Implemented the assert which checks that the output contains enough
replica RUVs



3)
assert(replica.hostname in result1.stdout_text)

I think that this is error prone. What if there is just error
'could not
connect to replica ', or something similar.
instead of
listing/cleaning/whatever operation was executed. I think that it
should
be more specific regexp than just finding a replica name substring
(Yes
In IPA we dont always print error so stderr)

I'm not sure, but probably there might be cases when non critical
error
happen and exist status is still 0


Agree. Implemented a regex-based search



4)

+replica.run_command(['poweroff'])
+time.sleep(3)

There should not be poweroff, probably sleep could be removed too.


Gone




  *   Automated clean-ruv subcommand test*

1) PEP8, 2 new lines expected
./ipatests/test_integration/test_topology.py:163:1: E302 expected 2
blank lines, found 0
./ipatests/test_integration/test_topology.py:182:80: E501 line too
long
(85 > 79 characters)


Fixed




2)
I dont like doing assert just with count of occurences of 
substring in

STDOUT, would be possible to improve this somehow?


Maybe, but frankly, I don't see how. In this case we are making sure
that both simple and CA-specific RUVs of a replica are displayed. The
format of the output is strict:
Replica Update Vectors:
replica1_hostname:389: RUV_id
replica2_hostname:389: RUV_id
Certificate Server Replica Upda

Re: [Freeipa-devel] links to docs in the messages from code

[Martin Basti]

On 12.10.2016 19:56, Petr Spacek wrote:

Hello FreeIPA developers,

looking at freeipa-users mailing list, a lot of questions could be answered by
just quick glance to the docs.

I wonder if we should add links HTML version of docs on access.redhat.com to
the messages generated by the code.

If we really want, we can make these platform-specific, but I would not even
bother with it. Fedora & CentOS & RHEL users end up on the very same page,
only the way how then find it is different (mailing list vs. Google vs. paid
support).


Examples:

a) Installation without DNS could end up with message like this:
Do not forget to finish post-installation steps listed on
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Linux_Domain_Identity_Authentication_and_Policy_Guide/install-server.html#install-server-without-dns


b) Failed connection check could print link to
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Linux_Domain_Identity_Authentication_and_Policy_Guide/installing-ipa.html#prereq-ports


c) Failed DNS check could mention link
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Linux_Domain_Identity_Authentication_and_Policy_Guide/installing-ipa.html#dns-reqs


d) Failed attempt to find AD DC could print a link to:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Windows_Integration_Guide/trust-requirements.html#dns-realm-settings

etc.

What do you think about this?



I'm afraid that those links can change over time, so we have to check 
them regularly otherwise we may end up with links pointing to nowhere. 
I'm not excited too much with this idea.


Martin^2

--
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] [Test][Patch-0047] Added a test for Ticket N 5964

[Martin Basti]

1)

Can you just turn off dirsrv on replica instead of doing iptables magic?


2) NACK

No more eval() ever in code, use 'getattr', 'get' or whatever in the 
object that can be used.


+evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:

Hi Martin,

After extensive discussion with Ludwig, I finally got the clue on how 
does this feature work. When we uninstall the replica, the master 
cleans the replication agreements with this replica and automatically 
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the replica, 
the replica's RUVs get recreated on master (replication works!). So, 
the only way to test the clean-ruv subcommand is to turn off the 
replica, or block the traffic on it so it gets inaccessible to updates 
from master.

The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1] 
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended_to_handle_CA-specific_RUVs


[2] 
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand


On 08/05/2016 06:36 PM, Martin Basti wrote:



On 03.08.2016 14:45, Oleg Fayans wrote:

Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:



On 08.07.2016 15:41, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:



On 27.06.2016 13:53, Oleg Fayans wrote:

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get reviewed
before
4.4 release? They cover a good part of the Managed Topology 4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:

One more test was added to the patch-0048

On 06/17/2016 09:43 AM, Oleg Fayans wrote:
Fixed a bug in the previous patch, automated 2 more testcases 
from
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan 






On 06/16/2016 04:46 PM, Oleg Fayans wrote:










IIUC, this will turn off the machine completely, how is cleanup done
then.  AFAIK our tests cannot turn on machine again and run
cleanup, so
you will not be able to run more tests on the same topology without
manual cleanup and manual start.

+replica = self.replicas[0]
+replica.run_command(['poweroff'])

IMO would be better to just call 'ipactl stop' instead of 'poweroff'


Agreed! Fixed.



Martin^2






*Automated ipa-replica-manage del tests*

1)
+replica.run_command(['ipactl', 'stop'])
+time.sleep(3)

Why do you need sleep here?


Removed, it was left from the old "poweroff" approach




2)
+ruvid_re = re.compile(".*%s:389: (\d+).*" % replica.hostname)
+replica_ruvs = ruvid_re.findall(result.stdout_text)
+master.run_command(['ipa-replica-manage', 'clean-ruv', 'f',
+'-p', master.config.dirman_password,
+replica_ruvs[0]])

Because you are using re.findall(), without any match you will receive
IndexError here replica_ruvs[0]. IMO it deserves assert before


Implemented the assert which checks that the output contains enough
replica RUVs



3)
assert(replica.hostname in result1.stdout_text)

I think that this is error prone. What if there is just error 
'could not
connect to replica ', or something similar. 
instead of
listing/cleaning/whatever operation was executed. I think that it 
should
be more specific regexp than just finding a replica name substring  
(Yes

In IPA we dont always print error so stderr)

I'm not sure, but probably there might be cases when non critical 
error

happen and exist status is still 0


Agree. Implemented a regex-based search



4)

+replica.run_command(['poweroff'])
+time.sleep(3)

There should not be poweroff, probably sleep could be removed too.


Gone




  *   Automated clean-ruv subcommand test*

1) PEP8, 2 new lines expected
./ipatests/test_integration/test_topology.py:163:1: E302 expected 2
blank lines, found 0
./ipatests/test_integration/test_topology.py:182:80: E501 line too 
long

(85 > 79 characters)


Fixed




2)
I dont like doing assert just with count of occurences of substring in
STDOUT, would be possible to improve this somehow?


Maybe, but frankly, I don't see how. In this case we are making sure
that both simple and CA-specific RUVs of a replica are displayed. The
format of the output is strict:
Replica Update Vectors:
replica1_hostname:389: RUV_id
replica2_hostname:389: RUV_id
Certificate Server Replica Update Vectors:
replica1_hostname:389: RUV_id
replica2_hostname:389: RUV_id
If we do not see 2 occurrences of the replica hostname than definitely
something went wrong



3)
I'm not sure if clean-ruv is instant operations or there is some magic
happening in background (we have abort-clean-ruv). Maybe some sleep
should be there, but this needs investigation.

+a

Re: [Freeipa-devel] [PATCH 0497] Py3: fix unicode/str error in LDAP*ReverseMember

[Martin Basti]

On 10.10.2016 07:57, Jan Cholasta wrote:

On 7.6.2016 10:35, Martin Basti wrote:



On 07.06.2016 10:35, Jan Cholasta wrote:

On 7.6.2016 10:29, Martin Basti wrote:



On 07.06.2016 09:08, Jan Cholasta wrote:

On 6.6.2016 14:33, Martin Basti wrote:

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

Patch attached.


Could we drop the error message parsing and do something sane 
instead?




Not now, we can do it later and push this patch just as workaround


What's the point of that?


Point is that it will work as before, and I don't have to time to fix it
nicely now.


Bump. Do you now have time to fix it nicely, or should we move the 
ticket to 4.5 or later?




We should move it to milestone where we will fix/enable py3.

--
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] Broken IPA installation caused by new python-dns package

[Martin Basti]

https://bodhi.fedoraproject.org/updates/FEDORA-2016-1857421df6


Please set karma accordingly


Traceback:

...

  File 
"/usr/lib/python2.7/site-packages/ipaserver/dns_data_management.py", 
line 426, in update_dns_records

self.update_base_records(),
  File 
"/usr/lib/python2.7/site-packages/ipaserver/dns_data_management.py", 
line 377, in update_base_records

base_zone = self.get_base_records()
  File 
"/usr/lib/python2.7/site-packages/ipaserver/dns_data_management.py", 
line 328, in get_base_records

include_kerberos_realm=include_kerberos_realm
  File 
"/usr/lib/python2.7/site-packages/ipaserver/dns_data_management.py", 
line 179, in _add_base_dns_records_for_server

self.__add_kerberos_txt_rec(zone_obj)
  File 
"/usr/lib/python2.7/site-packages/ipaserver/dns_data_management.py", 
line 165, in __add_kerberos_txt_rec

rdataset.add(rd, ttl=86400)  # FIXME: use TTL from config
  File "/usr/lib/python2.7/site-packages/dns/rdataset.py", line 129, in add
super(Rdataset, self).add(rd)
  File "/usr/lib/python2.7/site-packages/dns/set.py", line 49, in add
if item not in self.items:
  File "/usr/lib/python2.7/site-packages/dns/rdata.py", line 217, in __eq__
return self._cmp(other) == 0
  File "/usr/lib/python2.7/site-packages/dns/rdata.py", line 203, in _cmp
our = self.to_digestable(dns.name.root)
  File "/usr/lib/python2.7/site-packages/dns/rdata.py", line 174, in 
to_digestable

self.to_wire(f, None, origin)
  File "/usr/lib/python2.7/site-packages/dns/rdtypes/txtbase.py", line 
75, in to_wire

file.write(s)

2016-10-10T04:44:05Z DEBUG The ipa-replica-install command failed, 
exception: TypeError: 'unicode' does not have the buffer interface

2016-10-10T04:44:05Z ERROR 'unicode' does not have the buffer interface


I'll investigate if IPA using it wrong or there is new error introduced 
in pyhton-dns


--
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] Build system refactoring - design document

[Martin Basti]

On 07.10.2016 11:56, Petr Spacek wrote:

Dear FreeIPA developers and packagers,

you can find first version of the Build system refactoring design document on:
http://www.freeipa.org/page/V4/Build_system_refactoring

If you do not care about implementation details, please be so kind and quickly
scan through chapter
http://www.freeipa.org/page/V4/Build_system_refactoring#Feature_Management

I'm not an FreeIPA packager so I might miss some important thing which needs
to be configurable.


Also, I would appreciate ideas how to handle build versioning:
http://www.freeipa.org/page/V4/Build_system_refactoring#Versioning

My main questions are:
* What is triggering IPA upgrade?
* Would it be sufficient to bump release in RPM? (I mean - theoretically.
Could the code be modified to detect this?)

Here I'm trying to avoid unnecessary rebuilds caused by changes to
IPA_VENDOR_VERSION during each build.


Timo, what can I do to help you with packaging for Ubuntu/Debian?

Dream big, even wild ideas are welcome!



Thank you for nice design,

1)
I'd like to have make clean as well (we have it there now, but I don't 
think that it works well)


2)
Pylint:

currently we have (almost) python2/3 compatible code so what is the 
reason to have python2 and python3 separated checks? Pylint is doing 
that at once


--
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] python-nss-1.0.0-2.fc24.x86_64 from updates-testing breaks FreeIPA client API

[Martin Basti]

On 29.09.2016 10:14, Alexander Bokovoy wrote:

On to, 29 syys 2016, Martin Babinsky wrote:

Hi list,

today I noticed the following exceptions in my VMs when 
installing/using FreeIPA:


"""
# ipa ping
exception in SSLSocket.handshake_callback
Traceback (most recent call last):
 File "/usr/lib/python2.7/site-packages/ipapython/nsslib.py", line 
258, in handshake_callback

   channel = sock.get_ssl_channel_info()
nss.error.NSPRError: (SEC_ERROR_INVALID_ARGS) security library: 
invalid arguments.


IPA server version 4.4.90. API version 2.215

"""

This was caused by python-nss-1.0.0-2.fc24.x86_64 which was pushed to 
updates-testing. Reverting the package to previous versions fixed the 
problem.

python-nss-1.0.0-1.fc25 (note fc25) works fine. There is no 1.0.0-2.fc25
which is a packaging bug, but that's should not be bringing any
difference as the tarball (1.0.0) is the same and no additional patches
were applied.

Also, we didn't have any changes between 4.4.1 and git master that could
have affected ipapython/nsslib.py other than 
0f88f8fe889ae4801fc8d5ece1ad51c5246718ac,

which is this chunk of changes:

diff --git a/ipapython/nsslib.py b/ipapython/nsslib.py
index 1573de9..f9f64c1 100644
--- a/ipapython/nsslib.py
+++ b/ipapython/nsslib.py
@@ -234,7 +234,7 @@ class NSSConnection(httplib.HTTPConnection,
NSSAddressFamilyFallback):
self.sock.set_ssl_option(ssl.SSL_HANDSHAKE_AS_CLIENT, True)
try:
self.sock.set_ssl_version_range(self.tls_version_min, 
self.tls_version_max)

-except NSPRError as e:
+except NSPRError:
root_logger.error('Failed to set TLS range to %s, %s' % 
(self.tls_version_min, self.tls_version_max))

raise
self.sock.set_ssl_option(ssl_require_safe_negotiation, False)

e.g. nothing that is relevant to the trace you provided.



Sorry I cannot reproduce it as well

[root@vm-058-017 ~]# ipa ping

IPA server version 4.4.90. API version 2.215


[root@vm-058-017 ~]# dnf upgrade python-nss ...
Running transaction
  Upgrading   : python-nss-1.0.0-2.fc24.x86_64 1/4
  Upgrading   : python3-nss-1.0.0-2.fc24.x86_64 2/4
  Cleanup : python3-nss-1.0.0-beta1.2.fc24.1.x86_64 3/4
  Cleanup : python-nss-1.0.0-beta1.2.fc24.1.x86_64 4/4
  Verifying   : python3-nss-1.0.0-2.fc24.x86_64 1/4
  Verifying   : python-nss-1.0.0-2.fc24.x86_64 2/4
  Verifying   : python-nss-1.0.0-beta1.2.fc24.1.x86_64 3/4
  Verifying   : python3-nss-1.0.0-beta1.2.fc24.1.x86_64

[root@vm-058-017 ~]# ipa ping

IPA server version 4.4.90. API version 2.215


--
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] pylint: remove unused variables

[Martin Basti]

On 23.09.2016 14:12, Jan Cholasta wrote:

On 23.9.2016 13:23, Standa Laznicka wrote:

On 09/23/2016 07:28 AM, Jan Cholasta wrote:

On 22.9.2016 16:39, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and 
enable

pylint check. Due to big amount of unused variables in the code this
will be longterm effort.

Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an 
agreement

what to do with unpacking or iteration with unused variables


For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree 
on a

way how to tell pylint that this variable is unused on purpose:


1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


+1




2)

Use defined keyword: 'dummy' is default in pylint, we can set our own,
like ignored, unused

name, dummy, gender = user['mbasti']


-1, not visible enough.




3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


This. We have already been using it in new code for quite some time,
and it's common in other Python projects too. Don't reinvent the wheel.




4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


Use '_dummy', it's both :-)


+1. I would rather use _meh as it's easier to type but perhaps not that
self-explanatory and not established at all, so _dummy is just fine :)


What I'm actually suggesting is that any local variable with "_" 
prefix should be considered unused, so _meh would be fine as well.




+1 regexp '_.+' works for me

--
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] FedoraHosted.org sunset

[Martin Basti]

On 23.09.2016 09:54, Jakub Hrozek wrote:

On Thu, Sep 22, 2016 at 06:09:43PM +0200, Petr Vobornik wrote:

Hi all,

As you know, FedoraHosted.org will be decommissioned.
  https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/

We use Trac instance there. Let's discuss where we should migrate and
what are our requirements. Then put results on one place. For that I've
created:
   http://www.freeipa.org/page/FedoraHosted_Migration

That you for writing this up, there are some good points I didn't think
about, like migrating the ticket numbers. Did you already file an issue
that tracks this in Pagure (or asked if this is already possible)?



Do we need review by field? It is recorded in commit and for ongoing 
reviews we are assigning ourselves to pull requests, so everybody knows 
if somebody is reviewing a PR.


Martin^2

--
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] pylint: remove unused variables

[Martin Basti]

On 22.09.2016 18:05, Tomas Krizek wrote:

On 09/22/2016 06:00 PM, Martin Basti wrote:



On 22.09.2016 17:59, Tomas Krizek wrote:

On 09/22/2016 04:39 PM, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and 
enable pylint check. Due to big amount of unused variables in the 
code this will be longterm effort.


Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an 
agreement what to do with unpacking or iteration with unused variables



For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree 
on a way how to tell pylint that this variable is unused on purpose:



1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


2)

Use defined keyword: 'dummy' is default in pylint, we can set our 
own, like ignored, unused


name, dummy, gender = user['mbasti']


3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


As first step I'll enable pylint check and disable it locally per 
module where unused variables are, to avoid new regressions. Then I 
will fix it module by module.



I'm open to suggestions

Martin^2


I'd use a double underscore variable:

name, __, gender = user['mbasti']

It is quicker to write than _dummy and it also provides a better 
readability, because I can immediately identify the symbol as 
special. Unlike _dummy which I have to read to understand (simply 
because I'm used to _something to indicate a 'protected' variable).




With double underscores there is a risk, that typo will be just one 
underscore and _ means ugettext in IPA :)
I think pylint would detect a redefinition in that case, since this 
check was added in today's PR#97:


https://github.com/freeipa/freeipa/pull/97/commits/06f35e5bdcb9f3ea42145de253674fda06b43d30 





So much trust in pylint :), I tested it and it passed with redefinition 
of '_'. It works only for some cases.


--
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] pylint: remove unused variables

[Martin Basti]

On 22.09.2016 17:59, Tomas Krizek wrote:

On 09/22/2016 04:39 PM, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and 
enable pylint check. Due to big amount of unused variables in the 
code this will be longterm effort.


Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an 
agreement what to do with unpacking or iteration with unused variables



For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree on 
a way how to tell pylint that this variable is unused on purpose:



1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


2)

Use defined keyword: 'dummy' is default in pylint, we can set our 
own, like ignored, unused


name, dummy, gender = user['mbasti']


3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


As first step I'll enable pylint check and disable it locally per 
module where unused variables are, to avoid new regressions. Then I 
will fix it module by module.



I'm open to suggestions

Martin^2


I'd use a double underscore variable:

name, __, gender = user['mbasti']

It is quicker to write than _dummy and it also provides a better 
readability, because I can immediately identify the symbol as special. 
Unlike _dummy which I have to read to understand (simply because I'm 
used to _something to indicate a 'protected' variable).




With double underscores there is a risk, that typo will be just one 
underscore and _ means ugettext in IPA :)


--
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] pylint: remove unused variables

[Martin Basti]

Hello all,

In 4.5, I would like to remove all unused variables from code and enable 
pylint check. Due to big amount of unused variables in the code this 
will be longterm effort.


Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an agreement 
what to do with unpacking or iteration with unused variables



For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree on a 
way how to tell pylint that this variable is unused on purpose:



1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


2)

Use defined keyword: 'dummy' is default in pylint, we can set our own, 
like ignored, unused


name, dummy, gender = user['mbasti']


3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


As first step I'll enable pylint check and disable it locally per module 
where unused variables are, to avoid new regressions. Then I will fix it 
module by module.



I'm open to suggestions

Martin^2

--
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] Suspicious IPA cert test fail after upgrade to pki-ca-10.3.5-6

[Martin Basti]

On 22.09.2016 13:56, Martin Babinsky wrote:

On 09/22/2016 01:41 PM, Martin Basti wrote:

Hello all,


Following test is failing:


 


test_cert_find.test_0007_find_revocation_reason_0
 




self = 

def test_0007_find_revocation_reason_0(self):
"""
Find all certificates with revocation reason 0
"""
res = api.Command['cert_find'](revocation_reason=0)

  assert 'count' in res and res['count'] == 0

E   assert ('count' in {'count': 4, 'result': ({'cacn': 'ipa',
'issuer': 'CN=Certificate
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.CBRQ.REDHAT.COM',
'revoked': True, 'serial_number': 85, ...}), 'summary': '4 certificates
matched', 'truncated': False} and 4 == 0)

test_xmlrpc/test_cert_plugin.py:302: AssertionError
== 


1 failed, 38 passed in 10.77 seconds
=== 





Steps to reproduce:

1. upgrade to pki-ca-10.3.5-6

2. run all xmlrpc_tests (ipa-run-test test_xmlrpc)

3. ipa-run-tests test_xmlrpc/test_cert_plugin.py  will always fail with
error above


The curious thing is that with pki-ca-10.3.5-1, I'm not able to
reproduce this. Probably something was changed on pki-ca side.

[root@vm-058-017 ~]# ipa cert-find --revocation-reason=0
--
4 certificates matched
--
  Issuing CA: ipa
  Subject: CN=crud subca test,O=crud testing inc
  Issuer: CN=Certificate
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
  Serial number: 78
  Serial number (hex): 0x4E
  Status: REVOKED
  Revoked: True

  Issuing CA: ipa
  Subject: CN=crud subca test,O=crud testing inc
  Issuer: CN=Certificate
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
  Serial number: 79
  Serial number (hex): 0x4F
  Status: REVOKED
  Revoked: True

  Issuing CA: ipa
  Subject: CN=caacl test subca,O=test industries inc.
  Issuer: CN=Certificate
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
  Serial number: 80
  Serial number (hex): 0x50
  Status: REVOKED
  Revoked: True

  Issuing CA: ipa
  Subject: CN=SMIME CA,O=test industries Inc.
  Issuer: CN=Certificate
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
  Serial number: 85
  Serial number (hex): 0x55
  Status: REVOKED
  Revoked: True

Number of entries returned 4


My question is, should we update tests, or is it a bug on PKI-CA side??
I actually dont know why certificates are present there, it needs more
investigation.


Martin^2



Seeing that all the certs are actually intermediary CA certs and 
seeing the following line:


"""
- PKI TRAC Ticket #1638 - Lightweight CAs: revoke certificate on CA 
deletion (ftweedal)


"""

in pki-core 10.3.5-6 release notes, I would guess that these are 
leftover certificates from sub-CA tests which were previously just 
sitting there but are now marked as revoked with reason 0 - 
unspecified (as a side note, shouldn't there be different reason, i.e. 
5 -cessationOfOperation?).


Seems like we need to fix our tests to cleanup sub-CA certificates as 
well, should I open a ticket for this?




Yes please, thank you

--
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] Suspicious IPA cert test fail after upgrade to pki-ca-10.3.5-6

[Martin Basti]

Hello all,


Following test is failing:


 
test_cert_find.test_0007_find_revocation_reason_0 



self = 0x7f1bf4532f90>


def test_0007_find_revocation_reason_0(self):
"""
Find all certificates with revocation reason 0
"""
res = api.Command['cert_find'](revocation_reason=0)
>   assert 'count' in res and res['count'] == 0
E   assert ('count' in {'count': 4, 'result': ({'cacn': 'ipa', 
'issuer': 'CN=Certificate 
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.CBRQ.REDHAT.COM', 
'revoked': True, 'serial_number': 85, ...}), 'summary': '4 certificates 
matched', 'truncated': False} and 4 == 0)


test_xmlrpc/test_cert_plugin.py:302: AssertionError
== 
1 failed, 38 passed in 10.77 seconds 
===



Steps to reproduce:

1. upgrade to pki-ca-10.3.5-6

2. run all xmlrpc_tests (ipa-run-test test_xmlrpc)

3. ipa-run-tests test_xmlrpc/test_cert_plugin.py  will always fail with 
error above



The curious thing is that with pki-ca-10.3.5-1, I'm not able to 
reproduce this. Probably something was changed on pki-ca side.


[root@vm-058-017 ~]# ipa cert-find --revocation-reason=0
--
4 certificates matched
--
  Issuing CA: ipa
  Subject: CN=crud subca test,O=crud testing inc
  Issuer: CN=Certificate 
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM

  Serial number: 78
  Serial number (hex): 0x4E
  Status: REVOKED
  Revoked: True

  Issuing CA: ipa
  Subject: CN=crud subca test,O=crud testing inc
  Issuer: CN=Certificate 
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM

  Serial number: 79
  Serial number (hex): 0x4F
  Status: REVOKED
  Revoked: True

  Issuing CA: ipa
  Subject: CN=caacl test subca,O=test industries inc.
  Issuer: CN=Certificate 
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM

  Serial number: 80
  Serial number (hex): 0x50
  Status: REVOKED
  Revoked: True

  Issuing CA: ipa
  Subject: CN=SMIME CA,O=test industries Inc.
  Issuer: CN=Certificate 
Authority,O=DOM-058-017.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM

  Serial number: 85
  Serial number (hex): 0x55
  Status: REVOKED
  Revoked: True

Number of entries returned 4


My question is, should we update tests, or is it a bug on PKI-CA side?? 
I actually dont know why certificates are present there, it needs more 
investigation.



Martin^2



--
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] What would break if loopback addresses were allowed for IPA server?

[Martin Basti]

On 21.09.2016 12:01, Jan Pazdziora wrote:

Hello,

I've recently hit again the situation of IPA installer not happy
about the provided IP address not being local to it, this time in
containerized environment:

https://bugzilla.redhat.com/show_bug.cgi?id=1377973

During the discussion, we came to an interesting question:

What would break if loopback addresses were allowed for IPA
server?

Of course, the idea is that it would only be used for installation and
then IPA would change its IP address in DNS to whatever is the real IP
address under which it is accessible.

Where does the allow_loopback=False requirement in the installer come
from and what would break if it was removed altogether?

Thanks,



I'm not aware of anything that should prevent us to have just loopback 
address (installation without DNS) on server. It is somehow weird to not 
have any other address unicast address assigned, but cloud world strikes.


IIRC in past there might be issue with some services (KDC? not sure) 
that cannot run only with loopback address, but I dont think that this 
is an issue nowadays.


This needs investigation, please file a ticket and we may allocate human 
and time for this :)


Martin^2

--
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] pylint fixes

[Martin Basti]

On 01.07.2016 15:51, Florence Blanc-Renaud wrote:

On 06/21/2016 01:51 PM, Martin Basti wrote:



On 21.06.2016 08:38, Florence Blanc-Renaud wrote:

On 06/20/2016 07:08 PM, Martin Basti wrote:



On 20.06.2016 19:06, Martin Basti wrote:




On 20.06.2016 12:00, Florence Blanc-Renaud wrote:

On 06/09/2016 05:10 PM, Petr Spacek wrote:

Hello,

I've received a bunch of pylint fixes produced by upstream
contributor who is
not subscribed to the list so I'm resending them here.

All credit goes to Bárta Jan <55042ba...@sstebrno.eu>.

Flo, if you have time for it I think that it could be a good
exercise which
will lead you to various dark corners in IPA :-)

Petr^2 Spacek


 Forwarded Message 
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan <55042ba...@sstebrno.eu>
To: pspa...@redhat.com

___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_

diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
 res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)

-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode

should be instead: return res.returncode *== 0* (otherwise the 
return

type is an int and not a boolean).

In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
 zone = ".".join(client.hostname.split('.')[1:])
 result = master.run_command(["ipa", "dnszone-show", zone],
 raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0

should be instead: *return* result.returncode == 0 (otherwise there
is no return statement)

diff --git a/ipaserver/plugins/dogtag.py 
b/ipaserver/plugins/dogtag.py

index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
 # Return command result
 cmd_result = {}

-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')

Should be instead: cmd_result['revoked'] =
parse_result.get('revoked') *== 'yes'* (otherwise the type is a
string and not a boolean)

_- in the patch 00__04-pylint-fix-unneeded-not.patch_

@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
 options['ip_address'],
 check_forward=True,
 check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in
options:

Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence

I will review patches 0005 to 0010 later today.
Flo.




How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.

Martin^2




Sorry, I just noticed that there is no patch 1 :)



Patch 0003 is OK, ACK for this one.
Flo.


Patch 0003: Pushed to master: 94909d21dbf033cbe34089782c430ec25b9ad0bc


Hi,

please find my comments on the remaining patches.

- Patch 0005 must be rebased because of changes in 
ipatests/test_integration/tasks.py

the patch can also modify pylintrc (remove pointless-statement)

- Patch 0006:
no need to rename the items in "for e in ...", renaming the Exception 
as exc should be enough


- Patch 0007:
pylintrc should remove old-style-class instead of 
bad-classmethod-argument


- Patch 0008:
this one should remove bad-classmethod-argument in pylintrc

- Patch 0009:
ok

- Patch 0010:
In the __bind method(self, obj_type), cls variable is already used 
thus replacing self with cls can be done only if cls is also renamed 
into something else.


Flo.



Please follow new changes in this PR 
https://github.com/freeipa/freeipa/pull/97

It will be easier to review.

Martin^2

--
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] Github review feature

[Martin Basti]

Sorry for stealing your thread, but you started asking about github 
review emails :)



Standard review inline comments are disabled on purpose, each comment 
generates one email, so we decided that is better after review to write 
a regular comment "NACK, please see inline comments" or so.


I would expecting that the new review feature is sending all comments in 
one batch, but I was wrong. I used the new review feature (with the 
pending comments) but when I sent it I received all comments as single 
notifications again, so again one inline comment = one email


Even worse it is with states of review (approved, required change). I 
didn't received any notification from github related to this (not sure 
if is part of any inline comment message or just not implemented yet). 
This is not documented in their API docs (according David) so we cannot 
use it our tools yet.


Generally adding Labels ACK/Rejected are more visible and filters can be 
made easily.



So for now I would stay with our old workflow and not extend email 
notifications. We can play with new review feature for longer time and 
decide if it is worth to use it (and change email notification accordingly)



Martin^2


On 15.09.2016 22:49, Ben Lipton wrote:

On 09/15/2016 02:12 AM, jcholast wrote:

jcholast commented on a pull request

"""
In addition to my inline comments above:

1. "Certificate mapping" does not really evoke "certificate request templating" 
to me, and is also used in the context of mapping identities to certificates. Could we use a more 
suitable name to avoid confusion?
2. The `ipalib.certmapping` module is used only in `ipaclient`, so that's where 
it should be located. It can be moved to `ipalib` later if necessary.
3. I don't think `IPAExtension` deserves it's own module, at least not now.
"""

See the full comment 
athttps://github.com/freeipa/freeipa/pull/10#issuecomment-247244120


Tried sending my comments as a "review" (new Github feature) and it 
seems they don't get sent to the list that way. So:


Thanks for the comments! I've fixed the simple ones and replied to the 
rest. Regarding your comments about file organization:


 1. I quite agree that certmapping isn't a good name for what this
turned out to be. With the convention of naming modules after the
objects they model, perhaps a good name would
be|certrequest|or|csr|? The command could be renamed to something
like|certrequest-get-data|(or|certrequest-get-script|).
 2. Just to confirm, you're suggesting just moving these classes to
the|ipaclient.plugins.|module?
 3. Seems reasonable, I've moved it into the ipalib module for now. It
will go wherever the contents of that module end up.

Logistical stuff:

  * Now that this is under review I won't add any more content. Are
you ok with the two commits about testing being part of this
review or should I remove them?
  * If you run rebase --autosquash with the latest commit it doesn't
actually apply cleanly, but I'm trying not to change history while
it's being reviewed, so I'll do the rebase later on if that's ok?






-- 
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] [Test][Patch-0049, 0050] Certs in ID overrides test

[Martin Basti]

On 14.09.2016 18:53, Sumit Bose wrote:

On Wed, Sep 14, 2016 at 06:03:37PM +0200, Martin Basti wrote:


On 14.09.2016 17:53, Alexander Bokovoy wrote:

On Wed, 14 Sep 2016, Martin Basti wrote:


On 14.09.2016 17:41, Alexander Bokovoy wrote:

On Wed, 14 Sep 2016, Martin Basti wrote:

1)
I still don't see the reason why AD trust is needed. Default
trust ID view is added just by ipa-adtrust-install, adding
trust is not needed for current implementation. You don't
need AD for this, IDviews is generic feature not just for
AD. Is that user configured on AD side?

You cannot add non-AD user to 'default trust view', so you will not be
able to set up certificates to ID override which does not exist.

For non-'default trust view' you can add both IPA and AD users,
so using
some other view and then assign certificate for a ID override in that
one.


Ok then, but anyway I would like to see API/CLI tests for this
feature with proper output validation.


How can be this tested with SSSD?

You need to log into the system with a certificate...

Is this possible from test? We are logged remotely as root, is there any
cmdline util which allows us to test certificate against AD user?


You can use 'sss_ssh_authorizedkeys aduser@ad.domain' which should
return the ssh key derived from the public key in the certificate. This
should work for certificate stored in AD as well as for overrides.

You can also you the DBus lookup by certificate as described in
https://fedorahosted.org/sssd/wiki/DesignDocs/LookupUsersByCertificate .

HTH

bye,
Sumit


Thank you Alexander and Summit for hints.

Oleg I realized we don't have any other idviews integration tests

So I propose to rename test file you are adding to test_idviews.py. We 
can add more testcases for idviews there later


Martin^2

Martin^2

--
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


--
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] [Test][Patch-0049, 0050] Certs in ID overrides test

[Martin Basti]

On 14.09.2016 17:41, Alexander Bokovoy wrote:

On Wed, 14 Sep 2016, Martin Basti wrote:

1)
I still don't see the reason why AD trust is needed. Default trust ID 
view is added just by ipa-adtrust-install, adding trust is not needed 
for current implementation. You don't need AD for this, IDviews is 
generic feature not just for AD. Is that user configured on AD side?

You cannot add non-AD user to 'default trust view', so you will not be
able to set up certificates to ID override which does not exist.

For non-'default trust view' you can add both IPA and AD users, so using
some other view and then assign certificate for a ID override in that
one.



Ok then, but anyway I would like to see API/CLI tests for this feature 
with proper output validation.



How can be this tested with SSSD?

--
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] [Test][Patch-0049, 0050] Certs in ID overrides test

[Martin Basti]

On 06.09.2016 13:57, Oleg Fayans wrote:

The test is updated to clean up after itself

On 09/06/2016 12:57 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. The updated patches are attached. Please, see my
comments below

On 08/30/2016 01:58 PM, Martin Basti wrote:



On 22.08.2016 13:18, Oleg Fayans wrote:

ping for review

On 08/02/2016 01:11 PM, Oleg Fayans wrote:

Hi Martin,

I did! Thank you!

On 08/02/2016 12:31 PM, Martin Basti wrote:



On 01.08.2016 22:46, Oleg Fayans wrote:
The test was redesigned so that it actually tests against an AD 
user.

cleanly applies, passes lint and passes

https://paste.fedoraproject.org/399504/00843641/


Okay

Did you forget to send patches?

Martin^2



On 06/28/2016 01:40 PM, Oleg Fayans wrote:

Patch-0050 rebased against latest upstream branch

On 06/28/2016 10:45 AM, Oleg Fayans wrote:

Passing test output:

https://paste.fedoraproject.org/385774/71035231/



















NACK for 0049.1

1)
PEP8: you must use 2 empty lines between functions


Fixed



2)
+new_args = " ".join(new_args + args)

you don't need this, run_command takes list as argument too
new_args.extend(args)


The list-based approach does not work with shell redirects which are
heavily used in the certs_id_idoverrides test. Thus, this trick is
really needed



3)
To make it more usable you should add raiseonerr as kwarg to
run_certutil (True as default)


Done



NACK for 0050.2

1)
+tasks.run_certutil(master, ['-L', '-n', cls.adcert1, '-a', 
'>',

+cls.adcert1_file], cls.reqdir)
+tasks.run_certutil(master, ['-L', '-n', cls.adcert2, '-a', 
'>',

+cls.adcert2_file], cls.reqdir)

IMO thus should raise an error if failed, but previously you set
raiseonerr=False (multiple times)


Agreed. Done



2)
+cls.ad = cls.ad_domains[0].ads[0]
+cls.ad_domain = cls.ad.domain.name
+cls.aduser = "testuser@%s" % cls.ad_domain
+cls.adcert1 = 'MyCert1'
+cls.adcert2 = 'MyCert2'
+cls.adcert1_file = cls.adcert1 + '.crt'
+cls.adcert2_file = cls.adcert2 + '.crt'

New definitions of variables/constants should be directly in class not
in install method, adding new class variables in classmethod is the 
same

evil as adding instance variables outside __init__


Fair point. Fixed



3)
I have question, why do you need AD for this test? AFAIK you can use ID
overrides without AD


Correct. You can, but the workflow would be slightly different. For
example, you can not issue and sign cert requests for AD-users the way
you would do it for local users. We want to have tests that can be taken
by end-users as example how to use our software, that's why it is better
to be as close to real-world use-cases as it is possible.



Martin^3












1)
I still don't see the reason why AD trust is needed. Default trust ID 
view is added just by ipa-adtrust-install, adding trust is not needed 
for current implementation. You don't need AD for this, IDviews is 
generic feature not just for AD. Is that user configured on AD side?


2)
The test itself looks for me as just API/CLI test. IMO it can be in 
ipatests/test_xmlrpc/test_idviews_plugin.py or 
ipatests/test_xmlrpc/test_add_remove_cert_cmd.py


3)
I don't see any integration with SSSD in that test, just pure IPA CLI 
test, shouldn't be this tested against SSSD here?



Martin^2
-- 
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] Announcing FreeIPA 4.4.1

[Martin Basti]

The FreeIPA team would like to announce FreeIPA v4.4.1 release!

It can be downloaded from http://www.freeipa.org/page/Downloads.

Builds for Fedora 24 will be available in the official COPR repository 
.


== Highlights in 4.4.1 ==
=== Enhancements ===
* Kerberos KDC now takes Authentication Indicators into account when 
issuing service tickets. This allows, for example, to require two-factor 
authenticated Kerberos credentials prior to obtaining tickets to a VPN 
service.
* FreeIPA Certificate Authority now is able to create subordinate CAs to 
issue certificates with a specific scope
* Web UI and API end-points now can be configured to log-in with client 
certificates and smart cards. Additional configuration details are 
described in the External Authentication design page 
.

* Web UI now suggests to have redundancy in Certificate Authority topology
* Custom FreeIPA plugins can now be built without modifying core FreeIPA 
code
* When establishing trust to an Active Directory forest, FreeIPA now is 
capable on automatically resolving DNS namespace conflicts with another 
Active Directory forest.


=== Known Issues ===
* Interactive CLI input for dnsrecord-* commands does not work properly 
for multipart records 
* ipa-ca-install fails on replica when master is CA-less 

* Lightweight sub-CA certs are not tracked by certmonger after 
`ipa-replica-install` 
* Certificate revocation in service-del and host-del isn't aware of Sub 
CAs and causes command to fail when Sub CA cert is used 



=== Bug fixes ===
FreeIPA 4.4.1 is a stabilization release for the features delivered as a 
part of 4.4.0. There are more than 140 bug-fixes which details can be 
seen in the list of resolved tickets below.


== Upgrading ==
Upgrade instructions are available on [[Upgrade]] page.

== Feedback ==
Please provide comments, bugs and other feedback via the freeipa-users 
mailing list (http://www.redhat.com/mailman/listinfo/freeipa-users) or 
#freeipa channel on Freenode.


== Detailed changelog since 4.4.0 ==
=== Abhijeet Kasurde (4) ===
* Minor fix in ipa-replica-manage MAN page
* Corrected minor spell check in AD Trust information doc messages
* Removed unwanted line break from RefererError Dialog message
* Handled empty hostname in server-del command

=== Alexander Bokovoy (9) ===
* service: add flag to allow S4U2Self
* support schema files from third-party plugins
* ipaserver/dcerpc: reformat to make the code closer to pep8
* trust: automatically resolve DNS trust conflicts for triangle trusts
* trust: make sure external trust topology is correctly rendered
* trust: make sure ID range is created for the child domain even if it 
exists

* ipa-kdb: simplify trusted domain parent search
* support multiple uid values in schema compatibility tree
* freeipa.spec.in: move ipa CLI utility to freeipa-client

=== Ben Lipton (3) ===
* Fix several small typos
* Use existing HostKey config to test sshd
* Silence sshd messages during install

=== Christian Heimes (5) ===
* Correct path to HTTPD's systemd service directory
* RedHatCAService should wait for local Dogtag instance
* Remove Custodia server keys from LDAP
* Secure permissions of Custodia server.keys
* Require httpd 2.4.6-31 with mod_proxy Unix socket support

=== David Kupka (21) ===
* schema: Fix subtopic -> topic mapping
* help: Add dnsserver commands to help topic 'dns'
* vault: Catch correct exception in decrypt
* schema: Speed up schema cache
* frontend: Change doc, summary, topic and NO_CLI to class properties
* schema: Introduce schema cache format
* schema: Generate bits for help load them on request
* help: Do not create instances to get information about commands and topics
* compat: Save server's API version in for pre-schema servers
* schema cache: Do not reset ServerInfo dirty flag
* schema cache: Do not read fingerprint and format from cache
* Access data for help separately
* frontent: Add summary class property to CommandOverride
* schema cache: Read server info only once
* schema cache: Store API schema cache in memory
* client: Do not create instance just to check isinstance
* schema cache: Read schema instead of rewriting it when SchemaUpToDate
* schema check: Check current client language against cached one
* compat: Fix ping command call
* schema cache: Fallback to 'en_us' when locale is not available
* otptoken, permission: Convert custom type parameters on server

=== Florence Blanc-Renaud (4) ===
* Show full error message for selinuxusermap-add-hostgroup
* server uninstall fails to remove krb principals
* Fix session cookies
* Fix ipa hbactest output

=== Fraser Tweedale (11) ===
* uninstall: untrack lightweight CA certs
* caacl: expand plugin 

[Freeipa-devel] [PATCH] Bump master IPA devel version to 4.4.90

[Martin Basti]

Pushed under oneliner rule

Pushed to master: 371254fc4b36cb4d89351edb19c88a85e5a33a1b


From 17553b8e5d4a58fda8e9f8ad6427366e17aedb29 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 1 Sep 2016 16:20:41 +0200
Subject: [PATCH] Bump master IPA devel version to 4.4.90

---
 VERSION | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/VERSION b/VERSION
index bcc455663855bd03318fbe0ddfbcfee81c67dea8..bf8160a5deb1f7a5148ef6833cec318af144b5d7 100644
--- a/VERSION
+++ b/VERSION
@@ -21,7 +21,7 @@
 
 IPA_VERSION_MAJOR=4
 IPA_VERSION_MINOR=4
-IPA_VERSION_RELEASE=1
+IPA_VERSION_RELEASE=90
 
 
 # For 'alpha' releases the version will be #
-- 
2.7.4

-- 
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] restrict setkeytab operation

[Martin Basti]

On 26.07.2016 13:38, Simo Sorce wrote:

On Mon, 2016-07-25 at 11:26 -0400, Simo Sorce wrote:

On Mon, 2016-07-25 at 11:10 -0400, Rob Crittenden wrote:

Simo Sorce wrote:

On Mon, 2016-07-25 at 10:55 -0400, Rob Crittenden wrote:

Simo Sorce wrote:

As described in #232 start restricting the use of the setkeytab
operation to just the computers objects.

I haven't tested this with older RHEL/CentOS machines that actully use
the setkeytab operation as I do not have such an old VM handy right now.

Meanwhile I'd like to know if ppl agree with this approach.

What about services?

Do we automatically acquire keytab for services in the old clients ?

Are you thinking about scripted ipa-getkytab callouts ?

You are limiting access to host keytabs, what about service keytabs?
Should they be or are they now similarly restricted?

Installers for something like Foreman may try to generate a service
keytab in its installer, probably using admin credentials. I am planning
to do the same in Openstack.

Ok I'll amend the patch to allow service keytabs to still use the
setkeytab control still, and restrict only users.
However note that the idea of using this method is that admin can change
this default on their own, so they can restrict more or less if they
want, to that end I need to remember how to set a default that we do not
override in the update file.

Simo.


Amended patch to allow services too.
Only users are excluded.

Simo.





bump for review
-- 
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] Release 4.4.1 planning

[Martin Basti]

On 30.08.2016 15:00, Alexander Bokovoy wrote:

Hi,

we have a plan to release FreeIPA 4.4.1 on Wednesday, Aug 31st.

I started preparing a release page:
http://www.freeipa.org/page/Releases/4.4.1

It has staggering 140+ closed tickets already.

Please help me with filling in enhancements and bug fixes sections.



We must postpone release today, fix for 
https://fedorahosted.org/freeipa/ticket/6226 pushed yesterday causes 
replica installation issues


Sorry :-(

--
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] 0024 memory leak in ipapwd plugin

[Martin Basti]

On 30.08.2016 13:27, Martin Basti wrote:



On 11.08.2016 16:39, Alexander Bokovoy wrote:

On Thu, 11 Aug 2016, thierry bordaz wrote:
+/* rc should always be 0 (else slapi_sdn_new_dn_byref 
should have sigsev)
+ * but if we end in rc==LDAP_OPERATIONS_ERROR be sure to 
stop here

+ * because ret is not significant */

A short note here. You talk about slapi_sdn_new_dn_byref() but your
patch replaces that with slapi_sdn_new_dn_byval(). Does the comment
still apply?


+if (rc != 0) {
+LOG_OOM();
+goto free_and_return;
+}
+
   if (ret == 0) {
   Slapi_Value *cpw[2] = { NULL, NULL };
   Slapi_Value *pw;
--
2.7.4





Good catch Alexander. Yes the comment contained a wrong cut/paste

ACK.



Is there a ticket for this?


Pushed to master: b942b00ac7bca7e2864c7dc513d25983556916ff

--
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] 0220 move /bin/ipa to freeipa-client

[Martin Basti]

On 30.08.2016 09:27, Jan Cholasta wrote:

On 25.8.2016 13:09, Alexander Bokovoy wrote:

On Thu, 25 Aug 2016, Jan Cholasta wrote:

Hi,

On 25.8.2016 11:27, Alexander Bokovoy wrote:

Hi,

attached patch moves ipa CLI to freeipa-client and obsoletes
freeipa-admintools


The Obsoletes (both) should be on version < 4.4.1 rather than
%{version}, as per Fedora packaging guidelines [1].

Please move the Obsoletes and Provides on %{name}-admintools right
below Group (Obsoletes first) and put a newline between the
%{alt_name}-client and %{alt_name}-admintools blocks, for consistent
layout accross all subpackages in the spec file.



Solves https://fedorahosted.org/freeipa/ticket/5934

Here is how upgrade looks when running 'dnf':

Upgrading:
freeipa-client x86_64
4.4.0.201608250913GIT9c20682-0.fc24 @commandline
146 k
   replacing  freeipa-admintools.noarch
4.4.0.201608051228GIT590e30f-0.fc24


I'm going to test with yum as well, for RHEL and CentOS.

Updated patch attached.


Thanks, ACK.

Pushed to master: b91ba39d627d36d1a62ebf97a987a2579404395e


I'm experiencing this:

[root@vm-058-017 ~]# dnf reinstall /home/mbasti/freeipa-rpms/* -y
Last metadata expiration check: 1:32:14 ago on Tue Aug 30 12:32:32 2016.
Dependencies resolved.
===
 Package Arch Version Repository
Size

===
Reinstalling:
 freeipa-client x86_64 4.4.0-0.fc24 
@commandline 146 k

 replacing  freeipa-admintools.noarch 4.4.0-0.fc24
 freeipa-client-common noarch 4.4.0-0.fc24 
@commandline  28 k
 freeipa-common noarch 4.4.0-0.fc24 
@commandline 386 k
 freeipa-debuginfo x86_64 4.4.0-0.fc24 
@commandline 934 k
 freeipa-python-compat noarch 4.4.0-0.fc24 
@commandline  22 k
 freeipa-server x86_64 4.4.0-0.fc24 
@commandline 350 k
 freeipa-server-common noarch 4.4.0-0.fc24 
@commandline 524 k
 freeipa-server-dns noarch 4.4.0-0.fc24 
@commandline  26 k
 freeipa-server-trust-ad x86_64 4.4.0-0.fc24 
@commandline 116 k
 python2-ipaclient noarch 4.4.0-0.fc24 
@commandline 443 k
 python2-ipalib noarch 4.4.0-0.fc24 
@commandline 560 k
 python2-ipaserver noarch 4.4.0-0.fc24 
@commandline 1.2 M
 python2-ipatests noarch 4.4.0-0.fc24 
@commandline 840 k
 python3-ipaclient noarch 4.4.0-0.fc24 
@commandline 626 k
 python3-ipalib noarch 4.4.0-0.fc24 
@commandline 568 k
 python3-ipatests noarch 4.4.0-0.fc24 
@commandline 842 k


Transaction Summary
===

Total size: 7.4 M
Downloading Packages:
Running transaction check
Error: transaction check vs depsolve:
ipa-admintools conflicts with freeipa-client-4.4.0-0.fc24.x86_64
freeipa-admintools < 4.4.1 is obsoleted by 
freeipa-client-4.4.0-0.fc24.x86_64
ipa-admintools conflicts with (installed) 
freeipa-admintools-4.4.0-0.fc24.noarch

To diagnose the problem, try running: 'rpm -Va --nofiles --nodigest'.
You probably have corrupted RPMDB, running 'rpm --rebuilddb' might fix 
the issue.



--
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] [Test][Patch-0049, 0050] Certs in ID overrides test

[Martin Basti]

On 22.08.2016 13:18, Oleg Fayans wrote:

ping for review

On 08/02/2016 01:11 PM, Oleg Fayans wrote:

Hi Martin,

I did! Thank you!

On 08/02/2016 12:31 PM, Martin Basti wrote:



On 01.08.2016 22:46, Oleg Fayans wrote:

The test was redesigned so that it actually tests against an AD user.
cleanly applies, passes lint and passes

https://paste.fedoraproject.org/399504/00843641/


Okay

Did you forget to send patches?

Martin^2



On 06/28/2016 01:40 PM, Oleg Fayans wrote:

Patch-0050 rebased against latest upstream branch

On 06/28/2016 10:45 AM, Oleg Fayans wrote:

Passing test output:

https://paste.fedoraproject.org/385774/71035231/



















NACK for 0049.1

1)
PEP8: you must use 2 empty lines between functions

2)
+new_args = " ".join(new_args + args)

you don't need this, run_command takes list as argument too
new_args.extend(args)

3)
To make it more usable you should add raiseonerr as kwarg to 
run_certutil (True as default)


NACK for 0050.2

1)
+tasks.run_certutil(master, ['-L', '-n', cls.adcert1, '-a', '>',
+cls.adcert1_file], cls.reqdir)
+tasks.run_certutil(master, ['-L', '-n', cls.adcert2, '-a', '>',
+cls.adcert2_file], cls.reqdir)

IMO thus should raise an error if failed, but previously you set 
raiseonerr=False (multiple times)


2)
+cls.ad = cls.ad_domains[0].ads[0]
+cls.ad_domain = cls.ad.domain.name
+cls.aduser = "testuser@%s" % cls.ad_domain
+cls.adcert1 = 'MyCert1'
+cls.adcert2 = 'MyCert2'
+cls.adcert1_file = cls.adcert1 + '.crt'
+cls.adcert2_file = cls.adcert2 + '.crt'

New definitions of variables/constants should be directly in class not 
in install method, adding new class variables in classmethod is the same 
evil as adding instance variables outside __init__


3)
I have question, why do you need AD for this test? AFAIK you can use ID 
overrides without AD


Martin^3

--
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] 0024 memory leak in ipapwd plugin

[Martin Basti]

On 11.08.2016 16:39, Alexander Bokovoy wrote:

On Thu, 11 Aug 2016, thierry bordaz wrote:
+/* rc should always be 0 (else slapi_sdn_new_dn_byref 
should have sigsev)
+ * but if we end in rc==LDAP_OPERATIONS_ERROR be sure to 
stop here

+ * because ret is not significant */

A short note here. You talk about slapi_sdn_new_dn_byref() but your
patch replaces that with slapi_sdn_new_dn_byval(). Does the comment
still apply?


+if (rc != 0) {
+LOG_OOM();
+goto free_and_return;
+}
+
   if (ret == 0) {
   Slapi_Value *cpw[2] = { NULL, NULL };
   Slapi_Value *pw;
--
2.7.4





Good catch Alexander. Yes the comment contained a wrong cut/paste

ACK.



Is there a ticket for this?

--
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] [Test][patch-0061] Fixed error in teardown method of replica_promotion tests

[Martin Basti]

On 24.08.2016 16:26, Oleg Fayans wrote:





ACK

Pushed to master: 5812af84a4a12528e969f14017e9675160b3faef

-- 
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] [DESIGN][UPDATE] Time-Based HBAC Policies

[Martin Basti]

On 30.08.2016 11:51, Standa Laznicka wrote:

On 08/30/2016 09:34 AM, Standa Laznicka wrote:

On 08/30/2016 09:23 AM, Alexander Bokovoy wrote:

On Tue, 30 Aug 2016, Jan Cholasta wrote:

On 30.8.2016 08:47, Standa Laznicka wrote:

On 08/26/2016 05:37 PM, Simo Sorce wrote:

On Fri, 2016-08-26 at 11:26 -0400, Simo Sorce wrote:

On Fri, 2016-08-26 at 18:09 +0300, Alexander Bokovoy wrote:

On Fri, 26 Aug 2016, Simo Sorce wrote:

On Fri, 2016-08-26 at 12:39 +0200, Martin Basti wrote:
I miss "why" part of "To be able to handle backward 
compatibility

with

ease, a new object called ipaHBACRulev2 is introduced. " in the

design
page. If the reason is the above - old client's should 
ignore time

rules
then it has to be mentioned there. Otherwise I don't see a 
reason to

introduce a new object type instead of extending the current.
How do you want to enforce HBAC rule that have set time from 
10 to 14
everyday? With the same objectclass old clients will allow 
this HBAC

for
all day. Isn't this CVE?

This is a discussion worth having.

In general it is a CVE only if an authorization mechanism 
fails to

work
as advertised.

If you make it clear that old clients *DO NOT* respect time 
rules then

there is no CVE material, it is working as "described".

The admins already have a way to not set those rules for older 
clients

by simply grouping newer clients in a different host group and
applying
time rules only there.

So the question really is: should we allow admins to apply an 
HBAC

Rule
potentially to older clients that do not understand it and will
therefore allow access at any time of the day, or should we 
prevent

it ?

This is a hard question to answer and can go both ways.

A time rule may be something that admins want to enforce at all
cost or
deny access. In this case a client that fails to handle it 
would be a

problem.

But it may be something that is just used for defense in depth 
and

not a
strictly hard requirement. In this case allowing older clients 
would
make it an easy transition as you just set up the rule and the 
client
will start enforcing the time when it is upgraded but work 
otherwise

with the same rules.

I am a bit conflicted on trying to decide what scenario we should
target, but the second one appeals to me because host groups do
already
give admins a good way to apply rules to a specific set of 
hosts and

exclude old clients w/o us making it a hard rule.
OTOH if an admin does not understand this difference, they may be
surprised to find out there are clients that do not honor it.

Perhaps we could find a way to set a flag on the rule such that
when set
(and only when set) older clients get excluded by way of 
changing the

objectlass or something else to similar effect.

Open to discussion.
At this point using new object class becomes an attractive 
approach. We

don't have means to exclude HBAC rules other than applying them
per-host/hostgroup. We also have no deny rules.

I have another idea: what about enforcing time rules always to 
apply
per-host or per-hostgroup by default? Add --force option to 
override

the
behavior but default to not allow --hostcat=all. This would raise
awareness and make sure admins are actually applying these 
rules with

intention.
This sounds like a good idea, but it is not a silver bullet I am 
afraid.


Simo.
I was thinking that for future proofing we could add a version 
field,

then reasoned more and realized that changing the object class is
basically the same thing.

There is only one big problem, ipaHBACRule is a STRUCTURAL 
objectclass.
(I know 389ds allows us to do an LDAPv3 illegal operation and 
change it,

but I do not like to depend on that behavoir).

Now looking into this I had an idea to solve the problem of legacy
clients without having to swap classes.
We can redefine the accessRuleType attribute to be a "capability" 
type.


Ie rules that have a timeAccess component will be of type
"allow_with_time" instead of just "allow".
Old clients are supposed to search with accessRuleType=allow (and 
I can

see that SSSD does that), so an older client will fail to get those
rules as they won't match.

New clients instead can recognize both types.

Also if we need a future extension we will simpy add a new access 
rule

type and we can have the same effect.
The nice thing is that accessRyleType is defined as multivalue (no
SINGLE in schema) so we may actually create compatible rules if 
we want

to.
Ie we could set both "allow" and "allow_with_time" on an object for
cases where the admin wants to enforce the time part only o newer 
client

but otherwise apply the rule to any client.

This should give us the best of all options at once.

Thoughts ?

Simo.


Sorry to join the discussion so late, I was away yesterday.

I have to say I too like this idea much better than fiddling with the
objectClasses.


Note that the resulting code will be exactly 

Re: [Freeipa-devel] [PATCH 0159] Tests: fix test_forward_zones in test_xmlrpc/test_dns_plugin

[Martin Basti]

On 16.08.2016 13:55, Martin Basti wrote:




On 16.08.2016 13:30, Martin Basti wrote:




On 12.08.2016 20:00, Petr Spacek wrote:

Hello,

this is the last patch necessary to get all test_xmlrpc/test_dns_plugin tests
to pass! (I hope :-)

 Tests: fix test_forward_zones in test_xmlrpc/test_dns_plugin

 Class test_forward_zones in ipatests/test_xmlrpc/test_dns_plugin
 was using DNS zone 'fwzone2.test.' and expected to get warning
 'Forwarding policy conflicts with some automatic empty zones.'
 (aka 'DNSForwardPolicyConflictWithEmptyZone').

 This does not make sense because 'test.' zone is not listed in IANA 
registry
 'Locally-Served DNS Zones':

http://www.iana.org/assignments/locally-served-dns-zones/locally-served-dns-zones.xhtml

 To fix this I simply removed the warning from set of expected results.

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






NACK

raise AssertionError(
>   VALUE % (doc, expected, got, stack)
E   AssertionError: assert_deepequal: expected != got.
E 0014: dnsforwardzone_add: Create forward zone 
u'fwzone2.test.' with forwarders with default ("first") policy

E expected =  at 0x7f76f1225500>
E got = u"query 'fwzone2.test. SOA': The DNS 
operation timed out after 10.0012469292 seconds"

E path = (u'messages', 0, u'data', u'error')


I don't think that this will work, you must use function to check 
list, not list containing lambda functions



Martin^2




Sorry, my bad, it is just wrong expected message in lambda




This was resolved in:
https://github.com/freeipa/freeipa/pull/36

Please don't mix PR and patches

-- 
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] [DESIGN][UPDATE] Time-Based HBAC Policies

[Martin Basti]

On 26.08.2016 12:37, Petr Vobornik wrote:

On 08/26/2016 12:23 PM, Martin Basti wrote:


On 26.08.2016 12:20, Alexander Bokovoy wrote:

On Fri, 26 Aug 2016, Jan Cholasta wrote:

On 26.8.2016 11:55, Martin Basti wrote:


On 26.08.2016 11:43, Jan Cholasta wrote:

Hi,

On 11.8.2016 12:34, Stanislav Laznicka wrote:

Hello,

I updated the design of the Time-Based HBAC Policies according to the
discussion we led here earlier. Please check the design page
http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The
biggest
changes are in the Implementation and Feature Management sections. I
also added a short How to Use section.

1) Please use the 'ipa' prefix for new attributes: memberTimeRule ->
ipaMemberTimeRule


2) Source hosts are deprecated and thus should be removed from
ipaHBACRuleV2.


3) Since time rules are defined by memberTimeRule, accessTime should
be removed from ipaHBACRuleV2.

ad 2) 3)

Because backward compatibility, ipaHBACRuleV2 must contain all
attributes from ipaHBACRule as MAY

Not true.


With current approach, when timerule is added to HBAC, we just change
objectclass from 'ipahbacrule' to 'ipahbacrulev2' so we keep all
attributes that was defined in older HBAC. Removing any attrs from
ipaHBACRuleV2 can cause schema violation.

Which is perfectly fine.



I'm not sure if want to handle this in code (removing deprecated
attributes from HBAC entry when timerule is added)

We don't have to do anything. If any of the deprecated attributes are
present when you change the object class (which they shouldn't
anyway), you'll get schema violation, otherwise it will work just fine.


I realized that AccessTime is MUST for 'ipahbacrule', so when timerule
('ipahbacrulev2') is removed and somebody deleted accesstime we have to
add it back.

It is MAY. The only MUST attribute is accessRuleType, but that is
deprecated as well and should be removed from ipaHBACRuleV2. We only
support allow rules, so when timerule is removed, that's the value
you set accessRuleType to.

SSSD does search for HBAC rules by '(objectclass=ipaHBACRule)' filter.
Changing to ipaHBACRuleV2 means these rules will not be found by older
SSSD versions and therefore people will experience problems with older
clients not being able to use new rules even if they would lack time
component.


Older client do not support timerules, so they should not search for
them. HBAC without timerules will be still have 'ipaHBACRule'
objectclass and will work with old clients. Only HBAC with timerules
will have assigned 'ipaHBACRuleV2'

Martin^2

I miss "why" part of "To be able to handle backward compatibility with
ease, a new object called ipaHBACRulev2 is introduced. " in the design
page. If the reason is the above - old client's should ignore time rules
then it has to be mentioned there. Otherwise I don't see a reason to
introduce a new object type instead of extending the current.


How do you want to enforce HBAC rule that have set time from 10 to 14 
everyday? With the same objectclass old clients will allow this HBAC for 
all day. Isn't this CVE?





2. About API and CLI: wasn't there an idea to hide/not provide
--icalfile=file.ics and --time=escaped_icalstring  options in the first
implementation. So that we can limit the support scope to only a subset
of option(the  OPTS part). If arbitrary ical is allowed since the
beginning then we are asking for a lot of bugs filed.



--
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] [DESIGN][UPDATE] Time-Based HBAC Policies

[Martin Basti]

On 26.08.2016 12:20, Alexander Bokovoy wrote:

On Fri, 26 Aug 2016, Jan Cholasta wrote:

On 26.8.2016 11:55, Martin Basti wrote:



On 26.08.2016 11:43, Jan Cholasta wrote:

Hi,

On 11.8.2016 12:34, Stanislav Laznicka wrote:

Hello,

I updated the design of the Time-Based HBAC Policies according to the
discussion we led here earlier. Please check the design page
http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The 
biggest

changes are in the Implementation and Feature Management sections. I
also added a short How to Use section.


1) Please use the 'ipa' prefix for new attributes: memberTimeRule ->
ipaMemberTimeRule


2) Source hosts are deprecated and thus should be removed from
ipaHBACRuleV2.


3) Since time rules are defined by memberTimeRule, accessTime should
be removed from ipaHBACRuleV2.


ad 2) 3)

Because backward compatibility, ipaHBACRuleV2 must contain all
attributes from ipaHBACRule as MAY


Not true.



With current approach, when timerule is added to HBAC, we just change
objectclass from 'ipahbacrule' to 'ipahbacrulev2' so we keep all
attributes that was defined in older HBAC. Removing any attrs from
ipaHBACRuleV2 can cause schema violation.


Which is perfectly fine.




I'm not sure if want to handle this in code (removing deprecated
attributes from HBAC entry when timerule is added)


We don't have to do anything. If any of the deprecated attributes are 
present when you change the object class (which they shouldn't 
anyway), you'll get schema violation, otherwise it will work just fine.




I realized that AccessTime is MUST for 'ipahbacrule', so when timerule
('ipahbacrulev2') is removed and somebody deleted accesstime we have to
add it back.


It is MAY. The only MUST attribute is accessRuleType, but that is 
deprecated as well and should be removed from ipaHBACRuleV2. We only 
support allow rules, so when timerule is removed, that's the value 
you set accessRuleType to.


SSSD does search for HBAC rules by '(objectclass=ipaHBACRule)' filter.
Changing to ipaHBACRuleV2 means these rules will not be found by older
SSSD versions and therefore people will experience problems with older
clients not being able to use new rules even if they would lack time
component.



Older client do not support timerules, so they should not search for 
them. HBAC without timerules will be still have 'ipaHBACRule' 
objectclass and will work with old clients. Only HBAC with timerules 
will have assigned 'ipaHBACRuleV2'


Martin^2

--
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] [DESIGN][UPDATE] Time-Based HBAC Policies

[Martin Basti]

On 26.08.2016 12:13, Jan Cholasta wrote:

On 26.8.2016 11:55, Martin Basti wrote:



On 26.08.2016 11:43, Jan Cholasta wrote:

Hi,

On 11.8.2016 12:34, Stanislav Laznicka wrote:

Hello,

I updated the design of the Time-Based HBAC Policies according to the
discussion we led here earlier. Please check the design page
http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The 
biggest

changes are in the Implementation and Feature Management sections. I
also added a short How to Use section.


1) Please use the 'ipa' prefix for new attributes: memberTimeRule ->
ipaMemberTimeRule


2) Source hosts are deprecated and thus should be removed from
ipaHBACRuleV2.


3) Since time rules are defined by memberTimeRule, accessTime should
be removed from ipaHBACRuleV2.


ad 2) 3)

Because backward compatibility, ipaHBACRuleV2 must contain all
attributes from ipaHBACRule as MAY


Not true.



With current approach, when timerule is added to HBAC, we just change
objectclass from 'ipahbacrule' to 'ipahbacrulev2' so we keep all
attributes that was defined in older HBAC. Removing any attrs from
ipaHBACRuleV2 can cause schema violation.


Which is perfectly fine.




I'm not sure if want to handle this in code (removing deprecated
attributes from HBAC entry when timerule is added)


We don't have to do anything. If any of the deprecated attributes are 
present when you change the object class (which they shouldn't 
anyway), you'll get schema violation, otherwise it will work just fine.


I'm not sure if this is user friendly.





I realized that AccessTime is MUST for 'ipahbacrule', so when timerule
('ipahbacrulev2') is removed and somebody deleted accesstime we have to
add it back.


It is MAY. The only MUST attribute is accessRuleType, but that is 
deprecated as well and should be removed from ipaHBACRuleV2. We only 
support allow rules, so when timerule is removed, that's the value you 
set accessRuleType to.



Right, sorry.
Martin^2








4) The CLI sections needs more work, especially for non-standard
commands like timerule-test.



On the link below is a PROTOTYPE-patched FreeIPA that covers most 
of the

CLI functionality (except for the creation of iCalendar strings from
options) for better illustration of the design.

https://github.com/stlaz/freeipa/tree/timerules_2

I will add FreeIPA people that recently had some say about this to 
CC so

that we can get the discussion flowing.


Honza








--
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] [DESIGN][UPDATE] Time-Based HBAC Policies

[Martin Basti]

On 26.08.2016 11:43, Jan Cholasta wrote:

Hi,

On 11.8.2016 12:34, Stanislav Laznicka wrote:

Hello,

I updated the design of the Time-Based HBAC Policies according to the
discussion we led here earlier. Please check the design page
http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The biggest
changes are in the Implementation and Feature Management sections. I
also added a short How to Use section.


1) Please use the 'ipa' prefix for new attributes: memberTimeRule -> 
ipaMemberTimeRule



2) Source hosts are deprecated and thus should be removed from 
ipaHBACRuleV2.



3) Since time rules are defined by memberTimeRule, accessTime should 
be removed from ipaHBACRuleV2.


ad 2) 3)

Because backward compatibility, ipaHBACRuleV2 must contain all 
attributes from ipaHBACRule as MAY


With current approach, when timerule is added to HBAC, we just change 
objectclass from 'ipahbacrule' to 'ipahbacrulev2' so we keep all 
attributes that was defined in older HBAC. Removing any attrs from 
ipaHBACRuleV2 can cause schema violation.



I'm not sure if want to handle this in code (removing deprecated 
attributes from HBAC entry when timerule is added)


I realized that AccessTime is MUST for 'ipahbacrule', so when timerule 
('ipahbacrulev2') is removed and somebody deleted accesstime we have to 
add it back.







4) The CLI sections needs more work, especially for non-standard 
commands like timerule-test.




On the link below is a PROTOTYPE-patched FreeIPA that covers most of the
CLI functionality (except for the creation of iCalendar strings from
options) for better illustration of the design.

https://github.com/stlaz/freeipa/tree/timerules_2

I will add FreeIPA people that recently had some say about this to CC so
that we can get the discussion flowing.


Honza



--
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 0060] Add --force-join option to ipa-replica-install

[Martin Basti]

On 10.08.2016 07:53, Stanislav Laznicka wrote:

On 08/10/2016 07:31 AM, Jan Cholasta wrote:

On 9.8.2016 18:52, Petr Vobornik wrote:

On 08/09/2016 04:18 PM, Martin Basti wrote:



On 09.08.2016 16:07, Stanislav Laznicka wrote:

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



Didn't we agreed that --force-join should be always used (without 
extra

replica-install option)


+1


Did we?

IMO the default behavior should be the same as in domain level 0 when 
trying to install replica on an already enrolled host.

That was my impression as well.


OK then, I don't like to add mostly useless option, but client install 
is broken by design so whatever.


Martin^2

--
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 0213] support multiple uid values in slapi-nis users map

[Martin Basti]

On 25.08.2016 10:32, Alexander Bokovoy wrote:

On Tue, 23 Aug 2016, thierry bordaz wrote:

acceptance is now completed (successfully).  ACK


bump

so ACKed ab's 213-1 fixes 
https://fedorahosted.org/freeipa/ticket/6138 ?



Yes that is my understanding. patch 213-1 fixes #6138.
I verified that lookup of UPN entries does return the domain. But did 
not know how to check that entries with multiple uid values only 
returns the first value.

Can we push 0213-1?

Pushed to master: fab1f798ed6dfdb0b7de7c4fc4fe353f1d97177b

--
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] 0004 Fix ipa-server-install in pure IPv6 environment

[Martin Basti]

On 24.08.2016 18:41, Martin Basti wrote:




On 19.08.2016 14:09, Tomas Krizek wrote:

Hi,

please review the attached patch.

Make sure the hostname isn't resolved to link local IPv6(feXX:...) 
during testing, which doesn't work (and isn't supposed to).





It did not work for me,

pki-ca-spawn.log:
/ca/getStatus (Caused by 
NewConnectionError('object at 0x7f3d35854310>: Failed to establish a new connection: 
[Errno 111] Connection refused',))
2016-08-24 18:07:12 pkispawn: ERROR... server failed to 
restart

2016-08-24 18:07:12 pkispawn: DEBUG... Error Type: Exception
2016-08-24 18:07:12 pkispawn: DEBUG... Error Message: 
server failed to restart
2016-08-24 18:07:12 pkispawn: DEBUG...   File 
"/usr/sbin/pkispawn", line 528, in main

scriptlet.spawn(deployer)
  File 
"/usr/lib/python2.7/site-packages/pki/server/deployment/scriptlets/configuration.py", 
line 375, in spawn

raise Exception("server failed to restart")


journalctl:
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: Java virtual machine used: 
/usr/lib/jvm/jre-1.8.0-openjdk/bin/java
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: classpath used: 
/usr/share/tomcat/bin/bootstrap.jar:/usr/share/tomcat/bin/tomcat-juli.jar:/usr/lib/java/commons-daemon.jar
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: main class used: org.apache.catalina.startup.Bootstrap
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: flags used: -DRESTEASY_LIB=/usr/share/java/resteasy 
-Djava.library.path=/usr/lib64/nuxwdog-jni
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: options used: -Dcatalina.base=/var/lib/pki/pki-tomcat 
-Dcatalina.home=/usr/share/tomcat -Djava.endorsed.dirs= 
-Djava.io.tmpdir=/var/lib/pk
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: arguments used: stop
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: Aug 24, 2016 6:06:22 PM 
org.apache.catalina.startup.Catalina stopServer
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: SEVERE: Catalina.stop:
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: java.net.SocketException: Network is unreachable
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.PlainSocketImpl.socketConnect(Native Method)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.connect(Socket.java:589)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.connect(Socket.java:538)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.(Socket.java:434)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.(Socket.java:211)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
org.apache.catalina.startup.Catalina.stopServer(Catalina.java:450)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.lang.reflect.Method.invoke(Method.java:498)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
org.apache.catalina.startup.Bootstrap.stopServer(Bootstrap.java:400)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
org.apache.catalina.startup.Bootstrap.main(Bootstrap.java:487)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com systemd[1]: 
pki-tomcatd@pki-tomcat.service: Control process exited, code=exited 
status=1
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com systemd[1]: 
pki-tomcatd@pki-tomcat.service: Unit entered failed state.
Aug 24 18:06:22 vm-058-1

Re: [Freeipa-devel] [PATCH] 0004 Fix ipa-server-install in pure IPv6 environment

[Martin Basti]

On 19.08.2016 14:09, Tomas Krizek wrote:

Hi,

please review the attached patch.

Make sure the hostname isn't resolved to link local IPv6(feXX:...) 
during testing, which doesn't work (and isn't supposed to).





It did not work for me,

pki-ca-spawn.log:
/ca/getStatus (Caused by 
NewConnectionError('object at 0x7f3d35854310>: Failed to establish a new connection: [Errno 
111] Connection refused',))

2016-08-24 18:07:12 pkispawn: ERROR... server failed to restart
2016-08-24 18:07:12 pkispawn: DEBUG... Error Type: Exception
2016-08-24 18:07:12 pkispawn: DEBUG... Error Message: server 
failed to restart
2016-08-24 18:07:12 pkispawn: DEBUG...   File 
"/usr/sbin/pkispawn", line 528, in main

scriptlet.spawn(deployer)
  File 
"/usr/lib/python2.7/site-packages/pki/server/deployment/scriptlets/configuration.py", 
line 375, in spawn

raise Exception("server failed to restart")


journalctl:
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
Java virtual machine used: /usr/lib/jvm/jre-1.8.0-openjdk/bin/java
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
classpath used: 
/usr/share/tomcat/bin/bootstrap.jar:/usr/share/tomcat/bin/tomcat-juli.jar:/usr/lib/java/commons-daemon.jar
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
main class used: org.apache.catalina.startup.Bootstrap
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
flags used: -DRESTEASY_LIB=/usr/share/java/resteasy 
-Djava.library.path=/usr/lib64/nuxwdog-jni
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
options used: -Dcatalina.base=/var/lib/pki/pki-tomcat 
-Dcatalina.home=/usr/share/tomcat -Djava.endorsed.dirs= 
-Djava.io.tmpdir=/var/lib/pk
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
arguments used: stop
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
Aug 24, 2016 6:06:22 PM org.apache.catalina.startup.Catalina stopServer
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
SEVERE: Catalina.stop:
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com server[58257]: 
java.net.SocketException: Network is unreachable
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.PlainSocketImpl.socketConnect(Native 
Method)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.connect(Socket.java:589)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.connect(Socket.java:538)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.(Socket.java:434)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.net.Socket.(Socket.java:211)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
org.apache.catalina.startup.Catalina.stopServer(Catalina.java:450)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at java.lang.reflect.Method.invoke(Method.java:498)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
org.apache.catalina.startup.Bootstrap.stopServer(Bootstrap.java:400)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com 
server[58257]: at 
org.apache.catalina.startup.Bootstrap.main(Bootstrap.java:487)
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com systemd[1]: 
pki-tomcatd@pki-tomcat.service: Control process exited, code=exited status=1
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com systemd[1]: 
pki-tomcatd@pki-tomcat.service: Unit entered failed state.
Aug 24 18:06:22 vm-058-188.abc.idm.lab.eng.brq.redhat.com systemd[1]: 
pki-tomcatd@pki-tomcat.service: 

Re: [Freeipa-devel] [PATCH 0036, 0037][Tests] Host/service tests do not recognize newly added attribute

[Martin Basti]

On 24.08.2016 15:49, Ganna Kaihorodova wrote:

Hello!

[0036] ACK
[0037] ACK

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Lenka Doudova" 
To: "freeipa-devel" 
Sent: Monday, August 22, 2016 12:17:23 PM
Subject: [Freeipa-devel] [PATCH 0036, 0037][Tests] Host/service tests do not 
recognize newly added attribute

Hi,

attached patches fix test fails occuring since patch for [1] was pushed.

Ticket for tests: https://fedorahosted.org/freeipa/ticket/6240

Lenka


[1] https://fedorahosted.org/freeipa/ticket/5764



Pushed to master: 9021b649661ed135a4ee18ffe3728d661e6674a6

--
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] 0003 Validate key in otptoken-add

[Martin Basti]

On 24.08.2016 13:32, Tomas Krizek wrote:

Fixed the typo in error message.

On 08/23/2016 12:15 PM, Tomas Krizek wrote:

In that case, the first version of the patch solves the issue.

I'm attaching the patch once again, but it's the same as the one in 
the original message.



On 08/23/2016 11:53 AM, Jan Cholasta wrote:

On 22.8.2016 19:08, Tomas Krizek wrote:
I've attached the updated patch. Hopefully I didn't forget anything 
else

this time.


On 08/22/2016 05:48 PM, Martin Basti wrote:


On 22.08.2016 10:22, Tomas Krizek wrote:


Seems like a good idea, I'm attaching the updated patch. Autofill
does work when the param is required.


On 08/19/2016 04:19 PM, Martin Basti wrote:




On 16.08.2016 17:35, Tomas Krizek wrote:

Hi,

the attached patch fixes an error message when user provides an
empty key while adding otp token.

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





I'm curious why we don't fix it here:

OTPTokenKey('ipatokenotpkey?',
cli_name='key',
label=_('Key'),
doc=_('Token secret (Base32; default: random)'),
default_from=lambda: os.urandom(KEY_LENGTH),
autofill=True,
flags=('no_display', 'no_update', 'no_search'),
),


If OTPTokenKey is mandratory, it should be required param (autofill
should work in this case too)

Martin^2


--
Tomas Krizek


You changed API, you must regenerate API.txt (./makeapi) and 
increment

minor version in VERSION file

Option 'ipatokenotpkey?' in command 'otptoken_add/1' in API file 
not found

Options count in otptoken_add of 22 doesn't match expected: 23
Option ipatokenotpkey of command otptoken_add in ipalib, not in 
API file:

OTPTokenKey('ipatokenotpkey', autofill=True, cli_name='key')


NACK, this is a backward incompatible change.

AFAICT the option should remain optional, see the doc string:

Token secret (Base32; default: random)
  ^^^







--
Tomas Krizek


ACK

Pushed to master: 6f9a029bf5d33e6c8267cb330bd48033c5517188


http://www.freeipa.org/page/Pull_request_on_Github
-- 
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] 0003 Validate key in otptoken-add

[Martin Basti]

On 24.08.2016 13:32, Tomas Krizek wrote:

Fixed the typo in error message.

On 08/23/2016 12:15 PM, Tomas Krizek wrote:

In that case, the first version of the patch solves the issue.

I'm attaching the patch once again, but it's the same as the one in 
the original message.



On 08/23/2016 11:53 AM, Jan Cholasta wrote:

On 22.8.2016 19:08, Tomas Krizek wrote:
I've attached the updated patch. Hopefully I didn't forget anything 
else

this time.


On 08/22/2016 05:48 PM, Martin Basti wrote:


On 22.08.2016 10:22, Tomas Krizek wrote:


Seems like a good idea, I'm attaching the updated patch. Autofill
does work when the param is required.


On 08/19/2016 04:19 PM, Martin Basti wrote:




On 16.08.2016 17:35, Tomas Krizek wrote:

Hi,

the attached patch fixes an error message when user provides an
empty key while adding otp token.

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





I'm curious why we don't fix it here:

OTPTokenKey('ipatokenotpkey?',
cli_name='key',
label=_('Key'),
doc=_('Token secret (Base32; default: random)'),
default_from=lambda: os.urandom(KEY_LENGTH),
autofill=True,
flags=('no_display', 'no_update', 'no_search'),
),


If OTPTokenKey is mandratory, it should be required param (autofill
should work in this case too)

Martin^2


--
Tomas Krizek


You changed API, you must regenerate API.txt (./makeapi) and 
increment

minor version in VERSION file

Option 'ipatokenotpkey?' in command 'otptoken_add/1' in API file 
not found

Options count in otptoken_add of 22 doesn't match expected: 23
Option ipatokenotpkey of command otptoken_add in ipalib, not in 
API file:

OTPTokenKey('ipatokenotpkey', autofill=True, cli_name='key')


NACK, this is a backward incompatible change.

AFAICT the option should remain optional, see the doc string:

Token secret (Base32; default: random)
  ^^^







--
Tomas Krizek

Pushed to master: 6f9a029bf5d33e6c8267cb330bd48033c5517188

-- 
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 0039][Tests] ID views tests do not recognize 'krbcanonicalname' attribute

[Martin Basti]

On 22.08.2016 15:46, Lenka Doudova wrote:

Hi,

ID views tests still do not recognize 'krbcanonicalname' attribute - 
fix attached.


Lenka




ACK

Pushed to master: 775c37bb812604496594524d8c6c7d936b4d3b15

-- 
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 0035] Remove Custodia server keys from LDAP

[Martin Basti]

On 24.08.2016 11:25, Christian Heimes wrote:

On 2016-08-23 12:42, Petr Vobornik wrote:

On 08/11/2016 04:13 PM, Martin Basti wrote:


On 08.08.2016 16:10, Christian Heimes wrote:

The server-del plugin now removes the Custodia keys for encryption and
key signing from LDAP.

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



ACK for master

For 4.3, it requires new patch

Martin


bump

I haven't worked out a patch for 4.3. The server_del plugin in 4.3 is
much simpler than in 4.4. It's not possible to hook the clean-up code to
server_del like I did for 4.4. I would have to rewrite and redesign the
patch completely which I neither have the time nor resources to at the
moment.

I vote for WONTFIX for 4.3.

+1

Martin^2


Christian




--
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] 0003 Validate key in otptoken-add

[Martin Basti]

On 22.08.2016 19:05, Martin Basti wrote:




On 22.08.2016 17:48, Martin Basti wrote:




On 22.08.2016 10:22, Tomas Krizek wrote:


Seems like a good idea, I'm attaching the updated patch. Autofill 
does work when the param is required.



On 08/19/2016 04:19 PM, Martin Basti wrote:




On 16.08.2016 17:35, Tomas Krizek wrote:

Hi,

the attached patch fixes an error message when user provides an 
empty key while adding otp token.


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





I'm curious why we don't fix it here:

OTPTokenKey('ipatokenotpkey?',
cli_name='key',
label=_('Key'),
doc=_('Token secret (Base32; default: random)'),
default_from=lambda: os.urandom(KEY_LENGTH),
autofill=True,
flags=('no_display', 'no_update', 'no_search'),
),


If OTPTokenKey is mandratory, it should be required param (autofill 
should work in this case too)


Martin^2


--
Tomas Krizek


You changed API, you must regenerate API.txt (./makeapi) and 
increment minor version in VERSION file


Option 'ipatokenotpkey?' in command 'otptoken_add/1' in API file not 
found

Options count in otptoken_add of 22 doesn't match expected: 23
Option ipatokenotpkey of command otptoken_add in ipalib, not in API file:
OTPTokenKey('ipatokenotpkey', autofill=True, cli_name='key')

Martin^2




[root@vm-058-107 ~]# ipa otptoken-add --key='ORSXG5DFON2AU==='
Usage: ipa [global-options] otptoken-add [ID] [options]

ipa: error: --key option does not take a value


Well patch doesnt work for me, Honza may know if this is expected 
behavior of framework or just params bug


Martin62




My, bad this is expected.

Martin^2
-- 
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] 0003 Validate key in otptoken-add

[Martin Basti]

On 22.08.2016 17:48, Martin Basti wrote:




On 22.08.2016 10:22, Tomas Krizek wrote:


Seems like a good idea, I'm attaching the updated patch. Autofill 
does work when the param is required.



On 08/19/2016 04:19 PM, Martin Basti wrote:




On 16.08.2016 17:35, Tomas Krizek wrote:

Hi,

the attached patch fixes an error message when user provides an 
empty key while adding otp token.


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





I'm curious why we don't fix it here:

OTPTokenKey('ipatokenotpkey?',
cli_name='key',
label=_('Key'),
doc=_('Token secret (Base32; default: random)'),
default_from=lambda: os.urandom(KEY_LENGTH),
autofill=True,
flags=('no_display', 'no_update', 'no_search'),
),


If OTPTokenKey is mandratory, it should be required param (autofill 
should work in this case too)


Martin^2


--
Tomas Krizek


You changed API, you must regenerate API.txt (./makeapi) and increment 
minor version in VERSION file


Option 'ipatokenotpkey?' in command 'otptoken_add/1' in API file not found
Options count in otptoken_add of 22 doesn't match expected: 23
Option ipatokenotpkey of command otptoken_add in ipalib, not in API file:
OTPTokenKey('ipatokenotpkey', autofill=True, cli_name='key')

Martin^2




[root@vm-058-107 ~]# ipa otptoken-add --key='ORSXG5DFON2AU==='
Usage: ipa [global-options] otptoken-add [ID] [options]

ipa: error: --key option does not take a value


Well patch doesnt work for me, Honza may know if this is expected 
behavior of framework or just params bug


Martin62
-- 
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] 0003 Validate key in otptoken-add

[Martin Basti]

On 22.08.2016 10:22, Tomas Krizek wrote:


Seems like a good idea, I'm attaching the updated patch. Autofill does 
work when the param is required.



On 08/19/2016 04:19 PM, Martin Basti wrote:




On 16.08.2016 17:35, Tomas Krizek wrote:

Hi,

the attached patch fixes an error message when user provides an 
empty key while adding otp token.


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





I'm curious why we don't fix it here:

OTPTokenKey('ipatokenotpkey?',
cli_name='key',
label=_('Key'),
doc=_('Token secret (Base32; default: random)'),
default_from=lambda: os.urandom(KEY_LENGTH),
autofill=True,
flags=('no_display', 'no_update', 'no_search'),
),


If OTPTokenKey is mandratory, it should be required param (autofill 
should work in this case too)


Martin^2


--
Tomas Krizek


You changed API, you must regenerate API.txt (./makeapi) and increment 
minor version in VERSION file


Option 'ipatokenotpkey?' in command 'otptoken_add/1' in API file not found
Options count in otptoken_add of 22 doesn't match expected: 23
Option ipatokenotpkey of command otptoken_add in ipalib, not in API file:
OTPTokenKey('ipatokenotpkey', autofill=True, cli_name='key')

Martin^2
-- 
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 0019] Corrected minor spell check in AD Trust information doc messages

[Martin Basti]

On 22.08.2016 17:05, Abhijeet Kasurde wrote:

Hi All,


On 08/22/2016 05:47 PM, Martin Basti wrote:



On 22.08.2016 14:07, Alexander Bokovoy wrote:

On Mon, 22 Aug 2016, Abhijeet Kasurde wrote:

Hi All,

Please find the patch attached.

It's a minor spelling correction so, I have not created ticket for 
this.



ACK.



Please don't update .pot files, we are doing it before release 
automatically using Zanata.

Please find updated patch.


Thanks

master:
* c9419411c95baa67a5bf61fad0adc239e289e4dc Corrected minor spell check 
in AD Trust information doc messages


--
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 0019] Corrected minor spell check in AD Trust information doc messages

[Martin Basti]

On 22.08.2016 14:07, Alexander Bokovoy wrote:

On Mon, 22 Aug 2016, Abhijeet Kasurde wrote:

Hi All,

Please find the patch attached.

It's a minor spelling correction so, I have not created ticket for this.


ACK.



Please don't update .pot files, we are doing it before release 
automatically using Zanata.


--
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 0038][Tests] ID views does not recognize ipakrboktoauthasdelegate attribute

[Martin Basti]

On 22.08.2016 14:06, Alexander Bokovoy wrote:

On Mon, 22 Aug 2016, Lenka Doudova wrote:

Hi,

due to implementation of [1] some ID views tests fail because they do 
not recognize ipakrboktoauthasdelegate attribute. Providing fix for 
this.


Ticket: https://fedorahosted.org/freeipa/ticket/6241

ACK.


Pushed to master: 3d159c39c72ac43ae502f0cb978e534aa37f3b20

--
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] [DESIGN][UPDATE] Time-Based HBAC Policies

[Martin Basti]

On 19.08.2016 12:37, Pavel Vomacka wrote:



On 08/16/2016 08:21 AM, Stanislav Laznicka wrote:

On 08/12/2016 06:48 PM, Petr Spacek wrote:

On 11.8.2016 12:34, Stanislav Laznicka wrote:

Hello,

I updated the design of the Time-Based HBAC Policies according to the
discussion we led here earlier. Please check the design page
http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The 
biggest
changes are in the Implementation and Feature Management sections. 
I also

added a short How to Use section.

Thank you for the review! I will add some comments inline.

Nice page!

On the high level it all makes sense.

ad LDAP schema
==
1) Why accessTime attribute is MAY in ipaTimeRule object class?
Does it make sense to have the object without accessTime? I do not 
think so.
My idea was that we allow users prepare a few time rule objects 
before filling them with the actual times.
Also, it could be good to add description attribute to the object 
class and

incorporate it into commands (including find).


Definitely a good idea, I will work that in.

2) Besides all this, I spent few minutes in dark history of IPA. The
accessTime attribute was introduced back in 2009 in commit
"55ba300c7cb59cf05b16cc01281f51d93eb25acf" aka "Incorporate new 
schema for IPAv2".


The commit does not contain any reasoning for the change but I can 
see that
the attribute is already used as MAY in old object classes 
ipaHBACRule and

ipaSELinuxUserMap.

Is any of these a problem?
I believe that the accessTime attribute was originally brought to IPA 
when there was an implementation of time policies for HBAC objects 
and it's been rotting there ever since those capabilities were 
removed. We may eventually use a new attribute for storage of the 
time strings as accessTime by definition is multi-valued which is not 
what's currently desired (although we may end up with it some day in 
the future). However, I don't think any other use of accessTime 
should be a problem as it's been obsoleted for a long time.

Why is it even in ipaSELinuxUserMap object class?
I'm sorry to say I have no idea. I used it for what it originally was 
- a means for storing time strings at HBAC rules.

Commit
55512dc938eb4a9a6655e473beab587e340af55c does not mention any reason 
for doing so.


I cannot see any other problem so the low-level stuff is good and 
can be

implemented.


ad User interface
=
We need to polish the user interface so it really usable.

At least the web interface should contain some shortcuts. E.g. when 
I'm adding

a new HBAC rule, the "time" section should contain also "something" to
immediately add new time rule so I do not need to go to time rules 
first and

then go back to HBAC page.
I'm definitely for creating a field where admin can choose a existing 
time rule when creating a new HBAC. But I'm not sure about possibility 
to create and define new time rule in dialog for creating new HBAC. I 
think that mixing these two things together is like a possibility to 
create a new user when you are creating a user group. Which is mixing 
two different things together. I can imagine a button like "Create 
HBAC and add a new time rule to it" which could store new HBAC rule 
and immediately take admin to the page (or dialog) where admin can 
create a new time rule with prefilled HBAC rule. But as you write 
below it would be good to discuss it with some UX designer.


I'm not UX guru, but if you add button there and show dialog window to 
create new timerule and then automatically assign it to the HBACrule it 
may work for me :)




Similarly, dialog for rule modification should allow to easily 
change all the

values, warn if time rules is shared, and also have an easy way to
'disconnect' the time rule, i.e. make a copy of it and edit only the 
new copy

(instead of the shared original).


All of these points are really good.



All these are user interface things not affecting the low-level stuff.


Maybe you should sat down with some UX designer, talk about these 
cases and

draw some hand-made pictures.

I will add Pavel V. to CC, we may want to discuss this.
I do not believe that this will require any changes in schema so you 
can

polish SSSD and framework implementation in meantime.

On the link below is a PROTOTYPE-patched FreeIPA that covers most of 
the CLI
functionality (except for the creation of iCalendar strings from 
options) for

better illustration of the design.

https://github.com/stlaz/freeipa/tree/timerules_2
Honestly I did not look at the code today :-)

Overall, I'm glad to see current proposal. After so many iteration, 
we reached

something which does not have any glaring problem :-)
It definitely felt better to be writing it with all the previous 
knowledge. Thank you! :-)




LGTM with all previous comments


(Nitpick mode enabled: True)
1.
It may not be clear from design that client is actually SSSD, and not 
IPA CLI client. Please write explicitly there that HBAC time rules are 

Re: [Freeipa-devel] [PATCH 0214] Support schema files for external plugins

[Martin Basti]

On 19.08.2016 15:26, Alexander Bokovoy wrote:

On Fri, 19 Aug 2016, Martin Basti wrote:



On 19.08.2016 11:43, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Petr Vobornik wrote:

On 08/08/2016 12:26 PM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Alexander Bokovoy wrote:

Hi!

Attached patch is what is needed to allow external plugins for 
FreeIPA

framework to be functional if they need to extend a schema.

The idea is that we would have a separate directory as
/usr/share/ipa/schema.d and will allow to use schema (*.ldif) 
files from

it and its subdirectories during install and upgrade stages.

Without the patch only selected schema files from /usr/share/ipa 
are
used during install and upgrade. This leads to a failure to 
install IPA

server (or upgrade it) if a new plugin is added. If plugin defines
managed permissions, upgrade tool will generate ACIs which will 
fail to
be inserted into LDAP store due to references to missing 
attributes and

object classes.

The patch adds a directory to be installed and a helper utility 
that
loads files from the directory and adds them to the list of 
schema files

used during update of dsinstance and upgrade of the server.

With this patch I'm successfully managed to make FleetCommander
integration plugin completely independent of FreeIPA.

Patch attached now. ;)



I'll assume that we want to target 4.4.x therefore it can be 
pushed to

master, right? I.e. no need for creating ipa-4-4 branch atm.

Right.


Reasoning is that currently F24 has 4.3.x. F25 will most likely have
4.4.x because 4.5 is still in planning.

Sounds good to me. FleetCommander (which is one of drivers behind the
patches) is targeting F25 as well.

Can we get the patch reviewed?


ACK

However ticket is in future releases, so we have to branch master and 
ipa 4.4 before push

Why? We agreed above to get the patch into 4.4. Moving ticket to 4.4.1
milestone is certainly possible and does not require branching.


OK, you agreed but nobody changed milestone of ticket

Pushed to master: 7bec8a246d6712f749ec331f5bf066e3357c4ce7

Martin^2

--
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 0214] Support schema files for external plugins

[Martin Basti]

On 19.08.2016 11:43, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Petr Vobornik wrote:

On 08/08/2016 12:26 PM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Alexander Bokovoy wrote:

Hi!

Attached patch is what is needed to allow external plugins for 
FreeIPA

framework to be functional if they need to extend a schema.

The idea is that we would have a separate directory as
/usr/share/ipa/schema.d and will allow to use schema (*.ldif) 
files from

it and its subdirectories during install and upgrade stages.

Without the patch only selected schema files from /usr/share/ipa are
used during install and upgrade. This leads to a failure to 
install IPA

server (or upgrade it) if a new plugin is added. If plugin defines
managed permissions, upgrade tool will generate ACIs which will 
fail to
be inserted into LDAP store due to references to missing 
attributes and

object classes.

The patch adds a directory to be installed and a helper utility that
loads files from the directory and adds them to the list of schema 
files

used during update of dsinstance and upgrade of the server.

With this patch I'm successfully managed to make FleetCommander
integration plugin completely independent of FreeIPA.

Patch attached now. ;)



I'll assume that we want to target 4.4.x therefore it can be pushed to
master, right? I.e. no need for creating ipa-4-4 branch atm.

Right.


Reasoning is that currently F24 has 4.3.x. F25 will most likely have
4.4.x because 4.5 is still in planning.

Sounds good to me. FleetCommander (which is one of drivers behind the
patches) is targeting F25 as well.

Can we get the patch reviewed?


ACK

However ticket is in future releases, so we have to branch master and 
ipa 4.4 before push


Martin^2

--
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] [freeipa/freeipa #2] Remove forgotten print from DN.__str__ implementation (comment)

[Martin Basti]

On 19.08.2016 13:05, freeipa-github-notificat...@redhat.com wrote:

mbasti-rh commented on a pull request

m
a
s
t
e
r
:


*
  
8

6
e
1
5
6
c
3
c
5
f
3
3
1
e
3
f
1
6
9
b
9
4
1
b
e
2
d
9
f
7
2
e
7
c
8
f
0
0
0
  
R

e
m
o
v
e
  
f

o
r
g
o
t
t
e
n
  
p

r
i
n
t
  
f

r
o
m
  
D

N
.
_
_
s
t
r
_
_
  
i

m
p
l
e
m
e
n
t
a
t
i
o
n

See the full comment at 
https://github.com/freeipa/freeipa/pull/2#issuecomment-240990609




Sorry, this is testing phase of tools :)

Pushed to master: 86e156c3c5f331e3f169b941be2d9f72e7c8f000

-- 
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 0004] [Test] Test for caacl-add-service: incorrect error message when service does not exists

[Martin Basti]

On 18.08.2016 15:02, Tomas Krizek wrote:


Hi,

NACK.

The issue is not that the error message contains the "no such entry" 
string. That is actually a valid part of the error message if the 
service indeed does not exist.


The problem is that the error message contained only the first 
character of the service's name instead of the entire name of the 
service. For example, the command


ipa caacl-add-service test_caacl --services svc/master2.ipa.test --services 
svc/master1.ipa.test

should end with these error messages if those services do not exist:

 member service:svc/master2.ipa.t...@ipa.test: no such entry
 member service:svc/master1.ipa.t...@ipa.test: no such entry

There is also a typo in the file name of the test and I'm also not sure if 
there isn't a better place to put the test.


+1

This should not be integration_test but only test_xmlrpc

Martin^2


On 08/18/2016 01:48 PM, Ganna Kaihorodova wrote:

Hello!

Test for caacl-add-service: incorrect error message when service does not exists
https://fedorahosted.org/freeipa/ticket/6171

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


:




--
Tomas Krizek




-- 
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