[Taps] Paul Wouters' Discuss on draft-ietf-taps-interface-22: (with DISCUSS and COMMENT)

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

Paul Wouters has entered the following ballot position for
draft-ietf-taps-interface-22: Discuss

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/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

[ this review is still incomplete, but I wanted to at least file what I have here ]

- netflows vs IP flows?

Is TAPS only meant for flows or does it also support IP transport.
I don't see an example that would embody "setup a connection to 192.0.1.1 port
80, but require IPsec. I guess it could be added, eg a WithVPN("IPsec") or
something, but I'm not sure you can specify credentials without those being
interpreted to be for the port 80 in this example ? Is this specifically out
of scope, or just not (yet) specified?

Related, lets say you want to only use a flow if it goes over TOR, I guess one
could introduce a WithTOR() transport requirement. I'm a bit concerned that
without an IANA registry for functions, this might cause conflicts in the future.

- single credentials for multiple protocols?

Does this document cause encouraging of re-using of key pairs for
different protocols (eg TLS and IKEv2, or TLS 1.2 and QUIC). This
might have security implications (eg using RSA as RSA-PKCS1.5 as
well as RSA-PSS)

- identity and server-certificate forces same use over different protocols?

The current API kind of forces re-use of credentials for different protocols, eg:

SecurityParameters.Set(server-certificate, myCertificate)

        // Specifying a Remote Endpoint is optional when using Listen
        Preconnection := NewPreconnection(LocalSpecifier,
                                  TransportProperties,
                                  SecurityParameters)

Let's say that a server only supports TLS 1.2 with RSA-PKCS-1.5 and TLS
1.3 with RSASSA-PSS. We shouldn't use 1 RSA key from 1 certificate for
these two different RSA operations. Would this mean we could never tell
the TAPS API "TLS 1.2 or 1.3 is fine". Maybe that is okay and we should
pick an keypair that works the same in 1.2 and 1.3, but we don't know
what the future will hold.

If we could set multiple identities, TAPS could pick the appropriate one.
eg:
        SecurityParameters.Set(identity, myIdentity)
        SecurityParameters.Set(server-certificate, []myCertificate)

(and possibly multiple identities too?)

- Hostname vs FQDN

Can WithHostname be unqualified? (God I hope not!)
If not, can the call WithHostname be renamed to WithFQDN ?
If it can be unqualified, how does one deal with credentials
and identifiers, eg with a 'search domain' containing multiple
domains, RemoteSpecifier.WithHostname("mail") could end up on
either mail.example.com or mail.example.net (or heaven forbid,
the .mail TLD)

- WithPort and WithService, but not WithProtocol

How does one choose their syslog service transport protocol, since syslog
over udp and tcp are valid and both have the same service name? Or does
WithService accept values like "514/udp" ?

- ALPN

I don't see a WithALPN method for setting the ALPN.
- Preconnection vs Listener

The arch document talks about Preconnection, Connection and Listener
but this document in Section 3 places Listener as a Preconnection.


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

- Camelcase vs Dromedariescase

Why is it RemoteSpecifier.WithHostname and not RemoteSpecifier.WithHostName,
while it is RemoteSpecifier.WithIPAddress and not RemoteSpecifier.WithIPaddress ?

        String: Instances are represented in a way that is common to
        the programming language, e.g. UTF-8.

Why doesn't it define this just like the other basic types?
eg: "Instances are represented in UTF-8"
Also, how would that work for things like DNS (Ulabel vs Alabel)


        IP Address: An IPv4 or IPv6 address.

Maybe reference RFC5952 ?


        It is not necessary for an application to handle all events;
        some events may have implementation-specific default handlers. The
        application should not assume that ignoring events (e.g., errors)
        is always safe.

What is "ignoring events" vs "having a default handler for events" ? Wouldn't
default handlers be handled outside the application, and thus not trigger an
event towards the application? So that applications "ignoring events" would
be unrelated to "implementation-specific default handlers"? Perhaps the last
sentence should just start in a new paragraph to indicate these two things
are separate unrelated notes?

        SecurityParameters.Set(identity, myIdentity)

I assume myIdentity would be an array of Type and Value? Eg type=fqdn or type=RDN ?


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 ?

What is "credentials" (especially because previously we have myCertificate,
so it is unclear if this now contains myKey and if so, where in the previous
example myKey was conveyed to TAPS.

        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 4.1:

        For the purposes of this document, these names are alphanumeric strings

Is there another purpose? If it is outside this document, why does this document
care? Does it try to set a standard here to ensure interoperable names? Maybe
just remove the "For the purposes of this document," ?

        Vendor or implementation specific properties MUST use a string
        identifying the vendor or implementation as the Namespace

What if Vendor Foo has a non-standard tcp property MagicSmoke? Does it go
under tcp.Foo.MagicSmoke or under Foo.tcp.MagicSmoke or something else?

Section 6.1

        An Endpoint object can be configured with the following identifiers:
        * Hostname (string):

This starts with listing endpoint identifiers with types (eg string and 16-bit
integers) but then stops specifying their types further down. I guess WithService(),
WithIPAddress() and WithInterface take a (string)

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.

        If security is opportunistic, it will allow Connections without
        transport security, but will still attempt to use security
        if available.

I assume what is meant is "but will still attempt to use unauthenticated security
if available" ?

       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 ?

       A higher value reflects a higher priority.

This is uncommon in IETF protocols. Normally priority is down with lower numbers
being higher priority. Why reverse it here?

[ review to be continued from 8.1.3 ]



NITS:

Wouldn't the American spelling be "rendezvouzing" instead of "rendezvousing" :-)