Bug 92908 - Functionality to update the keytab
Summary: Functionality to update the keytab
Status: RESOLVED FIXED
Alias: None
Product: realmd
Classification: Unclassified
Component: adcli (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Stef Walter
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-11 20:25 UTC by Stef Walter
Modified: 2015-12-08 21:56 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
library: Refactor out some kerberos common code (7.08 KB, patch)
2015-11-11 20:30 UTC, Stef Walter
Details | Splinter Review
library: Add ability to do computer login with a keytab (4.68 KB, patch)
2015-11-11 20:30 UTC, Stef Walter
Details | Splinter Review
library: Add some string helpers (2.82 KB, patch)
2015-11-11 20:30 UTC, Stef Walter
Details | Splinter Review
Implement 'adcli update' (13.08 KB, patch)
2015-11-11 20:30 UTC, Stef Walter
Details | Splinter Review
library: Add ability to do computer login with a keytab (5.14 KB, patch)
2015-11-12 10:08 UTC, Stef Walter
Details | Splinter Review
Implement 'adcli update' (12.99 KB, patch)
2015-11-12 10:08 UTC, Stef Walter
Details | Splinter Review
Various fixes (4.89 KB, patch)
2015-11-19 16:04 UTC, Sumit Bose
Details | Splinter Review
Drop host/ prefix when deriving fqdn (916 bytes, patch)
2015-12-04 14:04 UTC, Sumit Bose
Details | Splinter Review
Allow domain option for updates (776 bytes, patch)
2015-12-04 14:04 UTC, Sumit Bose
Details | Splinter Review
Allow default keytab and additional ccache for updates (2.24 KB, patch)
2015-12-04 14:05 UTC, Sumit Bose
Details | Splinter Review
Check host password lifetime (5.85 KB, patch)
2015-12-04 14:05 UTC, Sumit Bose
Details | Splinter Review
Fix segfault in _adcli_krb5_open_keytab() if keytab is NULL (728 bytes, patch)
2015-12-04 14:06 UTC, Sumit Bose
Details | Splinter Review
doc: add update sub-command to man page (4.32 KB, patch)
2015-12-08 21:12 UTC, Sumit Bose
Details | Splinter Review

Description Stef Walter 2015-11-11 20:25:03 UTC
Implement functionality to update the keytab after a join, effectively renewing the keytab by choosing a new password.
Comment 1 Stef Walter 2015-11-11 20:30:26 UTC
Created attachment 119570 [details] [review]
library: Refactor out some kerberos common code

Add function for creating a new context, and opening a keytab
in the adkrb5.c file. These basically add minimal wrappers that
log failures properly, and handle default keytabs.
Comment 2 Stef Walter 2015-11-11 20:30:28 UTC
Created attachment 119571 [details] [review]
library: Add ability to do computer login with a keytab

This will be used by the 'adcli update' command.
Comment 3 Stef Walter 2015-11-11 20:30:30 UTC
Created attachment 119572 [details] [review]
library: Add some string helpers

These will be used by the logic to load settings from a keytab,
and used by 'adcli update' functionality.
Comment 4 Stef Walter 2015-11-11 20:30:32 UTC
Created attachment 119573 [details] [review]
Implement 'adcli update'

This is similar to an 'adcli join' but expects to use settings and
authentication from a keytab. It also expects to find a computer
account already present, rather than creating one.
Comment 5 Sumit Bose 2015-11-12 09:55:23 UTC
The patches can update the keytab entry but I found the following issues while testing.

Without arguments 'adcli update' seg-faults because keytab_name is NULL

(gdb) run
Starting program: /usr/sbin/adcli update
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
_adcli_krb5_open_keytab (k5=0x80020358, keytab_name=0x0, keytab=keytab@entry=0xbffff468) at adkrb5.c:187
187             if (keytab_name || strcmp (keytab_name, "") == 0) {



It looks adcli tries to lookup the DCs with the help of the domain name derived from the keytab entries. I think it would be useful to fall back to a lookup with the discovered realm (if it is different from the domain name) if the lookup with the domain name fails.
Comment 6 Stef Walter 2015-11-12 10:00:13 UTC
(In reply to Sumit Bose from comment #5)
> The patches can update the keytab entry but I found the following issues
> while testing.

Thanks for the testing and review.

> Without arguments 'adcli update' seg-faults because keytab_name is NULL
> 
> (gdb) run
> Starting program: /usr/sbin/adcli update
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/libthread_db.so.1".
> 
> Program received signal SIGSEGV, Segmentation fault.
> _adcli_krb5_open_keytab (k5=0x80020358, keytab_name=0x0,
> keytab=keytab@entry=0xbffff468) at adkrb5.c:187
> 187             if (keytab_name || strcmp (keytab_name, "") == 0) {

Good catch. That condition is backwards.

> It looks adcli tries to lookup the DCs with the help of the domain name
> derived from the keytab entries. I think it would be useful to fall back to
> a lookup with the discovered realm (if it is different from the domain name)
> if the lookup with the domain name fails.

Why would the realm be different from the domain on an AD server?

But one can specify --domain-controller=XXXXX in order to override this. I think that in the case that the keytab realm is somehow different from the domain or not resolveable, then we shouldn't make any guesses at that point.
Comment 7 Stef Walter 2015-11-12 10:08:20 UTC
Created attachment 119586 [details] [review]
library: Add ability to do computer login with a keytab

This will be used by the 'adcli update' command.
Comment 8 Stef Walter 2015-11-12 10:08:30 UTC
Created attachment 119587 [details] [review]
Implement 'adcli update'

This is similar to an 'adcli join' but expects to use settings and
authentication from a keytab. It also expects to find a computer
account already present, rather than creating one.
Comment 9 Sumit Bose 2015-11-12 12:25:38 UTC
(In reply to Stef Walter from comment #6)

> 
> Why would the realm be different from the domain on an AD server?

There can be multiple DNS domains in a AD domain/realm. If some of the DNS domains are not managed by the AD DNS service chances are that the SRV records for this domain are missing.

> 
> But one can specify --domain-controller=XXXXX in order to override this. I
> think that in the case that the keytab realm is somehow different from the
> domain or not resolveable, then we shouldn't make any guesses at that point.
Comment 10 Stef Walter 2015-11-12 12:37:10 UTC
> There can be multiple DNS domains in a AD domain/realm. If some of the DNS domains are not managed by the AD DNS service chances are that the SRV records for this domain are missing.

Okay, then I should add back the option to specify the domain explicitly on the command line. adcli has no other permanent store of information on the client side, other than the keytab, so any domain info would need to be passed in again in that scenario.
Comment 11 Sumit Bose 2015-11-13 15:03:38 UTC
Ok, make sense. 

While talking of command line options:

# adcli update --help
usage: adcli update

  -S, --domain-controller=<...>
                            domain controller to connect to
  -H, --host-fqdn=<...>     override the fully qualified domain name of the
                            local machine
  -K, --host-keytab=<...>   filename for the host kerberos keytab
  -N, --computer-name=<...> override the netbios short name of the local
                            machine
  -V, --service-name=<...>  additional service name for a kerberos
                            service principal to be created on the account
  --os-name=<...>           the computer operating system name
  --os-version=<...>        the computer operating system version
  --os-service-pack=<...>   the computer operating system service pack
  --user-principal=<...>    add an authentication principal to the account
  --show-details            show information about joining the domain after
                            a successful join
  --show-password           show computer account password after after a
                            successful join
  -v, --verbose             show verbose progress and failure messages


Is it expected that e.g. --service-name or --os-version work with 'update'? It would be very nice e.g. to add new service principals during the life-time of the computer account. But since in general the computer account is not allowed to modify its own LDAP object I think it would be good to add the --login-ccache options as well so that the attributes can be updated if suitable credentials are available.
Comment 12 Sumit Bose 2015-11-19 16:04:04 UTC
Created attachment 119944 [details] [review]
Various fixes

Hi,

I tried to fix some issues and add some new features around updates. My current state can be seen in the attached patch. It needs some more work, I just wanted to let you know that I'm working on it.

bye,
Sumit
Comment 13 Stef Walter 2015-11-19 16:18:40 UTC
Comment on attachment 119944 [details] [review]
Various fixes

Review of attachment 119944 [details] [review]:
-----------------------------------------------------------------

Awesome. These are great fixes. As requested, I'll hold off review. Splitting it into two patches (one with FIXUPS, and the other with the expiry logic) would make sense.
Comment 14 Sumit Bose 2015-12-04 14:04:22 UTC
Created attachment 120344 [details] [review]
Drop host/ prefix when deriving fqdn
Comment 15 Sumit Bose 2015-12-04 14:04:49 UTC
Created attachment 120345 [details] [review]
Allow domain option for updates
Comment 16 Sumit Bose 2015-12-04 14:05:14 UTC
Created attachment 120346 [details] [review]
Allow default keytab and additional ccache for updates
Comment 17 Sumit Bose 2015-12-04 14:05:48 UTC
Created attachment 120347 [details] [review]
Check host password lifetime
Comment 18 Sumit Bose 2015-12-04 14:06:22 UTC
Created attachment 120349 [details] [review]
Fix segfault in _adcli_krb5_open_keytab() if keytab is NULL
Comment 19 Sumit Bose 2015-12-04 14:07:06 UTC
Please find attached the split and improved patches.
Comment 20 Stef Walter 2015-12-07 07:55:35 UTC
Comment on attachment 120349 [details] [review]
Fix segfault in _adcli_krb5_open_keytab() if keytab is NULL

This patch is no longer necessary with the updated earlier patch. It also no longer applies.
Comment 21 Stef Walter 2015-12-07 08:02:30 UTC
Comment on attachment 120346 [details] [review]
Allow default keytab and additional ccache for updates

Review of attachment 120346 [details] [review]:
-----------------------------------------------------------------

Other than the krb5_kt_default_name() stuff, this patch looks good.

::: tools/computer.c
@@ +448,3 @@
>  	ktname = adcli_enroll_get_keytab_name (enroll);
> +	if (ktname == NULL) {
> +		kerr = krb5_kt_default_name (adcli_conn_get_krb5_context (conn),

I think _adcli_krb5_open_keytab() should handle this case. It calls krb5_kt_default(). There was a broken condition in an earlier version af a patch in this patch set, but now that has been fixed, and _adcli_krb5_open_keytab() should work correctly with regard to the default keytab. Therefore this code should not be necessary.
Comment 22 Stef Walter 2015-12-07 08:06:11 UTC
Comment on attachment 120344 [details] [review]
Drop host/ prefix when deriving fqdn

Review of attachment 120344 [details] [review]:
-----------------------------------------------------------------

Will merge after fixing whitespace around '+5'
Comment 23 Stef Walter 2015-12-07 09:01:28 UTC
Attachment 119570 [details] pushed as 60e9562 - library: Refactor out some kerberos common code
Attachment 119572 [details] pushed as e45e8b9 - library: Add some string helpers
Attachment 119586 [details] pushed as 9a5654f - library: Add ability to do computer login with a keytab
Attachment 119587 [details] pushed as 9086d3b - Implement 'adcli update'
Attachment 120344 [details] pushed as a518423 - Drop host/ prefix when deriving fqdn
Attachment 120345 [details] pushed as 4694aa7 - Allow domain option for updates

Great. Merged most of the patches, as noted, lets move the pwdLastSet code into a separate bugzilla bug and track that work there.
Comment 24 Stef Walter 2015-12-07 09:03:57 UTC
Filed additional bug: https://bugs.freedesktop.org/show_bug.cgi?id=93282
Comment 25 Sumit Bose 2015-12-08 21:12:23 UTC
Created attachment 120419 [details] [review]
doc: add update sub-command to man page

Please have a look at this additional patch. It adds a man page section for the new command.
Comment 26 Stef Walter 2015-12-08 21:56:53 UTC
Attachment 120419 [details] pushed as a96e9ef - doc: add update sub-command to man page

Looks good. Merged.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.