On 25.2.2016 14:36, Jan Cholasta wrote:
> Hi,
> 
> On 25.2.2016 14:23, Martin Basti wrote:
>>
>>
>> On 22.02.2016 22:13, Martin Štefany wrote:
>>> Hi,
>>>
>>> please, review the attached patch which adds --ssh-update to ipa-client-
>>> install.
>>>
>>> Ticket:https://fedorahosted.org/freeipa/ticket/2655
>> Hello,
>> thank you for your patch.
>> Please attach a patch as a file next time.
>>
>> I have doubts that this should be part of ipa-client-install, this needs
>> a broader discussion.
> 
> +1, I think it should be a separate command (ignore my earlier suggestion from
> Trac to incorporate this into ipa-client-install, I was young and stupid).
> 
> See client/ipa-certupdate and ipaclient/ipa_certupdate.py for an example of
> how such a command should be implemented.

Or we can have idempotent client installer as we have ipa-dns-install ...

Petr^2 Spacek

>> Code comments inline:
>>>
>>> ---
>>> Martin
>>>
>>> >From 4974a57f48a0cd48b83724297ae2af572bc530eb Mon Sep 17 00:00:00 2001
>>> From: Martin Stefany <martin stefany eu>
>>> Date: Mon, 22 Feb 2016 20:58:13 +0000
>>> Subject: [PATCH] Add new parameter --ssh-update to ipa-client-install
>>>
>>> Add a new parameter '--ssh-update' which can be used later after freeipa
>>> client is installed to update SSH hostkeys and SSHFP DNS records for
>>> host.
>>>
>>> https://fedorahosted.org/freeipa/ticket/2655
>>> ---
>>>   ipa-client/ipa-install/ipa-client-install | 102
>>> +++++++++++++++++++++++++++++-
>>>   1 file changed, 99 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-
>>> install/ipa-client-install
>>> index
>>> 789ff591591673744ee3b922e5c0181233ad553c..97adfb6b449fb441bddada89a3b151
>>> 33e398ca50 100755
>>> --- a/ipa-client/ipa-install/ipa-client-install
>>> +++ b/ipa-client/ipa-install/ipa-client-install
>>> @@ -71,6 +71,7 @@ CLIENT_INSTALL_ERROR = 1
>>>   CLIENT_NOT_CONFIGURED = 2
>>>   CLIENT_ALREADY_CONFIGURED = 3
>>>   CLIENT_UNINSTALL_ERROR = 4 # error after restoring files/state
>>> +CLIENT_SSHUPDATE_ERROR = 5 # error during update of SSH public keys
>>>
>>>   def parse_options():
>>>       def validate_ca_cert_file_option(option, opt, value, parser):
>>> @@ -215,6 +216,12 @@ def parse_options():
>>>                                             "be run with --unattended
>>> option")
>>>       parser.add_option_group(uninstall_group)
>>>
>>> +    sshupdate_group = OptionGroup(parser, "SSH key update options")
>>> +    sshupdate_group.add_option("--ssh-update", dest="ssh_update",
>>> +                      action="store_true", default=False,
>>> +                      help="update local host's SSH public keys in host
>>> entry and DNS.")
>>> +    parser.add_option_group(sshupdate_group)
>>> +
>>>       options, args = parser.parse_args()
>>>       safe_opts = parser.get_safe_opts(options)
>>>
>>> @@ -840,6 +847,92 @@ def uninstall(options, env):
>>>
>>>       return rv
>>>
>>> +def sshupdate(options, env):
>>> +    if not is_ipa_client_installed():
>>> +        root_logger.error("IPA client is not configured on this
>>> system.")
>>> +        return CLIENT_NOT_CONFIGURED
>>> +
>>> +    api.bootstrap(context='cli_installer', debug=options.debug)
>>> +    api.finalize()
>>> +    if 'config_loaded' not in api.env:
>>> +        root_logger.error("Failed to initialize IPA API.")
>>> +        return CLIENT_SSHUPDATE_ERROR
>>> +
>>> +    # Now, let's try to connect to the server's RPC interface
>>> +    connected = False
>>> +    try:
>>> +        api.Backend.rpcclient.connect()
>>> +        connected = True
>>> +        root_logger.debug("Try RPC connection")
>>> +        api.Backend.rpcclient.forward('ping')
>>> +    except errors.KerberosError as e:
>>> +        if connected:
>>> +            api.Backend.rpcclient.disconnect()
>>> +        root_logger.info(
>>> +            "Cannot connect to the server due to Kerberos error: %s. "
>>> +            "Trying with delegate=True", e)
>>> +        try:
>>> +            api.Backend.rpcclient.connect(delegate=True)
>>> +            root_logger.debug("Try RPC connection")
>>> +            api.Backend.rpcclient.forward('ping')
>>> +
>>> +            root_logger.info("Connection with delegate=True
>>> successful")
>>> +
>>> +            # The remote server is not capable of Kerberos S4U2Proxy
>>> +            # delegation. This features is implemented in IPA server
>>> +            # version 2.2 and higher
>>> +            root_logger.warning(
>>> +                "Target IPA server has a lower version than the
>>> enrolled "
>>> +                "client")
>>> +            root_logger.warning(
>>> +                "Some capabilities including the ipa command capability
>>> "
>>> +                "may not be available")
>>> +        except errors.PublicError as e2:
>>> +            root_logger.warning(
>>> +                "Second connect with delegate=True also failed: %s",
>>> e2)
>>> +            root_logger.error(
>>> +                "Cannot connect to the IPA server RPC interface: %s",
>>> e2)
>>> +            return CLIENT_SSHUPDATE_ERROR
>>> +    except errors.PublicError as e:
>>> +        root_logger.error(
>>> +            "Cannot connect to the server due to generic error: %s", e)
>>> +        return CLIENT_SSHUPDATE_ERROR
>> I think you should be kinited with client keytab, client is allowed to
>> modify its SSHpublic keys in ldap. I'd only require to be root to
>> execute it.
>>
>> kinit -kt /etc/krb5.keytab host/`hostname`
>> ipa host-mod `hostname` --sshpubkey="something"
>>
>> Also this rpcconnection looks to me too much complicated, I think it
>> should be just simple rpcconnect
>>
>>> +
>>> +    # We need to pull IPA server address from default.conf
>>> +    try:
>>> +        parser = RawConfigParser()
>>> +        parser.read(paths.IPA_DEFAULT_CONF)
>>> +        cli_realm = parser.get('global', 'realm')
>>> +        hostname = parser.get('global', 'host')
>>> +    # TODO: consult with review team
>>> +    # except ConfigParser.NoSectionError as e:
>>> +    #     pass
>>> +    # except ConfigParser.ParsingError as e:
>>> +    #     pass
>>> +    finally:
>>> +        pass
>> You can raise error there.
>>
>>> +
>>> +    host_principal = 'host/%s@%s' % (hostname, cli_realm)
>>> +    # Obtain the TGT. We do it with the temporary krb5.conf, so that
>>> +    # only the KDC we're installing under is contacted.
>>> +    # Other KDCs might not have replicated the principal yet.
>>> +    # Once we have the TGT, it's usable on any server.
>> I don't think that temporary krb5.conf should be used here
>>> +    try:
>>> +        ipautil.kinit_keytab(host_principal, paths.KRB5_KEYTAB,
>>> +                             CCACHE_FILE,
>>> +                             # config=krb_name,
>>> +                             attempts=options.kinit_attempts)
>>> +        env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = CCACHE_FILE
>>> +    except Krb5Error as e:
>>> +        print_port_conf_info()
>>> +        root_logger.error("Failed to obtain host TGT: %s" % e)
>>> +        # failure to get ticket makes it impossible to login and bind
>>> +        # from sssd to LDAP, abort installation and rollback changes
>>> +        return CLIENT_INSTALL_ERROR
>> This is not install error.
>>
>>> +
>>> +    # passing server parameter seems unneccessary, thus passing only ""
>>> +    update_ssh_keys("", hostname,
>>> services.knownservices.sshd.get_config_dir(), options.create_sshfp)
>>> +
>>>   def configure_ipa_conf(fstore, cli_basedn, cli_realm, cli_domain,
>>> cli_server, hostname):
>>>       ipaconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Installer")
>>>       ipaconf.setOptionAssignment(" =") @@ -2797,7 +2890,7 @@ def
>>> install(options, env, fstore,
>>> statestore):              connected = True
>>>              root_logger.debug("Try RPC connection")
>>>               api.Backend.rpcclient.forward('ping')
>>> -        except errors.KerberosError, e:
>>> +        except errors.KerberosError as e:
>> Please don't modify code that already exists and it is not related to
>> this change
>>>               if connected:
>>>                   api.Backend.rpcclient.disconnect()
>>>               root_logger.info(
>>> @@ -2820,13 +2913,13 @@ def install(options, env, fstore, statestore):
>>>                   root_logger.warning(
>>>                       "Some capabilities including the ipa command
>>> capability "
>>>                       "may not be available")
>>> -            except errors.PublicError, e2:
>>> +            except errors.PublicError as e2:
>> Remove this from patch too
>>>                   root_logger.warning(
>>>                       "Second connect with delegate=True also failed:
>>> %s", e2)
>>>                   root_logger.error(
>>>                       "Cannot connect to the IPA server RPC interface:
>>> %s", e2)
>>>                   return CLIENT_INSTALL_ERROR
>>> -        except errors.PublicError, e:
>>> +        except errors.PublicError as e:
>> and this too
>>>               root_logger.error(
>>>                   "Cannot connect to the server due to generic error:
>>> %s", e)
>>>               return CLIENT_INSTALL_ERROR
>>> @@ -3088,6 +3181,9 @@ def main():
>>>       if options.uninstall:
>>>           return uninstall(options, env)
>>>
>>> +    if options.ssh_update:
>>> +        return sshupdate(options, env)
>>> +
>>>       if is_ipa_client_installed(on_master=options.on_master):
>>>           root_logger.error("IPA client is already configured on this
>>> system.")
>>>           root_logger.info(
>>> -- 
>>> 1.8.3.1
>>>
>>>
>>
>> Martin^2
>>
>>
> 
> Honza

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to