kdc: avoid re-encoding KDC-REQ-BODY

Use --preserve-binary=KDC-REQ-BODY option to ASN.1 compiler to avoid re-encoding KDC-REQ-BODYs for verification in GSS preauth, TGS and PKINIT.
heimdal · Sep 13, 2021 · ebfd48e · ebfd48e
1 parent 908ef18
commit ebfd48e
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 56 deletions.
diff --git a/kdc/fast.c b/kdc/fast.c
@@ -357,7 +357,6 @@ fast_unwrap_request(astgs_request_t r)
     krb5_keyblock armorkey;
     krb5_error_code ret;
     krb5_ap_req ap_req;
-    unsigned char *buf = NULL;
     KrbFastReq fastreq;
     size_t len, size;
     krb5_data data;
@@ -476,18 +475,10 @@ fast_unwrap_request(astgs_request_t r)
     krb5_free_keyblock_contents(r->context, &armorkey);
 
     /* verify req-checksum of the outer body */
-
-    ASN1_MALLOC_ENCODE(KDC_REQ_BODY, buf, len, &r->req.req_body, &size, ret);
-    if (ret)
-	goto out;
-    if (size != len) {
-	ret = KRB5KDC_ERR_PREAUTH_FAILED;
-	goto out;
-    }
-
     ret = krb5_verify_checksum(r->context, r->armor_crypto,
 			       KRB5_KU_FAST_REQ_CHKSUM,
-			       buf, len, 
+			       r->req.req_body._save.data,
+			       r->req.req_body._save.length,
 			       &fxreq.u.armored_data.req_checksum);
     if (ret) {
 	kdc_log(r->context, r->config, 2,
@@ -548,7 +539,6 @@ fast_unwrap_request(astgs_request_t r)
 	krb5_free_principal(r->context, armor_server);
     if(armor_user)
 	_kdc_free_ent(r->context, armor_user);
-    free(buf);
 
     return ret;
 }

diff --git a/kdc/gss_preauth.c b/kdc/gss_preauth.c
@@ -190,7 +190,6 @@ _kdc_gss_rd_padata(astgs_request_t r,
                    int *open)
 {
     krb5_error_code ret;
-    size_t size;
 
     OM_uint32 minor;
     gss_client_params *gcp = NULL;
@@ -230,12 +229,7 @@ _kdc_gss_rd_padata(astgs_request_t r,
         goto out;
 
     _krb5_gss_data_to_buffer(&pa->padata_value, &input_token);
-
-    ASN1_MALLOC_ENCODE(KDC_REQ_BODY, cb.application_data.value,
-                       cb.application_data.length, &r->req.req_body,
-                       &size, ret);
-    heim_assert(ret || size == cb.application_data.length,
-                "internal asn1 encoder error");
+    _krb5_gss_data_to_buffer(&r->req.req_body._save, &cb.application_data);
 
     gcp->major = gss_accept_sec_context(&gcp->minor,
                                         &gcp->context_handle,
@@ -263,7 +257,6 @@ _kdc_gss_rd_padata(astgs_request_t r,
 
 out:
     gss_release_cred(&minor, &cred);
-    gss_release_buffer(&minor, &cb.application_data);
 
     if (gcp && gcp->major != GSS_S_NO_CONTEXT)
         *pgcp = gcp;

diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c
@@ -1081,9 +1081,6 @@ tgs_check_authenticator(krb5_context context,
 			krb5_keyblock *key)
 {
     krb5_authenticator auth;
-    size_t len = 0;
-    unsigned char *buf;
-    size_t buf_size;
     krb5_error_code ret;
     krb5_crypto crypto;
 
@@ -1109,36 +1106,19 @@ tgs_check_authenticator(krb5_context context,
 	goto out;
     }
 
-    /* XXX should not re-encode this */
-    ASN1_MALLOC_ENCODE(KDC_REQ_BODY, buf, buf_size, b, &len, ret);
-    if(ret){
-	const char *msg = krb5_get_error_message(context, ret);
-	kdc_log(context, config, 4, "Failed to encode KDC-REQ-BODY: %s", msg);
-	krb5_free_error_message(context, msg);
-	goto out;
-    }
-    if(buf_size != len) {
-	free(buf);
-	kdc_log(context, config, 4, "Internal error in ASN.1 encoder");
-	*e_text = "KDC internal error";
-	ret = KRB5KRB_ERR_GENERIC;
-	goto out;
-    }
     ret = krb5_crypto_init(context, key, 0, &crypto);
     if (ret) {
 	const char *msg = krb5_get_error_message(context, ret);
-	free(buf);
 	kdc_log(context, config, 4, "krb5_crypto_init failed: %s", msg);
 	krb5_free_error_message(context, msg);
 	goto out;
     }
     ret = krb5_verify_checksum(context,
 			       crypto,
 			       KRB5_KU_TGS_REQ_AUTH_CKSUM,
-			       buf,
-			       len,
+			       b->_save.data,
+			       b->_save.length,
 			       auth->cksum);
-    free(buf);
     krb5_crypto_destroy(context, crypto);
     if(ret){
 	const char *msg = krb5_get_error_message(context, ret);

diff --git a/kdc/pkinit.c b/kdc/pkinit.c
@@ -113,10 +113,7 @@ pk_check_pkauthenticator(krb5_context context,
 			 PKAuthenticator *a,
 			 const KDC_REQ *req)
 {
-    u_char *buf = NULL;
-    size_t buf_size;
     krb5_error_code ret;
-    size_t len = 0;
     krb5_timestamp now;
     Checksum checksum;
 
@@ -128,22 +125,13 @@ pk_check_pkauthenticator(krb5_context context,
 	return KRB5KRB_AP_ERR_SKEW;
     }
 
-    ASN1_MALLOC_ENCODE(KDC_REQ_BODY, buf, buf_size, &req->req_body, &len, ret);
-    if (ret) {
-	krb5_clear_error_message(context);
-	return ret;
-    }
-    if (buf_size != len)
-	krb5_abortx(context, "Internal error in ASN.1 encoder");
-
     ret = krb5_create_checksum(context,
 			       NULL,
 			       0,
 			       CKSUMTYPE_SHA1,
-			       buf,
-			       len,
+			       req->req_body._save.data,
+			       req->req_body._save.length,
 			       &checksum);
-    free(buf);
     if (ret) {
 	krb5_clear_error_message(context);
 	return ret;

diff --git a/lib/asn1/krb5.opt b/lib/asn1/krb5.opt
@@ -4,3 +4,4 @@
 --sequence=METHOD-DATA
 --sequence=ETYPE-INFO
 --sequence=ETYPE-INFO2
+--preserve-binary=KDC-REQ-BODY