From 73940e4631d76cc586e870a7652c090af6721220 Mon Sep 17 00:00:00 2001 From: Tom Yu Date: Wed, 8 Oct 2003 23:53:23 +0000 Subject: [PATCH] Save encoded KRB-SAFE-BODY to avoid problems caused by re-encoding it. Also, handle correctly implemented RFC 1510 KRB-SAFE i.e., checksummed over KRB-SAFE-BODY only. ticket: 1893 tags: pullup git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@15831 dc483132-0cff-0310-8789-dd5450dbe970 --- src/include/ChangeLog | 5 ++++ src/include/k5-int.h | 6 +++++ src/lib/krb5/asn.1/ChangeLog | 16 ++++++++++++ src/lib/krb5/asn.1/asn1_k_encode.c | 17 +++++++++++++ src/lib/krb5/asn.1/asn1_k_encode.h | 3 +++ src/lib/krb5/asn.1/krb5_decode.c | 41 +++++++++++++++++++++++++++++- src/lib/krb5/asn.1/krb5_encode.c | 37 +++++++++++++++++++++++++++ src/lib/krb5/krb/ChangeLog | 7 +++++ src/lib/krb5/krb/rd_safe.c | 18 ++++++++++--- 9 files changed, 145 insertions(+), 5 deletions(-) diff --git a/src/include/ChangeLog b/src/include/ChangeLog index 0b0ed7d99..84602683f 100644 --- a/src/include/ChangeLog +++ b/src/include/ChangeLog @@ -1,3 +1,8 @@ +2003-10-08 Tom Yu + + * k5-int.h: Add prototypes for decode_krb5_safe_with_body and + encode_krb5_safe_with_body. + 2003-09-26 Ken Raeburn * Makefile.in ($(srcdir)/krb5/autoconf.h.in): Depend on diff --git a/src/include/k5-int.h b/src/include/k5-int.h index 0cb1aeecb..ebcd2135f 100644 --- a/src/include/k5-int.h +++ b/src/include/k5-int.h @@ -1221,6 +1221,9 @@ krb5_error_code encode_krb5_kdc_req_body krb5_error_code encode_krb5_safe (const krb5_safe *rep, krb5_data **code); +krb5_error_code encode_krb5_safe_with_body + (const krb5_safe *rep, const krb5_data *body, krb5_data **code); + krb5_error_code encode_krb5_priv (const krb5_priv *rep, krb5_data **code); @@ -1400,6 +1403,9 @@ krb5_error_code decode_krb5_kdc_req_body krb5_error_code decode_krb5_safe (const krb5_data *output, krb5_safe **rep); +krb5_error_code decode_krb5_safe_with_body + (const krb5_data *output, krb5_safe **rep, krb5_data *body); + krb5_error_code decode_krb5_priv (const krb5_data *output, krb5_priv **rep); diff --git a/src/lib/krb5/asn.1/ChangeLog b/src/lib/krb5/asn.1/ChangeLog index 578352bdb..520999d7f 100644 --- a/src/lib/krb5/asn.1/ChangeLog +++ b/src/lib/krb5/asn.1/ChangeLog @@ -1,3 +1,19 @@ +2003-10-08 Tom Yu + + * asn1_k_encode.c (asn1_encode_krb_saved_safe_body): New function; + kludge to insert a raw pre-encoded KRB-SAFE-BODY. + + * asn1_k_encode.h (asn1_encode_krb_saved_safe_body): Add + prototype. + + * krb5_decode.c (decode_krb5_safe_with_body): New function; saves + a copy of the encoding of the KRB-SAFE-BODY to avoid problems + caused by re-encoding it during verification. + + * krb5_encode.c (encode_krb5_safe_with_body): New function; + re-encode a KRB-SAFE using a saved KRB-SAFE-BODY encoding, to + avoid trouble with re-encoding a KRB-SAFE-BODY. + 2003-07-22 Sam Hartman * asn1_k_decode.c (asn1_decode_etype_info2_entry_1_3): Decoder for diff --git a/src/lib/krb5/asn.1/asn1_k_encode.c b/src/lib/krb5/asn.1/asn1_k_encode.c index 325a6ce77..00cfab032 100644 --- a/src/lib/krb5/asn.1/asn1_k_encode.c +++ b/src/lib/krb5/asn.1/asn1_k_encode.c @@ -942,3 +942,20 @@ asn1_error_code asn1_encode_predicted_sam_response(asn1buf *buf, const krb5_pred asn1_cleanup(); } + +/* + * Do some ugliness to insert a raw pre-encoded KRB-SAFE-BODY. + */ +asn1_error_code asn1_encode_krb_saved_safe_body(asn1buf *buf, const krb5_data *body, unsigned int *retlen) +{ + asn1_error_code retval; + + retval = asn1buf_insert_octetstring(buf, body->length, + (krb5_octet *)body->data); + if (retval){ + asn1buf_destroy(&buf); + return retval; + } + *retlen = body->length; + return 0; +} diff --git a/src/lib/krb5/asn.1/asn1_k_encode.h b/src/lib/krb5/asn.1/asn1_k_encode.h index a2429a778..caa46c570 100644 --- a/src/lib/krb5/asn.1/asn1_k_encode.h +++ b/src/lib/krb5/asn.1/asn1_k_encode.h @@ -266,4 +266,7 @@ asn1_error_code asn1_encode_predicted_sam_response (asn1buf *buf, const krb5_predicted_sam_response *val, unsigned int *retlen); +asn1_error_code asn1_encode_krb_saved_safe_body + (asn1buf *buf, const krb5_data *body, unsigned int *retlen); + #endif diff --git a/src/lib/krb5/asn.1/krb5_decode.c b/src/lib/krb5/asn.1/krb5_decode.c index 4172c882b..596997fe9 100644 --- a/src/lib/krb5/asn.1/krb5_decode.c +++ b/src/lib/krb5/asn.1/krb5_decode.c @@ -90,6 +90,7 @@ if((var) == NULL) clean_return(ENOMEM) construction = t2.construction; \ tagnum = t2.tagnum; \ indef = t2.indef; \ + taglen = t2.length; \ } #define get_eoc() \ @@ -107,6 +108,7 @@ if((var) == NULL) clean_return(ENOMEM) /* decode sequence header and initialize tagnum with the first field */ #define begin_structure()\ +unsigned int taglen;\ asn1buf subbuf;\ int seqindef;\ int indef;\ @@ -494,8 +496,26 @@ krb5_error_code decode_krb5_kdc_req_body(const krb5_data *code, krb5_kdc_req **r cleanup(free); } -krb5_error_code decode_krb5_safe(const krb5_data *code, krb5_safe **rep) +/* + * decode_krb5_safe_with_body + * + * Like decode_krb5_safe(), but grabs the encoding of the + * KRB-SAFE-BODY as well, in case re-encoding would produce a + * different encoding. (Yes, we're using DER, but there's this + * annoying problem with pre-1.3.x code using signed sequence numbers, + * which we permissively decode and cram into unsigned 32-bit numbers. + * When they're re-encoded, they're no longer negative if they started + * out negative, so checksum verification fails.) + * + * This does *not* perform any copying; the returned pointer to the + * encoded KRB-SAFE-BODY points into the input buffer. + */ +krb5_error_code decode_krb5_safe_with_body( + const krb5_data *code, + krb5_safe **rep, + krb5_data *body) { + krb5_data tmpbody; setup(); alloc_field(*rep,krb5_safe); clear_field(rep,checksum); @@ -511,12 +531,26 @@ krb5_error_code decode_krb5_safe(const krb5_data *code, krb5_safe **rep) if(msg_type != KRB5_SAFE) clean_return(KRB5_BADMSGTYPE); #endif } + /* + * Gross kludge to extract pointer to encoded safe-body. Relies + * on tag prefetch done by next_tag(). Don't handle indefinite + * encoding, as it's too much work. + */ + if (!indef) { + tmpbody.length = taglen; + tmpbody.data = subbuf.next; + } else { + tmpbody.length = 0; + tmpbody.data = NULL; + } get_field(**rep,2,asn1_decode_krb_safe_body); alloc_field((*rep)->checksum,krb5_checksum); get_field(*((*rep)->checksum),3,asn1_decode_checksum); (*rep)->magic = KV5M_SAFE; end_structure(); } + if (body != NULL) + *body = tmpbody; cleanup_manual(); error_out: if (rep && *rep) { @@ -526,6 +560,11 @@ error_out: return retval; } +krb5_error_code decode_krb5_safe(const krb5_data *code, krb5_safe **rep) +{ + return decode_krb5_safe_with_body(code, rep, NULL); +} + krb5_error_code decode_krb5_priv(const krb5_data *code, krb5_priv **rep) { setup(); diff --git a/src/lib/krb5/asn.1/krb5_encode.c b/src/lib/krb5/asn.1/krb5_encode.c index eed6faf45..ecdfa1878 100644 --- a/src/lib/krb5/asn.1/krb5_encode.c +++ b/src/lib/krb5/asn.1/krb5_encode.c @@ -478,6 +478,43 @@ krb5_error_code encode_krb5_safe(const krb5_safe *rep, krb5_data **code) krb5_cleanup(); } +/* + * encode_krb5_safe_with_body + * + * Like encode_krb5_safe(), except takes a saved KRB-SAFE-BODY + * encoding to avoid problems with re-encoding. + */ +krb5_error_code encode_krb5_safe_with_body( + const krb5_safe *rep, + const krb5_data *body, + krb5_data **code) +{ + krb5_setup(); + + if (body == NULL) { + asn1buf_destroy(&buf); + return ASN1_MISSING_FIELD; + } + + /* cksum[3] Checksum */ + krb5_addfield(rep->checksum,3,asn1_encode_checksum); + + /* safe-body[2] KRB-SAFE-BODY */ + krb5_addfield(body,2,asn1_encode_krb_saved_safe_body); + + /* msg-type[1] INTEGER */ + krb5_addfield(ASN1_KRB_SAFE,1,asn1_encode_integer); + + /* pvno[0] INTEGER */ + krb5_addfield(KVNO,0,asn1_encode_integer); + + /* KRB-SAFE ::= [APPLICATION 20] SEQUENCE */ + krb5_makeseq(); + krb5_apptag(20); + + krb5_cleanup(); +} + krb5_error_code encode_krb5_priv(const krb5_priv *rep, krb5_data **code) { krb5_setup(); diff --git a/src/lib/krb5/krb/ChangeLog b/src/lib/krb5/krb/ChangeLog index 5d777909a..57d63bfd7 100644 --- a/src/lib/krb5/krb/ChangeLog +++ b/src/lib/krb5/krb/ChangeLog @@ -1,3 +1,10 @@ +2003-10-08 Tom Yu + + * rd_safe.c (krb5_rd_safe_basic): Save the encoded KRB-SAFE-BODY + to avoid trouble caused by re-encoding. Also, handle correctly + implemented RFC 1510 KRB-SAFE, i.e., checksummed over + KRB-SAFE-BODY only. + 2003-09-02 Tom Yu * conv_creds.c (krb524_convert_creds_plain): Apply patch from diff --git a/src/lib/krb5/krb/rd_safe.c b/src/lib/krb5/krb/rd_safe.c index 41c2596b7..15dc6dccc 100644 --- a/src/lib/krb5/krb/rd_safe.c +++ b/src/lib/krb5/krb/rd_safe.c @@ -51,6 +51,7 @@ krb5_rd_safe_basic(krb5_context context, const krb5_data *inbuf, const krb5_keyb { krb5_error_code retval; krb5_safe * message; + krb5_data safe_body; krb5_checksum our_cksum, *his_cksum; krb5_octet zero_octet = 0; krb5_data *scratch; @@ -59,7 +60,7 @@ krb5_rd_safe_basic(krb5_context context, const krb5_data *inbuf, const krb5_keyb if (!krb5_is_krb_safe(inbuf)) return KRB5KRB_AP_ERR_MSG_TYPE; - if ((retval = decode_krb5_safe(inbuf, &message))) + if ((retval = decode_krb5_safe_with_body(inbuf, &message, &safe_body))) return retval; if (!krb5_c_valid_cksumtype(message->checksum->checksum_type)) { @@ -113,7 +114,7 @@ krb5_rd_safe_basic(krb5_context context, const krb5_data *inbuf, const krb5_keyb message->checksum = &our_cksum; - if ((retval = encode_krb5_safe(message, &scratch))) + if ((retval = encode_krb5_safe_with_body(message, &safe_body, &scratch))) goto cleanup; message->checksum = his_cksum; @@ -126,8 +127,17 @@ krb5_rd_safe_basic(krb5_context context, const krb5_data *inbuf, const krb5_keyb krb5_free_data(context, scratch); if (!valid) { - retval = KRB5KRB_AP_ERR_MODIFIED; - goto cleanup; + /* + * Checksum over only the KRB-SAFE-BODY, like RFC 1510 says, in + * case someone actually implements it correctly. + */ + retval = krb5_c_verify_checksum(context, keyblock, + KRB5_KEYUSAGE_KRB_SAFE_CKSUM, + &safe_body, his_cksum, &valid); + if (!valid) { + retval = KRB5KRB_AP_ERR_MODIFIED; + goto cleanup; + } } replaydata->timestamp = message->timestamp; -- 2.26.2