[Taps] Paul Wouters' Yes on draft-ietf-taps-interface-24: (with COMMENT)

[Paul Wouters via Datatracker <[e-mail]>]

Paul Wouters has entered the following ballot position for
draft-ietf-taps-interface-24: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-taps-interface/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I updated my discuss items to non-blocking comments. I'm still a bit concerned
that some security items are not fully addressed by the API or its Security
Considerations. Resolving my comments would make the document a bit more clear
and useful I think.

I'm not sure that the model really expands from netflows to IP flows (or TOR)
in the future.

I'm not sure the Security Considerations warn enough about re-using the same
credentials with different protocols/auth mechanisms. (eg TLS and IKEv2, or TLS
1.2 and QUIC, or using RSA as RSA-PKCS1.5 as well as RSA-PSS)

Unqualified Hostname vs FQDN seems a security risk punted mostly to the
application.

I don't understand this code:

        Connection := Preconnection.Initiate()
        Connection2 := Connection.Clone()

        Connection -> Ready<>
        Connection2 -> Ready<>

        //---- Ready event handler for any Connection C begin ----
        C.Send(messageDataRequest)

Where does "C" come from in "C.Send" ? The comment says "any Connection C"?

I have a question on this code:

        Preconnections are reusable after being used to initiate a
        Connection, whether this Connection was closed or not. Hence, it
        would be correct to continue as follows after the above example:

        //.. carry out adjustments to the Preconnection, if desired
        Connection := Preconnection.Initiate()

What would happen here? I can imagine a "compiler" turning this into
a noop.  I can also see it would kill the existing Connection state and
start a new one. This could be to a different IP address (eg if the DNS
name has A and AAAA). When starting a new one, what would happen to any
Message or Event queues for Connection ?

        Preconnection.AddRemote(RemoteCandidates)

Should this not technically be:

        Preconnection.AddRemote([]RemoteCandidates)

as the array contains at least a host and a stun server candidate?

Maybe this is just the difference between you using the variable you define
that has been assigned, versus a more C like prototype format, eg:

        Preconnection := NewPreconnection([]LocalEndpoint,

So I guess if your example here had set LocalEndpoint := [a,b] you would not
have used [] in the call ?

Section 6.1.4

Perhaps it would be useful to add a Local Endpoint with ephemeral port before
the Local Endpoint with static port example, as the ephemeral port should be
the far more common case. Right now the examples might give the wrong impression
a local port MUST be specified.

SecurityParameters.Set() seems to allow to set our identiy and our certificate,
but not the remote peer's identity or certificate? For example, one might want
to pin a remote certificate and not just rely on a WithHostname() identifier
being present as subjectAltname on a certificate.

       The Connection state, which can be one of the following:
        Establishing, Established, Closing, or Closed.

I think the text in Section 8 should more clearly show the property names if the
goal is to have different implementations use the identical name. Eg in this
case, why not write: The Connection state ("state"). The next two entries are
similarly lacking a clear keyword to use:

        Whether the Connection can be used to send data.

        Whether the Connection can be used to receive data.

eg. why not define words for these to implementations will use the same words,
in this case perhaps ReadySend and ReadyRcv ?
Writing that now, perhaps "state" should be "State" then ?

Section 8.1.1:

        If this property is an Integer

It is best to define the actual type in this document and not let
implementations choose, if the goal is to try and harmonize implementations. I
also see no non-integer value being given here ?