Subject: Re: [OPSAWG] Roman Danyliw's Discuss on draft-ietf-opsawg-tacacs-13: (with DISCUSS and COMMENT)

From: Warren Kumari <[email-masked]>
Date: Thu, 6 Feb 2020 11:03:49 -0500

Hi there Roman,

I recently took over this document from Igans - it has been stuck in
'IESG Evaluation::AD Followup'  for 266 days (at version -13).
This is an informational document, describing an existing, and widely
deployed protocol -- the intent was that, once this is published,
there would be a new document largely saying "... great, now take
RFCxxxx, and throw it over TLS. Done!" (as you can guess, there is
much history here as to why it had to happen in two documents instead
of one :-))

The authors have made significant updates
(https://www.ietf.org/rfcdiff?url1=draft-ietf-opsawg-tacacs-13&url2=draft-ietf-opsawg-tacacs-16),
and believe that they have now addressed all open comments.

I'd really like to get this document out the door soon - would you
mind prioritizing checking if these changes address your concerns?
W

On Mon, Jan 27, 2020 at 3:23 PM Douglas Gash (dcmgash)
<dcmgash=[email-masked]> wrote:
>
> Hi,
>
> I hope that in the last few versions we have updated the document to sufficiently answer the concerns raised, please let me know if any concerns remain, many thanks.
>
> The majority of the issues were answered last summer, but the balance should be by the latest version recently uploaded.
>
> Please see the point-by point descriptions of changes or responses.
>
> Many thanks,
>
>
>
> On 15/05/2019, 20:12, "Roman Danyliw via Datatracker" <[email-masked]> wrote:--------------------------------------------------------------------------------------------------
>
>     (1) I appreciate the deliberate and thoughtful attempt in this section to
>     enumerate the possible risks/attacks and mitigations of the protocol as is.  In
>     addition to the top-level risks in Section 10.1, I can see the value of
>     maintaining symmetry between Sections 5+10.2; 6+10.3 and 7+10.4.  In the spirit
>     of the middle ground this draft is trying to realize (document the as-is, but
>     highlight the issues), I have the following feedback:
>
>     (a) Section 10.1.  I recommend replacing the first three paragraphs of Section
>     10.1 (“TACACS+ protocol does not …”, “While the protocol …”, and “Even though
>     …”) with the following text synthesized from Joe Salowey’s LC review
>     (https://mailarchive.ietf.org/arch/msg/secdir/rsqrNbVEKph1RdWh836Ard73pHs) and
>     the current introduction:
>
>     TACACS+ protocol does not include a security mechanism that would meet
>     modern-day requirements.  These security mechanisms would be best referred to
>     as “obfuscation” and not “encryption” since they provide no meaningful
>     integrity, privacy or replay protection.  An attacker with access to the data
>     stream should be assumed to be able to read and modify all TACACS+ packets.
>     Without mitigation, a range of risks such as the following are possible:
>
>     Accounting information may be modified by the man-in-the-middle attacker,
>     making such logs unsuitable and untrustable for auditing purposes.
>
>     Invalid or misleading values may be inserted by the man-in-the-middle attacker
>     in various fields at known offsets to try and circumvent the authentication or
>     authorization checks even inside the obfuscated body.
>
> TA>  Many thanks, looks like a  sensible proposal [AI=TA]
> After consultation, the foll;owing chnages were made:
>
> Section 10.1., paragraph 1:
> OLD:
>
>     TACACS+ protocol does not include a security mechanism that would
>     meet modern-day requirements.  Support for MD5-based crypto pad
>     encryption fails to provide any kind of transport integrity, which
>     presents at least the following risks:
>
> NEW:
>
>     TACACS+ protocol does not include a security mechanism that would
>     meet modern-day requirements.  These security mechanisms would be
>     best referred to as "obfuscation" and not "encryption" since they
>     provide no meaningful integrity, privacy or replay protection.  An
>     attacker with access to the data stream should be assumed to be able
>     to read and modify all TACACS+ packets.  Without mitigation, a range
>     of risks such as the following are possible:
>
>
> Section 10.1., paragraph 2:
> OLD:
>
>        Accounting information may be modified by the man-in-the-middle
>        attacker, making such logs unsuitable and untrustable for auditing
>        purposes.
>
>        Only the body of the request is obfuscated which leaves all header
>        fields open to trivial modification by the man-in-the-middle
>        attacker.  For this reason, deployments SHOULD NOT use connections
>        with TAC_PLUS_UNENCRYPTED_FLAG, as mentioned in the Best Practices
>        section (Section 10.5) .
>
> NEW:
>
>        Accounting information may be modified by the man-in-the-middle
>        attacker, making such logs unsuitable and not trustable for
>        auditing purposes.
>
>     (b) I recommend finding an alternative home and strengthening the text “For
>     this reason, deployments SHOULD NOT use connections with
>     TAC_PLUS_UNENCRYPTED_FLAG, as mentioned in the Best Practices section (Section
>     10.5)”.  It seemed odd to mix deployment guidance in a list of risks as
>     currently written.  I take Andrej Ota’s point from
>     https://mailarchive.ietf.org/arch/msg/secdir/UgtsSfh1RaauNoMRi87FRqtI0YI that
>     there is no harm in requiring the obfuscation, such as it is.  Furthermore, why
>     couldn’t this be MUST NOT use?
>
> TA> Yes, I think we can move to MUST NOT, and we can remove this reference at this point  [AI=TA]
> Specifcially: Reference removed here, and reinforced in section 10.5.
>
>     (c) Section 10.5.3.  I concur with the SECDIR recommendation and the follow-up
>     discussion with Andrej Ota per
>     https://mailarchive.ietf.org/arch/msg/secdir/UgtsSfh1RaauNoMRi87FRqtI0YI which
>     would: s/stronger authentication/less weak/
>
> TA> Agreed, We will update [AI=TA]
>
> Section 10.5.3., paragraph 1:
> OLD:
>
>     To help TACACS+ administraots select the stronger authentication
>     options, TACACS+ servers MUST allow the administrator to configure
>     the server to only accept challenge/response options for
>     authentication (TAC_PLUS_AUTHEN_TYPE_CHAP or
>     TAC_PLUS_AUTHEN_TYPE_MSCHAP or TAC_PLUS_AUTHEN_TYPE_MSCHAPV2 for
>     authen_type).
>
> NEW:
>
>     To help TACACS+ administrators select less weak authentication
>     options, TACACS+ servers MUST allow the administrator to configure
>     the server to only accept challenge/response options for
>     authentication (TAC_PLUS_AUTHEN_TYPE_CHAP or
>     TAC_PLUS_AUTHEN_TYPE_MSCHAP or TAC_PLUS_AUTHEN_TYPE_MSCHAPV2 for
>     authen_type).
>
>     (2) Section 10.2.  I’m confused by the deprecation of
>     TAC_PLUS_AUTHEN_STATUS_FOLLOW but a seemingly weaker “SHOULD NOT be used in
>     modern deployments”.  I was expecting a MUST NOT.
>
> TA> Agreed, We will update [AI=TA]
> Section 10.2., paragraph 2:
> OLD:
>
>     This document deprecates the redirection mechanism using the
>     TAC_PLUS_AUTHEN_STATUS_FOLLOW option which was included in the
>     original draft.  As part of this process, the secret key for a new
>     server was sent to the client.  This public exchange of secret keys
>     means that once one session is broken, it may be possible to leverage
>     that key to attacking connections to other servers.  This mechanism
>     SHOULD NOT be used in modern deployments.  It MUST NOT be used
>     outside a secured deployment.
>
> NEW:
>
>     This document deprecates the redirection mechanism using the
>     TAC_PLUS_AUTHEN_STATUS_FOLLOW option which was included in the
>     original draft.  As part of this process, the secret key for a new
>     server was sent to the client.  This public exchange of secret keys
>     means that once one session is broken, it may be possible to leverage
>     that key to attacking connections to other servers.  This mechanism
>     MUST NOT be used in modern deployments.  It MUST NOT be used outside
>     a secured deployment.
>
>     (3) Section 10.4.  Why shouldn’t accounting sessions also use secure transport
>     per 10.5 (like 10.3 and 10.4) given the risks outlined in the text?  I was
>     expecting to see this section open with “Accounting Session SHOULD be used via
>     a secure transport (see Best Practices section (Section 10.5))".
>
> TA> We’ll bring this in line [AI=TA]
> Specifically:
>
> Section 10.4., paragraph 1:
> OLD:
>
>     Accounting sessions are not directly involved in authentication or
>     authorizing operations on the device.  However, man-in-the-middle
>     attacker may do any of the following:
>
> NEW:
>
>     Accounting sessions SHOULD be used via a secure transport (see Best
>     Practices section (Section 10.5).  Although Accounting sessions are
>     not directly involved in authentication or authorizing operations on
>     the device, man-in-the-middle attacker may do any of the following:
>
>
> Section 10.4., paragraph 2:
> OLD:
>
>        Replace accounting data with new valid or garbage which prevents
>        to provide distraction or hide information related to their
>        authentication and/or authorization attack attempts.
>
> NEW:
>
>        Replace accounting data with new valid or garbage which can
>        confuse auditors or hide information related to their
>        authentication and/or authorization attack attempts.
>
>     ----------------------------------------------------------------------
>     COMMENT:
>     ----------------------------------------------------------------------
>
>     (1) Editorial Nits:
>
>     ** Section 10.5.3.  Typo.  s/administraots/administrators/
>     ** Global.  Various places in the document have an extra space between the end
>     of a reference and the closing period.  Recommend: s/] ./]./g
>
> TA> Thanks, will fix [AI=TA]
>
>     (2) I endorse Mirja and Deborah’s point that strong text is needed in Section 1
>     to state that this document is describing the current deployment of the
>     protocol which has serious security issues.
>
> TA> Agreed, added to the introduction: [AI=TA]
>
> Specifically added to introduction: "This did not address all of the
>     key security concerns which are considered when designing modern
>     standards.  Deployment must therefore be executed with care.  These
>     concerns are addressed in the security section (Section 10)."
>
>
>

-- 
I don't think the execution is relevant when it was obviously a bad
idea in the first place.
This is like putting rabid weasels in your pants, and later expressing
regret at having chosen those particular rabid weasels and that pair
of pants.
   ---maf