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

[Brian Weis <[e-mail]>]

Hi Al,

Sorry for the late response. [Re-sent including the cc list.]

> On Jun 30, 2022, at 9:52 AM, MORTON JR., AL <[e-mail] <mailto:[e-mail]>> wrote:
> 
> 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.

While these are good goals, I’m not aware that the IESG has a strict requirement for a fully encrypted mode of operation for standards track protocols. If you have seen this stated, please point it out to me as I should know  it. But in any case, it is true that both security and privacy concerns should be addressed in a standards track protocol. If there are no privacy concerns with data in the measurement PDUs and there is no sensitive data being transferred in the PDUs, then I don’t believe confidentially is a requirement. You can analyze your PDUs to determine if any revealed data can be used to identify an individual or device that would be correlated with an individual, which would be a privacy issue. Also whether any of the data would if observed would provide value to an attacker, which would be a confidentiality issue. If there are no reasons to encrypt the exchanges, it would be useful to describe the rationale in Security Considerations to show you’ve considered it.

However, authentication of messages is always important to ensure that the messages have been sourced from an authenticated and authorized peer. For protocols between network devices, that authentication and authorization is usually embodied by associating an authentication key with an identity, e.g. IP address.

> 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] <mailto:[e-mail]>>
>> Sent: Wednesday, June 22, 2022 8:45 PM
>> To: [e-mail] <mailto:[e-mail]>
>> Cc: [e-mail] <mailto:[e-mail]>; [e-mail] <mailto:[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.

Yes, this seems reasonable. A firewall administrator is likely to agree to open a port to a well-known server if the protocol and server meet the local required security profile.

However, I understand Section 9.1 to say that there is also concern with  firewall being placed in front of the client. I.e., "Firewalls protecting each host”.  If that is so, then the switch from (server w-k port, client eph. port) to (server eph. port, client eph. port) could be problematic for the firewall in front of the client. If a firewall is actually not expected in front of the client then perhaps the text in Section 9.1 could be clarified. 
> 
> 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*). 

However, thinking about this operationally … I’m less confident that the firewall administrator in front of a server will agree to opening a “limited range of ephemeral ports” that it doesn’t have a guarantee of what server is actually listening on those ports. For example, what if the measurement server is down, and another piece of software attaches to that ephemeral port first? I think relying on ephemeral ports receiving packets without the server first having an outbound packet (and presumably opening a pinhole) is a risky operational approach.

But if the server were to send a message on the ephemeral port after having sent a Setup Response indicating the ephemeral port to the client, that would probably suffice to create the pinhole, and the ephemeral port range probably wouldn’t need to be configured on the firewall. I’m not a firewall expert, but I believe that is a common scenario that is allowed.

> 
> -=-=-=-=-=-=-=-=-=-
> 
>> (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.

Excellant, thanks for the clarification.

> 
>> 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.

From a protocol perspective this seems logical, but as mentioned above it may not be reliable to depend on the FW administrator leaving those ephemeral ports open at all times.

>> 
>> (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.

I believe that not providing for authDigest in all messages will be a problem, so I strongly suggest adding that option. I think that would be option D below.

I assume the above methods intend to use a manually configured keys. You could recommend that they are stored as defined in RFC 7210 ("Database of Long-Lived Symmetric Cryptographic Keys“), but this is just a suggestion. This document shouldn’t depend on how the keys are stored, but it is good advice to use RFC 7210.

However, I note that the current PDUs don’t provide a key identifier, which means that there’s no orderly key rollover method possible. That is, when there is a key change between the client and server, administrators must coordinate to replace the key on both devices at the same time, or at least between tests. Such coordination is typically not operationally feasible with network devices, so a key rollover without  a key identifier typically results in authentication failures until both devices have been updated. If the PDUs in this document did include a key identifier, then the clean key rollover and key selection methods described in RFC 7210 could be used. That would be significant added value.

If you don’t want to add a key identifier, then you do need to add a section for operators giving a list of instructions on how to do an orderly key rollover so that testing is not blocked.

Also, it would be good to add a reference to RFC 6234, which defines SHA-256 HMAC.

> 
> -=-=-=-=-=-=-=-=-=- 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.

I would not suggest inventing a new Encrypted Setup Exchange. If confidentiality is necessary, then I would suggest protecting the Setup Exchange with DTLS. However, that does not help protecting the other exchanges for either confidentiality or authentication.

It may be possible to derive keys from that  DTLS session which can be used with SHA-256 HMAC to provide authentication the other exchanges. See RFC 5705.  This would provide for a fully automated key management solution, which is generally more secure and easier to manage. I don’t know if OpenSSL supports this or not.

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

I’ve made suggestions for D at the end of this message.

> 
> 
> 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.

To be a little more precise,  if Authentication validation is successful for the Setup Request message, and the responder can successfully create an Authentication field in the Setup Response, then I don’t see any problem with returning an authenticated response containing an error code for other reasons, e.g. CHSR_CRSP_BADJS. That can be helpful. The issue is returning a message when authentication of the received message failed. So I would substitute the wording in the paragraph above to something like this: 

OLD
unless the server requires Authentication, in which case the Setup Request 
SHOULD fail silently. 

NEW
unless the server cannot successfully authenticate the Setup Request message, in which case the Setup Request 
SHOULD fail silently. 


> 
>>>>> 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
> 

This list doesn’t actually seem to check of the authDigest field, or if so I can’t identify it. Reading the text in Section 5.2 of -02 makes me wonder if you intend for the field to ever be populated, but I am confused as to why the client would not want to authenticate the server’s response. Could you clarify this in the document, please?

> 
>> 
>> (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 <<<<

This would be a good idea. Otherwise you’ll need  to defend in Security Considerations why it is never important for a server to validate (via authentication) that the Test Activation Request came from a legitimate client, and why it is never important for a client to validate that a Test Activation Response came from server rather than a device spoofing the server. The same would apply to the Status Feedback and Load PDU messages.

One note on the 5 minute window … strictly speaking, it isn’t a complete replay protection mechanism, since all replayed messages within the window will seem legitimate. A time window is useful, but in order to be effective does need to be accompanied by some record of previously received messages within that window. Otherwise, it serves only a a delay protection mechanism.

>> 
>> (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.

OK, sounds good.

> 
> 
>> 
>> (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.

These are important statements. I will note that because the authenticated portion of the message doesn’t include the ID of the client,  then the authentication material (e.g., key) must be associated with a client IP address.  This should be stated. This mitigates an attacker from stealing a key from a device and being able to send fake authenticated messages. 

> 
>> 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.

As a disclaimer, I cannot speak authoritatively for what the IESG or Security Area Directors will require, but I can give some advice that should be safe. That is:

(1) Define a method for strong authentication of all messages. SHA-256 HMAC is a good choice.
(2) Make that method required to be implemented for all messages, although it could be optional for a site to deploy it on all messages.
(3) Require authentication of test setup messages (i.e., Setup Request/Response, Test Activation Request/Response), except possibly for diagnostic purposes. This seems defensible to me if they they are “control plane” protocols, for which the added processing for authentication is feasible.
(4) Make authentication of “data plane” messages optional. This seems defensible to me, when accompanied with an explanation that the process of adding authentication to test messages can impact the test result accuracy.
(5) Since the devices performing measurement are network devices with constrained processing and operations, the required method  will likely use manually configured keys. Provide for an orderly key rollover by including key ids in the PDUs for the authentication keys, This will be helpful for both security and operations of the protocol. 
(6) Consider whether DTLS can be used as an option for the Setup Request/Response exchange, and possibly add extract keying material from that exchange using RFC 5705 for use of SHA-256 HMAC for the other exchanges.

I hope that helps.

Thanks,
Brian

> 
>> 
>> (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