[Pkg-swan-devel] Bug#848890: Bug#848890: polished remaining delta for re-review

Tue Dec 5 09:12:33 UTC 2017

On Tue, 2017-12-05 at 08:31 +0100, Christian Ehrhardt wrote:
> On Mon, Dec 4, 2017 at 9:56 PM, Yves-Alexis Perez <corsac at debian.org> wrote:
> > On Thu, 2017-11-30 at 16:31 +0100, Christian Ehrhardt wrote:
> > > Pushed it to the same debian-submission-nov2017 branch as before.
> > 
> > Thanks,
> > 
> > I don't really have good news though. I took a look and first, it seems
> > that
> > the git commit entries aren't really good. git log --format=oneline is
> > barely
> > readable, for example.
> 
> Yeah the format was meant for another tool which made debian/changelog
> entries out of it.
> I could rewrite them the next time we talk about this topic in
> probably 6 months :-)

Ok :)
> 
> > For the commit contents:
> > 
> > 1aa409a5 (mass plugin enable): NACK again; I won't enable that many stuff
> > (and
> > especially not in one go)
> 
> I can understand, this all is just a suggestion.
> Things came up (way before my time) due to user requests and if I did
> history research correctly at that point it was decided to enable a
> few more to not get requests for extra plugins all the time.
> You are not taking all - that is fine, if you take a few that is good
> enough.

It might help to do one commit per feature (maybe one commit for consistent
groups) too so I can cherry-pick commits.

> Since I wasn't part of that old enabling activity I wanted to sync
> with you here and even considered dropping a bunch of them next (post
> LTS) cycle.
> Nobody remembers the particular reasons for some of them, so for all
> that make no sense to you in a hard enough way to not even enable them
> for "-extra-plugins I'd consider triggering bug reports in Ubuntu if
> somebody really used them.

Yes, that could make sense. If no one asks for them, I prefer to keep them
disabled in order to reduce attack surface, because strongSwan is really a
beast. Even if there's the plugin architecture, plugins are default-enabled
and that means a lot of exposed stuff.

For stuff like algorithms, there's also the security tradeoffs of default
config. I don't want to endorse stuff that's too young for my test (like the
various PQ bits) or too exotic.
> 
> I'm fine either way - all I request is that we look and discuss about
> things every half year or so.
> So far we benefitted both each time we did, so it isn't wasted time.

Indeed :)
> 
> > d83c243b (duplicheck disable): one good reason for the NACK just above:
> > it's
> > not enabled in Debian, you just enabled it in 1aa409a5 :)
> 
> That is a bummer, at the time I saw it the first time I thought it was
> enabled in Debian and since then carried on.
> /me feels ashamed and obviously drops that part :-)

No problem :)
> 
> > 766d4f0d (ha disable): not really sure; it's way easier to have a custom
> > kernel than a custom strongSwan
> 
> Ok, so you had real cases where you or a Debian user used that?
> Very interesting POV, I think neither is easier than the other but I
> see your point.

I didn't, but I know for sure that building and using a custom kernel is way
easier than using a custom strongSwan package.
> 
> > 85150f06 (kernel-libipsec enable): for reference, this is #739641 and I'm
> > still not sure I like it. I might pick it but end up disabling it before
> > release
> 
> It is disabled by default as Simon already outlined for the reason to
> be an opt-in.

Yes, by “disabling” I meant more disabling at build time.
> 
> > a587dc11 (TNC move): I might pick this one because TNC is pretty
> > standalone
> > per-se and it might make sense to split it, but really that's a lot of new
> > binary packages.
> 
> I understand the new-queue fight, but it really came handy for users
> who wanted it standalone.

Do you have pointers on people asking for it?
> 
> > 7629c11a7 (test-vectors move): NACK, what's the justification? Also it'll
> > need
> > some breaks/replaces
> > f9e7f9007 (CCN move): NACK, what's the justification? Also, the
> > break/replace
> > have the ubuntu version in them, which is wrong for Debian.
> 
> Sorry for not converting the breaks/replaces for those two to Debian
> versions properly.
> @Simon - thanks for helping with arguments! (he is one of the
> known-to-me Ubuntu Strongswan users).
> But in this case it really is CCM.
> 
> Every time I come back to this I hate that I only inherited this delta
> - too often I don't know and sometimes even fail to find the reasons
> for them.
> In this case due to your Nack I once again spent some more time to
> search for its history.
> I didn't find any and honestly I don't care anymore - let me drop
> these two changes on my end (with Ubuntu breaks/replaces :-) ).

Ok :)
> 
> > 13300d3bf (libstrongswan.install reorder): ACK, I could pick it if there
> > was
> > an isolated changelog entry with it
> 
> I'll on rebase let this be a change+CL entry.
> I'll also reorder the likely to pick changes at the top so you have
> not CL conflicts.

Thanks, that'd be really helpful.
> 
> > 4fe0861e2 (kernel-netlink conffiles): NACK, these files are installed only
> > on
> > Linux and thus the handling is done in debian/rules
> 
> Ok, it seems the original author didn't think about non-linux in this case.
> I'll convert it to a d/rules change inside the DEB_HOST_ARCH_OS check

I'm confusing. I don't think you need to do anything here, we already handle
it properly as far as I can tell.
> 
> > 76535cba5 (libfast disable): Should be ok, if I have an isolated changelog
> > entry for this
> 
> I'll reorder and add CL for this as well

Good.
> 
> > 8dbf648b7 (libcharon-standard-plugin): I can understand the rationale
> > (plugins
> > for common password-based mobile VPN setup), but I don't really like it. I
> > don't really like adding a new binary package, and the name is definitely
> > not
> > good. Also, as far as I understand it, the plugins are useful when you're
> > actually configuring a client/roadwarrior to imitate a mobile client with
> > its
> > limitations. I don't think it's a good thing to do, I'd prefer simplifying
> > the
> > secure uses cases, like pubkeys-based ones.
> 
> <insert the argument by Simon Deziel's former reply here/>

See my answer above about good practice. Recent iOS can use pubkeys/certs
based setups, not sure about Android (but they can use the strongSwan
application), so there's really no reason to not use them.
> 
> > 9b5769771 (changelog update): NACK for obvious reason, I'd need isolated
> > changes to the changelog (although obviously it's not simple to cherry-
> > pick
> > them either, I think I prefer it that way).
> 
> On the rebase I will revise the changes into the format that you need.

Thanks.
> 
> > Regards, and again thanks for the work you do.
> 
> I have to thank you way more - for your review, your discussions and
> your patience with me trying to sync&sort-out this rather old delta.
> Every cycle we do on this helps me tremendously, and the 30% we take
> into Debian each time helps both of us.

You're really welcom.
> 
> I'll come back with the revised version later today with all we
> discussed here so far.

Ok

Regards,
-- 
Yves-Alexis
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.alioth.debian.org/pipermail/pkg-swan-devel/attachments/20171205/ee12f361/attachment.sig>