[Freeipa-devel] [PATCH] 89 ipa-adtrust-install: allow to reset te NetBIOS domain name

Wed Nov 7 13:03:46 UTC 2012

On 11/07/2012 01:54 PM, Sumit Bose wrote:
> On Mon, Nov 05, 2012 at 01:18:49PM +0100, Martin Kosek wrote:
>> On 11/02/2012 09:50 PM, Sumit Bose wrote:
>>> On Fri, Nov 02, 2012 at 02:54:32PM +0100, Martin Kosek wrote:
>>>> On 11/02/2012 12:54 PM, Sumit Bose wrote:
>>>>> On Wed, Oct 31, 2012 at 04:03:14PM +0100, Martin Kosek wrote:
>>>>>> On 10/30/2012 12:16 PM, Sumit Bose wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> this patch allows ipa-adtrust-install to reset the NetBIOS domain name
>>>>>>> and fixes https://fedorahosted.org/freeipa/ticket/3192 .
>>>>>>>
>>>>>>> bye,
>>>>>>> Sumit
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Hello Sumit,
>>>>>>
>>>>>> I found few issues with your patch:
>>>>>
>>>>> Thank you for the review.
>>>>>
>>>>>>
>>>>>> 1) It requires admin to be kinited ("conn.do_sasl_gssapi_bind()") I do not
>>>>>> think this is necessary, ipa-adtrust-install already requires admin password to
>>>>>> be passed and it already connects to LDAP with these credentials:
>>>>>>
>>>>>> api.Backend.ldap2.connect(ccache.name)
>>>>>>
>>>>>> You could use ipa.Backend.ldap2 object to do entry retrieval
>>>>>> (ipa.Backend.ldap2.get_entry) without a need to init IPAdmin at all.
>>>>>
>>>>> fixed
>>>>>
>>>>>>
>>>>>> 2) When doing try..except statement, rule of thumb says that it should be as
>>>>>> short as possible, so that it does not hide other potential errors and makes
>>>>>> clear what function raises the catched exception.
>>>>>>
>>>>>> In your case:
>>>>>>
>>>>>> try:
>>>>>>     entry = api.Backend.ldap2.get_entry(DN(('cn', api.env.domain),
>>>>>>                                         api.env.container_cifsdomains,
>>>>>>                                         self.api.env.basedn),
>>>>>>                                        ['ipantflatname'])
>>>>>> except errors.NotFound:
>>>>>>     reset_netbios_name = False
>>>>>> else:
>>>>>>     # process entry
>>>>>>
>>>>>> Should be a pattern that you want.
>>>>>
>>>>> fixed
>>>>>
>>>>> I also move all the NetBIOS name related code into a separate function.
>>>>>>
>>>>>> 3) I think this line is redundant:
>>>>>> +                    print "Say 'yes' if the NetBIOS shall be changed and " \
>>>>>> +                          "'no' if the old one shall be kept."
>>>>>>
>>>>>> IMO, the question:
>>>>>>
>>>>>> reset_netbios_name = ipautil.user_input( 'Reset NetBIOS domain name?',  default
>>>>>> = False, allow_empty = False)
>>>>>>
>>>>>> and the information printed before is enough.
>>>>>
>>>>> I would prefer to keep it this way to make clear that
>>>>> ipa-adtrust-install will continue processing, but the old name if kept
>>>>> even if a new name was given with --netbios-name on the command line.
>>>>>
>>>>> New version attached.
>>>>>
>>>>> bye,
>>>>> Sumit
>>>>>
>>>>>>
>>>>>> Martin
>>>>
>>>>
>>>> The new approach looks much better. Sending issues I found with the new patch:
>>>>
>>>> 1) When I run ipa-adtrust-install on a clean IPA, I can no longer enter NetBIOS
>>>> name interactively. I can only change it via script option...
>>>>
>>>
>>> fixed
>>>
>>>>
>>>> 2) I saw few typos:
>>>>
>>>> +        print "Current NetBIOS domain name is %s new name is %s.\n" % \
>>>> should be:
>>>> +        print "Current NetBIOS domain name is %s, new name is %s.\n" % \
>>>>
>>>> +            print "NetBIOS domain name will be changes to %s.\n" % \
>>>> should be:
>>>> +            print "NetBIOS domain name will be changed to %s.\n" % \
>>>>
>>>>
>>>
>>> fixed
>>>
>>> new version attached.
>>>
>>> bye,
>>> Sumit
>>>> Martin
>>
>> NetBIOS name is now asked when first installing ipa-adtrust-install.
>>
>> But I see that NetBIOS name is still not queried when I run re-install of
>> ADTRUST, I can only change current name via option. Is this is an intended
>> behavior so that people cannot mess it with by mistake?
> 
> Yes. The old code didn't check if the NetBIOS name was already set in
> LDAP or not, hence it always asked the user if the generated NetBIOS
> name is the one the user expected.
> 
> The new version looks up the NetBIOS name in the LDAP server and if set
> and no new name is given on the command line assumes that the admin
> does not want to change the NetBIOS name and skips the dialog.
> 
> I'll add the QE team to hear what they prefer.
> 
> Jenny, Scott, Steeve, assume ipa-adtrust-install is run after trust was
> already configured and no --netbios-name option is given on the command
> line. Shall the following dialog be shown:
> 
> .....
> Enter the NetBIOS name for the IPA domain.
> Only up to 15 uppercase ASCII letters and digits are allowed.
> Example: EXAMPLE.
> 
> 
> NetBIOS domain name [IPA17]:
> ....
> 
> The admin then has to press enter to confirm the current NetBIOS name or
> can enter a new one. Or shall the dialog be skipped which means that the
> NetBIOS can only be resetted if a new one is given at the command line
> with the --netbios-name option?

Ok, thanks from explanation. But from my devel perspective, since a change of
NetBIOS domain name can break existing trusts I would rather not offer a change
of NetBIOS name during interactive prompt. A mere stating of a current value
with asking to user to change it should be enough.

QE input is welcome, yes.

Martin

> 
> Thank you for your input.
> 
> bye,
> Sumit
> 
>>
>> # ipa-adtrust-install
>>
>> The log file for this installation can be found in /var/log/ipaserver-install.log
>> ==============================================================================
>> This program will setup components needed to establish trust to AD domains for
>> the FreeIPA Server.
>>
>> This includes:
>>   * Configure Samba
>>   * Add trust related objects to FreeIPA LDAP server
>>
>> To accept the default shown in brackets, press the Enter key.
>>
>> IPA generated smb.conf detected.
>> Overwrite smb.conf? [no]: y
>>
>> The following operations may take some minutes to complete.
>> Please wait until the prompt is returned.
>>
>> <<< no NetBIOS name asked interactively
>>
>> Configuring cross-realm trusts for IPA server requires password for user 'admin'.
>> This user is a regular system account used for IPA server administration.
>>
>> admin password:
>>
>> Configuring CIFS
>>   [1/18]: stopping smbd
>> ...
>>
>>
>> Otherwise the patch looks OK.
>>
>> Martin