[Freeipa-devel] [PATCH] #3859: Better mechanism to retrieve keytabs

[Rob Crittenden]

Simo Sorce wrote:
> This patch set is an initial implementation of ticket #3859
> 
> It seem to be working fine in my initial testing but I have not yet
> tested all cases.
> 
> However I wonted to throw it on the list to get some initial feedback
> about the choices I made wrt access control and ipa-getkeytab flags and
> default behavior.
> 
> In particular, the current patch set would require us to make
> host/service keytabs readable by the requesting party (whoever that is,
> admin or host itself) in order to allow it to get back the actual
> keytab. I am not sure this is ideal. Also write access to the keytab is
> still all is needed to allow someone to change it.
> 
> Neither is ideal, but it was simpler as a first implementation. In
> particular I think we should allow either permission indipendently, and
> it should be something an admin can change. However I do not like
> allowing normal writes or reads to these attributes, mostly because w/o
> access to the master key nobody can really make sense of actually
> reading out the contents of KrbPrincipalKey or could write a blob that
> can be successfully decrypted.
> 
> So I was wondering if we might want to prevent both reading and writing
> via LDAP (except via extended operations) and instead use another method
> to determine access patterns.
> 
> As for ipa-getkeytab is everyone ok with tryin the new method first and
> always falling back to the old one (if a password has been provided) ?

0001

Needs a rebase. I did my best for testing.

In check_service_name() why not just initialize name to NULL rather than
assigning it twice? Or is the value of name undefined if krb5_* fails?

get_realm_backend() should probably have a comment on why returning NULL
is ok (either because there is no sdn or because there is no be). It
appears that things will eventually fail in get_entry_by_principal()

I think the Access Strategy comment needs to be expanded (or dropped) in
is_allowed_to_access_keys()

In the fatal error in is_allowed_to_access_keys() should the error
include the bound user and the resource they attempted to modify?

0002

ACK

0003

Nit: typo in commit, extedned

I'd prefer unique error messages for each ber decode failure.

Why is write access always required in ipapwd_getkeytab()? Would there
be a case where a principal can only re-fetch existing keys?

Since getnew is defined as a boolean in the ASN.1, is the conditional

if (getnew == 0)

preferred over just

if (getnew)?

Some errors read "Out of Memory" and others "allocation failed". I think
with a fatal error the line number is included in the error. If I'm
wrong then perhaps something specific to the allocation should be included.

The error message to the call ber_decode_krb5_key_data() doesn't seem to
match.

0004

More duplicated error messages, e.g. Missing reply control

0005

More duplicated error messages, Failed to parse key data

Typo, Incopatible

Functionality-wise.

Retrieving a keytab from a service without one fails in a strange way:

# ipa-getkeytab -r -s `hostname` -p test1/`hostname` -k /tmp/test.kt
Operation failed! Insufficient access rights

Failed to get keytab

I suppose a new permission is needed to add the ability to re-fetch
keytabs. Should the admins group be able to do this out-of-the box?

Maybe this is ok, I don't know, but it looks wierd. I fetched with -r
the same keytab a number of times and this is what it contains:

# klist -kt /tmp/test.kt
Keytab name: FILE:/tmp/test.kt
KVNO Timestamp           Principal
---- -------------------
------------------------------------------------------
   1 06/09/2014 13:35:50 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:35:50 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:35:50 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:35:50 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:35:53 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:35:53 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:35:53 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:35:53 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:00 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:00 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:00 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:00 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:45 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:45 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:45 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:45 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:51 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:51 test1/grindle.example.com at EXAMPLE.COM
   1 06/09/2014 13:36:51 test1/grindle.example.com at EXAMPLE.COM

The krbPrincipalKey value remained constant.

rob