Re: [dhcwg] WGLC on draft-ietf-dhc-sedhcpv6-21 - Respond by March 29th

[Timothy Carlin <[e-mail]>]

I apologize for the late email, I have read the document and have some
thoughts.

It's not completely clear which part of the messages are encrypted and
integrity protected, or if there are parts that are only integrity
protected.  The Increasing-number Option is contained in the encrypted
DHCPv6 messages, which may be a concern.

An attacker can transmit replayed packets and the server must attempt to
decrypt the packet to determine if it is a replayed (or otherwise invalid)
packet and an attack.  The decryption process is an expensive one, so this
could be a denial-of-service vector.

I have two possible suggestions.  The first is to move the
Increasing-number Option to an integrity protected (but not encrypted)
portion of the packet.  This allows for a preliminary check of the sequence
number, prior to any decryption or integrity processing.  This is similar
to ESP's sequence number check (https://tools.ietf.org/html/
rfc4303#section-3.4.3).

The second option is to document the potential attack in the Security
Considerations.  Additionally, including text to limit the rate at which
encrypted messages may be sent by a client and subsequently processed.

I'm also wondering about using RSA for encryption, as the data size (and
thus message size) is limited to the key size that the client or server
provides.  This could cause problems with large DHCP messages.  Could a
symmetric key algorithm (e.g. AES) be used?  The keys for this would have
to be negotiated during the initial Information-request/Reply.

Tim Carlin
UNH-IOL

On Thu, Mar 30, 2017 at 2:00 PM, Tomek Mrugalski <[e-mail]
> wrote:

> I have reviewed the draft and in my opinion -21 is in great overall
> shape. The text reads so much better, the concept is clearly explained
> early and the whole text fits together well. I do have many comments,
> but great majority of them are clarifications and editorial corrections.
> Good work, authors!
>
> Ok, let's start with easy ones.
>
> idnits. There are a number of idnits errors and warnings: 3 errors (**),
> 0 flaws (~~), 8 warnings (==), 5 comments (--).. Please fix them. Ping
> me off the list if you need a little help with idnits tool.
>
> This draft defines a set of recommendations for secure environment. It
> is not expected to be used by all DHCP implementations is every
> deployment, just those who are interested in improved security. In
> certain way it is very similar to anonymity profile. Anonymity profile
> defined set or rules that are not expected to be used by all DHCP
> implementations in every deployment, just those who are interested in
> improved privacy. So maybe it should be called Security Profile for
> DHCPv6? I think this approach does not change anything from technical
> perspective, but simply frames the solution better. Described that way
> it may also be easier to accept to people who may afraid we want to
> force everyone to encrypt everything everywhere. It's just a suggestion
> to consider, I don't insist on it if authors or the WG disagree.
>
> I like section 5, especially 5.1 and 5.2 a lot. It's a great two-page
> intro to the concept.
>
> Section 5.3
> nit: mainly a algorithm => mainly an algorithm
>
> Section 5.4 should also discuss the problem with Confirm message
> mentioned earlier by Jinmei.
>
> Section 5.5
> nit: In some scenario => In some scenarios
>
> Section 6
> nit: is a Increasing-number option => is an Increasing-number option
>
> Section 6: Is Advertise protected as well? I think it definitely should
> be to the maximum extent possible. Once the initial info-request/reply
> is completed, the next step for the client is to send Solicit wrapped in
> Encryption-Query. The server will reply with Advertise wrapped in
> Encryption-Response. At this stage the received Advertise can be
> validated. Maybe not all of the checks are possible yet for Advertise,
> because of not all necessary information is available yet (e.g. there's
> not possible to do the increasing number check as this is the first
> packet coming back and the client doesn't have previous value to check
> it against), but some checks are definitely applicable (e.g. drop
> because signature is missing, multiple signatures or certificate missing).
>
> Unless there's a good reason to not do it, the rules described in on
> page 9 should apply to both Advertise and Reply.
>
> "beat out a busy "real" server" => "beat out a busy legitimate server"
>
> There is this text: "The Certificate option MUST be constructed as
> explained in Section 10.1.2.", but I don't see any "client MUST send
> Certificate option". It's also not shown on Figure 1. Yet, the server
> section says that incoming messages without Certificate option are to be
> dropped. This should be clarified.
>
> It's a matter of preference, but I think Section 6 is too long and
> should be split into several sub-sections. When someone references it
> ("see Section 6"), it's difficult to find what specific parts of the
> text is being pointed out to.
>
> "the subsequent encrypted DHCPv6 message sent from the client can also
> contain the Increasing-number option to defend against replay attack.".
> Is there a reason why "can" is used, not SHOULD or even MUST?
>
> "If the transaction-id is 0, the client also try to decrypt it." Two
> comments about this. First, what is the client supposed to decrypt - the
> transaction-id field or the whole message? Second, what should the
> client do if it's not 0?
>
> "In this document, it is assumed that the client will not have multiple
> DHCPv6 sessions with different DHCPv6 servers using different key pairs
> and only one key pair is used for the encrypted DHCPv6 configuration
> process." That's a good text, but it doesn't belong in the place where
> it is now. It should be moved to separate paragraph or even separate
> section. You may also consider adding something like "Secure DHCPv6 can
> maintain multiple DHCPv6 sessions, but such scenario is not considered
> here for clarity reasons."
>
> "The encryption text SHOULD...". What is encryption text? Did you mean
> encrypted message or the message to be encrypted?
>
> nit: "as explain in [RFC5652]" => "as explained in [RFC5652]"
>
> "After the decryption, it handles the message as per 3315". I'm not a
> security expert by any stretch of that word, so excuse my naive
> question. Anyway, can you always detect that the decryption was a
> success or failure? Some decryption techniques will produce output, even
> when the key is incorrect, but the output will be an almost random
> garbage. Can the mechanism defined in RFC5652 always reliably conclude
> the decoding operation was success/failure? If not, I'm afraid that
> asking clients and server to decode random garbage will break some
> implementations.
>
> If the decryption algorithm cannot give definite success/fail answer,
> there's a solution I have in mind. You may add a magic cookie (a well
> known value of perhaps 4 bytes) at the beginning before encrypting the
> actual message and encrypt (cookie,message) tuple. Then the first check
> after decryption the receiver would do is to check if the first 4 bytes
> match the cookie value. If not, then throw away the whole packet without
> trying to parse any of it. I discussed the concept with Bernie and he's
> in favour of it. Also discussed with Jinmei and he's against it. Sten
> posted his comment against it, but I'm not sure if Bernie's description
> of the concept was accurate enough. Any other thoughts on this?
>
> "switch to other DHCPv6 server if it has another one." => "switch to
> other DHCPv6 server if available."
>
> The text uses "one and only one" several times. You may consider
> replacing it with "exactly one". It reads easier that way, but I admit
> it's a matter of preference, so it's just a suggestion.
>
> "The DHCPv6 server drops the message that is not for it, thus not paying
> cost to decrypt messages."
> => "The DHCPv6 server drops the message if the value of Server
> Identifier option does not match its own, thus not paying the cost of
> message decryption."
>
> "If the server cannot find the corresponding private key of the key tag
> or the corresponding private key of the key tag is invalid for
> decryption, then the server drops the received message." Shouldn't the
> server send back AuthenticationFail status? The benefit is that the
> client could try again and use a different certificate.
>
> "For the signature failure, the server SHOULD send an encrypted Reply
> message with an UnspecFail (value 1, [RFC3315]) error status code to
> the client." Why use UnspecFail, if there's SingatureFail code for it?
>
> General comment: "cert" should be replaced with "certificate". Even
> though everyone understands this jargon, it should be avoided in formal
> documents.
>
> Section 9.1:
> nit: "The server should keep a record of the increasing number forever."
> Nothing is eternal, so I don't like the word forever. It would be better
> to say the server must be able to retain the increasing number across
> restarts.
>
> Remove "And " at the beginning of two sentences in first paragraph.
>
> Section 9.2:
> Thanks a lot for including the code. It's very helpful. However, please
> update the comments that mention RDATA, DNSKEY RR and RDLENGTH. That way
> fewer people will realized where it was borrowed from :)
>
> Nit: some of the values to be assigned by IANA are boilerplated as
> TBD[number] and some as TBA[number]. Typically it's TBD[number].
>
> Section 10.1.1
>
> The text describing EA-id List could use an update. First, it's not
> clear what EA stands for (it is explained further down, but requires a
> bit of hopping around the text). I would replace "EA-id: The format of
> the EA-id List field is shown in Figure 3." with "EA-id List: Contains a
> list of supported Encryption Algorithm identifiers. The format of the
> EA-id List field is shown in Figure 3." Similar text should be added for
> SHA-id List ("Contains a list of supported Signature/Hash Algorithm
> pairs.").
>
> Nit: I'm not sure, but it looks like you put the whole description of
> EA-id into the Figure 3. It's better to have it as text outside of
> Figure. That's probably also one of the reasons why idnits is unhappy.
>
> 10.1.2
> "The Certificate option carries the certificate of the client/server,
> which is contained in the Reply message." This is confusing. I could
> easily misinterpret this as "server is supposed to send client
> certificate in Reply message" or even "Client can send Reply message
> with a certificate". It would be better to say "The Certificate option
> carries the certificate of the sender. It can be included by the client
> (see Section 6) and by the server (see Section 7)." Also, see my earlier
> comment about Section 6 not explicitly saying if/when the Certificate
> option is to be sent.
>
> Sections 10.1.*
> As a convenience to the reader, I would add a text similar to "The
> Algorithm option is sent by the client, as described in Section 6.".
> Similar texts should be added in other sections that describe the
> options. It can be confusing to understand who sends what, so such
> pointers are useful in my opinion. This applies to all sections that
> describe options.
>
> Section 10.2
> "share the following format:" => "share the standard format of DHCPv6
> messages:".
>
> Section 11
> "There are some mandatory algorithm for encryption algorithm in this
> document.  It may be at some point that the mandatory algorithm is no
> longer safe to use." => "There are some mandatory algorithms specified
> in this document.  It may be at some point that the mandatory algorithm
> is no longer safe to use. To mitigate this problem, an extension
> mechanism was defined that allows usage of new encryption, signing and
> hash algorithms."
>
> Another problem that should be mentioned is that secure traffic cannot
> be snooped by the relay agents. This is of crucial importance for
> networks that use PD and use relay snooping. It would be even more
> appropriate to create separate section called Deployment Considerations
> for it. If the RAAN draft is resurrected before sedhcpv6 reaches IESG
> (it should), adding text similar to "This issue will be addressed in
> future IETF work and add reference to
> draft-ietf-dhc-dhcpv6-agentopt-delegate).
>
> Another possible topic for discussion in deployment considerations would
> be how to do Rebind. My understanding is that it would work as long as
> both servers would use the same certificates. This explanation is buried
> deep in section 5.4, but it's probably useful to mention it explicitly
> (or have a pointer to 5.4) if you decide to create deployment
> considerations section.
>
> Section 12
> Do you (that's a broad question to the WG, not just authors) think it
> would be useful to add extra column to the new registries, specifying
> whether the protocol is mandatory, optional or deprecated? We would
> start with one mandatory in each registry and all other entries being
> optional. When new protocols are defined in the future and flaws in
> existing ones are discovered, we could publish short documents to easily
> update the current state of affairs. It would also correlate well with
> the text you have in section 11.
>
> I agree with Jinmei-san that the Confirm message problem is something
> that should be tackled. I don't have any ready to use solution for you,
> but going in the direction that Jinmei suggested (doing separate
> inf-request/reply to discover available servers) seems the best way.
>
> Finally, to answer my own question I think dhc-dhcpv6-agentopt-delegate
> should be resurrected right now, so sedhcpv6 could reference to it, when
> it discusses the issue with snooping. This is something I hope to
> discuss in couple hours during meeting.
>
> Ok, that's it.
> Tomek
>
> _______________________________________________
> dhcwg mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/dhcwg
>