Sequence numbers are now unsigned. Implement lenient parser for

sequence numbers which folds received negative sequence numbers into
positive unsigned numbers.  Constrain the space of initial sequence
numbers to facilitate backwards compatibility.

ticket: 1262
status: open

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@15457 dc483132-0cff-0310-8789-dd5450dbe970

10 files changed, 115 insertions, 11 deletions

diff --git a/src/lib/krb5/asn.1/ChangeLog b/src/lib/krb5/asn.1/ChangeLog
index c4bd9bc08..dd85a3c7a 100644
--- a/src/lib/krb5/asn.1/ChangeLog
+++ b/src/lib/krb5/asn.1/ChangeLog
@@ -1,3 +1,25 @@
+2003-05-18  Tom Yu  <tlyu@mit.edu>
+
+	* asn1_decode.c (asn1_decode_maybe_unsigned): New function; decode
+	negative 32-bit numbers into positive unsigned numbers for the
+	sake of backwards compatibility with old code.
+
+	* asn1_decode.h: Add prototype for asn1_decode_maybe_unsigned.
+
+	* asn1_k_decode.c (asn1_decode_seqnum): New function; wrapper
+	around asn1_decode_maybe_unsigned.
+
+	* asn1_k_decode.h: Add prototype for asn1_decode_seqnum.
+
+	* krb5_decode.c (decode_krb5_authenticator) 
+	(decode_krb5_ap_rep_enc_part, decode_krb5_enc_priv_part): Sequence
+	numbers are now unsigned.  Use asn1_decode_seqnum to handle
+	backwards compat with negative sequence numbers.
+
+	* krb5_encode.c (encode_krb5_authenticator) 
+	(encode_krb5_ap_rep_enc_part, encode_krb5_enc_priv_part): Sequence
+	numbers are now unsigned.
+
 2003-05-06  Sam Hartman  <hartmans@mit.edu>
 
 	* krb5_decode.c (decode_krb5_etype_info2): New function; currently
diff --git a/src/lib/krb5/asn.1/asn1_decode.c b/src/lib/krb5/asn.1/asn1_decode.c
index 56904c5ee..65863202a 100644
--- a/src/lib/krb5/asn.1/asn1_decode.c
+++ b/src/lib/krb5/asn.1/asn1_decode.c
@@ -1,7 +1,7 @@
 /*
  * src/lib/krb5/asn.1/asn1_decode.c
  * 
- * Copyright 1994 by the Massachusetts Institute of Technology.
+ * Copyright 1994, 2003 by the Massachusetts Institute of Technology.
  * All Rights Reserved.
  *
  * Export of this software from the United States of America may
@@ -106,6 +106,50 @@ asn1_error_code asn1_decode_unsigned_integer(asn1buf *buf, long unsigned int *va
   cleanup();
 }
 
+/*
+ * asn1_decode_maybe_unsigned
+ *
+ * This is needed because older releases of MIT krb5 have signed
+ * sequence numbers.  We want to accept both signed and unsigned
+ * sequence numbers, in the range -2^31..2^32-1, mapping negative
+ * numbers into their positive equivalents in the same way that C's
+ * normal integer conversions do, i.e., would preserve bits on a
+ * two's-complement architecture.
+ */
+asn1_error_code asn1_decode_maybe_unsigned(asn1buf *buf, unsigned long *val)
+{
+  setup();
+  asn1_octet o;
+  unsigned long n, bitsremain;
+  unsigned int i;
+
+  tag(ASN1_INTEGER);
+  o = 0;
+  n = 0;
+  bitsremain = ~0UL;
+  for (i = 0; i < length; i++) {
+    /* Accounts for u_long width not being a multiple of 8. */
+    if (bitsremain < 0xff) return ASN1_OVERFLOW;
+    retval = asn1buf_remove_octet(buf, &o);
+    if (retval) return retval;
+    if (bitsremain == ~0UL) {
+      if (i == 0)
+	n = (o & 0x80) ? ~0UL : 0UL; /* grab sign bit */
+      /*
+       * Skip leading zero or 0xFF octets to humor non-compliant encoders.
+       */
+      if (n == 0 && o == 0)
+	continue;
+      if (n == ~0UL && o == 0xff)
+	continue;
+    }
+    n = (n << 8) | o;
+    bitsremain >>= 8;
+  }
+  *val = n;
+  cleanup();
+}
+
 asn1_error_code asn1_decode_oid(asn1buf *buf, unsigned int *retlen, asn1_octet **val)
 {
   setup();
diff --git a/src/lib/krb5/asn.1/asn1_decode.h b/src/lib/krb5/asn.1/asn1_decode.h
index 449a58900..cafbf3fd3 100644
--- a/src/lib/krb5/asn.1/asn1_decode.h
+++ b/src/lib/krb5/asn.1/asn1_decode.h
@@ -62,6 +62,8 @@ asn1_error_code asn1_decode_integer
 	(asn1buf *buf, long *val);
 asn1_error_code asn1_decode_unsigned_integer
 	(asn1buf *buf, unsigned long *val);
+asn1_error_code asn1_decode_maybe_unsigned
+	(asn1buf *buf, unsigned long *val);
 asn1_error_code asn1_decode_null
 	(asn1buf *buf);
 
diff --git a/src/lib/krb5/asn.1/asn1_k_decode.c b/src/lib/krb5/asn.1/asn1_k_decode.c
index e56832a7b..f00cb0161 100644
--- a/src/lib/krb5/asn.1/asn1_k_decode.c
+++ b/src/lib/krb5/asn.1/asn1_k_decode.c
@@ -320,6 +320,17 @@ integer_convert(asn1_decode_authdatatype,krb5_authdatatype)
 unsigned_integer_convert(asn1_decode_ui_2,krb5_ui_2)
 unsigned_integer_convert(asn1_decode_ui_4,krb5_ui_4)
 
+asn1_error_code asn1_decode_seqnum(asn1buf *buf, krb5_ui_4 *val)
+{
+  asn1_error_code retval;
+  unsigned long n;
+
+  retval = asn1_decode_maybe_unsigned(buf, &n);
+  if (retval) return retval;
+  *val = (krb5_ui_4)n & 0xffffffff;
+  return 0;
+}
+
 asn1_error_code asn1_decode_msgtype(asn1buf *buf, krb5_msgtype *val)
 {
   asn1_error_code retval;
diff --git a/src/lib/krb5/asn.1/asn1_k_decode.h b/src/lib/krb5/asn.1/asn1_k_decode.h
index 8f8b0bcff..e2639dda3 100644
--- a/src/lib/krb5/asn.1/asn1_k_decode.h
+++ b/src/lib/krb5/asn.1/asn1_k_decode.h
@@ -89,6 +89,8 @@ asn1_error_code asn1_decode_ui_2
 	(asn1buf *buf, krb5_ui_2 *val);
 asn1_error_code asn1_decode_ui_4
 	(asn1buf *buf, krb5_ui_4 *val);
+asn1_error_code asn1_decode_seqnum
+	(asn1buf *buf, krb5_ui_4 *val);
 asn1_error_code asn1_decode_kerberos_time
 	(asn1buf *buf, krb5_timestamp *val);
 asn1_error_code asn1_decode_sam_flags
diff --git a/src/lib/krb5/asn.1/krb5_decode.c b/src/lib/krb5/asn.1/krb5_decode.c
index b39bae5c4..1ca57e930 100644
--- a/src/lib/krb5/asn.1/krb5_decode.c
+++ b/src/lib/krb5/asn.1/krb5_decode.c
@@ -219,7 +219,7 @@ krb5_error_code decode_krb5_authenticator(const krb5_data *code, krb5_authentica
     get_field((*rep)->ctime,5,asn1_decode_kerberos_time);
     if(tagnum == 6){ alloc_field((*rep)->subkey,krb5_keyblock); }
     opt_field(*((*rep)->subkey),6,asn1_decode_encryption_key);
-    opt_field((*rep)->seq_number,7,asn1_decode_int32);
+    opt_field((*rep)->seq_number,7,asn1_decode_seqnum);
     opt_field((*rep)->authorization_data,8,asn1_decode_authorization_data);
     (*rep)->magic = KV5M_AUTHENTICATOR;
     end_structure();
@@ -440,7 +440,7 @@ krb5_error_code decode_krb5_ap_rep_enc_part(const krb5_data *code, krb5_ap_rep_e
     get_field((*rep)->cusec,1,asn1_decode_int32);
     if(tagnum == 2){ alloc_field((*rep)->subkey,krb5_keyblock); }
     opt_field(*((*rep)->subkey),2,asn1_decode_encryption_key);
-    opt_field((*rep)->seq_number,3,asn1_decode_int32);
+    opt_field((*rep)->seq_number,3,asn1_decode_seqnum);
     end_structure();
     (*rep)->magic = KV5M_AP_REP_ENC_PART;
   }
@@ -561,7 +561,7 @@ krb5_error_code decode_krb5_enc_priv_part(const krb5_data *code, krb5_priv_enc_p
     get_lenfield((*rep)->user_data.length,(*rep)->user_data.data,0,asn1_decode_charstring);
     opt_field((*rep)->timestamp,1,asn1_decode_kerberos_time);
     opt_field((*rep)->usec,2,asn1_decode_int32);
-    opt_field((*rep)->seq_number,3,asn1_decode_int32);
+    opt_field((*rep)->seq_number,3,asn1_decode_seqnum);
     alloc_field((*rep)->s_address,krb5_address);
     get_field(*((*rep)->s_address),4,asn1_decode_host_address);
     if(tagnum == 5){ alloc_field((*rep)->r_address,krb5_address); }
diff --git a/src/lib/krb5/asn.1/krb5_encode.c b/src/lib/krb5/asn.1/krb5_encode.c
index 7412209f6..eed6faf45 100644
--- a/src/lib/krb5/asn.1/krb5_encode.c
+++ b/src/lib/krb5/asn.1/krb5_encode.c
@@ -166,7 +166,7 @@ krb5_error_code encode_krb5_authenticator(const krb5_authenticator *rep, krb5_da
 
   /* seq-number[7]		INTEGER OPTIONAL */
   if(rep->seq_number != 0)
-    krb5_addfield(rep->seq_number,7,asn1_encode_integer);
+    krb5_addfield(rep->seq_number,7,asn1_encode_unsigned_integer);
 
   /* subkey[6]			EncryptionKey OPTIONAL */
   if(rep->subkey != NULL)
@@ -305,6 +305,7 @@ krb5_error_code encode_krb5_enc_kdc_rep_part(const krb5_enc_kdc_rep_part *rep, k
 #ifdef KRB5_ENCKRB5KDCREPPART_COMPAT
   krb5_apptag(26);
 #else
+  /* XXX WRONG!!! Should use 25 || 26, not the outer KDC_REP tags! */
   if (rep->msg_type == KRB5_AS_REP) { krb5_apptag(ASN1_KRB_AS_REP); }
   else if (rep->msg_type == KRB5_TGS_REP) { krb5_apptag(ASN1_KRB_TGS_REP); }
   else return KRB5_BADMSGTYPE;
@@ -395,7 +396,7 @@ krb5_error_code encode_krb5_ap_rep_enc_part(const krb5_ap_rep_enc_part *rep, krb
 
   /* seq-number[3]	INTEGER OPTIONAL */
   if(rep->seq_number)
-    krb5_addfield(rep->seq_number,3,asn1_encode_integer);
+    krb5_addfield(rep->seq_number,3,asn1_encode_unsigned_integer);
 
   /* subkey[2]		EncryptionKey OPTIONAL */
   if(rep->subkey != NULL)
@@ -510,7 +511,7 @@ krb5_error_code encode_krb5_enc_priv_part(const krb5_priv_enc_part *rep, krb5_da
 
   /* seq-number[3]	INTEGER OPTIONAL */
   if(rep->seq_number)
-    krb5_addfield(rep->seq_number,3,asn1_encode_integer);
+    krb5_addfield(rep->seq_number,3,asn1_encode_unsigned_integer);
 
   /* usec[2]		INTEGER OPTIONAL */
   if(rep->timestamp){
diff --git a/src/lib/krb5/krb/ChangeLog b/src/lib/krb5/krb/ChangeLog
index 5ceee6685..8f34cf0bb 100644
--- a/src/lib/krb5/krb/ChangeLog
+++ b/src/lib/krb5/krb/ChangeLog
@@ -1,3 +1,10 @@
+2003-05-18  Tom Yu  <tlyu@mit.edu>
+
+	* auth_con.h: Sequence numbers are now unsigned.
+
+	* gen_seqnum.c (krb5_generate_seq_number): Constrain initial
+	sequence number space to facilitate backwards compatibility.
+
 2003-05-16  Ken Raeburn  <raeburn@mit.edu>
 
 	* ser_ctx.c (krb5_context_internalize): Copy read-in OS context
diff --git a/src/lib/krb5/krb/auth_con.h b/src/lib/krb5/krb/auth_con.h
index 1dcfc89e7..362909ce7 100644
--- a/src/lib/krb5/krb/auth_con.h
+++ b/src/lib/krb5/krb/auth_con.h
@@ -13,8 +13,8 @@ struct _krb5_auth_context {
     krb5_keyblock     * recv_subkey;
 
     krb5_int32		auth_context_flags;
-    krb5_int32		remote_seq_number;
-    krb5_int32		local_seq_number;
+    krb5_ui_4		remote_seq_number;
+    krb5_ui_4		local_seq_number;
     krb5_authenticator *authentp;		/* mk_req, rd_req, mk_rep, ...*/
     krb5_cksumtype	req_cksumtype;		/* mk_safe, ... */
     krb5_cksumtype	safe_cksumtype;		/* mk_safe, ... */
diff --git a/src/lib/krb5/krb/gen_seqnum.c b/src/lib/krb5/krb/gen_seqnum.c
index 196a43744..1dc8a58b1 100644
--- a/src/lib/krb5/krb/gen_seqnum.c
+++ b/src/lib/krb5/krb/gen_seqnum.c
@@ -36,7 +36,7 @@
 #endif
 
 krb5_error_code
-krb5_generate_seq_number(krb5_context context, const krb5_keyblock *key, krb5_int32 *seqno)
+krb5_generate_seq_number(krb5_context context, const krb5_keyblock *key, krb5_ui_4 *seqno)
 {
     krb5_data seed;
     krb5_error_code retval;
@@ -48,5 +48,20 @@ krb5_generate_seq_number(krb5_context context, const krb5_keyblock *key, krb5_in
 
     seed.length = sizeof(*seqno);
     seed.data = (char *) seqno;
-    return(krb5_c_random_make_octets(context, &seed));
+    retval = krb5_c_random_make_octets(context, &seed);
+    if (retval)
+	return retval;
+    /*
+     * Work around implementation incompatibilities by not generating
+     * initial sequence numbers greater than 2^30.  Previous MIT
+     * implementations use signed sequence numbers, so initial
+     * sequence numbers 2^31 to 2^32-1 inclusive will be rejected.
+     * Letting the maximum initial sequence number be 2^30-1 allows
+     * for about 2^30 messages to be sent before wrapping into
+     * "negative" numbers.
+     */
+    *seqno &= 0x5ffffff;
+    if (*seqno == 0)
+	*seqno = 1;
+    return 0;
 }