tech-pkg archive

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

Re: Improving security for pkgsrc

Pierre Pronchery <> writes:

> On 07/23/15 07:23, David Holland wrote:
>> On Wed, Jul 22, 2015 at 07:25:08PM -0700, Alistair Crooks wrote:
>>  > It might be better, if this is just for pkgsrc, to call the definition

I had the same reaction as agc (but without a specific name).

> I do not mind, but:
> - there are only 6 options in all of mk/defaults/mk.conf that are
>   prefixed this way;

That can be viewed as a bug; namespacing for config variables feels like
collection of historical artifacts..  I have found it confusing meaning,
had to actually go look things up) about which options are or base and
which are for pkgsrc.

Generally, we declare each variable to be user-settable in mk.conf or
pkg-settable in Makefile, and never both.  See the FETCH_USING

> - it can easily be applied to pkgsrc only, in mk.conf:
> .ifdef BSD_PKG_MK
> USE_SSP=yes
> .endif
> - unprivileged builds will use a different mk.conf anyway.

That's true, but a) a person with USE_SSP in base will now get SSP in
pgksrc, which may break a lot of things, and b) I think it's better in
general to have separate names.

On the patch, I agree with the concept, but have a few questions

  I wonder if PKGSRC_USE_SSP should provoke an error if used with a
  compiler that doesn't support it, rather than silently failing.  Or
  perhaps a warning; an error seems too much.  But overall I think I
  agree with how you handled it.

  The hunks in that add CFLAGS and remove LDFLAGS aren't
  immediately understandable, but you didn't include the commit message.
  If this is fixing for an issue that's separate from adding SSP
  (and it seems it must be), I (with pmc hat OFF) would tend to want to
  see a separate commit for that.    If it's just that nothing sets
  _GCC_LDFLAGS now, I don't see why the LDFLAGS+= line should be

Attachment: pgpD1mqP9Lahc.pgp
Description: PGP signature

Home | Main Index | Thread Index | Old Index