[Freeipa-devel] [PATCHES] 122-123 Remove generation and handling of LM hashes

[Martin Kosek]

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.

>> 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.

Martin