[kitten] shepherd review of draft-ietf-kitten-krb-auth-indicator-02

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[kitten] shepherd review of draft-ietf-kitten-krb-auth-indicator-02

Benjamin Kaduk-2
Hi all,

I've completed my shepherd review of
draft-ietf-kitten-krb-auth-indicator-02, and unfortunately it seems that
an additional revision may be needed.

It feels like some parts of the document are written as if the "short
strings" for comparison against known values are only for use when
presented back to the KDC, but other parts of the document describe it as
being used for policy decisions made by application services as well as
the KDC.  (If it was just the KDC that was handling it, the document would
not need to specify, since KDC-internal things need not
necessarily interoperate the way that public protocol elements do.)  There
is also the possibility (probability, really), of site-local values that
are specific to a given realm.  This is perhaps implied by the mention of
"the name of the Pre-Authentication mechanism used" but not stated
explicitly.  The interoperability requirements for site-local values are
less stringent, but there still needs to be a clear understanding of what
the contents of the protocol field should be.

So, I think that section 3 should be more stringent, saying something like
"strings that contain a colon character MUST be URIs that reference a
Level of Assurance Profile, leaving other strings available for
site-local use".

I also suspect that the last paragraph of the security considerations
should be rewritten in a normative fashion and moved to section 3, to make
it clear what the presence of a given string in the authdata indicates.
(Do we know of reasons why someone might use this AD element to indicate
something other than a positive indication that the indicated requirements
were met during the initial authentication?)

There's also some matter of form regarding the string
"AD-AUTHENTICATION-INDICATOR", which should probably just be used for the
definition of the ASN.1 type; the value 97 is the ad-type for which the
contents of-the ad-data field will be the DER encoding of the
AD-AUTHENTICATION-INDICATOR object.  So, I would just say "The KDC MAY
include authorization data of ad-type 97, wrapped in AD-CAMMAC, [...]" and
skip the line that's just

AD-AUTHENTICATION-INDICATOR 97

(and replace the colon with a full stop).



Some other editorial notes:

In the Abstract, s/the service tickets/service tickets/

In the Introduction, hyphenate "password-based".  It's probably good to
mention "or other preauthentication schemes" after public-key cryptography
(which, hmm, should probably also be hyphenated), and make it into a
comma-separated list.  Is "have" the best word in "[i]mplmentations that
have pre-authentication-mechanisms [...]"?  Maybe "offer", "support", or
"provide"?  In the last sentence of the first paragraph, maybe "strength
of the authentication that was used, for use as an input" would remove
some ambiguity.  In the second paragraph, maybe "to convey authentication
strength information", and "Elements of this type [will|MUST] appear
within an AD-CAMMAC".  Also, it's probably worth a clause briefly saying
why the CAMMAC is needed (to provide authenticity/integrity when a ticket
is presented back to the KDC).

The 2119 boilerplate doesn't list "NOT RECOMMENDED" but you don't use it,
so maybe it doesn't need to change.

In Section 3, it may be better to split the first sentence into two, so
"The KDC MAY copy it from a ticket-granting ticket into service tickets"
is a separate sentence, since the KDC can do the first without ever
needing to do the latter.

Still in Section 3, put a comma before "such as the name of the
Pre-Authentication mechanism used".  (Also, I sort-of want to ask "how
short is a short string?", but there's not really a useful answer to give,
particularly since the LoA URIs are in play and are likely to be longer
than non-URI strings.)


Thanks,

Ben

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten
Reply | Threaded
Open this post in threaded view
|

Re: [kitten] shepherd review of draft-ietf-kitten-krb-auth-indicator-02

Greg Hudson
On 09/25/2016 07:20 PM, Benjamin Kaduk wrote:
> It feels like some parts of the document are written as if the "short
> strings" for comparison against known values are only for use when
> presented back to the KDC, but other parts of the document describe it as
> being used for policy decisions made by application services as well as
> the KDC.

Application servers can use auth indicators.  What parts of the document
imply that they are only used by the KDC?

> So, I think that section 3 should be more stringent, saying something like
> "strings that contain a colon character MUST be URIs that reference a
> Level of Assurance Profile, leaving other strings available for
> site-local use".

The existing text in section 3 is pretty clear to me, but if we need to
make it more clear, I suggest replacing:

    These
    strings MAY be site-defined strings that do not contain a colon such
    as the name of the Pre-Authentication mechanism used, or
    alternatively URIs that reference a Level of Assurance Profile
    [RFC6711].

with:

    Each string MUST be either:

    * A URI which references a Level of Assurance Profile [RFC6711].

    * A site-defined string, which MUST NOT contain a colon, whose
      meaning is determined by the realm administrator.

> I also suspect that the last paragraph of the security considerations
> should be rewritten in a normative fashion and moved to section 3, to make
> it clear what the presence of a given string in the authdata indicates.
> (Do we know of reasons why someone might use this AD element to indicate
> something other than a positive indication that the indicated requirements
> were met during the initial authentication?)

That seems like a reasonable change to me.

> There's also some matter of form regarding the string
> "AD-AUTHENTICATION-INDICATOR", which should probably just be used for the
> definition of the ASN.1 type; the value 97 is the ad-type for which the
> contents of-the ad-data field will be the DER encoding of the
> AD-AUTHENTICATION-INDICATOR object.  So, I would just say "The KDC MAY
> include authorization data of ad-type 97, wrapped in AD-CAMMAC, [...]" and
> skip the line that's just
>
> AD-AUTHENTICATION-INDICATOR 97
>
> (and replace the colon with a full stop).

That change also seems okay to me.

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten
Reply | Threaded
Open this post in threaded view
|

Re: [kitten] shepherd review of draft-ietf-kitten-krb-auth-indicator-02

Benjamin Kaduk-2
On Mon, 26 Sep 2016, Greg Hudson wrote:

> On 09/25/2016 07:20 PM, Benjamin Kaduk wrote:
> > It feels like some parts of the document are written as if the "short
> > strings" for comparison against known values are only for use when
> > presented back to the KDC, but other parts of the document describe it as
> > being used for policy decisions made by application services as well as
> > the KDC.
>
> Application servers can use auth indicators.  What parts of the document
> imply that they are only used by the KDC?

I don't think there's anything explicit, just some implicit implication by
way of not concretely specifying the structure enough to be fully portable
without site-local configuration.

> > So, I think that section 3 should be more stringent, saying something like
> > "strings that contain a colon character MUST be URIs that reference a
> > Level of Assurance Profile, leaving other strings available for
> > site-local use".
>
> The existing text in section 3 is pretty clear to me, but if we need to
> make it more clear, I suggest replacing:
>
>     These
>     strings MAY be site-defined strings that do not contain a colon such
>     as the name of the Pre-Authentication mechanism used, or
>     alternatively URIs that reference a Level of Assurance Profile
>     [RFC6711].
>
> with:
>
>     Each string MUST be either:
>
>     * A URI which references a Level of Assurance Profile [RFC6711].
>
>     * A site-defined string, which MUST NOT contain a colon, whose
>       meaning is determined by the realm administrator.

That seems like an improvement to me, in that it is more likely to result
in interoperable implementations.

> > I also suspect that the last paragraph of the security considerations
> > should be rewritten in a normative fashion and moved to section 3, to make
> > it clear what the presence of a given string in the authdata indicates.
> > (Do we know of reasons why someone might use this AD element to indicate
> > something other than a positive indication that the indicated requirements
> > were met during the initial authentication?)
>
> That seems like a reasonable change to me.
>
> > There's also some matter of form regarding the string
> > "AD-AUTHENTICATION-INDICATOR", which should probably just be used for the
> > definition of the ASN.1 type; the value 97 is the ad-type for which the
> > contents of-the ad-data field will be the DER encoding of the
> > AD-AUTHENTICATION-INDICATOR object.  So, I would just say "The KDC MAY
> > include authorization data of ad-type 97, wrapped in AD-CAMMAC, [...]" and
> > skip the line that's just
> >
> > AD-AUTHENTICATION-INDICATOR 97
> >
> > (and replace the colon with a full stop).
>
> That change also seems okay to me.

Thanks for the review.

-Ben

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten
Reply | Threaded
Open this post in threaded view
|

Re: [kitten] shepherd review of draft-ietf-kitten-krb-auth-indicator-02

Nathaniel McCallum-5
In reply to this post by Benjamin Kaduk-2
On Sun, 2016-09-25 at 19:20 -0400, Benjamin Kaduk wrote:

> I also suspect that the last paragraph of the security considerations
> should be rewritten in a normative fashion and moved to section 3, to
> make
> it clear what the presence of a given string in the authdata
> indicates.
> (Do we know of reasons why someone might use this AD element to
> indicate
> something other than a positive indication that the indicated
> requirements
> were met during the initial authentication?)

IMHO, I think this is not necessary. Section 3 already contains this
paragraph:

"Each UTF8String value is a short string that indicates that a
particular set of requirements was met during the initial
authentication."

This sentence is the normative definition of precisely what an
indicator is. The paragraph in the security considerations section is a
warning about what would happen if you tried to use indicators for
another purpose: ambiguity would arise. This is a security
consideration.

This warning is necessary precisely because the majority use of
indicators will be as site-defined strings.

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten
Reply | Threaded
Open this post in threaded view
|

Re: [kitten] shepherd review of draft-ietf-kitten-krb-auth-indicator-02

Nathaniel McCallum-5
In reply to this post by Greg Hudson
I submitted a new draft with these changes. I only omitted the change
to the security considerations section. We can discuss that further if
need be. I stated my reasons for keeping the existing wording in
another email.

On Mon, 2016-09-26 at 12:28 -0400, Greg Hudson wrote:

> On 09/25/2016 07:20 PM, Benjamin Kaduk wrote:
> > It feels like some parts of the document are written as if the
> > "short
> > strings" for comparison against known values are only for use when
> > presented back to the KDC, but other parts of the document describe
> > it as
> > being used for policy decisions made by application services as
> > well as
> > the KDC.
>
>
> Application servers can use auth indicators.  What parts of the
> document
> imply that they are only used by the KDC?
>
> > So, I think that section 3 should be more stringent, saying
> > something like
> > "strings that contain a colon character MUST be URIs that reference
> > a
> > Level of Assurance Profile, leaving other strings available for
> > site-local use".
>
>
> The existing text in section 3 is pretty clear to me, but if we need
> to
> make it more clear, I suggest replacing:
>
>     These
>     strings MAY be site-defined strings that do not contain a colon
> such
>     as the name of the Pre-Authentication mechanism used, or
>     alternatively URIs that reference a Level of Assurance Profile
>     [RFC6711].
>
> with:
>
>     Each string MUST be either:
>
>     * A URI which references a Level of Assurance Profile [RFC6711].
>
>     * A site-defined string, which MUST NOT contain a colon, whose
>       meaning is determined by the realm administrator.
>
> > I also suspect that the last paragraph of the security
> > considerations
> > should be rewritten in a normative fashion and moved to section 3,
> > to make
> > it clear what the presence of a given string in the authdata
> > indicates.
> > (Do we know of reasons why someone might use this AD element to
> > indicate
> > something other than a positive indication that the indicated
> > requirements
> > were met during the initial authentication?)
>
>
> That seems like a reasonable change to me.
>
> > There's also some matter of form regarding the string
> > "AD-AUTHENTICATION-INDICATOR", which should probably just be used
> > for the
> > definition of the ASN.1 type; the value 97 is the ad-type for which
> > the
> > contents of-the ad-data field will be the DER encoding of the
> > AD-AUTHENTICATION-INDICATOR object.  So, I would just say "The KDC
> > MAY
> > include authorization data of ad-type 97, wrapped in AD-CAMMAC,
> > [...]" and
> > skip the line that's just
> >
> > AD-AUTHENTICATION-INDICATOR 97
> >
> > (and replace the colon with a full stop).
>
>
> That change also seems okay to me.
>
> _______________________________________________
> Kitten mailing list
> [hidden email]
> https://www.ietf.org/mailman/listinfo/kitten
>

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten
Reply | Threaded
Open this post in threaded view
|

Re: [kitten] shepherd review of draft-ietf-kitten-krb-auth-indicator-02

Benjamin Kaduk-2
On Wed, 28 Sep 2016, Nathaniel McCallum wrote:

> I submitted a new draft with these changes. I only omitted the change
> to the security considerations section. We can discuss that further if
> need be. I stated my reasons for keeping the existing wording in
> another email.

Thanks for the updates.  I do not insist on changes to the security
considerations text, but will not be surprised if a GenART/Opsdir/secdir
reviewer re-raises the question.

I do have one question, though: the new version says that the CAMMAC
requirement "exists to provide integrity protection from man-in-the-middle
attacks", which is a bit odd, since AuthorizationData appear within the
EncTicketPart (and the auth-indicator is not expected to appear "bare" in
KDC-REQ or Authenticators, since it is supposed to be KDC-issued).  So,
unfortunately, that sentence still leaves me confused.

-Ben

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten
Reply | Threaded
Open this post in threaded view
|

[kitten] intended status and Updates: 4120 for draft-ietf-kitten-krb-auth-indicator-02

Benjamin Kaduk-2
Sorry for the long silence on this one; certain checklist steps
essentially require contiguous blocks of time that are sometimes hard
to come by.

One of the checklist items is to look at the intended status and any
documents updated by the draft; in a look through the archives it seems
that we never explicitly mentioned that this draft is labelled as
targetting Standards-Track and is marked as Updating RFC 4120.

I think that Standards-Track is fine, and there is not full agreement
in the IETF as to what exactly the "Updates" tag means (long thread at
https://www.ietf.org/mail-archive/web/wgchairs/current/msg14618.html).
The argument for keeping the "Updates" tag would be that it is adding
a new AD type that we want implementations of 4120 to also implement
(CAMMAC has a stronger argument for "Updates" since it replaces
KDC-ISSUED).

My default action is to leave the document as-is, with target status
of Standards-Track and Updates: 4120, but in lieu of having this
be part of a formal WGLC, I will explicitly call it out on the WG
list in case there are objections.


On Fri, Sep 30, 2016 at 12:01:58AM -0400, Benjamin Kaduk wrote:
>
> I do have one question, though: the new version says that the CAMMAC
> requirement "exists to provide integrity protection from man-in-the-middle
> attacks", which is a bit odd, since AuthorizationData appear within the
> EncTicketPart (and the auth-indicator is not expected to appear "bare" in
> KDC-REQ or Authenticators, since it is supposed to be KDC-issued).  So,
> unfortunately, that sentence still leaves me confused.

I think this comment remains unaddressed.

-Ben

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten
Reply | Threaded
Open this post in threaded view
|

Re: [kitten] intended status and Updates: 4120 for draft-ietf-kitten-krb-auth-indicator-02

Greg Hudson
On 11/17/2016 02:13 AM, Benjamin Kaduk wrote:
>> I do have one question, though: the new version says that the CAMMAC
>> requirement "exists to provide integrity protection from man-in-the-middle
>> attacks", which is a bit odd, since AuthorizationData appear within the
>> EncTicketPart (and the auth-indicator is not expected to appear "bare" in
>> KDC-REQ or Authenticators, since it is supposed to be KDC-issued).  So,
>> unfortunately, that sentence still leaves me confused.

I agree with Ben that this statement is incorrect.

If an auth indicator appeared with no container, or in an if-relevant
container, a server would not know if the value came from the KDC or
from the client (which can request arbitrary authdata at AS-REQ or
TGS-REQ time).

If an auth indicator appeared in a kdc-issued container, a server could
trust it, but the indicator couldn't be safely propagated over an
S4U2Proxy request.  Although that concern might not be relevant in all
deployments, we are requiring CAMMAC for simplicity, using the model
that CAMMAC is an upgraded kdc-issued.

I suggest removing the offending text; I cannot think of a pithy
replacement and I don't think we need it.

_______________________________________________
Kitten mailing list
[hidden email]
https://www.ietf.org/mailman/listinfo/kitten