auth: Force correct evenness on rxkad tokens

Rxkad tokens historically have forced odd lifetimes when the given
viceid is actually an AFS ID, and even lifetimes when it is not. Force
this when the new token-handling functions are used (so the viceid is
correctly interpreted by users of the old token format), by creating
rxkad tokens with token_importRxkadViceId.

Slightly reworked by Simon Wilkinson to provide a generic token
destructor function.

Change-Id: I9f6aa518b8ae51a3772b69a0722a28bff6b47128
Reviewed-on: http://gerrit.openafs.org/4481
Tested-by: Simon Wilkinson <sxw@inf.ed.ac.uk>
Reviewed-by: Derrick Brashear <shadow@dementix.org>

diff --git a/src/aklog/aklog.c b/src/aklog/aklog.c
index 4a0e14e..efcdf5d 100644 (file)
--- a/src/aklog/aklog.c
+++ b/src/aklog/aklog.c
@@ -657,13 +657,12 @@ static int
 rxkad_build_native_token(krb5_context context, krb5_creds *v5cred,
                         struct ktc_tokenUnion **tokenPtr, char **userPtr) {
     char username[BUFSIZ];
-    struct ktc_tokenUnion *token;
-    struct token_rxkad *rxkadToken;
+    struct ktc_token token;
+    int status;
 #ifdef HAVE_NO_KRB5_524
     char *p;
     int len;
 #else
-    int status;
     char k4name[ANAME_SZ];
     char k4inst[INST_SZ];
     char k4realm[REALM_SZ];
@@ -706,30 +705,21 @@ rxkad_build_native_token(krb5_context context, krb5_creds *v5cred,
     }
 #endif
 
-    token = malloc(sizeof(struct ktc_tokenUnion));
-    if (token == NULL)
-       return ENOMEM;
-
-    memset(token, 0, sizeof(struct ktc_tokenUnion));
-
-    token->at_type = AFSTOKEN_UNION_KAD;
-    rxkadToken = &token->ktc_tokenUnion_u.at_kad;
+    memset(&token, 0, sizeof(struct ktc_token));
 
-    rxkadToken->rk_kvno = RXKAD_TKT_TYPE_KERBEROS_V5;
-    rxkadToken->rk_begintime = v5cred->times.starttime;;
-    rxkadToken->rk_endtime = v5cred->times.endtime;
-    memcpy(&rxkadToken->rk_key, get_cred_keydata(v5cred),
+    token.kvno = RXKAD_TKT_TYPE_KERBEROS_V5;
+    token.startTime = v5cred->times.starttime;;
+    token.endTime = v5cred->times.endtime;
+    memcpy(&token.sessionKey, get_cred_keydata(v5cred),
           get_cred_keylen(v5cred));
-    rxkadToken->rk_ticket.rk_ticket_len = v5cred->ticket.length;
-    rxkadToken->rk_ticket.rk_ticket_val = malloc(v5cred->ticket.length);
-    if (rxkadToken->rk_ticket.rk_ticket_val == NULL) {
-       free(token);
-       return ENOMEM;
+    token.ticketLen = v5cred->ticket.length;
+    memcpy(token.ticket, v5cred->ticket.data, token.ticketLen);
+
+    status = token_importRxkadViceId(tokenPtr, &token, 0);
+    if (status) {
+       return status;
     }
-    memcpy(rxkadToken->rk_ticket.rk_ticket_val, v5cred->ticket.data,
-          rxkadToken->rk_ticket.rk_ticket_len);
 
-    *tokenPtr = token;
     *userPtr = strdup(username);
 
     return 0;
@@ -771,8 +761,7 @@ rxkad_get_converted_token(krb5_context context, krb5_creds *v5cred,
                          struct ktc_tokenUnion **tokenPtr, char **userPtr) {
     CREDENTIALS cred;
     char username[BUFSIZ];
-    struct ktc_tokenUnion *token;
-    struct token_rxkad *rxkadToken;
+    struct ktc_token token;
     int status;
 
     *tokenPtr = NULL;
@@ -794,16 +783,10 @@ rxkad_get_converted_token(krb5_context context, krb5_creds *v5cred,
        strcat (username, cred.pinst);
     }
 
-    token = malloc(sizeof(struct ktc_tokenUnion));
-    if (token == NULL)
-       return ENOMEM;
-    memset(token, 0, sizeof(struct ktc_tokenUnion));
-
-    token->at_type = AFSTOKEN_UNION_KAD;
+    memset(&token, 0, sizeof(struct ktc_token));
 
-    rxkadToken = &token->ktc_tokenUnion_u.at_kad;
-    rxkadToken->rk_kvno = cred.kvno;
-    rxkadToken->rk_begintime = cred.issue_date;
+    token.kvno = cred.kvno;
+    token.startTime = cred.issue_date;
     /*
      * It seems silly to go through a bunch of contortions to
      * extract the expiration time, when the v5 credentials already
@@ -812,18 +795,16 @@ rxkad_get_converted_token(krb5_context context, krb5_creds *v5cred,
      * Note that this isn't a security hole, as the expiration time
      * is also contained in the encrypted token
      */
-    rxkadToken->rk_endtime = v5cred->times.endtime;
-    memcpy(&rxkadToken->rk_key, cred.session, 8);
-    rxkadToken->rk_ticket.rk_ticket_len = cred.ticket_st.length;
-    rxkadToken->rk_ticket.rk_ticket_val = malloc(cred.ticket_st.length);
-    if (rxkadToken->rk_ticket.rk_ticket_val == NULL) {
-       free(token);
-       return ENOMEM;
+    token.endTime = v5cred->times.endtime;
+    memcpy(&token.sessionKey, cred.session, 8);
+    token.ticketLen = cred.ticket_st.length;
+    memcpy(token.ticket, cred.ticket_st.dat, token.ticketLen);
+
+    status = token_importRxkadViceId(tokenPtr, &token, 0);
+    if (status) {
+       return status;
     }
-    memcpy(rxkadToken->rk_ticket.rk_ticket_val, cred.ticket_st.dat,
-          rxkadToken->rk_ticket.rk_ticket_len);
 
-    *tokenPtr = token;
     *userPtr = strdup(username);
 
     return 0;
@@ -1070,8 +1051,13 @@ auth_to_cell(krb5_context context, const char *config,
 #endif /* ALLOW_REGISTER */
 
            if ((status == 0) && (viceId != ANONYMOUSID)) {
-               rxkadToken->ktc_tokenUnion_u.at_kad.rk_viceid = viceId;
-               token_replaceToken(token, rxkadToken);
+               status = token_setRxkadViceId(rxkadToken, viceId);
+               if (status) {
+                   fprintf(stderr, "Error %d setting rxkad ViceId\n", status);
+                   status = AKLOG_SUCCESS;
+               } else {
+                   token_replaceToken(token, rxkadToken);
+               }
            }
        }
 
@@ -1098,8 +1084,7 @@ auth_to_cell(krb5_context context, const char *config,
 
 out:
     if (rxkadToken) {
-       free(rxkadToken->ktc_tokenUnion_u.at_kad.rk_ticket.rk_ticket_val);
-       free(rxkadToken);
+       token_freeToken(&rxkadToken);
     }
 
     if (local_cell)

diff --git a/src/auth/ktc.h b/src/auth/ktc.h
index 3507639..167a8ff 100644 (file)
--- a/src/auth/ktc.h
+++ b/src/auth/ktc.h
@@ -27,9 +27,15 @@ extern int token_SetsEquivalent(struct ktc_setTokenData *,
                                struct ktc_setTokenData *);
 extern void token_setPag(struct ktc_setTokenData *, int);
 extern void token_FreeSet(struct ktc_setTokenData **);
+extern void token_freeToken(struct ktc_tokenUnion **);
+extern void token_freeTokenContents(struct ktc_tokenUnion *);
 
 struct ktc_token;
 struct ktc_principal;
 extern int token_extractRxkad(struct ktc_setTokenData *, struct ktc_token *,
                              int *, struct ktc_principal *);
+extern int token_importRxkadViceId(struct ktc_tokenUnion **,
+                                   struct ktc_token *,
+                                   afs_int32);
+extern int token_setRxkadViceId(struct ktc_tokenUnion *, afs_int32);
 #endif /* AFS_SRC_AUTH_KTC_H */

diff --git a/src/auth/token.c b/src/auth/token.c
index bf0d351..0f67c39 100644 (file)
--- a/src/auth/token.c
+++ b/src/auth/token.c
@@ -75,11 +75,6 @@ decodeToken(struct token_opaque *opaque, struct ktc_tokenUnion *token) {
     return code;
 }
 
-static void
-freeToken(struct ktc_tokenUnion *token) {
-    xdr_free((xdrproc_t)xdr_ktc_tokenUnion, token);
-}
-
 static int
 rxkadTokenEqual(struct ktc_tokenUnion *tokenA, struct ktc_tokenUnion *tokenB) {
     return (tokenA->ktc_tokenUnion_u.at_kad.rk_kvno ==
@@ -224,6 +219,99 @@ token_findByType(struct ktc_setTokenData *token,
     return 0;
 }
 
+static void
+SetRxkadViceId(struct token_rxkad *rxkadToken, afs_int32 viceId)
+{
+    rxkadToken->rk_viceid = viceId;
+    if (viceId) {
+       if (((rxkadToken->rk_endtime - rxkadToken->rk_begintime) & 1) == 0) {
+           rxkadToken->rk_begintime++; /* force lifetime to be odd */
+       }
+    } else {
+       if (((rxkadToken->rk_endtime - rxkadToken->rk_begintime) & 1) == 1) {
+           rxkadToken->rk_begintime++; /* force lifetime to be even */
+       }
+    }
+}
+
+/**
+ * Import an rxkad token with a ViceId into a unified token.
+ *
+ * @param[out] atoken
+ *           The resultant unified token. Free with token_freeToken.
+ * @param[in] oldToken
+ *          The rxkad token to import.
+ * @param[in] viceId
+ *          The optional rxkad ViceId to use. Specify 0 to explicitly not
+ *          specify a ViceId.
+ *
+ * @return operation status
+ *  @retval 0 success
+ */
+int
+token_importRxkadViceId(struct ktc_tokenUnion **atoken,
+                       struct ktc_token *oldToken,
+                       afs_int32 viceId)
+{
+    struct ktc_tokenUnion *token;
+    struct token_rxkad *rxkadToken;
+
+    token = malloc(sizeof(struct ktc_tokenUnion));
+    if (!token)
+       return ENOMEM;
+
+    token->at_type = AFSTOKEN_UNION_KAD;
+    rxkadToken = &token->ktc_tokenUnion_u.at_kad;
+
+    rxkadToken->rk_kvno = oldToken->kvno;
+    rxkadToken->rk_begintime = oldToken->startTime;
+    rxkadToken->rk_endtime = oldToken->endTime;
+    memcpy(&rxkadToken->rk_key, &oldToken->sessionKey,
+           sizeof(oldToken->sessionKey));
+    rxkadToken->rk_ticket.rk_ticket_len = oldToken->ticketLen;
+
+    rxkadToken->rk_ticket.rk_ticket_val = xdr_alloc(oldToken->ticketLen);
+    if (!rxkadToken->rk_ticket.rk_ticket_val) {
+       free(token);
+       return ENOMEM;
+    }
+    memcpy(rxkadToken->rk_ticket.rk_ticket_val, oldToken->ticket, oldToken->ticketLen);
+
+    SetRxkadViceId(rxkadToken, viceId);
+
+    *atoken = token;
+
+    return 0;
+}
+
+/**
+ * Set the optional ViceId for an rxkad token.
+ *
+ * @param[in] token
+ *          The token union to change.
+ * @param[in] viceId
+ *          The ViceId to set. Specify 0 to explicitly set no ViceId.
+ *
+ * @return operation status
+ *  @retval EINVAL  The given token union is not an rxkad token
+ *  @retval 0  success
+ */
+int
+token_setRxkadViceId(struct ktc_tokenUnion *token,
+                     afs_int32 viceId)
+{
+    struct token_rxkad *rxkadToken;
+
+    if (token->at_type != AFSTOKEN_UNION_KAD) {
+       return EINVAL;
+    }
+
+    rxkadToken = &token->ktc_tokenUnion_u.at_kad;
+    SetRxkadViceId(rxkadToken, viceId);
+
+    return 0;
+}
+
 /*!
  * Given an unified token, populate an rxkad token from it
  *
@@ -411,11 +499,11 @@ token_SetsEquivalent(struct ktc_setTokenData *tokenSetA,
                    found = 1;
                    break;
                }
-               freeToken(&tokenB);
+               token_freeTokenContents(&tokenB);
            }
        }
        if (decodedOK)
-           freeToken(&tokenA);
+           token_freeTokenContents(&tokenA);
 
        if (!found)
            return 0;
@@ -434,6 +522,22 @@ token_setPag(struct ktc_setTokenData *jar, int setpag) {
 }
 
 void
+token_freeTokenContents(struct ktc_tokenUnion *atoken)
+{
+    xdr_free((xdrproc_t)xdr_ktc_tokenUnion, atoken);
+}
+    
+void
+token_freeToken(struct ktc_tokenUnion **atoken)
+{
+    if (*atoken) {
+       token_freeTokenContents(*atoken);
+       free(*atoken);
+        *atoken = NULL;
+    }
+}
+
+void
 token_FreeSet(struct ktc_setTokenData **jar) {
     if (*jar) {
        xdr_free((xdrproc_t)xdr_ktc_setTokenData, *jar);

diff --git a/src/tsm41/aix_aklog.c b/src/tsm41/aix_aklog.c
index 079b309..958765f 100644 (file)
--- a/src/tsm41/aix_aklog.c
+++ b/src/tsm41/aix_aklog.c
@@ -690,7 +690,7 @@ auth_to_cell(krb5_context context, char *user, char *cell, char *realm)
 
  done:
     if (rxkadToken) {
-       token_freeImportedToken(&rxkadToken);
+       token_freeToken(&rxkadToken);
     }
     token_FreeSet(&token);
     return(status);