Refactor Wallet::Kadmin keytab_rekey to return keytab

Change the API for keytab_rekey to match keytab, returning the keytab
as data instead of writing it to a file.  This simplifies the wallet
object implementation and moves the logic for reading the temporary
file into Wallet::Kadmin and its child classes.  (Eventually, there may
be a kadmin backend that doesn't require using a temporary file.)

Setting KEYTAB_TMP is now required to instantiate either the ::MIT or
::Heimdal Wallet::Kadmin classes.

4 files changed, 95 insertions, 81 deletions

diff --git a/perl/Wallet/Kadmin.pm b/perl/Wallet/Kadmin.pm
index f3c2895..074dd1e 100644
--- a/perl/Wallet/Kadmin.pm
+++ b/perl/Wallet/Kadmin.pm
@@ -23,6 +23,33 @@ use Wallet::Config ();
 $VERSION = '0.03';
 
 ##############################################################################
+# Utility functions for child classes
+##############################################################################
+
+# Read the entirety of a possibly binary file and return the contents,
+# deleting the file after reading it.  If reading the file fails, set the
+# error message and return undef.
+sub read_keytab {
+    my ($self, $file) = @_;
+    local *TMPFILE;
+    unless (open (TMPFILE, '<', $file)) {
+        $self->error ("cannot open temporary file $file: $!");
+        return;
+    }
+    local $/;
+    undef $!;
+    my $data = <TMPFILE>;
+    if ($!) {
+        $self->error ("cannot read temporary file $file: $!");
+        unlink $file;
+        return;
+    }
+    close TMPFILE;
+    unlink $file;
+    return $data;
+}
+
+##############################################################################
 # Public methods
 ##############################################################################
 
@@ -84,9 +111,9 @@ Wallet::Kadmin - Kerberos administration API for wallet keytab backend
 
     my $kadmin = Wallet::Kadmin->new;
     $kadmin->create ('host/foo.example.com');
-    $kadmin->keytab_rekey ('host/foo.example.com', 'keytab',
-                           'aes256-cts-hmac-sha1-96');
-    my $data = $kadmin->keytab ('host/foo.example.com');
+    my $data = $kadmin->keytab_rekey ('host/foo.example.com',
+                                      'aes256-cts-hmac-sha1-96');
+    $data = $kadmin->keytab ('host/foo.example.com');
     my $exists = $kadmin->exists ('host/oldshell.example.com');
     $kadmin->destroy ('host/oldshell.example.com') if $exists;
 
@@ -101,9 +128,8 @@ interact with that implementation's kadmin interface.
 
 The class uses Wallet::Config to find which type of kadmin interface is in
 use and then returns an object to use for interacting with that interface.
-To use this object, several configuration parameters must be set.  See
-Wallet::Config(3) for details on those configuration parameters and
-information about how to set wallet configuration.
+See L<Wallet::Config/"KEYTAB OBJECT CONFIGURATION"> for details on how to
+configure this module.
 
 =head1 CLASS METHODS
 
@@ -174,7 +200,7 @@ Kerberos.  To create a keytab, the principal has to have previously been
 created in the Kerberos KDC.  Returns the keytab as binary data on success
 and undef on failure.
 
-=item keytab_rekey(PRINCIPAL, FILE [, ENCTYPE ...])
+=item keytab_rekey(PRINCIPAL [, ENCTYPE ...])
 
 Like keytab(), but randomizes the key for the principal before generating
 the keytab and writes it to the given file.  This will invalidate any
@@ -183,7 +209,19 @@ encryption types of the keys for that principal via the optional ENCTYPE
 arguments.  The enctype values must be enctype strings recognized by the
 Kerberos implementation (strings like C<aes256-cts-hmac-sha1-96> or
 C<des-cbc-crc>).  If none are given, the KDC defaults will be used.
-Returns true on success and false on failure.
+Returns the keytab as binary data on success and undef on failure.
+
+=back
+
+The following methods are utility methods to aid with child class
+implementation and should only be called by child classes.
+
+=over 4
+
+=item read_keytab(FILE)
+
+Reads the contents of the keytab stored in FILE into memory and returns it
+as binary data.  On failure, returns undef and sets the object error.
 
 =back
 
diff --git a/perl/Wallet/Kadmin/Heimdal.pm b/perl/Wallet/Kadmin/Heimdal.pm
index e066006..d1eecda 100644
--- a/perl/Wallet/Kadmin/Heimdal.pm
+++ b/perl/Wallet/Kadmin/Heimdal.pm
@@ -39,23 +39,6 @@ sub canonicalize_principal {
     return $principal;
 }
 
-# Read the entirety of a possibly binary file and return the contents.  If
-# reading the file fails, set the error message and return undef.
-sub slurp_file {
-    my ($self, $file) = @_;
-    unless (open (TMPFILE, '<', $file)) {
-        $self->error ("cannot open temporary file $file: $!");
-        return;
-    }
-    local $/;
-    my $data = <TMPFILE>;
-    unless (close TMPFILE) {
-        $self->error ("cannot read temporary file $file: $!");
-        return;
-    }
-    return $data;
-}
-
 ##############################################################################
 # Public interfaces
 ##############################################################################
@@ -132,17 +115,15 @@ sub keytab {
         $self->error ("error creating keytab for principal: $@");
         return;
     }
-    my $data = $self->slurp_file ($file);
-    unlink $file;
-    return $data;
+    return $self->read_keytab ($file);
 }
 
 # Create a keytab for a principal, randomizing the keys for that principal at
-# the same time.  Takes the principal, the file, and optionally a list of
-# encryption types to which to limit the keytab.  Return true if successful,
-# false otherwise.  If the keytab creation fails, sets the error.
+# the same time.  Takes the principal and an optional list of encryption types
+# to which to limit the keytab.  Return the keytab data on success and undef
+# on failure.  If the keytab creation fails, sets the error.
 sub keytab_rekey {
-    my ($self, $principal, $file, @enctypes) = @_;
+    my ($self, $principal, @enctypes) = @_;
     $principal = $self->canonicalize_principal ($principal);
 
     # The way Heimdal works, you can only remove enctypes from a principal,
@@ -188,12 +169,14 @@ sub keytab_rekey {
     }
 
     # Create the keytab.
+    my $file = $Wallet::Config::KEYTAB_TMP . "/keytab.$$";
+    unlink $file;
     eval { $kadmin->extractKeytab ($princdata, $file) };
     if ($@) {
         $self->error ("error creating keytab for principal: $@");
         return;
     }
-    return 1;
+    return $self->read_keytab ($file);
 }
 
 # Delete a principal from Kerberos.  Return true if successful, false
@@ -227,6 +210,9 @@ sub new {
             and defined ($Wallet::Config::KEYTAB_REALM)) {
         die "keytab object implementation not configured\n";
     }
+    unless (defined ($Wallet::Config::KEYTAB_TMP)) {
+        die "KEYTAB_TMP configuration variable not set\n";
+    }
     my @options = (RaiseError => 1,
                    Principal  => $Wallet::Config::KEYTAB_PRINCIPAL,
                    Realm      => $Wallet::Config::KEYTAB_REALM,
@@ -270,9 +256,8 @@ Wallet::Kadmin::Heimdal implements the Wallet::Kadmin API for Heimdal,
 providing an interface to create and delete principals and create keytabs.
 It provides the API documented in Wallet::Kadmin(3) for a Heimdal KDC.
 
-To use this object, several configuration parameters must be set.  See
-Wallet::Config(3) for details on those configuration parameters and
-information about how to set wallet configuration.
+To use this class, several configuration parameters must be set.  See
+L<Wallet::Config/"KEYTAB OBJECT CONFIGURATION"> for details.
 
 =head1 FILES
 
diff --git a/perl/Wallet/Kadmin/MIT.pm b/perl/Wallet/Kadmin/MIT.pm
index 1c6d2c1..434e93d 100644
--- a/perl/Wallet/Kadmin/MIT.pm
+++ b/perl/Wallet/Kadmin/MIT.pm
@@ -178,12 +178,11 @@ sub keytab {
 }
 
 # Create a keytab for a principal, randomizing the keys for that principal
-# in the process.  Takes the principal, the file, and optionally a list of
-# encryption types to which to limit the keytab.  Return true if
-# successful, false otherwise.  If the keytab creation fails, sets the
-# error.
+# in the process.  Takes the principal and an optional list of encryption
+# types to which to limit the keytab.  Return the keytab data on success
+# and undef otherwise.  If the keytab creation fails, sets the error.
 sub keytab_rekey {
-    my ($self, $principal, $file, @enctypes) = @_;
+    my ($self, $principal, @enctypes) = @_;
     unless ($self->valid_principal ($principal)) {
         $self->error ("invalid principal name: $principal");
         return;
@@ -191,6 +190,8 @@ sub keytab_rekey {
     if ($Wallet::Config::KEYTAB_REALM) {
         $principal .= '@' . $Wallet::Config::KEYTAB_REALM;
     }
+    my $file = $Wallet::Config::KEYTAB_TMP . "/keytab.$$";
+    unlink $file;
     my $command = "ktadd -q -k $file";
     if (@enctypes) {
         @enctypes = map { /:/ ? $_ : "$_:normal" } @enctypes;
@@ -203,7 +204,7 @@ sub keytab_rekey {
         $self->error ("error creating keytab for $principal: $1");
         return;
     }
-    return 1;
+    return $self->read_keytab ($file);
 }
 
 # Delete a principal from Kerberos.  Return true if successful, false
@@ -238,6 +239,9 @@ sub destroy {
 # kadmin directly.
 sub new {
     my ($class) = @_;
+    unless (defined ($Wallet::Config::KEYTAB_TMP)) {
+        die "KEYTAB_TMP configuration variable not set\n";
+    }
     my $self = {};
     bless ($self, $class);
     return $self;
@@ -261,9 +265,9 @@ Wallet::Kadmin::MIT - Wallet Kerberos administration API for MIT
 
     my $kadmin = Wallet::Kadmin::MIT->new;
     $kadmin->create ('host/foo.example.com');
-    $kadmin->keytab_rekey ('host/foo.example.com', 'keytab',
-                           'aes256-cts-hmac-sha1-96');
-    my $data = $kadmin->keytab ('host/foo.example.com');
+    my $data = $kadmin->keytab_rekey ('host/foo.example.com',
+                                      'aes256-cts-hmac-sha1-96');
+    $data = $kadmin->keytab ('host/foo.example.com');
     my $exists = $kadmin->exists ('host/oldshell.example.com');
     $kadmin->destroy ('host/oldshell.example.com') if $exists;
 
@@ -281,9 +285,20 @@ implemented using a remctl backend.  For that method (used for unchanging
 keytab objects) to work, the necessary wallet configuration and remctl
 interface on the KDC must be set up.
 
-To use this object, several configuration parameters must be set.  See
-Wallet::Config(3) for details on those configuration parameters and
-information about how to set wallet configuration.
+To use this class, several configuration parameters must be set.  See
+L<Wallet::Config/"KEYTAB OBJECT CONFIGURATION"> for details.
+
+=head1 FILES
+
+=over 4
+
+=item KEYTAB_TMP/keytab.<pid>
+
+The keytab is created in this file and then read into memory.  KEYTAB_TMP
+is set in the wallet configuration, and <pid> is the process ID of the
+current process.  The file is unlinked after being read.
+
+=back
 
 =head1 LIMITATIONS
 
diff --git a/perl/Wallet/Object/Keytab.pm b/perl/Wallet/Object/Keytab.pm
index 5c66967..edb26b3 100644
--- a/perl/Wallet/Object/Keytab.pm
+++ b/perl/Wallet/Object/Keytab.pm
@@ -323,43 +323,19 @@ sub get {
         return;
     }
     my $kadmin = $self->{kadmin};
+    my $result;
     if ($self->flag_check ('unchanging')) {
-        my $result = $kadmin->keytab ($self->{name});
-        if (defined $result) {
-            $self->log_action ('get', $user, $host, $time);
-        }
-        return $result;
-    }
-    unless (defined ($Wallet::Config::KEYTAB_TMP)) {
-        $self->error ('KEYTAB_TMP configuration variable not set');
-        return;
+        $result = $kadmin->keytab ($self->{name});
+    } else {
+        my @enctypes = $self->attr ('enctypes');
+        $result = $kadmin->keytab_rekey ($self->{name}, @enctypes);
     }
-    my $file = $Wallet::Config::KEYTAB_TMP . "/keytab.$$";
-    unlink $file;
-    my @enctypes = $self->attr ('enctypes');
-    if (not $kadmin->keytab_rekey ($self->{name}, $file, @enctypes)) {
+    if (defined $result) {
+        $self->log_action ('get', $user, $host, $time);
+    } else {
         $self->error ($kadmin->error);
-        return;
-    }
-    local *KEYTAB;
-    unless (open (KEYTAB, '<', $file)) {
-        my $princ = $self->{name};
-        $self->error ("error opening keytab for principal $princ: $!");
-        return;
-    }
-    local $/;
-    undef $!;
-    my $data = <KEYTAB>;
-    if ($!) {
-        my $princ = $self->{name};
-        $self->error ("error reading keytab for principal $princ: $!");
-        unlink $file;
-        return;
     }
-    close KEYTAB;
-    unlink $file;
-    $self->log_action ('get', $user, $host, $time);
-    return $data;
+    return $result;
 }
 
 1;