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

[Tomek Mrugalski <[e-mail]>]

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