[ech] ECH PR split/progression

[Stephen Farrell]

Hiya,

Thanks for delving a bit deeper, I think there are issues
to consider below...

On 22/01/2024 09:44, Matt Caswell wrote:
> 
> 
> On 22/01/2024 01:07, Stephen Farrell wrote:
> 
>> FWIW, I'm unclear if following this plan would mean that
>> (all going well) each PR would be merged with master before
>> the next is processed, or if the plan is really to help with
>> more tractable reviews before ending up again with one PR
>> that has all the functionality.
> 
> If we are aiming to have smaller PRs that can individually be merged to 
> master then each PR needs to be entirely stand alone and include all 
> tests etc relevant to it. 

Yeah, that'll be a challenge with pretty much any functional
split into separate PRs, e.g. IIRC boring only supports use
of x25519 for ECH and not other HPKE suites, and there are
aspects of inner CH handling that differ between boring and
my client and nss that'd make it hard/impossible to exercise
some server code paths with just any one of those as a client.
There'd also be a whole pile of inevitable duplication of
current test code (e.g. HRR/early-data) that'd be fairly
painful if we wanted a first PR to be merged to master and
then released. I think there'd be similar issues with any
functional split I can think of, which is how we ended up
with the mega-PR in the first place.

(Aside: now that I know external tests are ok, the above
reminds me I should add one for nss as well as one for
boring, so I'll do that.)

> From 3.3 we are doing "time based" releases 
> (with 3.3 being in April this year), which means that master must be in 
> a ready-to-go state at any point. We can't have "half finished" changes 
> in master.

Ack. That's probably gonna be another challenge - getting from
here to "done" in any 6 month period would call for a lot of
effort on your (the project's) side. Is that feasible?

> 
> The alternative is for us to create a "feature branch" for ech and put 
> reviewed PRs there, pending the complete set becoming available. We have 
> one such feature branch at the moment 
> (https://github.com/openssl/openssl/tree/feature/quic-server) - but 
> frankly we're still figuring out how to manage things via a feature branch.
> 
> There are pros and cons to each approach.
> 
> The stand alone approach is simplest. We understand what we are doing 
> with that. On the downside it might be more difficult to break things up 
> to be entirely stand alone. Additionally we might be less willing to 
> merge a big "high risk" PR the closer we get to April (or any subsequent 
> time-based release date).

Right. I guess you'd need to have a plan that allocates some
maintainer effort for reviewing this that'd fit within a release
cycle. (But maybe that's what's needed in the end?)
> OTOH the feature branch approach has the advantage that we can work on 
> the PRs without worrying about release dates etc, or ensuring that 
> everything is fully stand alone. We can have "half finished" features in 
> a feature branch. 

That does sound like it might be a way forward...

> On the downside we are still learning how to manage 
> feature branches - we haven't done them before. Such a feature branch 
> would need to be regularly rebased (probably by a committer) 

As it happens we now have a chap working with us on the
defo project who's a whizz at that kind of thing and we
have in any case planned to setup a CI thing that'd do
that rebasing (for openssl but also for our PoC code for
web servers and curl). So we'd at least get regular mail
when there's some conflict that needs resolving and could
get those fixed pronto I think. If a feature branch means
having to do those via individual PRs that get approved
though, yeah, that's annoying overhead.

> - and we're 
> not too clear on what the rules are for doing that or how it will work. 
> There is a certain "overhead" in having a feature branch.

Ack. I do think having a worked-out way to handle such
feature branches would be good for the project though,
so maybe ECH is a potential guinea-pig for figuring out
those issues? (FWIW, I'd be willing to play my part of
that game were it the right way forward.)

> I'd personally prefer the stand alone approach if it can be arranged - 
> just because it is simpler.

True, but it probably needs the project to allocate the
effort to allow people to do the review work in some
six month period. (In the end, making a release that
doesn't support both client and server sides of ECH
seems fairly odd to me.)

Cheers,
S.

> 
> Matt
> 
> 
>> I'm assuming the former, but
>> would appreciate confirmation that that's feasible. (E.g.
>> the 1st "server" PR would have to omit lots of test code
>> that exists and works today, but that wouldn't work if the
>> client code is temporarily omitted.)
>>
>> Thanks,
>> S.
>>
>>
>> [1] https://github.com/openssl/openssl/pull/22938
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE4D8E9F997A833DD.asc
Type: application/pgp-keys
Size: 1197 bytes
Desc: OpenPGP public key
URL: <https://mta.openssl.org/pipermail/ech/attachments/20240122/68569c57/attachment-0001.asc>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://mta.openssl.org/pipermail/ech/attachments/20240122/68569c57/attachment-0001.sig>