Subject: [tcpinc] Review of draft-ietf-tcpinc-api-03

From: Kyle Rose <[email-masked]>
Date: Fri, 28 Jul 2017 18:24:49 -0400

(Keepalive before list timeout.)

This is a review of draft-ietf-tcpinc-api-03. Chair hat off.

Overall, I think the doc is informative. I want to echo some
discussion from long ago (maybe in Berlin or even earlier?) re: the
abstractness of the API. In particular, section 4 begins:

q( The previous sections presented abstract APIs )

when they feel remarkably concrete in some ways: everything from
C_STYLE_CONSTANTS_IN_UPPERCASE_WITH_UNDERSCORES to suggestions about
the representation of bad ports. IIRC, one complaint was that an API
like this feels too concrete for the IETF, but is nowhere near precise
enough for POSIX. I agree with that assessment.

That said, there is a lot of value here, so I'm happy with something
that is basically socket API pseudocode because it's clear to anyone
who familiar with the Berkeley sockets API, which is most people who
write networking software. I don't know if that will be okay with the
IESG. Maybe Mirja can chime in on that.

For the following I'm going to assume we're okay with this
presentation style. On to the show. By section:

1:

* Not sure what versions "(and perhaps versions)" refers to in the
second to last bullet in the ENO section. If you replace "encryption
scheme" with TEP, the parenthetical can probably just go away.

* I don't understand the rationale behind the last bullet in the
tcpcrypt section. Is the presumption that an application will
authenticate the first connection and then resume without
authentication? If so, then it's more than prudent: it MUST purge that
session from the cache.

* Relatedly, does this suggestion apply only to the keys cached for
the current session? For performance reasons, this should probably be
the case.

2.1:

* q( Non-socket systems should use existing error codes corresponding
to the same conditions.) I would strike "existing". What if they don't
have existing error codes corresponding to the same conditions, for
whatever reason?

* EBADF should be used for attempts to operate on closed sockets.
EINVAL should be used when any other element of the input is invalid
(e.g., invalid TEP).

* Right under table 2, there is a set of conditions starting with
"Unless otherwise specified". Do any of them cover the case in which
ENO is being negotiated but is not yet either enabled or disabled? In
implementing this, ENO was really best modeled as a tri-state:
negotiating, disabled, enabled. I'm thinking in particular of NEGSPEC,
PEER_GOPT, ROLE, and PEER_NAME.

* Sort the option descriptions in the same order as given in the table.

* Throughout this section, there are double quotes used
inconsistently. I would just remove them all, but either way make them
consistent. See: "TCP_ENO_RAW", "EISCONN", "A", "B".

* Under TCP_ENO_NEGSPEC, I would strike the last sentence as redundant
with TCP-ENO.

2.2:

* Is "Global options" going to be confused with the global suboption
in TCP-ENO? Maybe "System-wide default options/configuration
settings".

* Does it make sense to prefix all of the global options with
"eno_default_" to make it clear they can be overridden? Or is that
just too wordy?

* "eno_specs" should probably be updated for the TEP nomenclature.

* What is "baddynamic"? A Google search was unrevealing. Does this
need a reference?

* I would replace "bad" with "disable" or "exclude", especially if we
go with "eno_default_".

* If we're getting into the format of each value by byte, anyway, why
not go with each word representing a range "lowport:highport"?

3.1:

* The tcpcrypt doc consciously does not normatively specify how a
server is to choose the symmetric cipher. TCP_CRYPT_BCONF, by
contrast, specifies a decreasing preference order. This may not be
necessary (though it might be easier to understand given that this API
straddles the line between abstract and concrete), but it's also
potentially confusing that this preference list is in decreasing order
while the one for TEPs is in increasing order. (I understand that the
ordering there relates to saving the length byte for the TEP you
expect, when it is most likely to contain a session ID, and maybe this
isn't a big deal. Happy to entertain rebuttals.)

4.2:

* q( For example...a suitable name would be "net.inet.tcp.eno.specs"
). Why "specs" (or even "teps")? Maybe "<option name>" or a clause
introducing the example.

5.1:

* TCP_ENO_LOCAL_NAME conflicts with SELF_NAME.

* Can you describe for me an attack that would be possible using an
authenticator taken out of context? I'm assuming that both ends of the
connection with the unique session ID already have the cookie, and
that the authenticator would be useless on any other tcpinc connection
because of the uniqueness property. Is the issue that the session ID
used in the PRF could collide with an entirely distinct (i.e.,
non-tcpinc) use? Something similar applies to 5.2, as well.

5.2:

* It might be worth pointing out that, in unilateral authentication,
the side that does not validate an authenticator relies on the peer
ending the connection if validation fails to prevent man-in-the-middle
attacks.

Kyle