Re: [kitten] AD review of draft-ietf-kitten-pkinit-alg-agility-03

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

Re: [kitten] AD review of draft-ietf-kitten-pkinit-alg-agility-03

Benjamin Kaduk-2
[adding the WG list, sigh]

On Wed, Jan 30, 2019 at 04:45:55PM -0600, Benjamin Kaduk wrote:

> Hi all,
>
> I just have a number of editorial comments that we should probably tighten
> up before we send this out for IETF-wide review.  Sorry for taking so long
> to get to this :(
>
> The term "HKDF" has been claimed by RFC 5869 for HMAC-based
> Extract-and-Expand, which is not what we're doing here; we should avoid
> using that acronym, perhaps in favor of "hash-based KDF".
>
> In the Abstract and Introduction, it looks like we need an "and" in the
> list of "PKINIT key derivation function is made negotiable, and the digest
> algorithms for signing the pre-authentication data and the client's X.509
> certificate", since the latter two items are both sub-items of "signing",
> thus there are only two items in the top-level listing.
>
> It's probably worth a(n informative) reference to BCP 201 when we say that
> the IETF "called for actions to update xisting protocols to provide crypto
> algorithm agility" in the Introduction.
>
> Basically every "official" reviewer is going to note that BCP 14 now
> includes RFC 8174 as well as RFC 2119, with updated boilerplate text.
> Let's save them the time of having to note it :)
>
> Section 3
>
>    and the corresponding Kerberos request body.  This also prevents the
>    KDC body from being tampered with.  SHA-1 is the only allowed
>
> Is this "KDC_REQ body"?
>
>    When the reply key delivery mechanism is based on public key
>    encryption as described in Section 3.2.3. of [RFC4556], the
>
> I can't decide if 3.2.3 or 3.2.3.2 is a better reference for this.
>
>               However, if the reply key delivery mechanism is based on
>    the Diffie-Hellman key agreement as described in Section 3.2.3.1 of
>    [RFC4556], the security provided by using SHA-1 in the paChecksum is
>    weak.  [...]
>
> It might be worth noting more explictly that "there is nothing else to
> cryptographically bind the AS_REQ to the ticket response", to emphasize the
> difference to the public-key-encryption method that already digests the
> entire AS_REQ into the authenticated content, using a (strong) negotiated
> algorithm.
>
> Section 4
>
>    TD-CMS-DIGEST-ALGORITHMS-DATA ::= SEQUENCE OF
>        AlgorithmIdentifier
>            -- Contains the list of CMS algorithm [RFC5652]
>            -- identifiers that identify the digest algorithms
>            -- acceptable by the KDC for signing CMS data in
>            -- the order of decreasing preference.
>
> nit: "identifiers that identify" is a somewhat unfortunate construction;
> perhaps we could go with "identifiers that indicate" instead.
>
> Section 5
>
>    When the client's X.509 certificate is rejected and the
>    KDC_ERR_DIGEST_IN_SIGNED_DATA_NOT_ACCEPTED error is returned as
>    described in section 3.2.2 of [RFC4556], conforming implementations
>    can OPTIONALLY send a list of digest algorithms acceptable by the KDC
>
> We should probably use "implementations that conform to this specification"
> to match Section 4.  (Though we may get some complaints about using
> "conforming" at all, too.)
>
> Setion 6
>
>    New KDFs defined in this document based on [SP80056A] can be used in
>    conjunction with any hash functions.
>
> There's some disagreement here between "KDFs defined in this document" and
> "any hash functions" -- either we treat a KDF as tied to a specific hash
> (which is how we allocate OIDs) or it is a generic construct that can be
> instantiated for any given hash (in which case there is only one KDF not
> multiple).  We also want to be consistent with how we treat KDFs as
> identified by OIDs in the following text, so I think something more like
> "The KDF algorithm described in this document (based on [SP80056A]) can be
> implemented using any cryptographic hash function" and "A new KDF for
> PKINIT usage is identified by an object identifier" in the next paragraph.
>
>    Where id-pkinit is defined in [RFC4556].  The id-pkinit-kdf-ah-sha1
>    KDF is the ASN.1 structured hash based KDF (HKDF) [SP80056A] that
>    uses SHA-1 [RFC6234] as the hash function.  Similarly id-pkinit-kdf-
>    ah-sha256 and id-pkinit-kdf-ah-sha512 are the ASN.1 structured HKDF
>    using SHA-256 [RFC6234] and SHA-512 [RFC6234] respectively.
>
> We seem to have not added SHA-384 to this paragraph when we added it
> elsewhere.
>
>    To name the input parameters, an abbreviated version of the ASN.1
>    version of the KDF from [SP80056A] is described below, use [SP80056A]
>    for the full reference.
>
> [SP80056A] seems to call this a "KDM" and not a "KDF"; we probably want to
> call out the terminology mismatch and perhaps also the section number
> (5.8?).  Also, the actual algorithm seems to be coming from Section 4 of
> SP800-56C, not SP800-56A, if we want to be accurate about our reference.
>
> On the other hand, we could also save ourselves from worrying about the
> precise references as much if we avoided describing ours as "an
> abbreviated version of" and just say something like "inspired by" or
> "similar to", which allows us to treat our formulation as the single
> authoritative specification for what to do, without leaving the reader
> needing to check back and see if there are any missing details.  (This
> would probably allow [SP80056A] (or C) to be only an informative reference,
> too.)
>
>    4.  Set key_material = Hash1 || Hash2 || ... so that length of key is
>        K bits.
>
> I think we need to say something about "truncating as needed".
>
>    o  The maximum hash input length is 2^64 in bits.
>
> I'm confused where this parameter comes from.  On the one hand, that's an
> absurdly huge value; on the other hand, why do we need to give a limit at
> all?
>
> I think we need to say specifically that we use the DER encoding of
> OtherInfo as input to the KDF.
>
>    PkinitSuppPubInfo ::= SEQUENCE {
>           enctype           [0] Int32,
>                  -- The enctype of the AS reply key
>           as-REQ            [1] OCTET STRING,
>               -- This contains the AS-REQ in the request.
>           pk-as-rep         [2] OCTET STRING,
>               -- Contains the DER encoding of the type
>               -- PA-PK-AS-REP [RFC4556] in the KDC reply.
>           ...
>    }
>
> A couple nits in here -- is the AS-REQ "in" the request or perhaps "from"
> or "of" it?  Also, do we need the word "type" in the pk-as-rep description?
>
> I'm not entirely sure why we need an extension marker in KDFAlgorithmId
> (since we could just allocate new OIDs for the specific parameters/etc. we
> would otherwise want to put in), but I don't object to it.
>
>    If the supportedKDFs field is not present in the request, the kdf
>    field in the reply MUST be absent.
>
> Somehwere in here we should probably say that if the supportedKDFs field is
> absent, implementations continue to use the RFC 4556 KDF (if they continue
> at all).
>
>    If the client fills the supportedKDFs field in the request, but the
>    kdf field in the reply is not present, the client can deduce that the
>    KDC is not updated to conform with this specification.  In that case,
>    it is a matter of local policy on the client whether to reject the
>    reply when the kdf field is absent in the reply.
>
> I think we need to mention that this implies the server is using the old,
> insecure KDF function or that there is an active downgrade attack, and that
> the expected steady state is to reject this reply as a downgrade attack.
>
> Section 7.1
>
> We may get someone asking us to use RFC 6761 example domain names instead
> of SU.SE in the examples, but I don't see a need to go through the effort
> of regenerating the test vectors right now as a precaution.
>
> Section 8
>
>    This document describes negotiation of checksum types, key derivation
>    functions and other cryptographic functions.  If negotiation is done
>    unauthenticated, care MUST be taken to accept only acceptable values.
>
> nit: I'd suggest rephrasing as "If a given negotiation is unauthenticated,
> care must be taken to accept only secure values; to do otherwise allows a
> MITM to perform a downgrade attack".
>
>
> Thanks for all the good work on this document!  Hopefully we can get it
> across the finish line soon.
>
> -Ben
>

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

Re: [kitten] AD review of draft-ietf-kitten-pkinit-alg-agility-03

Greg Hudson
On Wed, Jan 30, 2019 at 04:45:55PM -0600, Benjamin Kaduk wrote:
>    To name the input parameters, an abbreviated version of the ASN.1
>    version of the KDF from [SP80056A] is described below, use [SP80056A]
>    for the full reference.
>
> [SP80056A] seems to call this a "KDM" and not a "KDF"; we probably want to
> call out the terminology mismatch and perhaps also the section number
> (5.8?).  Also, the actual algorithm seems to be coming from Section 4 of
> SP800-56C, not SP800-56A, if we want to be accurate about our reference.

The draft reference was accurate for the original SP800-56A, but there
has been some drift in revisions to the NIST document.  Fortunately,
none of the changes invalidate our formulation of the KDF.

The revisions do seem to have introduced the term KDM as you say,
perhaps to distinguish key derivation beginning with a DH shared secret
from key derivation beginning with an existing key (SP800-108).
However, the terms "key-derivation function" and "KDF" still appear in
SP800-56C several times, such as at the top of section 4.  So I didn't
see a need to call out the terminology difference.

I updated the references to account for the SP800-56 revisions, and
changed the terminology in the abbreviated description of the KDF to
match the revised NIST recommendations.

>    o  The maximum hash input length is 2^64 in bits.
>
> I'm confused where this parameter comes from.  On the one hand, that's an
> absurdly huge value; on the other hand, why do we need to give a limit at
> all?

SP800-56C section 4.1 has max_H_inputBits as an implementation-dependent
parameter.  (The original SP800-56A had something similar.)  It is only
used for an error check.  Since SHA-1 and SHA-2 hash functions do not
limit the hash input size, we just need to set a value large enough not
to trigger the error check.

>    If the supportedKDFs field is not present in the request, the kdf
>    field in the reply MUST be absent.
>
> Somehwere in here we should probably say that if the supportedKDFs field is
> absent, implementations continue to use the RFC 4556 KDF (if they continue
> at all).

We might also give the KDC the option of rejecting the request, if it
does not expect requests from old clients.  (I didn't make that change
in my revision as that wouldn't have been editorial, but I did clarify
that the RFC 4556 KDF must be used.)

I believe I addressed all of your other comments in the -04 update.

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

Re: [kitten] AD review of draft-ietf-kitten-pkinit-alg-agility-03

Benjamin Kaduk-2
On Sun, Feb 03, 2019 at 11:52:54AM -0500, Greg Hudson wrote:

> On Wed, Jan 30, 2019 at 04:45:55PM -0600, Benjamin Kaduk wrote:
> >    To name the input parameters, an abbreviated version of the ASN.1
> >    version of the KDF from [SP80056A] is described below, use [SP80056A]
> >    for the full reference.
> >
> > [SP80056A] seems to call this a "KDM" and not a "KDF"; we probably want to
> > call out the terminology mismatch and perhaps also the section number
> > (5.8?).  Also, the actual algorithm seems to be coming from Section 4 of
> > SP800-56C, not SP800-56A, if we want to be accurate about our reference.
>
> The draft reference was accurate for the original SP800-56A, but there
> has been some drift in revisions to the NIST document.  Fortunately,
> none of the changes invalidate our formulation of the KDF.
>
> The revisions do seem to have introduced the term KDM as you say,
> perhaps to distinguish key derivation beginning with a DH shared secret
> from key derivation beginning with an existing key (SP800-108).
> However, the terms "key-derivation function" and "KDF" still appear in
> SP800-56C several times, such as at the top of section 4.  So I didn't
> see a need to call out the terminology difference.
>
> I updated the references to account for the SP800-56 revisions, and
> changed the terminology in the abbreviated description of the KDF to
> match the revised NIST recommendations.
>
> >    o  The maximum hash input length is 2^64 in bits.
> >
> > I'm confused where this parameter comes from.  On the one hand, that's an
> > absurdly huge value; on the other hand, why do we need to give a limit at
> > all?
>
> SP800-56C section 4.1 has max_H_inputBits as an implementation-dependent
> parameter.  (The original SP800-56A had something similar.)  It is only
> used for an error check.  Since SHA-1 and SHA-2 hash functions do not
> limit the hash input size, we just need to set a value large enough not
> to trigger the error check.
>
> >    If the supportedKDFs field is not present in the request, the kdf
> >    field in the reply MUST be absent.
> >
> > Somehwere in here we should probably say that if the supportedKDFs field is
> > absent, implementations continue to use the RFC 4556 KDF (if they continue
> > at all).
>
> We might also give the KDC the option of rejecting the request, if it
> does not expect requests from old clients.  (I didn't make that change
> in my revision as that wouldn't have been editorial, but I did clarify
> that the RFC 4556 KDF must be used.)
>
> I believe I addressed all of your other comments in the -04 update.

Thanks for all these updates (and explanation of non-updates); I've gone
ahead and requested the IETF Last Call.

-Ben

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