kauth: fix clock skew detection

Commit 5b3c1042969daec38ccb260e61d665eda0c713ea changed/removed some
uses of abs() on unsigned time values. While the previous use of abs()
was indeed incorrect, the result wasn't necessarily much better, even
though it built with recent compilers, since it only checked for skew
in one direction.

Define and use a  macro to correctly evaluate the conditionals in 64-bit
precision, avoiding C's integer promotion rules which prefer unsigned types
(Date) to signed types of the same width (time_t on 32-bit systems).

Reviewed-on: http://gerrit.openafs.org/11850
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 810f0ccd0354dac30af024ca7b5acf3ebabf5f4b)

Change-Id: I29337e1ecd410fcf7733408287930c50c055ff90
Reviewed-on: http://gerrit.openafs.org/11863
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

diff --git a/src/kauth/kaprocs.c b/src/kauth/kaprocs.c
index e055c71a25a6db4532704fc184e978ea7aa0a432..e2c058c6fbc3d54ab78c70e0c595e6645ff08524 100644 (file)
--- a/src/kauth/kaprocs.c
+++ b/src/kauth/kaprocs.c
@@ -711,7 +711,7 @@ ChangePassWord(struct rx_call *call, char *aname, char *ainstance,
     /* validate the request */
     request_time = ntohl(request.time);        /* reorder date */
     kvno = ntohl(request.kvno);
-    if (labs(request_time - time(NULL)) > KTC_TIME_UNCERTAINTY ||
+    if (check_ka_skew(request_time, time(NULL), KTC_TIME_UNCERTAINTY) ||
        strncmp(request.label, KA_CPW_REQ_LABEL, sizeof(request.label)) ||
        request.spare || kvno > MAXKAKVNO) {    /* these are reserved */
        code = KABADREQUEST;
@@ -1142,7 +1142,7 @@ Authenticate(int version, struct rx_call *call, char *aname, char *ainstance,
     }
 #endif /* EXPIREPW */
 
-    if (request.time - now > KTC_TIME_UNCERTAINTY) {
+    if (check_ka_skew(request.time, now, KTC_TIME_UNCERTAINTY)) {
 #if 0
        if (oanswer->MaxSeqLen < sizeof(afs_int32))
            code = KAANSWERTOOLONG;

diff --git a/src/kauth/kauth_internal.h b/src/kauth/kauth_internal.h
index 02c91e9738cda49b39fae38db56dcee4719708d1..150334e93697c5f0581459a827217a69e42839e0 100644 (file)
--- a/src/kauth/kauth_internal.h
+++ b/src/kauth/kauth_internal.h
@@ -69,4 +69,30 @@ ktc_to_EncryptionKey(struct ktc_encryptionKey *key) {
     return (EncryptionKey *)key;
 }
 
+/*
+ * Some of the RPCs need to verify that two times are within a given
+ * skew window (usually KTC_TIME_UNCERTAINTY, 15 minutes).  However,
+ * there are multiple sources of timestamps.  The "AFS-native" type,
+ * Date, is afs_uint32; timestamps fetched from the system will
+ * generally be the output of time(NULL), i.e., time_t.  However, the
+ * base type of time_t is platform-dependent -- on some systems, it
+ * is int32_t, and on others it is int64_t.  Arithmetic operations
+ * and comparisons between integers of different type are subject to
+ * the usual arithmetic promotions in C -- comparing a uint32_t and
+ * an int32_t results in the int32_t being "promoted" to uint32_t, which
+ * has disasterous consequences when the value being promoted is
+ * negative.  If, on the other hand, time_t is int64_t, then the promotions
+ * work the other way, with everything evaluated at int64_t precision,
+ * since int64_t has a higher conversion rank than int32_t (which has
+ * the same conversion rank as uint32_t).  In order to properly and
+ * portably check the time skew, it is simplest to cast everything to
+ * afs_int64 before evaluating any expressions.
+ *
+ * The expression evaluates to true if the absolute value of the difference
+ * between the two time inputs is larger than the skew.
+ */
+#define check_ka_skew(__t1, __t2, __skew) \
+       ((afs_int64)(__t1) - (afs_int64)(__skew) > (afs_int64)(__t2) || \
+       (afs_int64)(__t2) - (afs_int64)(__skew) > (afs_int64)(__t1))
+
 #endif

diff --git a/src/kauth/krb_udp.c b/src/kauth/krb_udp.c
index c988370505730db7accfa6c1565f80623faa33e3..d9ecd3c5ad60e4c51df9b7e8eb01528926355fc3 100644 (file)
--- a/src/kauth/krb_udp.c
+++ b/src/kauth/krb_udp.c
@@ -297,7 +297,7 @@ UDP_Authenticate(int ksoc, struct sockaddr_in *client, char *name,
            code = KERB_ERR_NAME_EXP;   /* XXX Could use another error code XXX */
            goto abort;
        }
-       if (startTime - now > KTC_TIME_UNCERTAINTY) {
+       if (check_ka_skew(startTime, now, KTC_TIME_UNCERTAINTY)) {
            code = KERB_ERR_SERVICE_EXP;        /* was KABADREQUEST */
            goto abort;
        }