[Freeipa-devel] [PATCH] 0156 extdom: add certificate request

Martin Basti mbasti at redhat.com
Fri Jun 24 13:09:46 UTC 2016 


On 24.06.2016 14:59, Sumit Bose wrote:
> On Fri, Jun 24, 2016 at 02:00:24PM +0200, Martin Basti wrote:
>>
>> On 22.06.2016 23:20, Lukas Slebodnik wrote:
>>> On (22/06/16 11:57), Martin Basti wrote:
>>>> On 09.06.2016 21:02, Martin Basti wrote:
>>>>> On 09.06.2016 14:45, Martin Basti wrote:
>>>>>> On 09.06.2016 14:42, Martin Basti wrote:
>>>>>>> On 09.06.2016 14:38, Lukas Slebodnik wrote:
>>>>>>>> On (09/06/16 14:29), Martin Basti wrote:
>>>>>>>>> On 09.06.2016 14:22, Alexander Bokovoy wrote:
>>>>>>>>>> On Thu, 09 Jun 2016, Jakub Hrozek wrote:
>>>>>>>>>>> On Fri, May 20, 2016 at 09:23:46PM +0200, Sumit Bose wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> this patch allows the extom plugin to lookup
>>>>>>>>>>>> users by certificate which
>>>>>>>>>>>> is needed in the case where a IPA client
>>>>>>>>>>>> wants to lookup an AD user who
>>>>>>>>>>>> has the certificate stored in AD. To make
>>>>>>>>>>>> this work the related patches
>>>>>>>>>>>> I just send to sssd-devel are needed as well.
>>>>>>>>>>>>
>>>>>>>>>>>> Currently the patches miss the change in the
>>>>>>>>>>>> required version of SSSD.
>>>>>>>>>>>> since the SSSD patches are not committed. But
>>>>>>>>>>>> the patches are needed to
>>>>>>>>>>>> fully test the SSSD patches. I will send a
>>>>>>>>>>>> new version with the needed
>>>>>>>>>>>> changes to the minimal SSSD version when the SSSD patches are
>>>>>>>>>>>> committed.
>>>>>>>>>>>>
>>>>>>>>>>>> bye,
>>>>>>>>>>>> Sumit
>>>>>>>>>>> The patch works fine (tested together with the corresponding SSSD
>>>>>>>>>>> patches), so ACK from me. The code also looks
>>>>>>>>>>> good to me, but I'm not
>>>>>>>>>>> sure if reviewing an IPA patch requires something
>>>>>>>>>>> more (CI? Coverity?)
>>>>>>>>>> ACK from me as well, I forgot to send email about it,
>>>>>>>>>> though I reviewed
>>>>>>>>>> this patch a week ago.
>>>>>>>>>>
>>>>>>>>> Pushed to master: aa734da49440c5d12c0f8d4566505adaeef254e8
>>>>>>>>>
>>>>>>>> It's very likey that this commit will break build of
>>>>>>>> freeipa-master. I didn't try.
>>>>>>>>
>>>>>>>> Because it uses new function sss_nss_getnamebycert
>>>>>>>> from the library libsss_nss_idmap which is not in fedora.
>>>>>>>> It was pushed to sssd master just today.
>>>>>>>>
>>>>>>>> LS
>>>>>>> If this is true, can you/somebody provide the SRPM of SSSD with
>>>>>>> the required functionality please? We may need to add it to
>>>>>>> @freeipa/freeipa-master copr and bump required version of SSSD.
>>>>>>>
>>>>>>> Martin^2
>>>>>>>
>>>>>> Yes, you were right, master build is broken.
>>>>>> Martin^2
>>>>>>
>>>>> SSSD master build has been added to @freeipa/freeipa-master copr as a
>>>>> workaround (to unblock automatic testing an developers)
>>>>>
>>>>> Please bump version in specfile accordingly (I don't know in which
>>>>> version of SSSD will be required function)
>>>>>
>>>>> Martin^2
>>>>>
>>>> Bumping SSSD version in requires and buildrequires
>>>> Patch attached
>>> >From f2b394085157954768bc93a73b854778c65bfdcd Mon Sep 17 00:00:00 2001
>>>> From: Martin Basti <mbasti at redhat.com>
>>>> Date: Wed, 22 Jun 2016 10:49:39 +0200
>>>> Subject: [PATCH] Bump SSSD requires
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4955
>>>> ---
>>>> freeipa.spec.in | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/freeipa.spec.in b/freeipa.spec.in
>>>> index 0d5c745d5306cd7141c573454bd1c1e6a78c7e7f..befc7af9ee2ceefa41b1b999df4bdb1c6607bea8 100644
>>>> --- a/freeipa.spec.in
>>>> +++ b/freeipa.spec.in
>>>> @@ -85,7 +85,7 @@ BuildRequires:  python-pyasn1 >= 0.0.9a
>>>> BuildRequires:  python-qrcode-core >= 5.0.0
>>>> BuildRequires:  python-dns >= 1.11.1
>>>> BuildRequires:  libsss_idmap-devel
>>>> -BuildRequires:  libsss_nss_idmap-devel >= 1.12.2
>>>> +BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
>>>> BuildRequires:  java-headless
>>>> BuildRequires:  rhino
>>>> BuildRequires:  libverto-devel
>>>> @@ -327,7 +327,7 @@ Requires: pam_krb5
>>>> Requires: curl
>>>> Requires: libcurl >= 7.21.7-2
>>>> Requires: xmlrpc-c >= 1.27.4
>>>> -Requires: sssd >= 1.13.3-5
>>>> +Requires: sssd >= 1.14.0
>>> NACK
>> Thank you.
>>> A) It's not explained in commit message why you need to bump Requires for sssd.
>>>      IIRC, you need just new libsss_nss_idmap-devel.
>> I don't know actually, would be nice if author of the original patch can
>> confirm if newer SSSD is required or not
> Currently both are required. 'BuildRequires:  libsss_nss_idmap-devel >=
> 1.14.0' is needed for the build because the new call
> sss_nss_getnamebycert() is needed to look up trusted users by
> certificate.
>
> At runtime 'Requires: sssd >= 1.14.0' is needed because currently
> libsss_nss_idmap does not have a dependency to sssd. If only the
> libsss_nss_idmap would be updated and not SSSD the
> sss_nss_getnamebycert() would just return a not implemented error code
> because the older versions of SSSD cannot handle the request.
>
> HTH
>
> bye,
> Sumit
Thank you for explanation, updated patch attached.

Martin^2
>>> B) You forgot add detection for newer version of libsss_nss_idmap at configure
>>>      time
>>> Hint: * daemons/configure.ac
>>>         * https://autotools.io/pkgconfig/pkg_check_modules.html#pkgconfig.pkg_check_modules.specification
>> Fixed
>>> LS
>> Updated patch attached.
>>  From 34c63f8ba5c478cd95a62bc3dffc6bfc7be3384b Mon Sep 17 00:00:00 2001
>> From: Martin Basti <mbasti at redhat.com>
>> Date: Wed, 22 Jun 2016 10:49:39 +0200
>> Subject: [PATCH] Bump libsss_nss_idmap-devel
>>
>> This is required by commit aa734da49440c5d12c0f8d4566505adaeef254e8
>>
>> https://fedorahosted.org/freeipa/ticket/4955
>> ---
>>   daemons/configure.ac | 2 +-
>>   freeipa.spec.in      | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/daemons/configure.ac b/daemons/configure.ac
>> index 2906def285a0f6ad9553fc07cbc59f7a7f7fd426..fa5eab829cad6718f21ec3d5569ffe1b0168e518 100644
>> --- a/daemons/configure.ac
>> +++ b/daemons/configure.ac
>> @@ -253,7 +253,7 @@ dnl -- dirsrv is needed for the extdom unit tests --
>>   PKG_CHECK_MODULES([DIRSRV], [dirsrv  >= 1.3.0])
>>   dnl -- sss_idmap is needed by the extdom exop --
>>   PKG_CHECK_MODULES([SSSIDMAP], [sss_idmap])
>> -PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap])
>> +PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap >= 1.14.0])
>>   
>>   dnl ---------------------------------------------------------------------------
>>   dnl - Check for systemd unit directory
>> diff --git a/freeipa.spec.in b/freeipa.spec.in
>> index d31ddfaf78a455f4e4d65724bbbe23461e1336e0..e82950d7f82fb5018d893a0644dd1a5931656e2d 100644
>> --- a/freeipa.spec.in
>> +++ b/freeipa.spec.in
>> @@ -85,7 +85,7 @@ BuildRequires:  python-pyasn1 >= 0.0.9a
>>   BuildRequires:  python-qrcode-core >= 5.0.0
>>   BuildRequires:  python-dns >= 1.11.1
>>   BuildRequires:  libsss_idmap-devel
>> -BuildRequires:  libsss_nss_idmap-devel >= 1.12.2
>> +BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
>>   BuildRequires:  java-headless
>>   BuildRequires:  rhino
>>   BuildRequires:  libverto-devel
>> -- 
>> 2.5.5
>>
>> -- 
>> 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0536.3-Bump-SSSD-version-in-requires.patch
Type: text/x-patch
Size: 1931 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160624/50352fab/attachment.bin>

More information about the Freeipa-devel mailing list