tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: fixing libfetch as a first-class object



Greg Troxel <gdt%lexort.com@localhost> writes:

[I have been prioritizing dealing with the branch and am paging this
back in.]

> The fetch(3) man page reads like a general purpose library that users
> are welcome to use, so I'm going to accept that it's a separate library
> usable for other purposes.

On 22 December I wrote about the concept of fixing the bug in libfetch
where it does not verify TLS by default, and suggesting that just doing
that would be entirely sufficient for the goal of having pkg_add verify
TLS certs by default.

Since then, I have come to like this approach more, as it seems much
simpler, fixes all uses of libfetch, not just one, is achitecturally
clean, and can be explained extremely simply as it changes exactly one
thing.  We have, I believe, agreed as a group that TLS connections
should be subject to proper as-specified-by-pkix validation, as a
general practice, not just when there is a specific heightened concern.

I just looked at NetBSD's ftp(1), and it documents that it verifies SSL
certs unless the sslnoverify option is set to 1.  Setting that variable
is done in config file or via the FTPSSLNOVERIFY environment variable,
and the default value is the empty string.  This is a precedent that
supports fixing libfetch, which is basically doing the same thing as
ftp.

While it's true that libfetch is a library and ftp is a program,
libfetch already has lots of environment variables, 15 when counting too
quickly.  Some of these are about providing username/password, setting
proxy, etc., so "don't validate TLS certs" certainly fits.

I believe that pkgin downloading packages and pkg_add have identical
concerns, and that this will resolve the same issue for pkgin.

Thus, I still think my original proposal is sound.   Quoted here:

> So I think we should:
>
>   Add HTTPS_VALIDATE_CERTS=yes/no to the environment section, which is
>   aligned with the other variables already declared.
>
>   Default this to on.  (When pulling up to 2023Q4, limit to netbsd-10,
>   but turn it on generally for pkgsrc-current.)
>
>   Explain this in the man page, which is exactly that if this is
>   enabled, validation is done, and giving the default.
>
> and if people generally feel strongly about it (there is broad agreement
> that https should be validated by default, but i have not seen anybody
> else comment about any of the details), also
>
>   change libfetch to fail on a redirect from https to non-https
>   (always).  (Really, fail redirect from TLS-like protocols (transport
>   authentication/confidentiality with endpoint authentication) to
>   non-TLS-like protocols.)
>
>   Explain this in the man page.
>
>   If thought necessary, add HTTPS_ALLOW_HTTP_REDIRECT=yes/no, defaulting
>   to no.  (I am for now ok with skipping this step as I expect zero
>   actual people to have enough trouble with this to make it worthwhile
>   to add the complexity.)
>
>   Explain the new variable in the man page.

Also, perhaps add to pkg_add(8), because pkg_add effectively vendors
libfetch rather than depending on it, and thus does not cause the man
page to be installed:

  pkg_add uses libfetch, normally documented in fetch(3), to download
  packages.  Because of bootstrapping concerns, the code is directly
  included, and thus the man page may not be installed.  By default,
  https: fetches will validate certificates, and this can be disabled by
  setting HTTPS_VALIDATE_CERTS=no in the environment if necessary.

Are there any concurrences, or arguments that this doesn't fix the
tls-non-validation bug in a reasonable way?


Home | Main Index | Thread Index | Old Index