[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug in _krb5_pk_rd_pa_reply

In testing pkinit support in the latest code drops, I noticed that in 
lib/krb5/pkinit.c:_krb5_pk_rd_pa_reply() the code attempts to decode the 
padata first with  decode_PA_PK_AS_REP(), and subsequently with 
decode_PA_PK_AS_REP_19(), and decode_PA_PK_AS_REP_Win2k() without any 
sort of test of the padata type.

In the case of a -25 reply from the kdc(20050516 snapshot) both the 
decode_PA_PK_AS_REP(), and the decode_PA_PK_AS_REP_19() calls will 
return 0, and as a result, the -19 handling code will be followed in 
addition to the -25 branch. This winds up with a rep19.element value not 
represented in the case statement(probably totally bogus) and thus the 
default failure case is followed even though there was perfectly valid 
-25 padata. making these section conditional on the value of 
pa->padata_type solved this value, and is implemented in the patch 
below, but I wasn't sure what the padata type was for a pkinit reply 
from a win2k kdc so that is the fall through case. if the reply value is 
the same as for one of -19 or -25, that will not be correct(I have no 
win2k kdc to test against.)

anyways, thanks for all of the work that's gone into this. I now have a 
working kdc and client, and will be working on getting proxy cert 
support at some point in the future(rfc3820 style proxies should be 
relatively straightforward, I believe that the openssl 0.9.8 branch has 
support, but I'm not sure if the legacy globus ProxyCertInfo extension 
will ever be supported in the mainline openssl, and I'm not entirely 
sure what'll be required to get heimdal's configure to use the globus 

-Matt Andrews

--- src/heimdal/heimdal-20050519/lib/krb5/pkinit.c      2005-05-10 
00000 -0700
+++ pkinit.c    2005-05-19 09:11:34.000000000 -0700
@@ -1739,7 +1739,7 @@
      size_t size;

      /* Check for PK-INIT -25 */
-    {
+    if ( pa->padata_type == KRB5_PADATA_PK_AS_REP ){
         PA_PK_AS_REP rep;

         memset(&rep, 0, sizeof(rep));
@@ -1779,7 +1779,7 @@

      /* Check for PK-INIT -19 */
-    {
+    else if ( pa->padata_type == KRB5_PADATA_PK_AS_REP_19 ) {
         PA_PK_AS_REP_19 rep19;

         memset(&rep19, 0, sizeof(rep19));
@@ -1813,7 +1813,7 @@

      /* Check for Windows encoding of the AS-REP pa data */
-    {
+    else {
         PA_PK_AS_REP_Win2k w2krep;

         memset(&w2krep, 0, sizeof(w2krep));