Re: [ippm] Secdir early review of draft-ietf-ippm-capacity-protocol-01

["MORTON JR., AL" <[e-mail]>]

Thank you Brian for your review and comments!

Summary of the replies below:

There are two categories of changes needed: text clarifications alone, and text+protocol modifications. We have noted when protocol changes are needed (have not executed protocol changes in the code or working text).

We use a conventional communications setup, with a well-known port at the server. We will clarify this.

The main comment where we are looking for additional feedback is your comment number 3). We understand (?) that there be a fully encrypted mode of operation in IETF Stds track protocols, and that this mode must be the default mode operation. We will gladly implement a strong recommendation from you/SEC-DIR in the protocol specification.

We appreciate your observation that the Authenticated mode can be expanded to use the authDigest field to achieve important message protections and features like bit error checking. 

We have implemented text clarifications in the working text for -02.

thanks for sharing your expertise; one more recommendation will help!

Al and Len



> -----Original Message-----
> From: Brian Weis via Datatracker <[e-mail]>
> Sent: Wednesday, June 22, 2022 8:45 PM
> To: [e-mail]
> Cc: [e-mail]; [e-mail]
> Subject: Secdir early review of draft-ietf-ippm-capacity-protocol-01
>
> Reviewer: Brian Weis
> Review result: Not Ready
>
> 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 primarily for the benefit of the security area
> directors.  Document editors and WG chairs should treat these comments
> just like any other last call comments.
>
> The summary of the review is Not ready. (However, as this is an early
> review, this should not be unexpected.)
>
> Here are some concerns, comments, and questions.
>
> (1) With the current network flows, there’s some doubt that flows will
> actually pass one or more firewalls between the client and server.  Section
> 3 begins by saying
>
>   “One end-point takes the role of server, awaiting connection requests
>    on a well-known port from the other end-point, the client.”
>
> As I understand the above text, the client sends from a well-known port
> rather than the conventional method of a server opening a well-known port
> and listening on it. So a firewall in front of the client may open an
> inbound pinhole of “ephemeral port -> well-known port” (since the client
> sent a message from "well-known port -> ephemeral port").  This ought to
> be sufficient, from the client’s perspective.  But Security Consideration
> also says this:
>
>      “Firewalls protecting each host can both continue to do
>       their job normally.  This aspect is similar to many other test
>       utilities available.”
>
> I don’t understand how any reasonable firewall policy would be defined on
> the server side, when a client is initiating a protocol to a server’s
> ephemeral port. That is, ephemeral ports are usually blocked unless the
> server sends an outgoing message from it.  So how the firewall policy is
> intended to work needs to be described much more clearly. For example, if
> there is a co-dependency that a firewall administrator be required to open
> all of the ephemeral ports that the server might use in advance, that
> needs to be stated.  (Having said that, such a policy might very well be
> unacceptable to the firewall administrator.) Or if the server is never
> expected to have a firewall in front of it this last quoted sentence needs
> to be changed to say that.
>

[acm] We need to clarify the wording in Section 3, specifically:

	One end-point takes the role of server, *listening for* connection requests 
	on a well-known *destination* port from the other end-point, the client.

This is the conventional situation at the server in the network; we follow 
convention.

In the Sec Considerations, I think you'll agree that firewalls can operate
conventionally with the clarification above. It's the server's firewall 
using an open pinhole on the w-k port.

We could reduce the range of open ephemeral ports available at the server FW,
and there is no application listening on any of those ports until a successful
Test Setup exchange takes place.

We can add:

The firewall at the server will need to open a limited range of ephemeral ports to complete
the second exchange: Test Activation (where the client communicates to the server
on an ephemeral destination port *assigned by the server*). 

-=-=-=-=-=-=-=-=-=-

> (2) Can you describe the server’s behavior more clearly, as to how as a
> server it listens on an ephemeral port, a range of ephemeral ports, or
> whatever strategy you have in mind? This seems backwards from the usual
> flows of client using an ephemeral port to contact a server on a well-known
> port. 

[acm] Somehow, we led you to the reverse of our actual operation. The protocol
works exactly as your last sentence above specifies. Hopefully, the clarification 
in Section 3 will do the trick.

> The last quote above includes the sentence:
>
>     "This aspect is similar to many other test utilities available.”
>
> and seems to indicate that there are existing examples of how this works.
> Do these “other test utilities” really have these same data flows? 

[acm] Yes, but they have the w-k destination port open on the server, same as 
this protocol. 
Other utilities mentioned include iperf, netperf, etc.  Note that this 
section (9) will not be part of the RFC. 


> If so,
> then please describe how an existing firewall placed in front of the server
> works when the server is receiving packets on ephemeral ports.

[acm] the FW at server allows an ephemeral port range, and the server is only 
listening on ports that have been properly activated by the Test Setup exchange.

>
> (3) The draft seems to define message authentication only for the Setup
> Request and Response messages. In this situation I assume the goal is to
> authenticate a peer with the goal of verifying peer authorization only.
> That is, when only these messages are authenticated then there is no
> real goal of knowing whether the measurements later sent are accurate
> since an attacker can theoretically modify messages in transit. The
> other messages seem to lack a authDigest field.


>
> But Security Considerations does state
>
>      “Client-server authentication and integrity protection for
>        feedback messages conveying measurements is RECOMMENDED. “
>
> So is message authentication intended as an option for all messages  but
> the PDUs in -01 just doesn’t show the authDigest field yet? If so, it would
> be good to explicitly add them.
>
> In any case, Security Considerations needs to state just what the security
> goals are when only some messages are authenticated, and defend why that
> is acceptable. For example, I understand that adding message authentication
> processing to feedback messages generated by a data plane can affect the
> quality of the measurements, and some people might accept that measurements
> are not necessarily correct if they have been tampered by an attacker.

[acm] This is one of the main reasons we requested an early SEC-DIR review, 
It appears that you understand the trade-off between measurement accuracy 
and measurement integrity. We did not plan to try to justify the current 
(limited) authentication capabilities of the protocol as the finished spec. 

Instead, we would like to provide additional/optional mode(s)
of operation where  (listed in Section 10)

WG ver 01 proposal below: 

A. Unauthenticated mode (for all phases)
AND
B. OPTIONAL Authenticated set-up only 
SHA-256 HMAC time-window verification (5 min time stamp verification)
(could add silent failure option)
New: we could add authDigest everywhere that is possible, as you suggested below.

 -=-=-=-=-=-=-=-=-=- Above options exist in Running Code -=-=-=-=-=-
 
 *** We would like a SEC-DIR recommendation to accomplish C and/or D below: 

C. Encrypted Setup Exchange in a tunnel to well-known port:
(remaining transmissions are on a new UDP port-pair, in the clear)
New: could combine Test Activation exchange with Setup, on the well-known port, encrypted.
Need a packet to open the firewall from client to server.

D. Encrypt "all the things"  
(Reduce the options, provide the required protocol protection)


while keeping the following design criteria in mind:
+ the accuracy <-> integrity trade-off (lightweight encryption may see more deployment)
+ synergy: we are already using the OpenSSL library in the running code (for Authentication)

New: we think this mode D might not be used very often, the demands on hosts to generate and 
measure at Gbps rates usually require all the cycles they can allocate to the measurement
process.

>
> (4) Section 3 says
[section 5]
>
>   “If the Setup Request must be rejected (due to any of the reasons in
>    the Command response codes listed below), a Setup Response SHALL be
>    sent back to the client with a corresponding command response value
>    indicating the reason for the rejection.”
>
> I note that some reasons have to do with the server not being able to
> authenticate the client. Assuming the server has an open port or set of
> ports that any network device can target, this seems like an opportunity
> for an arbitrary network device spoofing a victim IP address to use the
> server as a DDoS “bounce” address targeting that victim. That is, the
> attacker would create a message with the victim's IP address as source
> address, and use a destination address of the measurement server, causing
> the measurement server to reply to the intended victim.
>
> A more secure policy would be to silently drop messages that cannot be
> authenticated, even though this makes diagnosing the failure harder. The
> idea of silently dropping message is mentioned on Page 9, including when
> “a directed attack has been detected”, but as these attacks can do a lot
> of damage before being detected it’s not really sufficient to just say
> that “Attack detection is beyond the scope of this specification”. Yes,
> that’s true, but causing a vulnerability in protocol design and assuming
> some other system will detect and block it is not sufficient. This is
> why it's important to silently drop messages that fail authentication.
> Otherwise, the remaining threat needs to be defended in Security
> Considerations.

[acm] I see, silent AUTH failure could be a feature of the current AUTH mode
(the option we described above - would need to implement the silent failure).
We need to re-write the paragraph above to reflect the silent behavior when 
the AUTH option is used.  
So - re-wrote para above. default would be silent, but allow compile time 
#define for server requiring Authentication to use reject response.

If the Setup Request must be rejected (due to any of the reasons in the Command 
response codes listed below), a Setup Response SHALL be sent back to the client 
with a corresponding command response value indicating the reason for the rejection, 
unless the server requires Authentication, in which case the Setup Request 
SHOULD fail silently. The exception is for operations support: server administrators 
using Authentication are permitted to send a Setup Response to support operations 
and troubleshooting.

>>>> This is a protocol change <<<<

>
> (5) Section 5.1 begins by saying
>
>   “When the client receives the Setup response from the server it first
>    checks the cmdResponse value.”
>
> Actually, checking the validity of the  authDigest field in the Setup
> Response should be the first step. Otherwise, the cmdResponse value is not
> known to be trustable.

[acm] Agreed, we likely wrote this before we had an AUTH-mode option, we should
change the wording in the draft to reflect what the protocol actually does...
In fact, we need to first check that the message PDU is correctly formatted 
(size and message type)
with the fields we expect, then check the authDigest field, then the 
cmdResponse.
Text has been updated, as follows:

When the client receives the Setup response from the server, the client SHALL check:
><t
>the message PDU for correct length and formatting (fields have values in-range, beginning with the fields listed below)</t
><t
>the controlID, to validate the type of message (Setup)</t
><t
the cmdResponse value</t


>
> (6) Section 6.1 does mention using the “Authentication mode, and
> Authentication
> time stamp” in the context of the Test Activation Request. Can you clarify
> how these data fields are used? 

[acm] this is a good catch, Test Activation exchange currently does NOT use 
the authDigest field, so the initial clarification is to remove these items from the list
(until further changes to the protocol make them active).
We have clarified the list of client actions to populate the request as follows:


>The client SHALL use the configuration for
><t
>cmdRequest (upstream or downstream)</t
><t
>cmdResponse is cleared (all zeroes)</t
><t
>and the remaining fields are populated based on default values or command-line options</t
></list

requested in the Setup Request and confirmed by the server in the Setup Response.


> Would it be reasonable to add these fields
> to the  Test Activation Request and Response messages, since they seem to
> be control plane messages rather than data plane messages?

[acm] We could add the authDigest to the Test Activation request/response.
THEN, we would explain that 
+ Use of optional Authenticated mode requires checking the validity of authDigest in this phase
+ The time stamp in the PDU MUST be within 5 minutes (+/-2.5 minutes) of the current time at the recipient.
>>>> This is a protocol change <<<<


>
> (7) Section 7.2. There is a note that protection from bit errors could
> be desirable. I note that message authentication of these messsage would
> ensure that bit errors are detected.

[acm]  Ok, so in the optional Authenticated mode (currently in the spec and 
running code), authDigest *could be added* to the Status Feedback messages.
>>>> This is a protocol change <<<<

>
> (8) Section 8 says
>
>   “When the client receives a load or status PDU with the STOP1
>    indication, it SHALL finalize testing, ….”
>
> Stopping a test seems like an important message to verify that it actually
> came from the server rather than an attacker that doesn’t want the measurement
> to happen. (Maybe the measurement would result in discovery that the attacker
> is using some resources, or some such.) An authDigest field would resolve
> that threat.

[acm]  Ok, so in the optional Authenticated mode (currently in the spec and 
running code), authDigest *could be added* to the Load PDU messages to validate STOP1.
>>>> This is a protocol change <<<<

>
> (9) Section 10 says
>
>      “Cooperating source and destination hosts and agreements to test
>        the path between the hosts are REQUIRED.”
>
> I assume this cooperation is determined by authorization through the use
> of the authDigest field and the known identity associated with the key used to
> create the authDigest field. If so this sentence should say this more
> explicitly.

[acm] Yes, adding: 
One way to assure a cooperative agreement employs the optional Authorization mode
through the use of the authDigest field and the known identity associated with 
the key used to create the authDigest field. Other means are also possible, 
such as access control lists at the server.


>
> (10) The authDigest usage needs to be defined, i.e. what bytes are included
> in the digest, etc.  
[acm] Len confirms:
The entire control header for a Setup Request is covered by the digest. 
The current Unix time is inserted just before it is calculated.


> Also the choices that can be configured for authMode
> should be stated. Also, there should be a registry of authentication methods
> for interoperability.

[acm] Ok, right now we only have 2 modes, as described: unAUTH and AUTH (Setup 
exchange only).
We could implement the authDigest field in more aspects of the protocol, as 
noted above. But this would continue as the single AUTH mode in the next protocol version.

ASSUMING that the IESG will prefer that we have fully encrypted operation "on by default",
we would truly appreciate a recommendation for the encryption method that works with 
our UDP-based control and measurement protocol, AND satisfies all the future SEC reviewers!

IOW, we have running code and want to avoid re-do.
We will be glad to take a strong SEC-DIR recommendation 
in order to avoid blocking comments later. It's very simple. We are putty in your hands.

>
> (11) The use of authUnixTime is not stated anywhere. Why is time important,
> how does a receiver determine if the time is accuracy given latency in the
> network, and what threats does it resolve?

[acm]need to clarify:
The time stamp and the 5 minute window (+/- 2.5 seconds) is used to prevent the replay of a setup request.
All fields would otherwise be valid if the timestamp field was not included (which is also covered by the hash).

 Added text to explain above.


>
> Thanks,
> Brian
>