On Thu, Jul 30, 2015 at 11:10:33AM +0200, Jakub Hrozek wrote:
> On Wed, Jul 29, 2015 at 03:34:32PM +0200, Jakub Hrozek wrote:
> > On Tue, Jul 28, 2015 at 10:43:13PM +0200, Sumit Bose wrote:
> > > >
> > > > Hi,
> > > >
> > > > I started the review, but because the patches are quite big, I will
send
> > > > my comments in batches. I hope that's fine.
> > >
> > > Thank you for the review.
> > >
> > > >
> > > > CI failed in distcheck.
> > >
> > > The failure was due to distcheck's tendency to make the complete
source
> > > tree read-only and NSS's tendency open all databases in read-write
more.
> > > I added the needed read-onl options to the NSS code.
> > >
> > > >
> > > > Coverity found some issues that I forwarded to Sumit.
> > >
> > > Thank you, I fixed them and hopefully did not create new ones.
> >
> > Coverity is clean this time, but CI did not pass on Debian due to build
> > failure of a test:
> >
http://sssd-ci.duckdns.org/logs/job/19/78/debian_testing/ci-build-debug/c...
> > Normally this is caused by a missing dependency, which the Fedora/RHEL
> > linker can satisfy.
Please find attached a new set which should fix the debian build (only
the last patch changed). As you suspected it was a missing dependency in
Makefile.am. But this make me realize that I didn't implement a
OpenSSL/libcrypto version of cert_to_ssh_key() which would break the
OpenSSL build. I added an initial version without certificate
validation. If you don't like it it can simply replaced by 'return
ENOSUPP'.
I'm fine with the code as long as you keep the TODOs at least in some
todo list of yours.
ACK code wise, also my regression tests went fine.
I'm only waiting for the automated tests to finish.