[Kerberos] Flags rewrite

[Emmanuel Lecharny <el...@gmail.com>]

Hi,

to stick to the RFCs, and to ease the manipulation of kerberos flags
(TicketFlags, KDCOptions, APOptions), I suggest we modify the related
classes.

Considering that those three elements are KerberosFlags object, and
knowing that KerberosFlag is a 32 bits BIT STRING, we can use the
following structure :

- an interface KerberoFlags
- an abstract class AbstractKerberosFlags implementing the base
operations, and extending the asn1 BitString object
- 3 classes extending AbstractKerberosFlags, containing the
combinaison of flags (TicketFlags, KdcOptions and ApOptions)
- 3 enum containing the different individual flags (TicketFlag,
KdcOption and ApOption)

It will ease a lot flags manipulation in the code, and be coherent
with our current ASN.1 codec.

thoughts ?

Emmanuel
-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: [Kerberos] Flags rewrite

[Enrique Rodriguez <en...@gmail.com>]

On 8/15/07, Emmanuel Lecharny <el...@gmail.com> wrote:
> Hi Enrique
> ...

Thanks for responding with detailed reasons.  This will be great
clean-up.  You have obviously taken time to read up on Kerberos.  I am
looking forward to these changes.

Enrique

Re: [Kerberos] Flags rewrite

[Emmanuel Lecharny <el...@gmail.com>]

Hi Enrique

>
> In general I like the clean-up, but we do have enumeration using the
> older Java-style int's and we do have these classes and they already
> extend the common base class Options.

Yes, but it's all about manipulating int, when enum will allow us to
manupilate direct flags like TicketFlags.PROXIABLE and such. I also
added some direct methods like isProxiable(). The idea is to use Java
5 syntax.

> Regarding easing flag manipulation, I haven't found flag
> setting to be a problem and I'm not sure how what you propose helps.
> So, what's the major benefit of this work?  Is this part of the "make
> codecs 6x faster" and "remove BC deps" initiative?

Yes, because the encoding will be directly included in the messages
themselves. That might sound crazy, and we can find a better way to do
it (using a decorator), but I have tried both approach, and wrt
performances, this is the fastest way to handle encoding. More to come
...

> What happens with the concept of making Kerberos ASN.1 classes
> implement common codec interfaces, such as "Encodable" and
> "Decodable."  I thought one of the ideas you were working to make
> uniform was that the classes would know how to encode themselves,
> which to me ment something like a getEncoded() method that returned a
> byte array containing the DER or wrote to a ByteBuffer.

This is exacty what I'm working on, but I can't push it now, as it's
not done entirely. I'm around 60% done.

At the end, we will be able to totally discard all the
kerberos.io.encoder package, and rely only on the messages to encode
themselves, and also discard BC dependencies (assuming that the
decoders is ready too)

>
> Also, if I found the right BitString you're talking about, extending
> BitString would expose a public API that has nothing to do with flags,
> such as isStreamed().

That's true, but it's also very important to offer the server the
opportunity to deal with DOS attacks : if the decoded BITSTRING is up
in memory, you can easily break it sending a Gbyte of data. The idea
behind the curtain is to stream a BIT STRING when it's bigger that
1Kbyte, for instance, instead of discarding it (using a limitation).
Sadly enough, this is not already implemented... Last, not least,
classes inheriting from BIT STRING will be used on the server side, so
I don't really think it could be a burden for users.

>
> These classes really should be immutable, even though they aren't
> today.  After decoding off the wire, they should be immutable.  The
> client side needs to set flags during request building, though.

As for LDAP, we have to make a difference between client and server
side. This has not been done in LDAP (but we have discussed that a
lot), but our guess is that we will have a 99/1 ratio of usage in
favor of the server side. Even for the client side, those classes
won't be manipulated directly by any user, as they will generaly use a
layer like GSSAPI to communicate with our kerberos server (I may be
wrong, though). So I don't think it's a big deal to have mutable
classes.

>
> Do you have examples yet of how you've updated Kerberos message
> objects to use the newer ASN.1 codecs?  I thought you checked in a
> class or two, but I couldn't find them.

Not yet. By the end of the week, I think, I will be able to ci some of
them. However, I have added some tests for each new encoder, and those
tests are comparing the result with the previous encoder, to be sure
there is no problem. We will be able to keep both encoder but at the
end, I think it will be preferable to ditch one of them.

There will be still a lot of work to do, like adding some defensive
programming in the server : a lot of places are accepting data without
any control (the user may face a NPE which is wrong ...), not talking
about the semantic aspects (which data is correct in which message,
etc, but this is not something I can fix without spending a huge
amount of time reading the specs ...)


I also get rid of the modifiers (which seems to me a useless burden,
duplicating the effort and classes for an advantage I don't see,
beside the fact that 'using pattern is good' ), dividing the number of
classes by two.

I also tried to add as much comment as I can, in the headers and
private members, copying the text from the 4120 RFC. Don't be
surprised if I changed the names of those members, I prefer to use
cname instead of clientPrincipal, in order to stick to the RFC (it's
easier because you have to remember the name when doing the encoding).
However, the methods used to manipulate this member remains the same :
getClientPrincipal) and setClientPrincipal() (the API user has no
reason to suffer using cryptic names ...).

The last important thing is about KerberosName and PrincipalName (a
new class, added to manipulate the names without the realm). Messages
separate names and realms, so the PrincipalName us used internally. At
first, I thought we can get rid of KerberosName totally, but it was a
very bad idea. So now, you have the possibility to set the
clientPrincipal with a KerberosName or with the PrincipalName. You can
also get both objects using getClientPrincipal() -> KerberosName or
getClientprincipalName() -> PrincipalName.

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: [Kerberos] Flags rewrite

[Enrique Rodriguez <en...@gmail.com>]

On 8/14/07, Emmanuel Lecharny <el...@gmail.com> wrote:
> ...
> - an interface KerberoFlags
> - an abstract class AbstractKerberosFlags implementing the base
> operations, and extending the asn1 BitString object
> - 3 classes extending AbstractKerberosFlags, containing the
> combinaison of flags (TicketFlags, KdcOptions and ApOptions)
> - 3 enum containing the different individual flags (TicketFlag,
> KdcOption and ApOption)
>
> It will ease a lot flags manipulation in the code, and be coherent
> with our current ASN.1 codec.
>
> thoughts ?

In general I like the clean-up, but we do have enumeration using the
older Java-style int's and we do have these classes and they already
extend the common base class Options.  KerberosFlags is a new name
from RFC 4120 (as opposed to RFC 1510) and I agree the rename is in
order.  Regarding easing flag manipulation, I haven't found flag
setting to be a problem and I'm not sure how what you propose helps.
So, what's the major benefit of this work?  Is this part of the "make
codecs 6x faster" and "remove BC deps" initiative?

What happens with the concept of making Kerberos ASN.1 classes
implement common codec interfaces, such as "Encodable" and
"Decodable."  I thought one of the ideas you were working to make
uniform was that the classes would know how to encode themselves,
which to me ment something like a getEncoded() method that returned a
byte array containing the DER or wrote to a ByteBuffer.

Also, if I found the right BitString you're talking about, extending
BitString would expose a public API that has nothing to do with flags,
such as isStreamed().

These classes really should be immutable, even though they aren't
today.  After decoding off the wire, they should be immutable.  The
client side needs to set flags during request building, though.

Do you have examples yet of how you've updated Kerberos message
objects to use the newer ASN.1 codecs?  I thought you checked in a
class or two, but I couldn't find them.

Enrique