Project review: OTPOverRadius

[Dmitri Pal]

On 12/14/2012 10:34 PM, Nathaniel McCallum wrote:
> On Fri, 2012-12-14 at 16:20 -0500, Dmitri Pal wrote:
>>>> Secret should be optional in all cases because it is useful for debugging however every authentication via unprotected non RoUS channel should produce log messages.
>>>> Strip_realm should be yes by default. It is more likely that people have flattened the namespace in some way. 
>>> 1. There is no such thing as "no secret" only a "well-known, pre-defined
>>> secret"
>> I have implemented RADIUS servers in the past. Ability to send packets
>> that you can read visually without any other tools is very important. If
>> you always have to use tools to decompose the radius packet and see what
>> actually was sent you significantly slow down development and testing. I
>> was also shocked about the fact that secrets can be empty but then I
>> have seen several cases when removing the secret and seeing what
>> actually goes over the wire led to quick identification of root cause of
>> the problem.
> Can you point me to the proper section in the RFC on how to do this
> properly? So far as I have read, User-Password MUST be hashed and that
> hash requires the secret. Removing the secret is probably the same as an
> empty secret (NULL == ""), which is the default we use. But the hash is
> still performed. Which means you'd still need a dehashing tool.

I guess you are right. We are building client and I was building the
server. In the server testing with several clients without the need to
set secrets was a benefit during testing.

> tcpdump and wireshark are pretty trivial to use either with or without a
> secret without much additional development lag... But again, I think
> that refusing to send packets with a known secret over UDP/IP by default
> is a Good Thing.

OK
>
>>> If we use DNS round-robin, then having a separate radius config adds
>>> complexity just to save a little typing in a write-once config. I'm not
>>> inclined to complicate the code for that reason.
>> This is the assumption that needs to be reflected on the page.
> Added.
>
>>>> 2) It is unclear why ID is really needed.
>>>>
>>>> ||
>>>>
>>>> [{
>>>>    "type": <string>,
>>>>    "id": <string>,
>>>>    "username": <string>
>>>>  }, ...]
>>>>
>>>> The commens says "The *id* field identifies the unique id of the token and is sent in the PA-OTP-CHALLENGE." But it is really never used.
>>> This exists for interoperability. Some clients may require this and the
>>> admin should be able to just set a field.
>> We can always add it later if someone asks.
> At the cost of a 1 year release cycle and/or maintaining a fork for
> internal deployment. My attempt to tackle this stuff now was to be
> future-ready.

I am not against adding fields in advance but I am against adding fields
that might lead to the wrong assumptions at least without the proper
disclaimers.

>
>>>> I suspect that you expect the prompt to tell the user to a specific token but I am not sure that in practice it would ever be used.
>>>> Even in the migration case when company tries to move from one OTP vendor to another it would be very cumbersome to populate this field in the KDC. 
>>> Less cumbersome than having to create a patch, run their own build and
>>> still have to populate the field in the KDC.
>>>
>>>> If we want to support migration in future we probably would have to allow more then one type per user and would try several token configurations for the user until one RADIUS server returns OK.
>>>> I think you alluding to it in the following sentence : 
>>>> "All matches will then be used as configuration for RADIUS validation, in the order they were specified in the *otp* user string, stopping after the first Access-Accept response is received."
>>>> But it really requires a bit more clarity and explanation.
>>>> At least JSON syntax seems to allow more then one type per user but it is not clear what would happen if multiple types are specified. 
>>> How is that statement not clear? It eliminates the tokens that don't
>>> match 
>> Match how? What is compared to what?
> The PA-OTP-REQUEST would be matched with the vendor, length, format,
> algorithm and id fields, eliminating non-matches. Whatever tokens
> remain, the RADIUS servers for these will be tried. I'm pretty sure that
> this is strongly implied in the proposal and some basic knowledge of OTP
> preauth.

OK though in cases other than testing most likely none of these fields
will be populated so there would be nothing to match.

And I realize that knowledge of the OTP preauth is needed though others
might not so it might make sense to add it as a prerequisite on the page.
 
>
> Nathaniel
>


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/