Re: Cert validation in pkg_add

[Greg Troxel <gdt%lexort.com@localhost>]

Taylor R Campbell <riastradh%NetBSD.org@localhost> writes:

(I'm responding to the design discussion not the patch.)

> One can argue that (a) packages should be signed at the origin rather
> than in the transport, (b) you should be using pkgin rather than
> pkg_add, (c) you can just make your own packages and install them via
> file:// PKG_PATHs mounted with psshfs, (d) etc.
>
> But there's a lot of man pages and docs and copypasta out there to
> just use https://cdn.NetBSD.org/... or similar and I expect most users
> would be astonished (if not disgusted) that this doesn't do
> validation, not even when they have trust anchor certs installed.
> Certainly it came as a surprise to me when I discovered this today.
>
> And while the CDN isn't the TNF origin, transport authentication goes
> a long way toward plugging the gaping security hole here, with much
> less effort because almost all the infrastructure is already there.
>
> Plus even if you authenticate the origin with package signatures,
> there's a huge attack surface of complicated data transports and
> formats to get at the package signatures in the first place, which
> transport authentication helps to avoid.

Agreed that "packages should be signed" is a separate issue, and not
relevant for the https validation.
>
> So I propose the attached patch to implement the following:
>
> 1. New libfetch flag `V' to enable validation.
>
>    This is an opt-in so that existing clients of the _library_ don't
>    break -- no change to library semantics.

opt-in seems fine.  But it looks like the patch changes the signature of
an existing public function, so I'd expect this requires a shlib major
bump of libfetch, and accomodation in all users, and I don't understand
either how that's being done or why it isn't necessary.

> 2. pkg_add does cert validation by default, by setting `V' flag in
>    libfetch.  New option INSECURE_TRANSPORT=1 in pkg_install.conf to
>    allow insecure transports.
>
>    This is an opt-out because the reasonable modern expectation for
>    applications is to do cert validation out of the box -- and while
>    pkg_add has a bit of a dual purpose as a library subroutine and a
>    user interface, its nature as a user interface is baked into a lot
>    of documentation and fingers.
>
>    The option name is intentionally scary-sounding so you're tempted
>    to fix whatever symptom leads you to try it first before you are
>    tempted to enable the insecure option.

So pkg_add without the new option will fail for http and ftp?  Or do you
mean "TLS will require validation"?

Saying there is an expectation that pkix rules are followed when one
configures https: is one thing.  Saying that when someone asks to use
ftp: or http: it will fail is quite another.  That is sort of like
patching firefox to refuse to load pages from facebook because after all
nobody should be doing that.

I don't like INSECURE_TRANSPORT because transport isn't really what's
going on, bu more because I don't like picking names to give people an
impression to cause them to change their behavior.  I think we need
something like "DISABLE_TLS_CERT_VALIDATION", and ideally there's be a
consistent name across the various tools.

> I realize it's late in the branch cycle, but it would be embarrassing
> for this not to be fixed in NetBSD 10 (whose release is imminent) now
> that we have certs set up by default in base.

True, but it would also be embarassing if it broke other things.

> We could, of course, apply the changes to NetBSD first, and then
> pkgsrc, rather than the other way around, if pkgsrc-releng considers
> this change too risky for pkgsrc so close to branch-time.

How are you proposing to deal with this for systems that do and don't
have trust anchors configured?  I get it that on NetBSD 10, trust
anchors are configured in base.  But on older NetBSD -- and who knows
about the other ~20 platforms, it's hard to tell.

(A nit but stability heading to branch is pkgsrc-pmc.  Acceptability of
pullups is pkgsrc-releng, in theory with pmc oversight, but I have no
memory of *ever* needing to discuss a decision that was made my
pkgsrc-releng.)

I don't think it's wise to have different code in NetBSD base
pkg_install and in pkgsrc pkg_install, as that will cause behavior
changes when flipping between them, which could happen due to unrelated
fixes in pkg_install.

My quick reaction is that the default to validate TLS is easy to revert
if trouble.  When guarded by an ifdef on NetBSD 10, then it addresses
what I think you most care about, while not impacting the rest of the
systems where we don't have a good handle on the impacts.

We can certainly discuss rejecting TLS connections that don't validate
in general on all systems, but that's the kind of change to land in
pkgsrc-current just after a branch, not just before.

A change to fail ftp/http by default I see as more controversial, and
that's also a soon-after-branch thing, vs a just-before-the-bell thing.


Not quite on topic, but related to why this is perceived as urgent: I'm
not really clear on why we don't have  signed packages.  I see it as
"key management is hard" and we don't have a story for supercession and
revocation, distribution of trust anchors, etc., for the keys used or
signing.