Re: [Dime] Benjamin Kaduk's Discuss on draft-ietf-dime-rfc4006bis-08: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

Catching up on the non-redirect items...

On Thu, May 24, 2018 at 06:17:51AM +0000, Yuval Lifshitz wrote:
>  inline
>     On Wednesday, May 23, 2018, 4:58:32 p.m. GMT+3, Benjamin Kaduk <[e-mail]> wrote:  
>  
>  [re-sending since my client crashed during the first attempt and I
> don't see the message in the archives.  I attempted to reconstruct
> my message from a scrollback buffer, but there may be some artifacts
> with missing or duplicated text]
> 
> On Wed, May 23, 2018 at 08:31:51AM +0000, Yuval Lifshitz wrote:
> >  Thanks Ben and Benjamin! Comments inline
> >    On Wednesday, May 23, 2018, 6:32:47 a.m. GMT+3, Ben Campbell <[e-mail]> wrote:
> >
> >  Hi Benjamin,
> >
> > I’ve cherry-picked a few points, inline:
> >
> > Thanks!
> >
> > Ben.
> >
> > > On May 22, 2018, at 8:48 AM, Benjamin Kaduk <[e-mail]> wrote:
> > >
> > > Benjamin Kaduk has entered the following ballot position for
> > > draft-ietf-dime-rfc4006bis-08: Discuss
> > >
> > > When responding, please keep the subject line intact and reply to all
> > > email addresses included in the To and CC lines. (Feel free to cut this
> > > introductory paragraph, however.)
> > >
> > >
> > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> > > for more information about IESG DISCUSS and COMMENT positions.
> > >
> > >
> > > The document, along with other ballot positions, can be found here:
> > > https://datatracker.ietf.org/doc/draft-ietf-dime-rfc4006bis/
> > >
> > >
> > >
> > > ----------------------------------------------------------------------
> > > DISCUSS:
> > > ----------------------------------------------------------------------
> > >
> > > I support Alissa's Discuss point about sensitive AVPs.
> > >
> > > I appear to have a different understanding of Section 5.6.2, though,
> > > with a different potential issue (but again, it's possible that I'm
> > > confused to how things work):
> > >
> > > With the redirection schemes covered in Section 5.6.2, it sounds
> > > like the user can be redirected (at the application protocol level,
> > > e.g., HTTP or SIP) to a top-up server to purchase more credits.  I
> > > don't see a way described how user agents can distinguish between a
> > > Diameter-induced redirect and an attacker-induced redirect to a
> > > (potentially phishing) site that accepts payment information.  To be
> > > clear, the scenario here is that a user is using a credit-controlled
> > > application session to talk to "arbitrary application servers",
> > > including potentially attacker-controllled HTTP/SIP/etc. servers;
> > > the attacker could introduce a redirect to a phishing page that asks
> > > for payment information, and the user would be led to believe that
> > > this was the normal flow for "my prepaid balance has been used up",
> > > and give their payment information to the phishing site. I think
> > > that either user agents need to give some indication that this
> > > DIAMETER-level redirect is different than an
> > > application-protocol-level redirect (e.g., http) or the Security
> > > Considerations need to mention the risk of acclimating users to
> > > arbitrary websites redirecting to sites asking for payment
> > > credentials, which may or may not be legitimate sites.
> > >
> >
> > I think it’s reasonable to mention the concern in the security considerations. But I don’t see how a redirection based on this specification is any different than any other time an HTTP browser or SIP UA connects to a resource based on an arbitrary URL.
> 
> In some sense, it's not.  My claim is more that users are being
> conditioned to expect payment prompts to appear at "arbitrary times"
> than a specific flaw in this workflow.
> 
> 
> > But to put some more context around this, the Diameter client is typically a Network Access Server (NAS) that the end user is using for network access. The user is already at the mercy of that NAS, and that NAS is at the mercy of the credit-control application server. The user’s trust in that NAS seems to have the same issues whether or not this Diameter application is used.
> >
> > [yuval] agree with Ben, if someone bad took control over the NAS, they can do much worse
> 
> I agree that the NAS could send traffic to "wherever", but my
> objection could perhaps be categorized as more "social" than
> "technical".  Maybe I should attempt to give an example (and you can
> point out if I misunderstand things).
> 
> I believe that a "normal usage" of this technology could be for a
> user with a prepaid data plan to be using a web browser, and, e.g.,
> connect successfully to https://example.com.  Suppose that this
> request used up their last remaining credits, and they click on a
> link from that page.  Instead of going to the actual target of that
> link, they are redirected to the data plan owner's top-up page,
> which displays a message "your balance is zero; please enter credit
> card information to purchase more data", the user pays, and can
> potentially even be redirected back to the actual target of the link
> they clicked on (though that's not key to my example).  Let's
> consider what would happen in an "attack scenario", where the same
> user has plenty of data quota remaining, and goes to
> https://honest-achmed.com.  They click on a link from that page,
> which instead of going to an "actual site" that would be expected
> from the context surrounding the link, actually goes to
> http://[IP address controlled by attacker]/topup.html, which is
> designed to look as similar as possible to the actual data plan
> owner's top-up page.  The user may then enter their payment
> information, and get redirected to a site with actual content,
> believing that they have obtained more data quota from their
> provider, when in reality they have given their credit card
> information to a phisher.
> 
> I don't see what mechanism is in place to allow the user to be able
> to distinguish between the "normal usage" scenario and the "attack
> scenario".  I also understand that this sort of technology is
> already deployed and is seen as useful by both users and data
> providers, so it seems unrealistic to expect to be able to get rid
> of this usage entirely.  I do think that it is unreasonable for us
> to enable this behavior without documenting the risks to the user,
> though.
> 
> [yuval] I guess that there is nothing in the spec that technically enables phishing, however, it makes the social engineering part of it easier. How about the following addition to the security sections:"It is RECOMMENDED that operators put in place mechanisms that would help their subscribers identify a valid redirect operation from a malicious one"
> > > Separately, in Section 8.68 (for QoS-Final-Unit-Indication):
> > >
> > >  If the Final-Unit-Action AVP is set to REDIRECT at least the
> > >  Redirect-Server-Extension AVP MUST be present.
> > >
> > > This MUST seems in conflict with the text in 8.64 (actually, is
> > > section 8.64 even internally inconsistent, too?) about
> > > "Redirect-Server AVP, in addition to or instead of the
> > > Redirect-Server-Extension AVP", in particular, the "instead of"
> > > portion.  (The ABNF-based formal language spec in 8.68 does match up
> > > with the above MUST, though.)
> >
> > Would changing “in addition to or instead of” in 8.64 to just “in addition to” help?
> >
> > Authors: Please comment if that works, given the backwards-compatibility concern.
> > [yuval] the cumbersome text is because of backward compatibility issue. Would appriciate suggestion for better phrasing. The idea is the following:* We have an existing AVP that can carry some information* We need more information, but we cannot modify the existing one, so we added a new AVP (which, unlike the old one, is future proof)* If you have a peer that does not support the new spec, but only need the old info, you can use the old AVP. what makes the spec backward compatible to the old one* If you have need to send the new info, you have to use the new AVP, but in this case an older peer does not make sense
> 
> To Ben, I believe that just "in addition to" resolves the internal
> inconsistency.  However, it sounds like that may not be the best
> solution from a deployment perspective, and perhaps the formal
> definition in Section 8.68 (and the body text) should be relaxed to
> allow either the -Extension or non-Extension form.
> [yuval] will remove "instead of"

I guess my first response did not adequately respond to your request
for better phrasing.  Unfortunately, I don't think just different
phrasing would suffice if we want to retain the ability to use
just the legacy AVP.  (If we don't want to retain that ability, then
removing the "instead of" is the right thing to do.)

To regain the ability to just use Redirect-Server without
Redirect-Server-Extension, the text in section 8.68 would need to
have

OLD:
   If the Final-Unit-Action AVP is set to REDIRECT at least the
   Redirect-Server-Extension AVP MUST be present.  The Filter-Rule AVP

NEW:
   If the Final-Unit-Action AVP is set to REDIRECT, at least one of the
   Redirect-Server and Redirect-Server-Extension AVPs MUST be present.
   The Filter-Rule AVP

And probably also to change the formal syntax (though I guess
technically it is not necessary, it would be misleading to leave it
as-is):

OLD:
         QoS-Final-Unit-Indication ::= < AVP Header: TBD17 >
                                   { Final-Unit-Action }
                                  *[ Filter-Rule ]
                                  *[ Filter-Id ]
                                   [ Redirect-Server-Extension ]
                                  *[ AVP ]

NEW:
         QoS-Final-Unit-Indication ::= < AVP Header: TBD17 >
                                   { Final-Unit-Action }
                                  *[ Filter-Rule ]
                                  *[ Filter-Id ]
                                   [ Redirect-Server ]
                                   [ Redirect-Server-Extension ]
                                  *[ AVP ]

(Am I reading RFC 6733 correctly that the ABNF '/' operator is not
used in CCF?)

> > >
> > > ----------------------------------------------------------------------
> > > COMMENT:
> > > ----------------------------------------------------------------------
> > >
> > > Some additional significant but not necessarily DISCUSS-worthy
> > > comments, followed by more editorial- and nit-level things.
> > >
> > > In Section 1.3, "Advertising Application Support" uses the same
> > > Auth-Application-ID value as for RFC 4006; what are the interop
> > > consequences of this?
> > > [yuval] this was done to make interops easier - this is why we kept this RFC backward compatible with RFC4006
> > > Alissa already noted that the text about how to split (sub-)AVPs
> > > between the foo and foo-Extension AVPs is inconsistent among them --
> > > e.g., Redirect-Server-Extension and User-Equipment-Info say "SHOULD
> > > send [in the old one]", but Subscription-Id-Extension has separate
> > > text about "[i]f full backward compatibility with [RFC4006] is
> > > required" and slightly different behavior described.  We should try
> > > to catch all instances of this sort of backwards compatibility and
> > > make them consistent.
> >
> > I agree.
> > [yuval] will look to unify the text
> 
> I see that there was some discussion on Alissa's ballot thread that
> there may indeed be special considerations for one AVP.  If so,
> that's fine, but the reasoning should probably be included in the
> document to explain the apparent disparity.
> 
> [yuval] ok
> 
> > > The use of UTF8String for URLs and URIs (e.g., Redirect-Address-URL)
> > > might benefit from additional text about the expected contents.  A
> > > lot of the time when we use a UTF8 container for names (whether
> > > domain names or other kinds), we specify what normalization form and
> > > canonicalization are performed, whether domain names are A-labels or
> > > U-labels, etc., as there's a lot of potential flexibility not all of
> > > which is good.  In this case, since the communication is only
> > > between trusted actors, perhaps the additional restrictions are not
> > > needed, but I am curious if the topic was raised in the WG.
> >
> > On reflection, shouldn’t that be based on whatever rules already exist for the URL scheme? And if some scheme doesn’t have well defined behavior for non-ascii labels, is that the concern of this application?
> 
> It is probably a matter for the URL scheme, yes.  So at most we
> could note here that "individual URL schemes may restrict the
> contents of the UTF8String", but as I imply in my original comment,
> it's far from clear to me that we actually need to say anything in
> this specific case.
> 
> > >
> > > Thank you for adding the Privacy Considerations and list of
> > > Privacy-Sensitive AVPs!
> > >
> > > Section 14
> > >
> > >  [...] The Diameter credit-
> > >  control application is often used within one domain, and there may be
> > >  a single hop between the peers.  In these environments, the use of
> > >  TLS/TCP, DTLS/SCTP or IPsec is sufficient.
> > >
> > > I did not see any concrete guidance on what would suffice in other
> > > environments (with multiple hops between peers).  Is this the realm
> > > of the mythical "end-to-end security mechanism" for Diameter that is
> > > much-desired?  (The last paragraph does have guidance on what might
> > > *not* suffice, which is not exactly the same thing.)
> > >
> >
> > Sort of, but in real world deployments the “often used within one domain” (assuming domain means “trust domain”, which should be clarified) effectively overrides everything else. That is far from ideal, but it’s the reality. So it basically comes down to keep everything in the trust domain, or if you cross a non-trusted network then use TLS or IPSec.
> >
> > There’s no solution to safely traverse a non-trusted Diameter agent. The mythical e2e mechanism could conceivably give us that.  (someday, along with world peace and a post-scarcity economy).
> >
> > [yuval] as Ben and Lyle wrote, if your messages need to go through agents that belong to a 3rd party, you have risks. In this case, AVP level encryption (which is not standardized yet...) is the only option for to ensure privacy. So, the only thing we can do here is to highlight the risks. 
> 
> Do we want to say something about "at present, there is not a
> general reliable security solution for other environments"?  (To be
> clear, I do not insiste that we do so.)
> 
> [yuval] not sure if AVP level ancryption would ever happen... would keep text as is
> 
> > >  Even without any modification to the messages, an adversary can
> > >  eavesdrop on transactions that contain privacy-sensitive information
> > >  about the user.
> > >
> > > This ("an adversary can") makes it sounds as if there is no
> > > confidentiality protection anywhere in the system, but that's what
> > > TLS/DTLS/IPSec are supposed to provide.  So maybe "an adversary that
> > > can eavesdrop on transactions can obtain privacy-sensitive
> > > information [...]" is better.
> > >
> >
> > Good point, I agree your suggestion is better.[yuval] ok
> >
> > > (Editorial- and nit-level stuff follows.)
> > >
> > > Section 4
> > >
> > >  A flexible credit-control application specific failure handling is
> > >  defined in which the home service provider can model the credit-
> > >  control client behavior according to its own credit risk management
> > >  policy.
> > >
> > > This sentence is hard to parse -- in "credit-control application
> > > specific" what does "specific" bind to?  What is being modelled?  Is
> > > anything being controlled (as opposed to modelled)?
> > [yuval] ok. so how about:
> > "Flexible failure handling, specific to the credit-control, is defined in the application.This allows the the service provider to control the credit-control client behavior according to its ownrisk management policy"
> 
> That's much better; thanks!
> 
> > > Section 4.1.1
> > >
> > >  2.  The Service-Parameter-Info AVP MAY be used as a container to pass
> > >  legacy rating information in its original encoded form (e.g., ASN.1
> > >  BER).  [...]
> > >
> > > Why BER and not DER?  The unnecessary flexibility in BER vs. DER has
> > > been known to tickle parser bugs and create security
> > > vulnerabilities.
> > > 
> > [yuval] this is just an example of legacy stuff you have no control over
> >
> > > Section 4.1.2
> > >
> > >  [...] To
> > >  facilitate interoperability, it is RECOMMENDED that the rating input
> > >  and the values of the Service-Context-Id be coordinated via an
> > >  informational RFC or other permanent and readily available reference.
> > >  The specification of another cooperative standardization body (e.g.,
> > >  3GPP, OMA, or 3GPP2) SHOULD be used.
> > >
> > > The RECOMMENDED and SHOULD seem slightly in conflict.
> > > 
> > [yuval] yes, seems a bit awkward. How about:
> > "To facilitate interoperability, it is RECOMMENDED that the rating input
> > and the values of the Service-Context-Id be coordinated via an
> > informational RFC or other permanent and readily available reference,
> > preferably, of another cooperative standardization body (e.g.,
> >  3GPP, OMA, or 3GPP2)."
> 
> Sounds good.
> 
> > > Section 5.1.1
> > >
> > > As noted elsewhere, 60 seconds per minute does not always hold; it
> > > seems that this text could be reworded to just talk about a
> > > "constant rate" or "constant level per fixed time period" or
> > > similar.
> > > 
> > [yuval] "constant rate" could mean sub-second resolution as well. The grants are in seconds, so, IMO, this phrasing is more accurate
> 
> It seems that it's only more accurate if leap seconds are ignored.
> (I suppose you could explicitly disclaim "(ignoring leap seconds)"
> or similar.)
> 
> [yuval] will add
> 
> > > Section 5.1.2
> > >
> > >  [...]
> > >  timer (Section 13) every time a CCR message with the value
> > >  UPDATE_REQUEST is sent while they are in PendingU state.  When
> > >  answers to all pending messages are received, the state machine moves
> > >  to OPEN state, and Tx is stopped.
> > >
> > > Transmission, or the Tx timer (is stopped)?
> > > 
> > [yuval] well, "Tx" is overloaded :-( but we never use it in the sense of "transmission" in the RFC
> 
> (I think that "Tx timer" was fairly consistently used throughout the
> rest of the document; it may make sense to be fully consistent about
> it.)
> 
> [yuval] will fix in the doc
> 
> > > Using a different title for Section 5.2.2 might make the parallel
> > > between it and Section 5.2.1 more clear.  That is, perhaps something
> > > like "First Interrogation Included with Authorization Messages".
> > > 
> > [yuval] make sense
> >
> > > Section 5.7
> > >
> > >  [...] It is RECOMMENDED that the client complement the credit-
> > >  control failure procedures with backup accounting flow toward an
> > >  accounting server. [...]
> > >
> > > Nit: it looks like there's a missing article here, maybe "a backup
> > > accounting flow" is better.
> > > 
> > [yuval] the rest of the paragraph describe such "backup accounting flow". See what comes after "For example"
> 
> I just meant that it sounds like you need to add the word "a" in
> order for the grammar to make sense.  (But perhaps "the" is the
> right word; I wasn't sure.)
> 
> [yuval] ok
> > >  The AAA transport profile [RFC3539] defines the application layer
> > >  watchdog algorithm that enables failover from a peer that has failed
> > >  and is controlled by a watchdog timer (Tw) defined in [RFC3539].
> > >
> > > Nit: Is it "the" watchdog algorithm or "an" watchdog algorithm?
> > > 
> > [yuval] would imagine there are other algorithms out there... should fix
> >
> > > Section 6.2
> > >
> > > Should there be text indicating how the client indicates what
> > > service the balance check is being requested for?
> > > 
> > [yuval] didn't find any reference to service information for "EVET_REQUEST" type messages (direct debit, refund, balance check and price enquiry). Seems like that in the IEC section of 3GPP TS 32.299, they added their own mechanism for "direct debit", and "refund", and leave "balance check" and "price enquiry" as references to RFC4006. Unless I've missed something, this seems like a missing part of the spec.
> >
> [yuval] hmm. seems like "event requests" should be looked at (probably at the light of 3GPP 32.299 IEC concept). But for sure *not* as part of this work.

I was mostly wondering whether this section should say anything
about including the Service-Identifier AVP.  Looking back at the
document now, though, maybe this question does not actually make
sense.

-Benjamin

> > > Section 8.54
> > >
> > > Maybe give a section reference in RFC 3580 for how the formatting is
> > > done?
> > > 
> > [yuval] see section 3.21 in RFC3580, this is also true for section 8.50 in our RFC
> >
> > > Section 12
> > >
> > >  [...] Initially, such Expert discussions take place on the AAA
> > >  WG mailing list.
> > >
> > > That list appears to be dead, suggesting that this text should be
> > > updated.  (I also agree with Adam's comments about updating the IANA Considerations.)
> > > 
> > [yuval] i don't know the process here - not sure how to change it.
> 
> With respect to the "expert discussions take place" text, I think it
> could just be removed.
> 
> Discussion of the detailed updates to the IANA actions should
> probably happen in Adam's ballot thread.
> 
> > > Why is the disclaimer in Section B.4 (and other examples) not needed in Section B.3?
> > [yuval] should be added there as well
> 
> Great, thanks.
> 
> 
> -Benjamin
> 
> 
> | 
> | 
> |  | 
> Example Domain
> 
> 
>  |
> 
>  |
> 
>  |
> 
> 
> 
>