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

Re: preauth_always option?



On Sun, 8 Jun 2008 20:49:35 -0700
"Henry B. Hotz" <hotz@jpl.nasa.gov> wrote:

> Just playing devil's advocate, without thinking much about it, do we  
> do the "right" thing if the kdc in question decides to fudge the spec  
> and e.g. returns PREAUTH_REQUIRED in some state other than 2?

You mean in some state other than 3. Yes, the error block will not reset
'done' and so the loop will exit and that error will be returned.

>  Are  
> there other states than 3 where we ought to respond similarly to  
> PREAUTH_FAILED?

I think you mean states other than 2.

I don't think so. If the caller supplied padata (state 1) then I would
think they'd want to know straight away if it didn't work. If no padata
was used (state 3) then getting PREAUTH_FAILED is an invalid error. If
ETYPE_INFO was used to build padata (state 4) and the KDC returned
PREAUTH_FAILED that is also basically invalid. In all of these cases,
the 'done' sentinel is not reset so the loop just exits and the error
is returned.

> Thinking just a tiny bit more:  seems like we ought to test for those  
> error returns on the outside and do just enough state-checking on the  
> inside to guarantee we don't infinite-loop.  Without going all the way  
> to pseudo-code I can't say if that results in a practical difference  
> or not.

The error codes are part of the state machine logic so I don't think
they should be dealt with separately. If one were exceedingly paranoid
they could put in a limit counter to stop any possibility of an infinite
loop. But state machines are good at handling such details. The
'done' sentinel is only reset in the error block where the state is
always increased. So as it is an infinite loop is not possible.

Mike

> My first reaction was that the state machine was too complex, but I  
> now agree with you that it's a good idea.
> 
> I'd suggest making the state an enum would improve readability.
> 
> On Jun 8, 2008, at 2:16 PM, Michael B Allen wrote:
> 
> > On Wed, 28 May 2008 16:15:45 -0400
> > Michael B Allen <miallen@ioplex.com> wrote:
> >
> >> If not I'll make one and post it but I was hoping someone else had  
> >> done
> >> this already.
> >
> > I'm not in my environment right so I can't supply a real patch yet but
> > here's the pseudocode that will be the basis for it:
> >
> >  get_in_cred(padata)
> >  {
> >      error = 0;
> >      state = padata != NULL ? 1 : 2;
> >
> >      do {
> >          done = 1;
> >
> >          switch (state) {
> >              case 1: /* PA supplied as param */
> >                  break;
> >              case 2: /* Try optimistic PA */
> >                  padata = make_optimistic_padata();
> >                  if (pdata) {
> >                      break;
> >                  }
> >                  /* Cannot determine suitable optimistic
> >                   * padata, fall through to no PA
> >                   */
> >                  state = 3;
> >              case 3: /* No PA */
> >                  break;
> >              case 4: /* Extract from ETYPE_INFO */
> >                  padata = extract_etype_info_padata();
> >                  break;
> >          }
> >
> >          sendto_kdc(req, rep);
> >
> >          if (error) {
> >              if (state == 2) {
> >                  if (error == PREAUTH_FAILED) {
> >                      /* Optimistic PA failed, try no PA to get  
> > ETYPE_INFO */
> >                      state = 3;
> >                      done = 0;
> >                  }
> >              } else if (state == 3) {
> >                  if (error == PREAUTH_REQUIRED) {
> >                      if (is_etype_info_present) {
> >                          /* Try PA from ETYPE_INFO */
> >                          state = 4;
> >                          done = 0;
> >                      }
> >                  }
> >              }
> >          }
> >      } while (!done);
> >
> >      return error;
> >  }
> >
> > One could argue that the state machine isn't necessary but, for the  
> > long
> > term, I think it is warranted here.

-- 
Michael B Allen
PHP Active Directory SPNEGO SSO
http://www.ioplex.com/