Re: [radext] Shepherd review of draft-ietf-radext-tls-psk-03

[Valery Smyslov <[e-mail]>]

Hi Alan,

thank you for quick response.

> On Nov 1, 2023, at 10:33 AM, Valery Smyslov <[e-mail]> wrote:
> > this is a shepherd review of draft-ietf-radext-tls-psk-03.
> > First, as a shepherd, I have to ask the following questions:
> >
> > 1. According to BCP79 all authors and contributors should either disclose
> >    any known to them IPRs related to the draft or confirm that they are not aware of such IPRs.
> 
>   I have no IPR related to this draft.

This is not enough. Please confirm, that you are not _aware_ of any IPRs 
(which may be held by others, not only by you).

>   Unless noted below, I've followed the recommendations.
> 
> > 2. Are there existing implementations that follow this draft?
> 
>   FreeRADIUS has implemented TLS-PSK for a long time.  Radsecproxy also implements it.
> 
> > 5. Section 4.1
> >    I don't think that referencing RFC8937 is appropriate in the context of this document.
> >    The idea of improving randomness in RFC8937 is that a long term signature private
> >     key is used to add an entropy to a random value. And this document assumes that
> >     peers don't have certificates (thus don't have long term signature private keys).
> >     So, it's a bit weird to advice using them. Perhaps we could just reference RFC4086 instead?
> 
>   OK.
> 
> > 6.I think that a sentence should be added somewhere, that unless specifically clarified,
> >    by "TLS" both TLS and DTLS are meant in the document.
> >    For example, RFC 9325 has the following text:
> >
> >   Unless it is explicitly called out that a recommendation applies to
> >   TLS alone or to DTLS alone, each recommendation applies to both TLS
> >   and DTLS.
> 
>   I'll add that in the introduction.
> 
> > 7. Section 4.1
> >    I think that mentioning PAKE TLS needs reference (perhaps RFC5054 and RFC8492, something else?).
> 
>   Added.
> 
> > Text improvements.
> > 1. Section 4.1.1
> >    Just to avoid any confusion, can we clarify that by "Shared Secrets" here "RADIUS Shared Secrets" are
> meant?
> >    Because PSKs are also "shared secrets", so there is some overlap in terminology and a room for
> confusion.
> 
>   I've added a sentence in the introduction:
> 
> This document uses "shared secret" to mean "RADIUS shared secret", and Pre-Shared Key (PSK) to mean
> secrets which are used with TLS-PSK.

Works for me, thanks.

> > 2. Section 5.1
> >     In the text "Due to the security issues described above" it is not immediately clear
> >     what issues are meant. Can we replace "above" with a reference to a Section
> >     where these issues described?
> 
>   I've updated it to say instead:
> 
> The historic methods of signing RADIUS packets have not yet been cracked, but they are believed to be
> much less secure than modern TLS.  Theregore, when a RADIUS shared secret is used to sign
> RADIUS/UDP or RADIUS/TCP packets, that shared secret MUST NOT be used with TLS-PSK.  If the secrets
> were to be reused, then an attack on historic RADIUS cryptography could be trivially leveraged to
> decrypt TLS-PSK sessions.  Therefore in order to prevent confusion between shared secrets and TLS-PSKs,
> management interfaces and APIs need to label PSK fields as "PSK" or "TLS-PSK", rather than as "shared
> secret".

Fine.

> > 3. Section 5.1
> >     I think that the text:
> > ...
> >   can be rephrased for better clarity. Perhaps:
> >
> >   With TLS-PSK RADIUS/TLS clients MUST permit the configuration of a RADIUS
> >   server IP address or host name, because dynamic server lookups [RFC7585]
> >   can only be used if servers use certificates.
> 
>   Done.
> 
> > The following issues are not formal and probably require some discussion.
> > So I want to identify them here mostly from SECRIR reviewer point of view.
> >
> > 1. Section 4.1
> >    There are requirements on PSKs length, but it is not mentioned that
> >    they must be uniformly random, which is important for the security of TLS-PSK.
> >    Perhaps this is implied, but it's better to spell this out.
> 
>   I'll add a note.

Great.

> >    And then I'm a bit concerned that a script generated PSK is provided.
> >    First, the script "as is" is inconsistent with the requirement above
> >    (I assume that PSKs are random), because it generates only 12 bytes
> >    of random data, while the requirement is that PSKs MUST be at least 16 bytes.
> >    Yes, there is a text that if you want more, just modify the script,
> >    but most implementers will just copy-paste it. Please, don't provoke them
> >    to make wrong things.
> 
>   I can update that to generate 16 bytes of random data.  That change was already made in my local copy.

OK.

> >    In addition, I'm not sure that converting PSKs to a human-readable form is a good idea.
> >    This conversion makes PSKs weaker (the length is increased, while the initial entropy
> >    persists, the distribution becomes non-uniform). In my understanding PSKs often
> >    are entered to the systems just referencing a file containing them, not by entering them letter by
> letter.
> 
>   I suppose it depends on the UI how they are entered.  It's likely worth adding a note that PSKs should
> be taken either from a file, or if the admin needs to enter them, from a hex string.

Yes, please.

>   If the PSKs are hashed before they're used, a non-uniform distribution shouldn't cause issues, I think.

Sure. I just wanted to make a point that with uniform distribution we have the smallest PSKs containing 
the maximum entropy for their length.

> > 2. Section 6.2.
> >    In the text:
> >
> >   The alternative would be to suggest that RADIUS servers allow any
> >   source IP address to connect and try TLS-PSK, which could be a
> >   security risk.
> >
> >   I think that we should explain what security risk is meant.
> >   Otherwise it looks like handwaving.
> 
>    How about:
> 
> In order to securely support dynamic source IP addresses for clients, we also require that servers limit
> clients based on a network range.  The alternative would be to suggest that RADIUS servers allow any
> source IP address to connect and try TLS-PSK, which could be a security risk.  When RADIUS servers do
> no source IP address filtering, that opennes it easier for attackers to send malicious traffic to the server.
> An issue with a TLS library or even a TCP/IP stack could permit the attacker to gain unwarranted access.
> In contrast, when IP address filtering is done, attackers generally must first gain access to a secure
> network before attacking the RADIUS server.

OK, works for me.

> > 3. Section 6.2.
> >    The following assertion:
> >
> >   It is significantly easier for an attacker to crack a PSK than
> >   to forge a client certificate.
> >
> >   is not true in general (with sufficiently strong PSKs) and
> >   is plain wrong if we talk about quantum computers, which
> >   will break any certificate but cannot break PSKs. So, either
> >   more context should be given (e.g. we assume that
> >   certificate private keys are stored in HSMs, while
> >   PSKs are not etc.) or this assertion should be removed.
> 
>   OK.  How about:
> 
> Even where {{RFC7585}} dynamic discovery is not used, servers SHOULD NOT permit TLS-PSK to be used
> across the wider Internet.  The intent for TLS-PSK is for it to be used in internal / secured networks,
> where clients come from a small number of known locations.  In contrast, certificates can be generated
> and assigned to clients without any interaction with the RADIUS server.  Therefore if the RADIUS server
> needs to accept connections from clients at unknown locations, the only secure method is to use client
> certificates.

Works for me too.

> >   In my understanding this assertion justifies the requirement
> >   for servers to filter clients' IP addresses when PSKs are used.
> >   Perhaps it is better to justify this requirement by mentioning,
> >   that PSKs are provided to a known clients which location
> >   is usually well-known, thus we can add a safeguard on IP level.
> >   On contrast, certificates can be obtained by clients from CAs
> >   without servers prior knowledge, so it is generally impossible
> >   to filter out IP addresses of (not-yet-known) clients' in this case.
> 
>   Updated text as above to discuss this.
> 
> > 4. Section 6.2.1
> >   The text:
> >
> >   A server supporting this specification MUST be configurable with a
> >   set of "allowed" network ranges from which clients are permitted to
> >   connect.
> >
> >   needs a clarification that only clients wishing to use PSK are considered here.
> >   Perhaps:
> >
> >   A server supporting this specification MUST be configurable with a
> >   set of "allowed" network ranges from which clients wishing to use PSK in TLS
> >   are permitted to connect.
> 
>   How about:
> 
> A server supporting this specification MUST be configurable with a set of "allowed" network ranges from
> which clients are permitted to connect.  Any connection from outside of the allowed range(s) MUST be
> rejected before any PSK identity is checked.  It is RECOMMENDED that servers support IP address
> filtering even when TLS-PSK is not used.  There are few reasons for a server to have a default
> configuration which allows connections from any source IP address.

I think this text still doesn't address my point. The point is that servers
may support both this specification and certificates. In this case
they should be able to only filter out IP addresses for those clients,
which are connecting with TLS-PSK and should be configurable
to allow any IP address for clients wishing to use certificates.

How about:

A server supporting this specification MUST be configurable with a set of "allowed" network ranges from
which clients wishing to use TLS-PSK are permitted to connect.  Any connection from outside of the allowed range(s) MUST be
rejected before any PSK identity is checked.  It is RECOMMENDED that servers support IP address
filtering even when TLS-PSK is not used.  There are few reasons for a server to have a default
configuration which allows connections from any source IP address.

(MUST filter for TLS-PSK and RECOMMENDED for other cases).

> >   Then, an interesting question is what to do with clients that wish to use
> >   both certificates and PSK (see RFC 8773)? Should servers filter out
> >   IP addresses of these clients or not? I think some clarification is needed in the draft.
> 
>   See above.  It is useful to *always* filter by IPs, even when certificates are being used.  I can't see any
> reason why a server supporting TLS + certificates would default to allowing clients from any source IP.

Well, with the proposed text above, I think that my point is addressed.

Regards,
Valery.

>   Alan DeKok.