On 19.11.2015 17:43, Simo Sorce wrote:
On Thu, 2015-11-19 at 15:43 +0100, Jan Cholasta wrote:
Hi,

the attached patches fix <https://fedorahosted.org/freeipa/ticket/3416>
and <https://fedorahosted.org/freeipa/ticket/5401>.

I worked around the issue of checking if the user is privileged to
perform replica promotion by using host credentials instead. The host
must be a member of the IPA servers host group "ipaservers" in order to
be able to promote itself. Using host credentials will also allow
replica install using one-time password.

User credentials are still used for connection check and to
automatically add the host to ipaservers if the user is privileged to do
that.

Simo, is this approach OK? Could you check the new ACIs in patches 510
and 513?

I have a couple of questions:

1) Why are custodia keys for the replica added to LDAP using connection
to the remote master instead of local ldapi connection? Is it to
eliminate race conditions caused by replication timeout from the replica
to the remote master?

Yes it is critical for that reason.

If the code was changed to use ldapi and wait until the key appears in
custodia on the remote master, we could lose the "IPA server hosts can
create own Custodia secrets" and "IPA server hosts can manage own
Custodia secrets" ACIs from patch 510. Not sure if it's worth the change
though.

Not worth the change, as it has the same security properties.

2) Why is 'memberPrincipal' used in cn=custodia instead of 'member'?

If 'member' was used instead, we would gain referential integrity and
the ability to add ACIs based on the attribute (think
userattr="member#USERDN").

To avoid referential integrity and mixup with other group objects, it
was intentional.

3) Why is 'memberPrincipal' used in cn=custodia at all?

The hostname of the replica is already in 'cn', so instead of searching
cn=custodia for entries matching (memberPrincipal=host/$HOSTNAME), we
could get cn={enc,sig}/$HOSTNAME,cn=custodia directly.

They idea was to be flexible, in a future we might have some shared
keys, then the only way to properly resolve keys would be to use a
multivalued attribute.
Or we may want to have multiple keys sets for the same principal (it
doesn't have to be a host technically, it could be a service or even a
user in the future), and a naming based convention would make it harder
to deal with that.


On the patches
--
509:
- commit says only: "aci: add IPA servers host group 'ipaservers'"
but it does other things like changing how CA renewal certificate acis
are added, I think that must be separated in another patch.

- Why "Manage Host Keytab"  aci has been changed to exclude ipaservers ?

To allow only members of the admins group to manage keytabs of IPA servers. This is analogous to how only members of the admins group can change passwords of other admins.


- Why adding the host to the ipaservers group is done in the
krbinstance ? I would expect LDAP ops to be mostly done in the
DSinstance.

It is the same place where the host entry is created.


- I do not see where the host is added to the ipaservers group on
upgrade.

It's in install/updates/20-ipaservers_hostgroup.update:

add: member: fqdn=$FQDN,cn=computers,cn=accounts,$SUFFIX


510:
- We should probably tightenup the ACI to allos host X to only add
memberPrincipal = X and no other value, also the host should not be
allowed to change the memberPrincipal attribute only the keys.
If we can't express this in ACIs we can live with the ones you propose
though.

I think this can be done.

memberPrincipal is included in the ACI because KEMLdap.set_key() touches it. Perhaps it is not intentional?


511:
ack, but I think you should catch potential exceptions from the
os.rmdir, as it will throw if there is some other file in the dir (for
whatever reason). The exception can be safely ignored, we just do not
want to blow up with a traceback here.

OK.


512:
- I do not think this is correct, it seem to me you are overriding the
local ccache (if any) with the host keytab. If I read this right, this
means that if you run kinit admin && ipa-replica-install then you will
kind your admin credentials gone and a host TGT instead. Am I reading
this change correctly ?

No. :-) Temporary ccache is now used for the install and the host is kinited into it. The initial ccache is used only for connection check (and adding the host to ipaservers in patch 514). Your kinit admin will not be lost.


513:
Nack
- Should be folded into 510

These ACIs are required only for patch 514.

- Should not allow all hosts in the domain but only ipaservers for aal
three changes

If the host isn't member of ipaservers before ipa-replica-install is run, the "Check that we don't already have a replication agreement", "Detect if the other master can handle replication managers" and "Find if any server has a CA" steps will fail without these ACIs, which breaks patch 514.


514:
- Should be folded in 512, they are completely interdependent changes.

Patch 512 works fine without this patch, but the host has to be member of ipaservers before ipa-replica-install is run.

- I do not understand how this works. promote_check() runs before
promote() and will fail if the host is not already in the ipaservers
group, so you will never be able to actually add the host to the group ?
sounds like chicken-egg ... or what am I missing ?

The chicken-egg is solved by the ACIs from patch 513. They allow any host to perform the checks necessary for promote_check() to succeed, so the host can be added to ipaservers in promote().


Simo.



--
Jan Cholasta

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