Last Call Review of draft-ietf-taps-interface-20
review-ietf-taps-interface-20-genart-lc-fossati-2023-04-10-00

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-taps-interface-??
Reviewer: Thomas Fossati
Review Date: 2023-04-10
IETF LC End Date: 2023-04-14
IESG Telechat date: Not scheduled for a telechat

The document describes an API that abstracts the transport services
available on the platform and aims at replacing the BSD socket
interface.

The intended status is standards track which looks appropriate.

The document makes no requests to IANA.

The document is generally well written and easy to follow.

Internationalisation-related concerns are minimal.  Though maybe
something could be explicitly said about the parts of the API that can
become visible to human eyes, e.g., Message Contexts, and SendError
reasons.

Not a blocking issue - because in general the meaning can be inferred
quite easily - but I've found the typographic conventions to be not
super clear / consistent.  Some times input parameters are exemplary
(e.g., "https", 443), other times they are given generic names (e.g.,
GroupAddress).  It'd be probably neater if the API were described more
formally in terms of their (abstract) input and output types to prevent
the reader from inferring the wrong signatures.  Also, examples could
use a more self-consistent pseudo-code convention, but as per premise,
this is not blocking.  However, I'd suggest the authors to do a pass on
the whole document and see if they see opportunities to improve the API
presentation language.

The API objects can be quite complex, and there is no explicit
validation on construction.  Question: is the assumption that
implementations will do the same?  Or are they are free to, for example,
use, say, a builder pattern to provide a safer experience for their
users?

Question: should Appendix A be moved to draft-ietf-taps-impl?

Note: some examples have too long lines which will look suboptimally
when rendered in text and pdf.

I was curious about the implementation status but I couldn't find
further info.  If you happen to have implementations already, it'd be
nice to give them visibility in a BCP205 "Implementation Status"
section.

Eight authors are currently listed, but RFC7322 seems to allow at most
five [1].

[1] https://www.rfc-editor.org/rfc/rfc7322.html#section-4.1.1

I have made a PR that fixes a few typos I stumbled upon while reading:
https://github.com/ietf-tapswg/api-drafts/pull/1128

# 1.1

IPv4 and IPv6 addresses should be also listed among the basic types?
Also, strings (e.g., used for services, interfaces, hostnames).

# 3.1.1

HTTPS with raw public keys is not a great match ;-)
What about using a PK cert rather than raw PK?

# 3.1.2

I have found this example to be rather puzzling.  It looks to be sending
the same message on two different connections to the same HTTPS endpoint
at roughly the same time.  What is the use case for this?  Also, I
couldn't tell whether the same output buffer is used for both responses,
which added to my confusion.

The in last para:

  "Preconnections are reusable after being used to initiate a
   Connection.  Hence, for example, after the Connections were closed,
   the following would be correct:"

the positioning of "for example" is slightly ambiguous: it is not clear
whether it is necessary that all connections derived from a given
preconnection are closed before one is able to call Initiate() again, or
not.
Could you please clarify this aspect?

# 3.1.3

Apologies in advance, but my OCD is triggered by typographic
inconsistencies :-)

In this example, are the "..."-prefixed comments supposed to have
different semantics from those that don't?  If so, please explain,
otherwise remove the "...".

It was not immediately clear to me whether "sending the ResolvedLocal
list to peer via signalling channel" is taken care of by
Preconnection.AddRemote(RemoteCandidates).
Could you please clarify this?

Also, please remove the empty line before
Preconnection.AddRemote(RemoteCandidates), unless there's a good reason
for deviating from the rest of the examples, which I have missed.

# 4

The five paragraphs are slightly patchwork-y.  For example, the second
para fluctuates from Selection to Connection and then again to Selection
props.  

I suggest grouping more tightly around the different properties:

NEW

4.  Transport Properties

   Each application using the Transport Services API declares its
   preferences for how the Transport Services system should operate.
   This is done by using Transport Properties, as defined in
   [I-D.ietf-taps-arch], at each stage of the lifetime of a connection.

   Transport Properties are divided into Selection, Connection, and
   Message Properties.

   Selection Properties (see Section 6.2) can only be set during pre-
   establishment.  They are only used to specify which paths and
   protocol stacks can be used and are preferred by the application.
   Calling Initiate on a Preconnection creates an outbound Connection or
   a Listener, and the Selection Properties remain readable from the
   Connection or Listener, but become immutable.  Selection Properties
   can be set on Preconnections, and the effect of Selection Properties
   can be queried on Connections and Messages.

   Connection Properties (see Section 8.1) are used to inform decisions
   made during establishment and to fine-tune the established
   connection.  They can be set during pre-establishment and they may be
   changed later.  Connection Properties can be set on Connections and
   Preconnections; when set on Preconnections, they act as an initial
   default for the resulting Connections.

   Message Properties (see Section 9.1.3) control the behavior of the
   selected protocol stack(s) when sending Messages.  Message Properties
   can be set on Messages, Connections, and Preconnections; when set on
   the latter two, they act as an initial default for the Messages sent
   over those Connections.

   Note that configuring Connection Properties and Message Properties on
   Preconnections is preferred over setting them later.  Early
   specification of Connection Properties allows their use as additional
   input to the selection process.  Protocol-specific Properties, which
   enable configuration of specialized features of a specific protocol,
   see Section 3.2 of [I-D.ietf-taps-arch], are not used as an input to
   the selection process, but only support configuration if the
   respective protocol has been selected.

If you find the proposal above OK, I can PR this:
https://github.com/thomas-fossati/taps-api-drafts/tree/s4-reflow


I am not sure the following is entirely correct:

  "Calling Initiate on a Preconnection creates an outbound Connection or
   a Listener"

since a Listener is created with a call to Listen(), isn't it?

# 4.1

In this sentence:

  "alphanumeric strings in which words may be separated by hyphens."

It is not clear what is meant by "words"?  Also, s/hyphens/hyphens and
underscores"?

Note: wouldn't allowing only underscores as separators help uniformity
across languages?

In this sentence:

  "[...] and are defined in an RFC."

I am not sure why being defined in an RFC is relevant here?  Maybe
adding an example would help to understand the rationale..

The sentence:

  "Avoid using any [...]"

should probably use BCP14 language.

# 4.2

I got confused by the use of "Ignore" to mean "I have no preference, do
whatever".  Not a real problem, just a bit of cognitive dissonance.

# 6

5th para: why SHOULD and not MUST? Specifically, when is it OK for
multiple remote endpoints to represent different services?

# 6.1

Maybe use 4443 for WithPort(): I guess one would normally use
WithService("https") if they want to use the standard port for the
service, and WithPort(...) only for alternative/non-default.

# 6.1.1

The three options:
* Initiate() - send only
* Listen() - receive only
* Rendezvous() - send and receive
are well described.

One source of confusion for me though is the very first para.  Does it
apply to all three scenarios?  The sentence starts with the very generic
"To use multicast [...]" so it'd seem so, but then reading the second
para it looks like that preamble is specific to Initiate().

Could you please clarify?

# 6.1.3

Third para: It'd be probably more meaningful for:

  RemoteSpecifier.WithProtocol(QUIC)

to be

  AlternateRemoteSpecifier.WithProtocol(QUIC)

so that it chains up with the preceeding AddAlias() statement.


# 6.1.4

Same comment as before with regards to WithPort().


# 6.2.18

The reference to draft-ietf-taps-impl seems a bit too vague.  I have
scanned it but couldn't find anything unambiguously applicable.

# 6.3.1

Is there a reason for not providing a user-friendly way for configuring
X.509 verification?

# 6.3.2

The case of the trust callback does not match.

I think it should be one of:

SecurityParameters.SetTrustVerificationCallback(TrustCallback)

or 

trustCallback := NewCallback({
  // Handle trust, return the result
})


Ditto for the challenge callback.

# 8.1.5

Is this SCTP specific or could it apply more generally?

# 8.1.10

You could use "{{Section 4.2.3 of I-D.ietf-taps-arch}}" to make the
reference for connection contexts separation more precise: 

# 13

5th para: not clear to me what is meant for "a protocols or paths" to be
"raised"?  Same para: not clear what kind of "re-establishment is
supported"?

Last para, the bit on visibility, tracing and logging:

  "Further, a Transport Services system should provide an interface to
   poll information about which protocol and path is currently in use as
   well as provide logging about the communication events of each
   connection."

I reckon this is a very important point which is maybe worth BCP14 language.

# 15

I was a bit surprised that the TCP, SCTP and QUIC RFCs were not
referenced.  But more generally, I am unsure about the criteria used to
tell normative references from informative.  To me, BCP14 and
draft-ietf-taps-arch are the only two normative things, the rest could
be made informative.