On Wed, 2011-04-27 at 11:36 -0400, Stephen Gallagher wrote: > On Wed, 2011-04-27 at 14:00 +0200, Jakub Hrozek wrote: > > On 04/26/2011 08:04 PM, Stephen Gallagher wrote: > > > On Tue, 2011-04-26 at 13:59 -0400, Stephen Gallagher wrote: > > >> On Tue, 2011-04-26 at 11:01 +0200, Jakub Hrozek wrote: > > >>> [PATCH 1/2] Require openssl-devel is libcrypto backend is selected > > >>> > > >>> I've changed the configure flags a little. The previous --enable-crypto > > >>> sounded like something you should enable to get the functionality > > >>> compiled in rather than a selection of alternative crypto back end. > > >>> > > >>> https://fedorahosted.org/sssd/ticket/844 > > >>> > > >>> [PATCH 2/2] BUILD.txt: Warn that all crypto features are implemented > > >>> > > >>> Note that the recommended default is NSS. > > >> > > >> > > >> Nack. > > >> > > >> Please add a warning in the configure script as well when using > > >> libcrypto. No one EVER reads the BUILD.txt. > > >> > > > > Done. btw I've seen one user that read it (and saw a bug there) :-) > > > > >> Have you tested whether functionality works with libcrypto at all? What > > >> happens when cache_passwords = true? Do we store them plaintext or fail > > >> entirely? Is it a graceful failure? > > >> > > > > password caching works OK with libcrypto, it stores a hash in the cache > > file in the same way NSS does. The only feature that does not work at > > all with libcrypto is password obfuscation. > > > > >> I'm honestly not sure there's a good reason to allow the use of > > >> libcrypto at all at this point. I think we should consider disabling it > > >> (not removing it) until and unless someone else decides to maintain it. > > > > > > > > > > I agree, rotten code is bad. I would also like to get rid of the > > --enable-crypto switch one way or another, because the name if confusing. > > > > What about including the first two patches that fix the dependency > > detection and rename the configure switch and then include another patch > > (#3 in this case) to unconditionally build just NSS? > > > > This way, if someone steps up for libcrypto maintenance or contributes > > another crypto back end, we'll be able to just revert this one patch and > > get the fixed infrastructure? > > > > > Also, 'make check' fails if --with-crypto=libcrypto is used with: > > > Running suite(s): sss_crypto > > > 0%: Checks: 1, Failures: 1, Errors: 0 > > > ../src/tests/crypto-tests.c:69:F:sss crypto > > > tests:test_encrypt_decrypt:0: Failure 'ret != EOK' occured > > > FAIL: crypto-tests > > > > > > > Fixed, I've also discovered that HAVE_NSS is not #defined anywhere so > > the first testcase never ran. Both issues are fixed in patch #1. If you > > don't agree with the first two patches and prefer to just get rid of > > libcrypto, I'd still like to include these two changes. > > > Ack to all three.
Pushed to master.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
