Re: [secdir] secdir review of draft-ietf-sfc-problem-statement-10

["Paul Quinn (paulq)" <[e-mail]>]

Ben,

Thank you for your detailed review and sorry for the delayed reply due to the holidays.

I added some comments inline below.  I’ve also cc’d our document shepherd, Joel, since I’m not sure he was on the original distribution.

Paul

> On Jan 2, 2015, at 7:39 PM, Benjamin Kaduk <[e-mail]> wrote:
> 
> Hi all,
> 
> Sorry this is a bit late.
> 
> I have reviewed this document as part of the security directorate's
> ongoing effort to review all IETF documents being processed by the
> IESG.  These comments were written with the intent of improving
> security requirements and considerations in IETF drafts.  Comments
> not addressed in last call may be included in AD reviews during the
> IESG review.  Document editors and WG chairs should treat these
> comments just like any other last call comments.
> 
> I believe this document is not ready for publication.
> 
> The document attempts to disclaim the need for a security considerations
> section by virtue of being a problem-statement only document.  However,
> section 3 provides three components which are expected to be part of
> solutions to the problem, and the focus of future work in the working
> group.  Even at this abstract level, there are still security
> considerations that can be made, and I'm sure I will not think of all of
> them during the course of this review.
> 
> Additionally, there are sufficiently many grammar and language oddities
> and inconsistencies in the text to be distracting from the actual content
> of the document; I'll try to cover those at the end of this mail.  Using
> multiple terms for the same concept doesn't help, either (as is disclaimed
> in the definitions section), but I understand how that is unavoidable in
> this case.
> 
> 
> To re-summarize the document (section 5 notwithstanding), it lays out some
> terminology and provides a list of many of the issues involved in setting
> up a complicated traffic processing scheme involving multiple network
> service functions (firewalls, traffic accelerators, load balancing, etc.)
> using current technologies.  The difficulties largely stem from the tight
> coupling between the network topology and the way/order in which service
> functions are applied to traffic.  Three components are discussed which
> will help break this coupling: a "service overlay", which allows for the
> service-function-chain topology to be decoupled from the network topology;
> "service classification" to control traffic flow on the service overlay
> topology; and "dataplane metadata" to convey the classification data (and
> other data).
> 
> 
> 
> Here are the potential security considerations I came up with so far:
> 
> * An error in service classification could lead to untrusted traffic
> flowing through a service function chain intended for trusted traffic
> only.  In various hypothetical situations, this could lead to an attacker
> being able to execute privileged actions via a trusted service, execute a
> DoS attack against an internal service, etc.  It is important for service
> classification to "fail safe" to avoid this class of issues.  ("Failing
> safe" might or might not include just dropping such traffic.)

PQ>  This statement applies to all things that depend on classification, or configuration in general.  Perhaps a statement about “risk associated with configuration or other operational errors” and service chaining would help clarify.


> 
> * Similarly, errors in translating from an existing network/service
> topology to a separate service overlay (e.g., omission or addition of a
> firewall element) could lead to an attacker sending traffic in the real
> network to services which ought to be disallowed by the service overlay.

PQ>  The above suggested statement (or something similar) would cover this as well.



> 
> * The dataplane metadata could open up a giant rabbit hole of information
> leakage.  It is tempting to say that the metadata would only flow inside
> the network boundaries of a single (corporate) entity, but "SSL added and
> removed here :-)" should convince us that that's not a workable solution.
> Metadata could conceivably include the type of traffic, client information
> (i.e., personally identifying information), and more, some of which should
> receive protection from eavesdroppers on the dataplane which will not need
> to process the traffic in question.

PQ>  The “value” of the metadata is highly variable, in some environments, as you point out, it might be sensitive.  Perhaps we can add the following to the security considerations sections:

1.  If data plane confidentiality or integrity is required, a transport that supports the requisite functionality (e.g. IPSec) should be used to create the service overlay.

2.  Similarly, control plane messages may be authenticated or encrypted, as required by an operator, using existing mechanisms (e.g. SSL)



> 
> * Metadata can come from "external sources".  Those sources should be
> trustworthy and verified, or attackers could send traffic where they
> oughtn't be able to.

PQ>  Covered by the above control plane text.



> 
> 
> 
> Here's the grammar and language issues that bothered me, roughly sorted
> with the more important ones coming first and otherwise in order of
> appearance:
> 
> Section 3 has the text "the SFC working group will [...]", which seems
> more appropriate for a WG charter than an informational RFC.  I see this
> had been brought up in discussion of the -03, but I did not see any
> obvious resolution in a quick skim.  To be clear, I would be fine with
> something that didn't explicitly say what the WG would do, like "SFC may
> include solutions utilizing the following elements" or something similar.

PQ>  The complete current version states: “...will investigate solutions that address the following elements:..” which was widely reviewed and accepted the WG members.


> 
> The document uses some acronyms that are not expanded.  I expect most
> readers to be familiar with some, but not all, of:
> * Open Systems Interconnection
> * the OSI model, e.g., layer 3, layer 7, etc. (I had to think a moment
> about "L4-L7")
> * transmission control protocol
> * Virtual Local Area Network
> * Border Gateway Protocol
> * internet protocol
> * virtual private network
> * multiprotocol label switching
> * Wide Area Network
> * datacenter

PQ>  As you point out, many of these are well known terms to readers of the draft, as was the case with the reviewers.


> 
> I can't find a parse that I'm happy with of the first paragraph of section
> 3.3.  In particular, I'm not sure whether the leading "As such" is just a
> transition leading into a new sentence (equivalent to "Thus,"), in which
> case it should be offset by a comma, or if the "such" binds to "metadata"
> (equiavent to "Since such").  In the first version, I guess the metadata
> is supposed to be sent out-of-band and interpreted by functions along the
> service overlay, but I don't see how it would then *not* be used to
> control the route by which packets travel.  In the latter version, the
> sentence is incomplete, since there's no dependent clause.
> 

PQ>  As such does not mean “thus”, thus (;-)) the sentence can read:

"As such it is not used as forwarding information to deliver packets along the service overlay."

Having said that, it’s pretty minor point, and can be fixed several ways if reader find it confusing/offensive:

-  I believe the correct use of “as such” in this context is to remove the word metadata immediately following “as such”. 
- As such —> Metadata (i.e. remove as such)


> In the third paragraph of section 3.3, the sense of the two things
> addressing issues seems reversed: "the decoupling of policy from the
> network topology" is a good thing, which will be obtained by using
> metadata; on the contrary, "the need for per-service function
> classification (and re-classification)" is a bad thing, namely the problem
> mentioned in section 2.10.  I would probably resolve this with "[...] most
> notably by decoupling policy from the network topology, and by removing
> the need for per-service function classification (and re-classification)
> described in section 2.10.”

PQ>  I like your suggestion, easy to fix.


> 
> The abstract uses the phrase "ordered set of instances", but a set is
> explicitly unordered.  Is there a better term to use, like "graph",
> "network", or "structure"?  (The word "set" also appears in the second
> paragraph of the abstract, but may be more appropriate in that usage.)


PQ>  Is there not a concept of an ordered set as well?

> 
> I'm not sure I'm interpreting the third paragraph of section 2.1 correctly
> -- is the issue that the network topology needs to change in front and
> behind of each service function, whenever a new service function is
> required?  Or is it that the network topology must change before and after
> a new service function is introduced, in order to allow inserting the
> function without loss of serice?  I would also find the last sentence more
> clear if reorderd to be "[...] all traffic often passes through the same
> strict order, whether a given service needs to be applied or not.”

PQ>  In many cases, both.  But the intent here is to describe the “front” and “behind” issue.


> 
> In section 2.3, I don't understand what "constrained service function high
> availability" is.  Is the issue just that the topology forces a situation
> which could be subject to reduced availability because certain
> high-availability techniques are not usable in that topology?

PQ>  Correct, as described in the next paragraph:

"Since traffic reaches many service functions based on network
   topology, alternate, or redundant service functions must be placed in
   the same topology as the primary service."


> 
> The parenthetical in the abstract "such as firewalls, load balancers" is
> incorrect comma usage; I would tack on a ", etc." at the end.
> 

PQ>  Good catch, thank you.


> The second paragraph of the abstract is hard to parse; my best effort at
> cleaning it up is "The set of enabled service function chains reflects the
> service offerings of a given operator, and is designed in conjunction with
> application delivery, service, and network policy.", but I fear that has
> changed the meaning somehow.  ("chains" needs to be plural, as does
> "reflects", but there's also a missing article for "operator service
> offerings", and the two "ands" in the final clause read oddly.)


PQ>  I’m not sure what is hard to parse, it seemed well understood by the WG members.


> 
> First paragraph of section 1, "requires" should be plural.  (Comma after
> "functions" is optional, but might help readability.)

PQ>  Thank you.


> 
> Second paragraph of section 1, "the network topology" with the definite
> article.  Also add a comma after "limits" so the parenthetical statement
> is properly offset by commas.
> 
> The definition for "classification" doesn't seem grammatical.  Perhaps,
> "Locally instantiated policy to match traffic flows to the appropriate
> outbound action(s)"?  Additionally, should the definition be explicitly
> constrained to just forwarding actions (my proposal is not)?

PQ>  This is the definition that WG come up with after several rounds off back and forth (it’s used in other SFC drafts).  We tried to keep it generic, rather than constrained.  


> 
> In the definition for "service function", is "One of" needed?  Also,
> s/the Service Function/a Service Function/ in the last sentence of the
> first paragraph.  In the second paragraph, there's no need to say "etc."
> when prefaced with "includes:" -- just say "and TCP optimization" (note
> s/optimizer/optimization/ for tense consistency).

PQ> Did you mean “One or”?  If so, then yes.  


> 
> The definition for "Service Function Chain" is ... odd.  I believe that
> "their ordering requirements" refers to the ordering of service functions
> within the chain, but "their ordering requirements that must be applied to
> packets" actually reads that the ordering requirements come from the set
> of service functions but is applied to the packets and/or frames.  I would
> probably try to instead say something about "the constraints on the order
> in which service functions are applied to packets and/or frames".
> Additionally, "linear progression" is a bit vague; is the intention just
> to say that it may allow branching and need not be a complete/strict
> ordering (i.e., it could be a partial order)?

PQ>  This oddness can be quickly resolved I think:

A service Function chain defines an abstract set of service functions and associated ordering constraints
      that must be applied to packets and/or frames selected as a result
      of classification. 

As for the linear sentence, yes, it is to simply convey that there maybe a non-linear sequence of functions.


> 
> In the definition for "Service Topology", the phrase "service overlay
> connectivity" is a little odd; I might reword to "connectivity of the
> service overlay".
> 
> In section 2.1, remove the "the" from both "the service delivery" and "the
> flexibility”.

PQ>  Will update.


> 
> In the fourth paragraph of section 2.1, I'm failing to interpret the
> phrase "placement and service function selection taking into account
> network topology information is not viable."  Is it adding anything that
> is not said prior to the colon in that sentence?

PQ>  There is some redundancy there that can be cleaned up.


> 
> In the fifth paragraph of section 2.1, add a comma before "forcing", to
> separate the independent and dependent clauses.
> 
> In section 2.2, is "once installed, configured and deployed in production
> environments" needed?  I might add "the" before "use" in the last line to
> aid readability, but it is probably not strictly necessary for
> correctness.
> 
> In the second paragraph of section 2.3, add "the" before "network", and
> consider removing the comma after "alternate”.
> 
> In section 2.5, remove the space in "(re) classification".  Use a comma
> after "i.e." as well as before.  "increasingly less" is strange usage;
> consider just "decreasingly".  Use "topology information" consistently
> (not "topological information").  Use a comma after the sentence adverb
> "furthermore".
> 

PQ>  Will update.


> In section 2.6, add "network" before "under and overlays" (or mention that
> just "overlays" is valid usage in the definition in section 1.1).
> 
> In section 2.7, are "such change" the VLAN changes or the service
> deployment changes?  The current text has "such changes" bind to "changes
> to the service deployment", making the clause tautological.

PQ>  Both easy fixes.

> 
> In section 2.8, "traffic" is singular, so "traverses" to match (first
> sentence).  Consider expanding to "all service functions on that segment"
> as well, for clarity.  Also consider adding "which" after "data
> traverses".
> 
> In section 2.10, consider "between service functions" instead of "per
> service function", but in either case add a comma afterward.  I would also
> clarify by adding "classification" in "the results from other service
> functions".
> 
> In section 2.11, comma after "association" to separate the dependent and
> independent clauses.  (Also, should it be the plural "associations"?)
> 
> In section 3.1, the two "and"s is uncommon usage.  Most such cases would
> be condensed to a comma-separated list, but here I would recommend """The
> service overlay provides service function connectivity, building "on
> top" of the existing network topology to allow operators to use whatever
> overlay or underlay they prefer for creating a path between service
> functions, and to locate service functions in the network as needed."""
> 
> In the last paragraph of section 3.1, hyphenate "service-specific
> information".
> 
> In section 3.2, replace "service offered" with "the services offered".
> 
> Section 3.3 is inconsistent about the whitespace in "dataplane" between
> section title and body.
> 
> The second sentence of section 4 has what is basically a comma splice; I
> would fix it by "[...] exhaustive; rather, it [...]”.

PQ>  We’ll go through the corrections above and clean things as needed.


> 
> The enumerated entries in section 4 are inconsistent about whether the
> working group name is expanded in the body text, and even whether it is
> referred to as a working group (or just a proper noun in its own right,
> viz. "LISP").
> 
> In section 4, item 3, add "The" before "NVO3 WG".
> 
> 
> 
> -Ben Kaduk