Re: [Freeipa-devel] [PATCH] 0097 Add options to write lightweight CA cert or chain to file

2016-08-15 Thread Fraser Tweedale 
On Mon, Aug 15, 2016 at 08:19:33AM +0200, Jan Cholasta wrote:
> On 9.8.2016 16:47, Fraser Tweedale wrote:
> > On Mon, Aug 08, 2016 at 10:49:27AM +0200, Jan Cholasta wrote:
> > > On 8.8.2016 09:06, Fraser Tweedale wrote:
> > > > On Mon, Aug 08, 2016 at 08:54:05AM +0200, Jan Cholasta wrote:
> > > > > Hi,
> > > > > 
> > > > > On 8.8.2016 06:34, Fraser Tweedale wrote:
> > > > > > Please review the attached patch with adds --certificate-out and
> > > > > > --certificate-chain-out options to `ca-show' command.
> > > > > > 
> > > > > > Note that --certificate-chain-out currently writes a bogus file due
> > > > > > to a bug in Dogtag that will be fixed in this week's build.
> > > > > > 
> > > > > > https://fedorahosted.org/freeipa/ticket/6178
> > > > > 
> > > > > 1) The client-side *-out options should be defined on the client 
> > > > > side, not
> > > > > on the server side.
> > > > > 
> > > > Will option defined on client side be propagated to, and observable
> > > > in the ipaserver plugin?  The ipaserver plugin needs to observe that
> > > > *-out has been requested and executes additional command(s) on that
> > > > basis.
> > > 
> > > Is there a reason not to *always* return the certs?
> > > 
> > We hit Dogtag to retrieve them.
> 
> I don't think that's an issue in a -show command.
> 
cert_show is invoked by other commands (cert_find*, cert_show,
cert_request, cert_status, ca_del) but these all hit Dogtag anyway
so I suppose that's fine.  I'll return the cert *and* the chain in
separate attributes, unconditionally.

> > 
> > > > 
> > > > > 
> > > > > 2) I don't think there should be additional information included in 
> > > > > summary
> > > > > (and it definitely should not be multi-line). I would rather inform 
> > > > > the user
> > > > > via an error message when unable to write the files.
> > > > > 
> > > > I was just following the pattern of other commands that write certs,
> > > > profile config, etc.  Apart from consistency with other commands I
> > > > agree that there is no need to have it.  So I will remove it.
> > > > 
> > > > > If you think there is an actual value in informing the user about
> > > > > successfully writing the files, please use ipalib.messages for the 
> > > > > job.
> > > > > 
> > > > > 
> > > > > 3) IMO a better format for the certificate chain than PKCS#7 would be
> > > > > concatenated PEM, as that's the most commonly used format in IPA (in
> > > > > installers, there are no cert chains in API commands ATM).
> > > > > 
> > > > Sure, but the main use case isn't IPA.  Other apps require PKCS #7
> > > > or concatenated PEMs, but sometimes they must be concatenated
> > > > forward, and othertimes backwards.  There is no one size fits all.
> > > 
> > > True, which is exactly why I think we should at least be self-consistent 
> > > and
> > > use concatenated PEM (and multi-value DER over the wire).
> > > 
> > Dogtag returns a PKCS7 (either DER or PEM, according to HTTP Accept
> > header).
> > 
> > If we want list-of-PEMs between server and client we have to convert
> > on the server.  Do we have a good way of doing this without exec'ing
> > `openssl pkcs7' on the server?  Is it acceptable to exec 'openssl'
> > to do the conversion on the server?  python-nss does not have PKCS7
> > functions and I am not keen on adding a pyasn1 PKCS7 parser just for
> > the sake of pushing bits as list-of-PEMs.
> 
> I'm afraid we can't avoid conversion to/from PKCS#7 one way or the other.
> For example, if we added a call to retrieve external CA chain using certs
> from cn=certificates,cn=ipa,cn=etc, we would have to convert the result to
> PKCS#7 if it was our cert chain format of choice.
> 
> What we can avoid though is executing "openssl pkcs7" to do the conversion -
> we can use an approach similar to our DNSSEC code and use python-cffi to
> call libcrypto's PKCS#7 conversion routines instead.
> 
I had a look at the OpenSSL API for parsing PKCS #7; now I prefer to
exec `openssl' to do the job :)

I will transmit DER-encoded PKCS #7 object on the wire; we cannot
used multi-valued DER attribute because order is important.  Client
will convert to PEMs.

Should have new patch on list this afternoon.

Thanks,
Fraser

> > 
> > FWIW, man pages and code suggest that PKCS #7 is accepted in
> > installer, etc.
> 
> True, but that's a relatively new feature (since 4.1) and the installer
> internally executes "openssl pkcs7" to convert PKCS #7 to list of certs :-)
> 
> > 
> > > > We can add an option to control the format later, but for now,
> > > > Dogtag returns a PKCS #7 (PEM or DER) so let's go with that.  Worst
> > > > case is an admin has to invoke `openssl pkcs7' and concat the certs
> > > > themselves.
> > > 
> > > AFAIK none of NSS, OpenSSL or p11-kit can use PKCS#7 cert chains directly,
> > > so I'm afraid the worst case would happen virtually always.
> > > 
> > If you're OK with invoking OpenSSL on the client to convert PKCS #7
> > to list-of-PEMs (similar to what is done in
> >

Re: [Freeipa-devel] [PATCH 0004-0012] Automatic CSR generation

2016-08-15 Thread Fraser Tweedale 
On Mon, Aug 15, 2016 at 03:58:40PM +0200, Petr Spacek wrote:
> On 15.8.2016 15:54, Fraser Tweedale wrote:
> > On Mon, Aug 15, 2016 at 03:31:20PM +0200, Petr Spacek wrote:
> >> On 15.8.2016 15:16, Fraser Tweedale wrote:
> >>> On Mon, Aug 15, 2016 at 02:52:46PM +0200, Petr Spacek wrote:
>  On 2.8.2016 05:57, Fraser Tweedale wrote:
> >>> Hah! This is what I get for thinking I know what the output has to 
> >>> look
> >>> like, and not testing all the way through to requesting the cert. I'll
> >>> change the profile to generate a subject with CN= instead of UID=. 
> >>> Updated
> >>> patch is attached. Unfortunately these rules are only updated at
> >>> ipa-server-install time, so if you'd like to fix it without 
> >>> reinstalling:
> >>>
> > (Tangential commentary...) Yeah, currently cert-request demands the
> > CN.  There is a design to relax the requirement to handle empty
> > subject names (look at SAN only).  IMO it would make sense to accept
> > other "obvious" mappings in Subject DN like accepting UID instead of
> > CN for user subjects, but that would be a separate RFE.  Noone has
> > actually asked for it yet :)
> 
>  Side-note:
>  I thought that subject format is enforced by certificate profile on 
>  server.
>  Am I wrong?
> 
> >>> You are right - what I suggested above would (today) require a
> >>> custom profile.
> >>
> >> Sooo...
> >> can we just relax existing profiles not to require CN= but accept SAN-only 
> >> CSRs?
> >>
> >> :-)
> >>
> > That is absolutely going to happen as part of
> > http://www.freeipa.org/page/V4/RFC_2818_certificate_compliance :)
> 
> Good!
> 
> Is it still targeting 4.4.x?
> 
It's not going to make it.

-- 
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] 0207, 0218-0219 Solving trust conflicts and external trust topology fixes

2016-08-15 Thread Alexander Bokovoy 

On Mon, 15 Aug 2016, Alexander Bokovoy wrote:

Hi!

Attached are trust-related patches.

0207 is a pre-requisite. I did send it before, it is re-formatting of
the ipaserver/dcerpc.py to be close to PEP8 requirements.

0218 is an automated trust topology conflict resolver for DNS namespace
conflicts. Read the commit message for details, and also comments in the
code. With this patch FreeIPA becomes more smart than Windows Server
which doesn't have automated trust topology conflict resolution. ;)

0219 fixes issue of topology details leaking through external trust. The
problem is only in presentation as we store more data than needed -- it
is impossible to cross external trust boundary anyway as it is handled
by AD DC side, but one important consequence is that we need to store
UPN suffixes before we start storing information about child domains.
Again, read the commit message.

These patches also are on top of my previously sent patches 0215-0216.

Patches attached.

--
/ Alexander Bokovoy
From 356f5b81af065ee808ab953c7103ae536cd0136f Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Tue, 7 Jun 2016 22:41:10 +0300
Subject: [PATCH 07/10] ipaserver/dcerpc: reformat to make the code closer to
 pep8

Because Samba Python bindings provide long-named methods and constants,
sometimes it is impossible to fit into 80 columns without causing
damage to readability of the code. This patchset attempts to reduce
pep8 complaints to a minimum.
---
 ipaserver/dcerpc.py | 473 +---
 1 file changed, 298 insertions(+), 175 deletions(-)

diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 21ac89d..19be6bf 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -44,10 +44,12 @@ import samba
 import random
 from cryptography.hazmat.primitives.ciphers import Cipher, algorithms
 from cryptography.hazmat.backends import default_backend
+# pylint: disable=F0401
 try:
-from ldap.controls import RequestControl as LDAPControl #pylint: 
disable=F0401
+from ldap.controls import RequestControl as LDAPControl
 except ImportError:
-from ldap.controls import LDAPControl as LDAPControl#pylint: 
disable=F0401
+from ldap.controls import LDAPControl as LDAPControl
+# pylint: enable=F0401
 import ldap as _ldap
 from ipapython.ipaldap import IPAdmin
 from ipaserver.session import krbccache_dir, krbccache_prefix
@@ -74,7 +76,7 @@ and Samba4 python bindings.
 
 # Both constants can be used as masks against trust direction
 # because bi-directional has two lower bits set.
-TRUST_ONEWAY= 1
+TRUST_ONEWAY = 1
 TRUST_BIDIRECTIONAL = 3
 
 # Trust join behavior
@@ -91,31 +93,44 @@ def is_sid_valid(sid):
 return True
 
 
-access_denied_error =  errors.ACIError(info=_('CIFS server denied your 
credentials'))
+access_denied_error = errors.ACIError(
+  info=_('CIFS server denied your credentials'))
 dcerpc_error_codes = {
 -1073741823:
-errors.RemoteRetrieveError(reason=_('communication with CIFS server 
was unsuccessful')),
+errors.RemoteRetrieveError(
+reason=_('communication with CIFS server was unsuccessful')),
 -1073741790: access_denied_error,
 -1073741715: access_denied_error,
 -1073741614: access_denied_error,
 -1073741603:
-errors.ValidationError(name=_('AD domain controller'), 
error=_('unsupported functional level')),
--1073741811: # NT_STATUS_INVALID_PARAMETER
+errors.ValidationError(
+name=_('AD domain controller'),
+error=_('unsupported functional level')),
+-1073741811:  # NT_STATUS_INVALID_PARAMETER
 errors.RemoteRetrieveError(
-reason=_('AD domain controller complains about communication 
sequence. It may mean unsynchronized time on both sides, for example')),
--1073741776: # NT_STATUS_INVALID_PARAMETER_MIX, we simply will skip the 
binding
+reason=_('AD domain controller complains about communication '
+ 'sequence. It may mean unsynchronized time on both '
+ 'sides, for example')),
+-1073741776:  # NT_STATUS_INVALID_PARAMETER_MIX,
+  # we simply will skip the binding
 access_denied_error,
--1073741772: # NT_STATUS_OBJECT_NAME_NOT_FOUND
-errors.RemoteRetrieveError(reason=_('CIFS server configuration does 
not allow access to pipe\\lsarpc')),
+-1073741772:  # NT_STATUS_OBJECT_NAME_NOT_FOUND
+errors.RemoteRetrieveError(
+reason=_('CIFS server configuration does not allow '
+ 'access to pipe\\lsarpc')),
 }
 
 dcerpc_error_messages = {
 "NT_STATUS_OBJECT_NAME_NOT_FOUND":
- errors.NotFound(reason=_('Cannot find specified domain or server 
name')),
+errors.NotFound(
+reason=_('Cannot find specified domain or server name')),
 "WERR_NO_LOGON_SERVERS":
- errors.RemoteRetrieveError(reason=_('AD DC was

[Freeipa-devel] [PATCH] 0207, 0218-0219 Solving trust conflicts and external trust topology fixes

2016-08-15 Thread Alexander Bokovoy 

Hi!

Attached are trust-related patches.

0207 is a pre-requisite. I did send it before, it is re-formatting of
the ipaserver/dcerpc.py to be close to PEP8 requirements.

0218 is an automated trust topology conflict resolver for DNS namespace
conflicts. Read the commit message for details, and also comments in the
code. With this patch FreeIPA becomes more smart than Windows Server
which doesn't have automated trust topology conflict resolution. ;)

0219 fixes issue of topology details leaking through external trust. The
problem is only in presentation as we store more data than needed -- it
is impossible to cross external trust boundary anyway as it is handled
by AD DC side, but one important consequence is that we need to store
UPN suffixes before we start storing information about child domains.
Again, read the commit message.

These patches also are on top of my previously sent patches 0215-0216.


--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCH 0004-0012] Automatic CSR generation

2016-08-15 Thread Petr Spacek 
On 15.8.2016 15:54, Fraser Tweedale wrote:
> On Mon, Aug 15, 2016 at 03:31:20PM +0200, Petr Spacek wrote:
>> On 15.8.2016 15:16, Fraser Tweedale wrote:
>>> On Mon, Aug 15, 2016 at 02:52:46PM +0200, Petr Spacek wrote:
 On 2.8.2016 05:57, Fraser Tweedale wrote:
>>> Hah! This is what I get for thinking I know what the output has to look
>>> like, and not testing all the way through to requesting the cert. I'll
>>> change the profile to generate a subject with CN= instead of UID=. 
>>> Updated
>>> patch is attached. Unfortunately these rules are only updated at
>>> ipa-server-install time, so if you'd like to fix it without 
>>> reinstalling:
>>>
> (Tangential commentary...) Yeah, currently cert-request demands the
> CN.  There is a design to relax the requirement to handle empty
> subject names (look at SAN only).  IMO it would make sense to accept
> other "obvious" mappings in Subject DN like accepting UID instead of
> CN for user subjects, but that would be a separate RFE.  Noone has
> actually asked for it yet :)

 Side-note:
 I thought that subject format is enforced by certificate profile on server.
 Am I wrong?

>>> You are right - what I suggested above would (today) require a
>>> custom profile.
>>
>> Sooo...
>> can we just relax existing profiles not to require CN= but accept SAN-only 
>> CSRs?
>>
>> :-)
>>
> That is absolutely going to happen as part of
> http://www.freeipa.org/page/V4/RFC_2818_certificate_compliance :)

Good!

Is it still targeting 4.4.x?

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0004-0012] Automatic CSR generation

2016-08-15 Thread Fraser Tweedale 
On Mon, Aug 15, 2016 at 03:31:20PM +0200, Petr Spacek wrote:
> On 15.8.2016 15:16, Fraser Tweedale wrote:
> > On Mon, Aug 15, 2016 at 02:52:46PM +0200, Petr Spacek wrote:
> >> On 2.8.2016 05:57, Fraser Tweedale wrote:
> > Hah! This is what I get for thinking I know what the output has to look
> > like, and not testing all the way through to requesting the cert. I'll
> > change the profile to generate a subject with CN= instead of UID=. 
> > Updated
> > patch is attached. Unfortunately these rules are only updated at
> > ipa-server-install time, so if you'd like to fix it without 
> > reinstalling:
> >
> >>> (Tangential commentary...) Yeah, currently cert-request demands the
> >>> CN.  There is a design to relax the requirement to handle empty
> >>> subject names (look at SAN only).  IMO it would make sense to accept
> >>> other "obvious" mappings in Subject DN like accepting UID instead of
> >>> CN for user subjects, but that would be a separate RFE.  Noone has
> >>> actually asked for it yet :)
> >>
> >> Side-note:
> >> I thought that subject format is enforced by certificate profile on server.
> >> Am I wrong?
> >>
> > You are right - what I suggested above would (today) require a
> > custom profile.
> 
> Sooo...
> can we just relax existing profiles not to require CN= but accept SAN-only 
> CSRs?
> 
> :-)
> 
That is absolutely going to happen as part of
http://www.freeipa.org/page/V4/RFC_2818_certificate_compliance :)

-- 
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-0012] Automatic CSR generation

2016-08-15 Thread Petr Spacek 
On 15.8.2016 15:16, Fraser Tweedale wrote:
> On Mon, Aug 15, 2016 at 02:52:46PM +0200, Petr Spacek wrote:
>> On 2.8.2016 05:57, Fraser Tweedale wrote:
> Hah! This is what I get for thinking I know what the output has to look
> like, and not testing all the way through to requesting the cert. I'll
> change the profile to generate a subject with CN= instead of UID=. Updated
> patch is attached. Unfortunately these rules are only updated at
> ipa-server-install time, so if you'd like to fix it without reinstalling:
>
>>> (Tangential commentary...) Yeah, currently cert-request demands the
>>> CN.  There is a design to relax the requirement to handle empty
>>> subject names (look at SAN only).  IMO it would make sense to accept
>>> other "obvious" mappings in Subject DN like accepting UID instead of
>>> CN for user subjects, but that would be a separate RFE.  Noone has
>>> actually asked for it yet :)
>>
>> Side-note:
>> I thought that subject format is enforced by certificate profile on server.
>> Am I wrong?
>>
> You are right - what I suggested above would (today) require a
> custom profile.

Sooo...
can we just relax existing profiles not to require CN= but accept SAN-only CSRs?

:-)

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH] 0090, 0092..0094 cert-show: show subject alternative names

2016-08-15 Thread Petr Spacek 
On 15.8.2016 15:07, Fraser Tweedale wrote:
> On Mon, Aug 15, 2016 at 07:48:22AM +0200, Jan Cholasta wrote:
>> On 12.8.2016 18:57, Petr Spacek wrote:
>>> On 12.8.2016 11:33, Jan Cholasta wrote:
 On 4.8.2016 18:18, Petr Vobornik wrote:
> On 07/22/2016 07:13 AM, Fraser Tweedale wrote:
>> On Tue, Jul 19, 2016 at 08:50:34AM +0200, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 14.7.2016 13:44, Fraser Tweedale wrote:
 Hi all,

 The attached patch includes SANs in cert-show output.  If you have
 certs with esoteric altnames (especially any that are more than just
 ASN.1 string types), please test with those certs.

 https://fedorahosted.org/freeipa/ticket/6022
>>>
>>> I think it would be better to have a separate attribute for each 
>>> supported
>>> SAN type rather than cramming everything into subject_alt_name. That 
>>> way if
>>> you care only about a single specific type you won't have to go through 
>>> all
>>> the values and parse them. Also it would allow you to use param types
>>> appropriate to the SAN types (DNSNameParam for DNS names, Principal for
>>> principal names, etc.)
>>>
>>> Nitpick: please don't mix moving existing stuff and adding new stuff in 
>>> a
>>> single patch.
>>>
>> Updated patches attached.
>>
>> Patches 0092..0094 are refactors and bugfixes.
>> Patch 0090-2 is the main feature (depends on 0092..0094).
>>
>> Thanks,
>> Fraser
>>
>
> bump for review

 Patch 0092: ACK

 Patch 0093: ACK

 Patch 0094: ACK

 Patch 0090:

 1) Generic otherNames (san_other) do not work correctly. The OID is not
 included in the value and names with complex type other than 
 KerberosPrincipal
 are not parsed correctly. The value should include the OID and DER blob of 
 the
 name.

 2) With --all, san_other should be included in the result for all 
 otherNames,
 even the known ones, to provide (limited) forward compatibility.

 3) Do we have to support *all* the name types? I mean we could, for the 
 sake
 of completeness, but it might be easier to just keep the few ones we 
 actually
 care about (email, DNS name, principal name, UPN and directory name in your
 patch 0095).
>>>
>>> As far as I remember this reasoning usually comes back to bite us into butt.
>>>
>>> - "Implement only this subset, nobody else needs the other the of ...
>>> (whatever, e.g. DNS record type)."
>>> - "Yes my lord."
>>>
>>> Two months later:
>>>
>>> - "We need to support for XYZ. It was (already) late yesterday!"
>>>
>>> :-)
>>
>> Care to give a concrete example of when this actually happened? Because IIRC
>> this happened once or twice, not "usually".

I do not have list at hand, sorry. It is just my feeling.


>> Anyway, I'm fine with whatever, my point was that additional effort needs to
>> be put in to support "everything" and even then, we wouldn't actually
>> support everything (two months later: "we need to support extension XYZ!").
>>
> Sure, but in this case it was minimal additional effort to support
> even the esoteric name types - it is only for display after all; if
> an uncommon altname is (somehow) there we can display it.

That is the main point. The cost is minimal so why not to do it?

Petr^2 Spacek

>> (FWIW, I think it would be more useful to add support for Basic constraints
>> rather than X.400 SANs.)
>>
> I can only agree, and say: another RFE for another day :)
> 
>>>
>>> Petr^2 Spacek
>>>

 4)

 +obj.setdefault(attr_name, []).append(unicode(name))

 The value should not (always) be unicode, but of the type declared by the
 param (unicode or ipapython.kerberos.Principal or 
 ipapython.dnsutil.DNSName).

-- 
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-0012] Automatic CSR generation

2016-08-15 Thread Fraser Tweedale 
On Mon, Aug 15, 2016 at 02:52:46PM +0200, Petr Spacek wrote:
> On 2.8.2016 05:57, Fraser Tweedale wrote:
> >> > Hah! This is what I get for thinking I know what the output has to look
> >> > like, and not testing all the way through to requesting the cert. I'll
> >> > change the profile to generate a subject with CN= instead of UID=. 
> >> > Updated
> >> > patch is attached. Unfortunately these rules are only updated at
> >> > ipa-server-install time, so if you'd like to fix it without reinstalling:
> >> > 
> > (Tangential commentary...) Yeah, currently cert-request demands the
> > CN.  There is a design to relax the requirement to handle empty
> > subject names (look at SAN only).  IMO it would make sense to accept
> > other "obvious" mappings in Subject DN like accepting UID instead of
> > CN for user subjects, but that would be a separate RFE.  Noone has
> > actually asked for it yet :)
> 
> Side-note:
> I thought that subject format is enforced by certificate profile on server.
> Am I wrong?
> 
You are right - what I suggested above would (today) require a
custom profile.

-- 
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] 0090, 0092..0094 cert-show: show subject alternative names

2016-08-15 Thread Fraser Tweedale 
On Mon, Aug 15, 2016 at 07:48:22AM +0200, Jan Cholasta wrote:
> On 12.8.2016 18:57, Petr Spacek wrote:
> > On 12.8.2016 11:33, Jan Cholasta wrote:
> > > On 4.8.2016 18:18, Petr Vobornik wrote:
> > > > On 07/22/2016 07:13 AM, Fraser Tweedale wrote:
> > > > > On Tue, Jul 19, 2016 at 08:50:34AM +0200, Jan Cholasta wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On 14.7.2016 13:44, Fraser Tweedale wrote:
> > > > > > > Hi all,
> > > > > > > 
> > > > > > > The attached patch includes SANs in cert-show output.  If you have
> > > > > > > certs with esoteric altnames (especially any that are more than 
> > > > > > > just
> > > > > > > ASN.1 string types), please test with those certs.
> > > > > > > 
> > > > > > > https://fedorahosted.org/freeipa/ticket/6022
> > > > > > 
> > > > > > I think it would be better to have a separate attribute for each 
> > > > > > supported
> > > > > > SAN type rather than cramming everything into subject_alt_name. 
> > > > > > That way if
> > > > > > you care only about a single specific type you won't have to go 
> > > > > > through all
> > > > > > the values and parse them. Also it would allow you to use param 
> > > > > > types
> > > > > > appropriate to the SAN types (DNSNameParam for DNS names, Principal 
> > > > > > for
> > > > > > principal names, etc.)
> > > > > > 
> > > > > > Nitpick: please don't mix moving existing stuff and adding new 
> > > > > > stuff in a
> > > > > > single patch.
> > > > > > 
> > > > > Updated patches attached.
> > > > > 
> > > > > Patches 0092..0094 are refactors and bugfixes.
> > > > > Patch 0090-2 is the main feature (depends on 0092..0094).
> > > > > 
> > > > > Thanks,
> > > > > Fraser
> > > > > 
> > > > 
> > > > bump for review
> > > 
> > > Patch 0092: ACK
> > > 
> > > Patch 0093: ACK
> > > 
> > > Patch 0094: ACK
> > > 
> > > Patch 0090:
> > > 
> > > 1) Generic otherNames (san_other) do not work correctly. The OID is not
> > > included in the value and names with complex type other than 
> > > KerberosPrincipal
> > > are not parsed correctly. The value should include the OID and DER blob 
> > > of the
> > > name.
> > > 
> > > 2) With --all, san_other should be included in the result for all 
> > > otherNames,
> > > even the known ones, to provide (limited) forward compatibility.
> > > 
> > > 3) Do we have to support *all* the name types? I mean we could, for the 
> > > sake
> > > of completeness, but it might be easier to just keep the few ones we 
> > > actually
> > > care about (email, DNS name, principal name, UPN and directory name in 
> > > your
> > > patch 0095).
> > 
> > As far as I remember this reasoning usually comes back to bite us into butt.
> > 
> > - "Implement only this subset, nobody else needs the other the of ...
> > (whatever, e.g. DNS record type)."
> > - "Yes my lord."
> > 
> > Two months later:
> > 
> > - "We need to support for XYZ. It was (already) late yesterday!"
> > 
> > :-)
> 
> Care to give a concrete example of when this actually happened? Because IIRC
> this happened once or twice, not "usually".
> 
> Anyway, I'm fine with whatever, my point was that additional effort needs to
> be put in to support "everything" and even then, we wouldn't actually
> support everything (two months later: "we need to support extension XYZ!").
> 
Sure, but in this case it was minimal additional effort to support
even the esoteric name types - it is only for display after all; if
an uncommon altname is (somehow) there we can display it.

> (FWIW, I think it would be more useful to add support for Basic constraints
> rather than X.400 SANs.)
> 
I can only agree, and say: another RFE for another day :)

> > 
> > Petr^2 Spacek
> > 
> > > 
> > > 4)
> > > 
> > > +obj.setdefault(attr_name, []).append(unicode(name))
> > > 
> > > The value should not (always) be unicode, but of the type declared by the
> > > param (unicode or ipapython.kerberos.Principal or 
> > > ipapython.dnsutil.DNSName).
> > 
> 
> 
> -- 
> 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

Re: [Freeipa-devel] [PATCH] [WIP] Allow full customisability of CA subject name

2016-08-15 Thread Fraser Tweedale 
On Mon, Aug 15, 2016 at 02:08:54PM +0200, Jan Cholasta wrote:
> On 19.7.2016 12:05, Jan Cholasta wrote:
> > On 19.7.2016 11:54, Fraser Tweedale wrote:
> > > On Tue, Jul 19, 2016 at 09:36:17AM +0200, Jan Cholasta wrote:
> > > > Hi,
> > > > 
> > > > On 15.7.2016 07:05, Fraser Tweedale wrote:
> > > > > On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote:
> > > > > > The attached patch is a work in progress for
> > > > > > https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866).
> > > > > > 
> > > > > > I am sharing now to make the approach clear and solicit feedback.
> > > > > > 
> > > > > > It has been tested for server install, replica install (with and
> > > > > > without CA) and CA-replica install (all hosts running master+patch).
> > > > > > 
> > > > > > Migration from earlier versions and server/replica/CA install on a
> > > > > > CA-less deployment are not yet tested; these will be tested over
> > > > > > coming days and patch will be tweaked as necessary.
> > > > > > 
> > > > > > Commit message has a fair bit to say so I won't repeat here but let
> > > > > > me know your questions and comments.
> > > > > > 
> > > > > > Thanks,
> > > > > > Fraser
> > > > > > 
> > > > > It does help to attach the patch, of course ^_^
> > > > 
> > > > IMO explicit is better than implicit, so instead of introducing
> > > > additional
> > > > magic around --subject, I would rather add a new separate option for
> > > > specifying the CA subject name (I think --ca-subject, for consistency
> > > > with
> > > > --ca-signing-algorithm).
> > > > 
> > > The current situation - the --subject argument which specifies the
> > > not the subject but the subject base, is confusing enough (to say
> > > nothing of the limitations that give rise to the RFE).
> > > 
> > > Retaining --subject for specifying the subject base and adding
> > > --ca-subject for specifying the *actual* subject DN gets us over the
> > > line in terms of the RFE, but does not make the installer less
> > > confusing.  This is why I made --subject accept the full subject DN,
> > > with provisions to retain existing behaviour.
> > > 
> > > IMO if we want to have separate arguments for subject DN and subject
> > > base (I am not against it), let's bite the bullet and name arguments
> > > accordingly.  --subject should be used to specify full Subject DN,
> > > --subject-base (or similar) for specifying subject base.
> > 
> > IMHO --ca-subject is better than --subject, because it is more explicit
> > whose subject name that is (the CA's). I agree that --subject should be
> > deprecated and replaced with --subject-base.
> > 
> > > 
> > > (I intentionally defer discussion of specific behaviour if one, none
> > > or both are specified; let's resolve the question or renaming /
> > > changing meaning of arguments first).
> > > 
> > > 
> > > > By specifying the option you would override the default "CN=Certificate
> > > > Authority,$SUBJECT_BASE" subject name. If --external-ca was not used,
> > > > additional validation would be done to make sure the subject name meets
> > > > Dogtag's expectations. Actually, it might make sense to always do the
> > > > additional validation, to be able to print a warning that if a
> > > > Dogtag-incompatible subject name is used, it won't be possible to
> > > > change the
> > > > CA cert chaining from externally signed to self-signed later.
> > > > 
> > > > Honza
> 
> Bump, any update on this?
> 
I have an updated patch that fixes some issues Sebastian encountered
in testing, but I've not yet tackled the main change requested by
Honza (in brief: adding --ca-subject for specifying that, adding
--subject-base for specifying that, and deprecating --subject;
Sebastian, see discussion above and feel free to give your
thoughts).  I expect I'll get back onto this work within the next
few days.

Thanks,
Fraser

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

Re: [Freeipa-devel] [PATCH 0004-0012] Automatic CSR generation

2016-08-15 Thread Petr Spacek 
On 2.8.2016 05:57, Fraser Tweedale wrote:
>> > Hah! This is what I get for thinking I know what the output has to look
>> > like, and not testing all the way through to requesting the cert. I'll
>> > change the profile to generate a subject with CN= instead of UID=. Updated
>> > patch is attached. Unfortunately these rules are only updated at
>> > ipa-server-install time, so if you'd like to fix it without reinstalling:
>> > 
> (Tangential commentary...) Yeah, currently cert-request demands the
> CN.  There is a design to relax the requirement to handle empty
> subject names (look at SAN only).  IMO it would make sense to accept
> other "obvious" mappings in Subject DN like accepting UID instead of
> CN for user subjects, but that would be a separate RFE.  Noone has
> actually asked for it yet :)

Side-note:
I thought that subject format is enforced by certificate profile on server.
Am I wrong?

-- 
Petr^2 Spacek

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

[Freeipa-devel] [patch 0052] ipatests: Fix wrong fixture in kerberos principal alias test

2016-08-15 Thread Milan Kubík 

Fixes issue in ticket https://fedorahosted.org/freeipa/ticket/6197


--
Milan Kubik

From b0a731c2b655c331001c9cb217f66045c9c2fdb7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 15 Aug 2016 14:31:48 +0200
Subject: [PATCH] ipatests: Fix wrong fixture in kerberos principal alias test

https://fedorahosted.org/freeipa/ticket/6197
---
 ipatests/test_xmlrpc/test_kerberos_principal_aliases.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
index 3bbc641f1c5ea495d6a8e37e8aff67f1c67d5f4e..a1973af2c1f29e187d0bca8038c5452b1bd0b6cc 100644
--- a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
+++ b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
@@ -41,7 +41,7 @@ def trusted_domain():
 are deleted from the directory.
 """
 
-trusted_dom = TRUSTED_DOMAIN_MOCK['ldif']
+trusted_dom = TRUSTED_DOMAIN_MOCK
 
 # Write the changes
 with mocked_trust_containers(), MockLDAP() as ldap:
-- 
2.9.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

[Freeipa-devel] [PATCH 0033][Tests] Fix test_ipalib/test_output failing due to change of Output class behaviour

2016-08-15 Thread Lenka Doudova 

Hi,

attaching patch for https://fedorahosted.org/freeipa/ticket/6189


Lenka

From 9e0591f6099c587218aa8155113d3e7bd85fe9f4 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 15 Aug 2016 14:24:11 +0200
Subject: [PATCH] Tests: test_ipalib/test_output fails due to change of Output
 behaviour

https://fedorahosted.org/freeipa/ticket/6189
---
 ipatests/test_ipalib/test_output.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_ipalib/test_output.py b/ipatests/test_ipalib/test_output.py
index 5741637f9734e33be1e1a7e1f44099c53c8789c8..ed734ffd4b9ad99657ada0d3bcfc35db6c4b8c73 100644
--- a/ipatests/test_ipalib/test_output.py
+++ b/ipatests/test_ipalib/test_output.py
@@ -51,16 +51,16 @@ class test_Output(ClassChecker):
 Test the `ipalib.output.Output.__repr__` method.
 """
 o = self.cls('aye')
-assert repr(o) == "Output('aye', None, None)"
+assert repr(o) == "Output('aye')"
 o = self.cls('aye', type=int, doc='An A, aye?')
-assert repr(o) == "Output('aye', %r, 'An A, aye?')" % int
+assert repr(o) == "Output('aye', type=[%r], doc='An A, aye?')" % int
 
 class Entry(self.cls):
 pass
 o = Entry('aye')
-assert repr(o) == "Entry('aye', None, None)"
+assert repr(o) == "Entry('aye')"
 o = Entry('aye', type=int, doc='An A, aye?')
-assert repr(o) == "Entry('aye', %r, 'An A, aye?')" % int
+assert repr(o) == "Entry('aye', type=[%r], doc='An A, aye?')" % int
 
 
 class test_ListOfEntries(ClassChecker):
-- 
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 0063] Raise error on topology disconnect/last-role-host removal during server uninstall

2016-08-15 Thread Martin Babinsky 

On 08/15/2016 02:13 PM, Martin Babinsky wrote:

On 08/12/2016 12:08 PM, Stanislav Laznicka wrote:

Hello,

topology disconnect/last-role-host removal errors would just be logged
during server uninstall even if ignore options are not present. The host
would still appear in the topology even after "successful" uninstall.

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





The patch seems to be ok, however shouldn't we use sys.exit() when
handling ServerRemovalError? Yes raising SystemExit from within a
function is a horrible practice, but it is already done on several other
places instead of letting the exception bubble up to the main handler.

CC'ing Jan for his thoughts on this since I may be wrong.

Hmm, you will definitely need sys.exit() here since otherwise 
ipa-server-install reports 0 exit code even if there was an exception 
thrown:


"""
[root@master1 ~]# ipa-server-install --uninstall -U
ipa : ERRORServer removal aborted: Deleting this server will 
leave your installation without a DNS..

[root@master1 ~]# echo $?
0
"""

--
Martin^3 Babinsky

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

Re: [Freeipa-devel] [PATCH 0063] Raise error on topology disconnect/last-role-host removal during server uninstall

2016-08-15 Thread Martin Babinsky 

On 08/12/2016 12:08 PM, Stanislav Laznicka wrote:

Hello,

topology disconnect/last-role-host removal errors would just be logged
during server uninstall even if ignore options are not present. The host
would still appear in the topology even after "successful" uninstall.

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





The patch seems to be ok, however shouldn't we use sys.exit() when 
handling ServerRemovalError? Yes raising SystemExit from within a 
function is a horrible practice, but it is already done on several other 
places instead of letting the exception bubble up to the main handler.


CC'ing Jan for his thoughts on this since I may be wrong.

--
Martin^3 Babinsky

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

Re: [Freeipa-devel] [PATCH] [WIP] Allow full customisability of CA subject name

2016-08-15 Thread Jan Cholasta 

On 19.7.2016 12:05, Jan Cholasta wrote:

On 19.7.2016 11:54, Fraser Tweedale wrote:

On Tue, Jul 19, 2016 at 09:36:17AM +0200, Jan Cholasta wrote:

Hi,

On 15.7.2016 07:05, Fraser Tweedale wrote:

On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote:

The attached patch is a work in progress for
https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866).

I am sharing now to make the approach clear and solicit feedback.

It has been tested for server install, replica install (with and
without CA) and CA-replica install (all hosts running master+patch).

Migration from earlier versions and server/replica/CA install on a
CA-less deployment are not yet tested; these will be tested over
coming days and patch will be tweaked as necessary.

Commit message has a fair bit to say so I won't repeat here but let
me know your questions and comments.

Thanks,
Fraser


It does help to attach the patch, of course ^_^


IMO explicit is better than implicit, so instead of introducing
additional
magic around --subject, I would rather add a new separate option for
specifying the CA subject name (I think --ca-subject, for consistency
with
--ca-signing-algorithm).


The current situation - the --subject argument which specifies the
not the subject but the subject base, is confusing enough (to say
nothing of the limitations that give rise to the RFE).

Retaining --subject for specifying the subject base and adding
--ca-subject for specifying the *actual* subject DN gets us over the
line in terms of the RFE, but does not make the installer less
confusing.  This is why I made --subject accept the full subject DN,
with provisions to retain existing behaviour.

IMO if we want to have separate arguments for subject DN and subject
base (I am not against it), let's bite the bullet and name arguments
accordingly.  --subject should be used to specify full Subject DN,
--subject-base (or similar) for specifying subject base.


IMHO --ca-subject is better than --subject, because it is more explicit
whose subject name that is (the CA's). I agree that --subject should be
deprecated and replaced with --subject-base.



(I intentionally defer discussion of specific behaviour if one, none
or both are specified; let's resolve the question or renaming /
changing meaning of arguments first).



By specifying the option you would override the default "CN=Certificate
Authority,$SUBJECT_BASE" subject name. If --external-ca was not used,
additional validation would be done to make sure the subject name meets
Dogtag's expectations. Actually, it might make sense to always do the
additional validation, to be able to print a warning that if a
Dogtag-incompatible subject name is used, it won't be possible to
change the
CA cert chaining from externally signed to self-signed later.

Honza


Bump, any update on this?

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0003][Tests] Fix for integration tests replication layouts

2016-08-15 Thread Ganna Kaihorodova 
Hello!
I fixed typo in commit message.

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Ganna Kaihorodova" 
To: "Petr Spacek" 
Cc: freeipa-devel@redhat.com
Sent: Monday, August 15, 2016 10:55:08 AM
Subject: Re: [Freeipa-devel] [PATCH 0003][Tests] Fix for integration tests 
replication layouts

Hello, Petr!

Yes, this is exactly what I meant. 
Martin Basti educated me with that.

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Petr Spacek" 
To: freeipa-devel@redhat.com
Sent: Friday, August 12, 2016 6:58:54 PM
Subject: Re: [Freeipa-devel] [PATCH 0003][Tests] Fix for integration tests 
replication layouts

On 9.8.2016 16:55, Ganna Kaihorodova wrote:
> Hello!
> 
> Domain level 0 doesn't allow to create replica file on CA master, testcase 
> was skipped with Domain level 0

You mean on CA-less master, right?

Petr^2 Spacek

> https://fedorahosted.org/freeipa/ticket/6134

-- 
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
From 852040e1cfbdc7636f92ac509c08782d6c83347c Mon Sep 17 00:00:00 2001
From: Ganna Kaihorodova 
Date: Fri, 12 Aug 2016 13:14:10 +0200
Subject: [PATCH] Fix for integration tests replication layouts

Domain level 0 doesn't allow to create replica file on CA-less master, testcases were skipped with Domain level 0

[https://fedorahosted.org/freeipa/ticket/6134]
---
 ipatests/test_integration/test_replication_layouts.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_replication_layouts.py b/ipatests/test_integration/test_replication_layouts.py
index 9fd4ddca299ca99053aae17ae1d43d3c64650f40..c1788153032ffb9125bd9139e70adad78c78607e 100644
--- a/ipatests/test_integration/test_replication_layouts.py
+++ b/ipatests/test_integration/test_replication_layouts.py
@@ -3,10 +3,13 @@
 #
 
 import time
-
+import pytest
+from ipalib.constants import DOMAIN_LEVEL_0
+from ipatests.test_integration.env_config import get_global_config
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration import tasks
 
+config = get_global_config()
 
 class LayoutsBaseTest(IntegrationTest):
 
@@ -27,6 +30,8 @@ class LayoutsBaseTest(IntegrationTest):
 r.run_command(['ipa', 'user-show', test_user])
 
 
+@pytest.mark.skipif(config.domain_level == DOMAIN_LEVEL_0,
+reason='does not work on DOMAIN_LEVEL_0 by design')
 class TestLineTopologyWithoutCA(LayoutsBaseTest):
 
 num_replicas = 3
@@ -87,6 +92,8 @@ class TestCompleteTopologyWithCA(LayoutsBaseTest):
 self.replication_is_working()
 
 
+@pytest.mark.skipif(config.domain_level == DOMAIN_LEVEL_0,
+reason='does not work on DOMAIN_LEVEL_0 by design')
 class Test2ConnectedTopologyWithoutCA(LayoutsBaseTest):
 num_replicas = 33
 
@@ -105,6 +112,8 @@ class Test2ConnectedTopologyWithCA(LayoutsBaseTest):
 self.replication_is_working()
 
 
+@pytest.mark.skipif(config.domain_level == DOMAIN_LEVEL_0,
+reason='does not work on DOMAIN_LEVEL_0 by design')
 class TestDoubleCircleTopologyWithoutCA(LayoutsBaseTest):
 num_replicas = 29
 
-- 
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

[Freeipa-devel] [PATCH 0031, 0032][Tests] Fixes for failing test_ipalib/test_messages tests

2016-08-15 Thread Lenka Doudova 

Hi,

attached are patches that are fixing 3 failing tests in 
test_ipalib/test_messages.py.



Lenka

From 11bd09d8b82630b959deebe265320221db815540 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 15 Aug 2016 11:10:57 +0200
Subject: [PATCH 1/2] Fix malformed or missing docstrings in ipalib/messages

Some of the docstrings in ipalib/messages.py are malformed or missing
entirely. This causes test_ipalib/test_messages to fail due to non-matching
regex.

https://fedorahosted.org/freeipa/ticket/6215
---
 ipalib/messages.py | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 6abad64a8259a8e164db60f63e75bbb9c230e7bf..02b0a0e1021fc2c59a139746eb35fb2d24b78945 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -397,7 +397,7 @@ class DNSForwardPolicyConflictWithEmptyZone(PublicMessage):
 
 class DNSUpdateOfSystemRecordFailed(PublicMessage):
 """
-** 13022 ** Update of a DNS system record failed
+**13022** Update of a DNS system record failed
 """
 errno = 13022
 type = "warning"
@@ -408,7 +408,7 @@ class DNSUpdateOfSystemRecordFailed(PublicMessage):
 
 class DNSUpdateNotIPAManagedZone(PublicMessage):
 """
-** 13023 ** Zone for system records is not managed by IPA
+**13023** Zone for system records is not managed by IPA
 """
 errno = 13023
 type = "warning"
@@ -419,6 +419,9 @@ class DNSUpdateNotIPAManagedZone(PublicMessage):
 
 
 class AutomaticDNSRecordsUpdateFailed(PublicMessage):
+"""
+**13024** Automatic update of DNS records failed
+"""
 errno = 13024
 type = "warning"
 format = _(
@@ -429,6 +432,9 @@ class AutomaticDNSRecordsUpdateFailed(PublicMessage):
 
 
 class ServiceRestartRequired(PublicMessage):
+"""
+**13025** Service restart is required
+"""
 errno = 13025
 type = "warning"
 format = _(
@@ -438,6 +444,9 @@ class ServiceRestartRequired(PublicMessage):
 
 
 class LocationWithoutDNSServer(PublicMessage):
+"""
+**13026** Location without DNS server
+"""
 errno = 13026
 type = "warning"
 format = _(
@@ -464,7 +473,7 @@ class ServerRemovalWarning(PublicMessage):
 
 class CertificateInvalid(PublicMessage):
 """
-***13029 Failed to parse a certificate
+**13029** Failed to parse a certificate
 """
 errno = 13029
 type = "error"
-- 
2.7.4

From 11f4236f73bcff46297b24c5fde5f30e637d9b0c Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 15 Aug 2016 11:19:38 +0200
Subject: [PATCH] Tests: Add data attribute to messages

Tests test_ipalib/test_messages.py are failing because messages now contain
also 'data' attribute, which is not yet reflected in tests.

https://fedorahosted.org/freeipa/ticket/6185
---
 ipatests/test_ipalib/test_messages.py | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/ipatests/test_ipalib/test_messages.py b/ipatests/test_ipalib/test_messages.py
index dad0e988a6f0da1486e02af6c35e6657029e07ac..f6508b98a3cc2ae2d734fc92300fe858c59d6f58 100644
--- a/ipatests/test_ipalib/test_messages.py
+++ b/ipatests/test_ipalib/test_messages.py
@@ -55,10 +55,11 @@ class test_PublicMessages(test_errors.BaseMessagesTest):
 
 def test_to_dict():
 expected = dict(
-name='HelloMessage',
-type='info',
-message='Hello, world!',
+name=u'HelloMessage',
+type=u'info',
+message=u'Hello, world!',
 code=1234,
+data={'greeting': 'Hello', 'object': 'world'},
 )
 
 assert HelloMessage(greeting='Hello', object='world').to_dict() == expected
@@ -78,15 +79,17 @@ def test_add_message():
 
 assert result == {'messages': [
 dict(
-name='HelloMessage',
-type='info',
-message='Hello, world!',
+name=u'HelloMessage',
+type=u'info',
+message=u'Hello, world!',
 code=1234,
+data={'greeting': 'Hello', 'object': 'world'},
 ),
 dict(
-name='HelloMessage',
-type='info',
-message='Hi, version!',
+name=u'HelloMessage',
+type=u'info',
+message=u'Hi, version!',
 code=1234,
+data={'greeting': 'Hi', 'object': 'version'},
 )
 ]}
-- 
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 0003][Tests] Fix for integration tests replication layouts

2016-08-15 Thread Ganna Kaihorodova 
Hello, Petr!

Yes, this is exactly what I meant. 
Martin Basti educated me with that.

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Petr Spacek" 
To: freeipa-devel@redhat.com
Sent: Friday, August 12, 2016 6:58:54 PM
Subject: Re: [Freeipa-devel] [PATCH 0003][Tests] Fix for integration tests 
replication layouts

On 9.8.2016 16:55, Ganna Kaihorodova wrote:
> Hello!
> 
> Domain level 0 doesn't allow to create replica file on CA master, testcase 
> was skipped with Domain level 0

You mean on CA-less master, right?

Petr^2 Spacek

> https://fedorahosted.org/freeipa/ticket/6134

-- 
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] default debug_level of sssd

2016-08-15 Thread Alexander Bokovoy 

On Mon, 15 Aug 2016, Oleg Fayans wrote:

Hi all,

Does anyone know what is the default debug_level for sssd daemon in 
ipa? We've found out that some tests (mainly basic-trust) generate 
huge volumes of sssd logs which we have to store. A quick glance into 
the logs show that these log every tiny bit of really low level 
information that we probably never gonna need. We'd like to tweak the 
tests to configure sssd for less logging, but I was unable to find 
info on default debug_level. The sssd configuration file does not 
explicitly specify it.

man page sssd.conf:
Options usable in all sections
  debug_level (integer)
  

  Currently supported debug levels:

   0, 0x0010: Fatal failures. Anything that would prevent SSSD
 from starting up or causes it to cease running.

  
  Default: 0


--
/ Alexander Bokovoy

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

[Freeipa-devel] default debug_level of sssd

2016-08-15 Thread Oleg Fayans 

Hi all,

Does anyone know what is the default debug_level for sssd daemon in ipa? 
We've found out that some tests (mainly basic-trust) generate huge 
volumes of sssd logs which we have to store. A quick glance into the 
logs show that these log every tiny bit of really low level information 
that we probably never gonna need. We'd like to tweak the tests to 
configure sssd for less logging, but I was unable to find info on 
default debug_level. The sssd configuration file does not explicitly 
specify it.

Thanks!

--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

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

Re: [Freeipa-devel] [PATCH] 0090, 0092..0094 cert-show: show subject alternative names

2016-08-15 Thread Fraser Tweedale 
Thanks for reviews.  Rebased and updated patches attached (and one
new patch).  No substantive changes to 92..94.  Patch order is:

92-2, 93-2, 94-2, 98, 90-3

Other comments inline.

Thanks,
Fraser

On Fri, Aug 12, 2016 at 11:33:28AM +0200, Jan Cholasta wrote:
> Patch 0092: ACK
> 
> Patch 0093: ACK
> 
> Patch 0094: ACK
> 
> Patch 0090:
> 
> 1) Generic otherNames (san_other) do not work correctly. The OID is not
> included in the value and names with complex type other than
> KerberosPrincipal are not parsed correctly. The value should include the OID
> and DER blob of the name.
> 
Updated to use "OID:b64(DER)" as the attribute value.

> 2) With --all, san_other should be included in the result for all
> otherNames, even the known ones, to provide (limited) forward compatibility.
> 
Done; when --all given, known otherName kinds are included in
'san_other' attribute in addition to their own attribute.

> 3) Do we have to support *all* the name types? I mean we could, for the sake
> of completeness, but it might be easier to just keep the few ones we
> actually care about (email, DNS name, principal name, UPN and directory name
> in your patch 0095).
> 
Yeah, why not support them all?  See also Petr's comments in other
branch of thread.

> 4)
> 
> +obj.setdefault(attr_name, []).append(unicode(name))
> 
> The value should not (always) be unicode, but of the type declared by the
> param (unicode or ipapython.kerberos.Principal or
> ipapython.dnsutil.DNSName).
> 
I now pass the value to the constructor of whatever type the
parameter uses:

attr_value = self.params[attr_name].type(name_formatted)
obj.setdefault(attr_name, []).append(attr_value)
From 17e5515ab0eeb92d87091eb00a26dcf358060dba Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 21 Jul 2016 16:27:49 +1000
Subject: [PATCH 92/94] Move GeneralName parsing code to ipalib.x509

GeneralName parsing code is primarily relevant to X.509.  An
upcoming change will add SAN parsing to the cert-show command, so
first move the GeneralName parsing code from ipalib.pkcs10 to
ipalib.x509.

Part of: https://fedorahosted.org/freeipa/ticket/6022
---
 ipalib/pkcs10.py  |  93 ++---
 ipalib/x509.py| 114 +-
 ipaserver/plugins/cert.py |   8 ++--
 3 files changed, 120 insertions(+), 95 deletions(-)

diff --git a/ipalib/pkcs10.py b/ipalib/pkcs10.py
index 
e340c1a2005ad781542a32a0a76753e80364dacf..158ebb3a25be2bd292f3883545fe8afe49b7be8c
 100644
--- a/ipalib/pkcs10.py
+++ b/ipalib/pkcs10.py
@@ -22,9 +22,10 @@ from __future__ import print_function
 import sys
 import base64
 import nss.nss as nss
-from pyasn1.type import univ, char, namedtype, tag
+from pyasn1.type import univ, namedtype, tag
 from pyasn1.codec.der import decoder
 import six
+from ipalib import x509
 
 if six.PY3:
 unicode = str
@@ -32,11 +33,6 @@ if six.PY3:
 PEM = 0
 DER = 1
 
-SAN_DNSNAME = 'DNS name'
-SAN_RFC822NAME = 'RFC822 Name'
-SAN_OTHERNAME_UPN = 'Other Name (OID.1.3.6.1.4.1.311.20.2.3)'
-SAN_OTHERNAME_KRB5PRINCIPALNAME = 'Other Name (OID.1.3.6.1.5.2.2)'
-
 def get_subject(csr, datatype=PEM):
 """
 Given a CSR return the subject value.
@@ -72,78 +68,6 @@ def get_extensions(csr, datatype=PEM):
 return tuple(get_prefixed_oid_str(ext)[4:]
  for ext in request.extensions)
 
-class _PrincipalName(univ.Sequence):
-componentType = namedtype.NamedTypes(
-namedtype.NamedType('name-type', univ.Integer().subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
-),
-namedtype.NamedType('name-string', 
univ.SequenceOf(char.GeneralString()).subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
-),
-)
-
-class _KRB5PrincipalName(univ.Sequence):
-componentType = namedtype.NamedTypes(
-namedtype.NamedType('realm', char.GeneralString().subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
-),
-namedtype.NamedType('principalName', _PrincipalName().subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
-),
-)
-
-def _decode_krb5principalname(data):
-principal = decoder.decode(data, asn1Spec=_KRB5PrincipalName())[0]
-realm = (str(principal['realm']).replace('\\', '')
-.replace('@', '\\@'))
-name = principal['principalName']['name-string']
-name = '/'.join(str(n).replace('\\', '')
-  .replace('/', '\\/')
-  .replace('@', '\\@') for n in name)
-name = '%s@%s' % (name, realm)
-return name
-
-class _AnotherName(univ.Sequence):
-componentType = namedtype.NamedTypes(
-namedtype.NamedType('type-id', univ.ObjectIdentifier()),
-namedtype.NamedType('value', univ.Any().subtype(
-

Re: [Freeipa-devel] [PATCH] 0078-82: webui tests: tests for new certificate widget

2016-08-15 Thread Martin Kosek 
On 07/29/2016 03:00 PM, Pavel Vomacka wrote:
> 
> 
> On 07/28/2016 08:16 AM, Lenka Doudova wrote:
>>
>>
>>
>> On 07/20/2016 04:51 PM, Pavel Vomacka wrote:
>>> Please review attached patches, which add tests for new certificate widget 
>>> in 
>>> WebUI.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6064
>>>
>>>
>>>
>> Hi,
>> thanks for patches.
>> Functionally ok, but you have lots of PEP8 errors in patches 78, 80, 81 and 
>> 82 
>> -> NACK.
>> Also in patch 82, method test_arbitrary_certificate, comment says user needs 
>> to have "arbitrary_cert" configured, but the property in config file is 
>> correctly "arbitrary_cert_path", so it's a bit misleading.
>>
>> Patch 79 is OK, ACK.
>>
>> Lenka
>>
>>
> Thank you for review. Attaching patches which have fixed all pep8 erros. Bad 
> property of config file was also mentioned in patch 81. These are also fixed.

It looks like these patches had wrong author set. Pavel, you may want to
revisit your work environment management scripts :-)

(I wonder if this is worth updating .mailmap to avoid wrong git shortlog)

-- 
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] 0097 Add options to write lightweight CA cert or chain to file

2016-08-15 Thread Jan Cholasta 

On 9.8.2016 16:47, Fraser Tweedale wrote:

On Mon, Aug 08, 2016 at 10:49:27AM +0200, Jan Cholasta wrote:

On 8.8.2016 09:06, Fraser Tweedale wrote:

On Mon, Aug 08, 2016 at 08:54:05AM +0200, Jan Cholasta wrote:

Hi,

On 8.8.2016 06:34, Fraser Tweedale wrote:

Please review the attached patch with adds --certificate-out and
--certificate-chain-out options to `ca-show' command.

Note that --certificate-chain-out currently writes a bogus file due
to a bug in Dogtag that will be fixed in this week's build.

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


1) The client-side *-out options should be defined on the client side, not
on the server side.


Will option defined on client side be propagated to, and observable
in the ipaserver plugin?  The ipaserver plugin needs to observe that
*-out has been requested and executes additional command(s) on that
basis.


Is there a reason not to *always* return the certs?


We hit Dogtag to retrieve them.


I don't think that's an issue in a -show command.







2) I don't think there should be additional information included in summary
(and it definitely should not be multi-line). I would rather inform the user
via an error message when unable to write the files.


I was just following the pattern of other commands that write certs,
profile config, etc.  Apart from consistency with other commands I
agree that there is no need to have it.  So I will remove it.


If you think there is an actual value in informing the user about
successfully writing the files, please use ipalib.messages for the job.


3) IMO a better format for the certificate chain than PKCS#7 would be
concatenated PEM, as that's the most commonly used format in IPA (in
installers, there are no cert chains in API commands ATM).


Sure, but the main use case isn't IPA.  Other apps require PKCS #7
or concatenated PEMs, but sometimes they must be concatenated
forward, and othertimes backwards.  There is no one size fits all.


True, which is exactly why I think we should at least be self-consistent and
use concatenated PEM (and multi-value DER over the wire).


Dogtag returns a PKCS7 (either DER or PEM, according to HTTP Accept
header).

If we want list-of-PEMs between server and client we have to convert
on the server.  Do we have a good way of doing this without exec'ing
`openssl pkcs7' on the server?  Is it acceptable to exec 'openssl'
to do the conversion on the server?  python-nss does not have PKCS7
functions and I am not keen on adding a pyasn1 PKCS7 parser just for
the sake of pushing bits as list-of-PEMs.


I'm afraid we can't avoid conversion to/from PKCS#7 one way or the 
other. For example, if we added a call to retrieve external CA chain 
using certs from cn=certificates,cn=ipa,cn=etc, we would have to convert 
the result to PKCS#7 if it was our cert chain format of choice.


What we can avoid though is executing "openssl pkcs7" to do the 
conversion - we can use an approach similar to our DNSSEC code and use 
python-cffi to call libcrypto's PKCS#7 conversion routines instead.




FWIW, man pages and code suggest that PKCS #7 is accepted in
installer, etc.


True, but that's a relatively new feature (since 4.1) and the installer 
internally executes "openssl pkcs7" to convert PKCS #7 to list of certs :-)





We can add an option to control the format later, but for now,
Dogtag returns a PKCS #7 (PEM or DER) so let's go with that.  Worst
case is an admin has to invoke `openssl pkcs7' and concat the certs
themselves.


AFAIK none of NSS, OpenSSL or p11-kit can use PKCS#7 cert chains directly,
so I'm afraid the worst case would happen virtually always.


If you're OK with invoking OpenSSL on the client to convert PKCS #7
to list-of-PEMs (similar to what is done in
ipapython.certdb.NSSDatabase) then we can have the client perform
the conversion.


See above.







4) Over the wire, the certs should be DER-formatted, as that's the most
common wire format in other API commands.


OK.



5) What is the benefit in having the CA cert and the rest of the chain
separate? For end-entity certs it makes sense to separate the cert from the
CA chain, but for CA certs, you usually want the full chain, no?


If you want to anchor trust directly at a subca (e.g. restrict VPN
login to certs issued by VPN sub-CA) then you often just want the
cert.  The chain option does subsume it, at cost of more work for
administrators with this use case.  I think it makes sense to keep
both options.


Does it? From what you described above, you either want just the sub-CA
cert, or the full chain including the sub-CA cert, in which case it might
make more sense to have a single --out option and a --chain flag.


How about --certificate-out which defaults to single cert, but does
chain (as list-of-PEMs) when --chain flag given.

Per https://fedorahosted.org/freeipa/ticket/5166 let's not add more
`--out' options.


+1



Thanks,
Fraser




--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list: