On 10/30/2013 10:44 PM, Sumit Bose wrote: > On Wed, Oct 30, 2013 at 02:12:23PM +0100, Martin Kosek wrote: >> On 10/30/2013 01:28 PM, Alexander Bokovoy wrote: >>> On Wed, 30 Oct 2013, Sumit Bose wrote: >>> >>>> Hi, >>>> >>>> those two patches try to fix >>>> https://fedorahosted.org/freeipa/ticket/3795 (Remove LANMAN hash >>>> support). The first patch removes to option to enable the support while >>>> the second removes all the related C-code. >>> ACK on these patches but see below. >> >> I have few comments on the patches: >> >> 1) In util/ipa_pwd_ntlm.c, we can now also remove parity_table. >> >> 2) In util/ipa_pwd_ntlm.c, in encode_ntlm_keys, upperPasswd is no longer >> needed >> (i.e. the UTF upper-casing calls in caller functions are not needed either). >> I >> am thinking we could simplify the function just to: >> >> int encode_nt_key(char *newPasswd, >> uint8_t *ntHash) >> >> i.e. it seems to me that ntlm_keys structure may not be needed now, since we >> removed one item of two in it. keys->lm is not used anywhere anyway. > > I removed/changed the code as you suggested. New version attached. > >> >>>> Although the ticket is schedule for the 3.3.x bugfix release I'm not >>>> sure if it is a good idea to remove the support in a minor release. >>>> Since the LM hashes are not enabled by default I would expect that in >>>> setups where it is enabled the hashes are needed one way or the other. >>>> Those setup should get time to adopt. >>> We should add removal of the 'allowlmhash' from the IPA config with >>> upgrade plugin. >> >> Not sure this is the best way. With Sumit's patches, generation of the LM >> hash >> is not stopped despite the configuration. So if someone still needs an old >> IPA >> server where these hashes are used, they are still generated and used there. >> >> If you remove allowlmhash from the config, once you install a patched IPA >> replica, the value would get replicated and old IPA server would not generate >> the hashes. > > We discussed this and came to the conclusion that we might want to add a > script which removes existing LM hashes and config entries from the > directory tree. This way the admin can decide based on his environment > when is the best time to remove them. > > bye, > Sumit
ACK, pushed your 2 patches to master, ipa-3-3. I also pushed a small patch removing the config string allowing LM hashes (attached) which was ACK-ed via IRC by Alexander. Note that I also filed https://fedorahosted.org/freeipa/ticket/4009 to let us prepare a script removing existing LM hashes for all users. Martin
From 8d3230bab79576e6ee8a863b3900bd50cc7c51e0 Mon Sep 17 00:00:00 2001 From: Martin Kosek <[email protected]> Date: Fri, 1 Nov 2013 09:25:33 +0100 Subject: [PATCH] Remove deprecated AllowLMhash config Remove this ipaConfigString value as LM hash is deprecated and in fact even insecure. https://fedorahosted.org/freeipa/ticket/3795 --- install/updates/50-ipaconfig.update | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/updates/50-ipaconfig.update b/install/updates/50-ipaconfig.update index 69783f13261cfd969d37fdd0e00f2adf8bd66355..ce617fe0db0159ac8bc382b947dba359cde3c0ec 100644 --- a/install/updates/50-ipaconfig.update +++ b/install/updates/50-ipaconfig.update @@ -1,5 +1,5 @@ dn: cn=ipaConfig,cn=etc,$SUFFIX add:ipaSELinuxUserMapOrder: guest_u:s0$$xguest_u:s0$$user_u:s0$$staff_u:s0-s0:c0.c1023$$unconfined_u:s0-s0:c0.c1023 add:ipaSELinuxUserMapDefault: unconfined_u:s0-s0:c0.c1023 - add:ipaUserObjectClasses: ipasshuser +remove:ipaConfigString:AllowLMhash -- 1.8.3.1
_______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
