Subject: Re: [secdir] SECDIR review of draft-ietf-kitten-krb-auth-indicator-04

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 3 Jan 2017 00:20:02 -0600

Hi Christian,

Thanks for the review!

On Sat, Dec 31, 2016 at 06:39:21PM -0800, Christian Huitema wrote:
> Copying to Nathan Kinder and Nathaniel McCallum, since their mail server
> rejects messages relayed by the IETF server.
> 
> -----Original Message-----
> From: secdir [mailto:[email-masked]] On Behalf Of Christian Huitema
> Sent: Saturday, December 31, 2016 6:20 PM
> To: 'IESG' <[email-masked]>; 'secdir' <[email-masked]>;
> [email-masked]
> Subject: [secdir] SECDIR review of draft-ietf-kitten-krb-auth-indicator-04
> 
[...]
> The document is almost ready, by I wish a few issues were addressed before
> publication.
> 
> My first issue is that the document describes an update to the Kerberos
> protocol specification, RFC 4120, but does not define the specific way in
> which RFC 4120 is updated. Could the draft be updated to include something
> like the section "6. Assigned Numbers" of RFC 7751? If I understand
> correctly, the changes are a new ad-type number 97, pointing to a CAMMAC
> container, in which the "elements" are encoded according to the syntax
> specified in Appendix A of the draft. Having that explained succinctly would
> help future readers.

I noticed the "Updates but doesn't really update" issue while preparing the
shepherd review, and opted to leave in the "Updates" marker since it's
probably something an implementor of 4120 should know about.
The "Assigned Numbers" section is a good idea, thanks for pointing it out
(and yes, you understand correctly).

Authors, can you prepare another update?

> My second issue is with the use of site-defined strings. I understand that
> the site defined strings are defined by the administrator of a realm. What
> happens if these strings appear outside the original realm, for example in
> an environment connecting multiple realms? Don't we have a potential there
> for name collision? Should there not be some guidance to implementers? 

There is maybe some potential for confusion, though not, I think, at the
protocol level.  The authentication indicator should always originate from
the realm of orignial authentication, which is the realm of the client
principal (in general).  Even with some of the more exotic flows, like
anonymous (or semi-anonymous) principals and making cross-realm TGS
requests for foreign-realm TGTs, the client principal's realm is unchanged,
so at a protocol level, the meaning of "this realm asserts that this
authentication mechanism was used" remains clear.  The confusion is when
applications just check strings against a table without special-casing
foreign-realm principals (which is likely to happen and the natural thing
for application authors to do; I am not trying to belittle the issue
you raised).

In many cases, cross-realm operations occur when the administrators
of the different realms are tightly coordinated (or even the same
group), in which case they probably use the same semantics for the
authentication indicator.  In cases where the administrators of the
different realms are genuinely different organizations, there are already
risks for application services in such realms, such as for applications
that grant access to "valid user".  That said, the authentication indicator
does introduce a new type of risk, and it is appropriate to have some
text about it in the security considerations.

Authors, do you think you can come up with text, or should one of us
try to make a contribution?

> I note that the proposed short string syntax forbids use of the ":"
> character in site-defined strings. Did the WG look at the consequences of
> that choice? If site administrators cannot use the URI like syntax, what is
> the preferred way of defining unique strings and preventing collisions?

I don't think the WG looked at the consequences, no -- IIRC this requirement
was introduced at my urging due to the shepherd review, in order to
avoid conflict between the two classes of possible values.  If URIs must
be LoA profiles and site-local values must be not-URIs, then there is
no conflict.

My expectation is that what will happen in practice is that the site-local
short strings will actually be implementation-local, and the name of the
preauthentication plugin or module will be used, like "otp" or "pkinit"
or "spake".  I don't expect anyone to try to make globally unique values,
but of course there are always options like UUIDs or using alternate
separator characters for those who wish to try.  (It is debatable whether
UUIDs count as "short", but there is no enforcement on "short", so
they are in practice fair game.)

> What are application services supposed to do when they encounter URI or
> site-defined strings that they do not understand?

The same thing they do now (in practice) when receiving other unknown
authorization data types: ignore it.  (This is in violation of the
spec, that says unknown types should be treated as critical unless
wrapped in AD-IF-RELEVANT, but that behavior is not implemented in the
major implementations.)  That may end up being a default-deny or
default-permit mode, depending on the application service's configuration.

> The ASN.1 syntax defines the element as a "SEQUENCE OF UTF8String". The
> document mentions that "Each UTF8String value is a short string". How short
> exactly should these strings be? How many of them should an application
> expect in the "SEQUENCE OF" element? The syntax itself does not constrain
> the length or number of these strings. Are we not worried with potential
> interoperability issues? Could this be abused in some attacks? Should the
> security considerations mention that?

If I remember the history of the document correctly, there is intentionally
no limit.  URIs for LoA profiles could end up being pretty long, and
there was a desire to not artificially limit those; it doesn't seem
worth complicating the semantics of the indicator just to impose a length
restriction on the non-URI strings.  As far as the number of elements in
the sequence, in practice there is probably no issue, since the
authentication indicator is issued by the KDC in response to the actual
authentication that occurred -- well-behaved KDCs should only include
as many strings as authentication methods were used (which is in practice
one or two at the moment, and probably not going to get much above three
ever).  There is always the concern about a client parsing
untrusted/unvalidated input, but the consumer should be validating the
MAC(s) in the CAMMAC container before parsing, and the implementation
ticket size (and similar) constraints will also limit the possible
size here.

So, probably no attacks (absent compromised KDCs, which have other
ways to wreak havoc) and probably no need for security consideration
mention.  I can't come up with any potential interoperability issues,
either, but I didn't spend a whole lot of time thinking about it.

Thanks again,

Ben

---

From: "Christian Huitema" <[email-masked]>
Date: Thu, 5 Jan 2017 07:13:55 -0800

Thanks for the corrections. I checked the new draft version,
draft-ietf-kitten-krb-auth-indicator-06, and the changes address my concern.
The new section "4.  Assigned Numbers" provides a clear update to RFC 4120,
and the added paragraph in the security section addresses cross-realm
indicator collisions.

One point, though. The new section 4 states:

   o  The table in Section 5.2.6 of RFC 4120 [RFC4120] is updated to map
      the ad-type 97 to "DER encoding of AD-AUTHENTICATION-INDICATOR".

Should that not be "DER encoding of AD-AUTHENTICATION-INDICATOR wrapped in a
CANMAC container"?

-- Christian Huitema

-----Original Message-----
From: Benjamin Kaduk [mailto:[email-masked]] 
Sent: Monday, January 2, 2017 10:20 PM
To: Christian Huitema <[email-masked]>
Cc: 'IESG' <[email-masked]>; 'secdir' <[email-masked]>;
[email-masked]; [email-masked];
[email-masked]
Subject: Re: [secdir] SECDIR review of
draft-ietf-kitten-krb-auth-indicator-04

Hi Christian,

Thanks for the review!

[...]

Thanks again,

Ben

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 5 Jan 2017 13:47:29 -0600

> and the added paragraph in the security section addresses cross-realm

Thanks for finding the new document -- I was going to send you a pointer
today to confirm that it addressed your concerns, but you beat me to it.

> Should that not be "DER encoding of AD-AUTHENTICATION-INDICATOR wrapped in a

I don't think so, but will loop in the WG to confirm.
The ad-type should indicate what is immediately inside the next encoding
layer of the ad-data.  So a Ticket might have an AuthorizationData that
contains ad-type 1 (AD-IF-RELEVANT), that itself contains AuthorizationData
with ad-type 96 (AD-CAMMAC), that in turn contains AuthorizationData with
ad-type 97 (AD-AUTHENTICATION-INDICATOR).  So, 97 should appear only at
the lowest level, and correspond to ad-data that's just the
AD-AUTHENTICATION-INDICATOR itself.

But thanks again for double-checking!

-Ben

---

From: Greg Hudson <[email-masked]>
Date: Thu, 5 Jan 2017 14:57:48 -0500

> AD-AUTHENTICATION-INDICATOR itself.

I agree with Ben.

---

From: "Christian Huitema" <[email-masked]>
Date: Thu, 5 Jan 2017 12:18:40 -0800

> today to confirm that it addressed your concerns, but you beat me to it.

Blame Tero Kivinen. He sent me a reminder this morning.

>> CAMMAC container"?

OK, I get that now. It was not entirely obvious from reading the text.

What is supposed to happen if the outside Authorization Data type is set to
97 instead of 96? Should that be specified somewhere? The text says:

   Authorization data elements of type AD-AUTHENTICATION-INDICATOR MUST
   be included in an AD-CAMMAC container so that their contents can be
   verified as originating from the KDC.

That's a fine constraint for the sender, but what about receivers?

-- Christian Huitema

---

From: Nathaniel McCallum <[email-masked]>
Date: Thu, 5 Jan 2017 15:39:36 -0500

> That's a fine constraint for the sender, but what about receivers?

5.  Security Considerations

   ... Application servers MUST validate the AD-CAMMAC container before
   making authorization decisions based on AD-AUTHENTICATION-INDICATOR
   elements.  Application servers MUST NOT make authorization decisions
   based on AD-AUTHENTICATION-INDICATOR elements which appear outside of
   AD-CAMMAC containers. ...

---

From: "Christian Huitema" <[email-masked]>
Date: Thu, 5 Jan 2017 14:34:47 -0800

>   AD-CAMMAC containers. ...

You are right, and I was confused. 

As far as I am concerned, the draft is fine and ready for publication. The "reserved number" section and the additional paragraph in the security consideration addressed the concerned that I raised in the initial review.

-- Christian Huitema