Re: [secdir] Secdir early review of draft-ietf-taps-interface-13

[Sean Turner <[e-mail]>]

I went ahead and submitted Issues/PRs for these.

Note that some were addressed by other merged PRs, some I reconsidered and withdrew as comments, and some I revised after re-reading them.  And, I came up with a new one about whether the PSK modes need to be included in s6.3.1:
https://github.com/ietf-tapswg/api-drafts/issues/999

Cheers,
spt

> On Oct 19, 2021, at 22:42, Sean Turner via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Sean Turner
> Review result: Has Issues
> 
> This is an early sector review. Based on the ARTART early reviews of this I-D
> and the -arch I-D, there is no doubt going to need to be another review after
> the authors get done restructuring both I-Ds.
> 
> BTW: Theresa thank you for the heads up about that restructuring. It forced me
> to read the -arch and -impl I-Ds, but that probably helped me not make silly
> requests of the authors/WG.
> 
> tl;dr: I picked "has issues" knowing there is a rewrite on the way and I would
> like to reserve final judgement until then.
> 
> I apologize upfront because I am going to ramble a bit before I get to the
> specifics:
> 
> The taps I-Ds are about “a protocol-independent Transport Services Application
> Programming Interface (API).” I took a pretty liberal approach when reviewing
> this I-D from a security perspective because these I-Ds are somewhat different
> than the “normal” protocol I-Ds. As noted in the -arch I-D: these I-Ds do not
> specify “specific protocols or algorithms” … gasp. But, that’s probably okay in
> this context.
> 
> I wondered what you would say about an API:
> - Apps need to use the API properly! check: see -arch I-D
> - Implementation/Library needs to be from trust source! check: see -interface
> I-D - Apps need to use the right keys at the right time! check: see -arch I-D -
> Avoid fallback/downgrade to insecure protocols! check: see -arch I-D and -impl
> I-D - Deal with 0-RTT! check: see -impl I-D - Address privacy aspects! check:
> see -interface I-D
> 
> I mean maybe you could add something about:
> 
> - randomness for keys - but that better already be in the security protocol
> specifications so I do not think it is absolutely needed here.
> 
> - following good programming techniques - but I am not sure where you would
> point nor whether it really makes sense to do so.
> 
> I think I found what I was looking for is somewhere in the stack of I-Ds. Am I
> worried somebody can implement this wrong. Sure. But, not any more than I would
> be for any other protocol.
> 
> Now on to the specifics:
> 
> NOTE: I tried not repeat anything from the ARTART review.
> 
> 0) s6: maybe this wording would be better?
> 
> For these two sentences are you trying to say that MUST else if?
> 
> OLD: At least one Local Endpoint MUST be specified if the Preconnection is
>   used to Listen() for incoming Connections, but the list of Local
>   Endpoints MAY be empty if the Preconnection is used to Initiate()
>   connections.
> 
> NEW: At least one Remote Endpoint MUST
>   be specified if the Preconnection is used to Initiate() Connections,
>   but the list of Remote Endpoints MAY be empty if the Preconnection is
>   used to Listen() for incoming Connections

https://github.com/ietf-tapswg/api-drafts/issues/973

> 1) s6.3.1 I know this is an example, but can we replace:
> 
> SecurityParameters.Set(supported-group, "secp256k1")
> 
> with
> 
> SecurityParameters.Set(supported-group, "secp256r1")
> 
> r1 the current MTI for TLS. k1 is for bitcoin :)
> 
> Or, is this where I get a bitcoin because I caught the shiny object? :)

https://github.com/ietf-tapswg/api-drafts/issues/975

> 2) s6.3.2: Is this like checking the certificate status?

https://github.com/ietf-tapswg/api-drafts/issues/977

> 3) s8.1.1- I read this definition a bunch. Are there two special values? The
> Type line says “Full Coverage” is special, but then the description says “0” is
> special too. Maybe look at s91.3.6 for inspiration?

https://github.com/ietf-tapswg/api-drafts/issues/978

> 4) s8.1.2: Any reason it’s not called connPriority? Seems arbitrary to drop the
> end of the word when connTimeout is even longer.

https://github.com/ietf-tapswg/api-drafts/issues/979

> 5) s8.1.2-should “Type” line also include (non-negative) like s8.1.1 and
> s9.1.3.2 do?

https://github.com/ietf-tapswg/api-drafts/issues/981

> 6) s8.1.3/.4: The Numeric values specifies seconds, minutes, hours, or
> millennia?

https://github.com/ietf-tapswg/api-drafts/issues/983

> 7) s8.1.6: don’t you need to specify the Type? I guess Enumeration is what
> you’re after?

https://github.com/ietf-tapswg/api-drafts/issues/984

> 8) s8.1.11.2/.3/.4: Are these sizes in bytes too like 8.1.11.1?

https://github.com/ietf-tapswg/api-drafts/issues/986

> 9) s8.2.1: RFC 5482 allows granularity of minutes or seconds don’t you need to
> say which it is here?

https://github.com/ietf-tapswg/api-drafts/issues/988

> 10) s8.2.2: Is there some reason it’s not called tcp.userTimeoutEnabled?

https://github.com/ietf-tapswg/api-drafts/issues/989

> 11) s8.2.3: Is there some reason it’s not called tcp.userTimeoutChangable?

https://github.com/ietf-tapswg/api-drafts/issues/991

> 12) s9.1.3.2: Why 100?

https://github.com/ietf-tapswg/api-drafts/issues/993

> 13) s9.1.3.2: Why shorten to msrPrio when msgOrdered is almost as long and
> safelyReplayable is longer?

https://github.com/ietf-tapswg/api-drafts/issues/994

> 14) A.1: Could the caution about freeing memory be generalized?

https://github.com/ietf-tapswg/api-drafts/issues/996

> 15) What follows are the I-D nits references to check. Some of these might be
> on purpose:
> 
>  == Outdated reference: A later version (-11) exists of
>     draft-ietf-taps-arch-10
> 
>  ** Obsolete normative reference: RFC 4941 (Obsoleted by RFC 8981)
> 
>  == Outdated reference: A later version (-06) exists of
>     draft-ietf-httpbis-priority-03
> 
>  == Outdated reference: A later version (-10) exists of
>     draft-ietf-taps-impl-09
> 
>  -- Obsolete informational reference (is this intentional?): RFC 5245
>     (Obsoleted by RFC 8445, RFC 8839)
> 
>  -- Obsolete informational reference (is this intentional?): RFC 5766
>     (Obsoleted by RFC 8656)

https://github.com/ietf-tapswg/api-drafts/issues/997

> _______________________________________________
> secdir mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/secdir
> wiki: https://trac.ietf.org/trac/sec/wiki/SecDirReview