--AhhlLboLdkugWU4S
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Due to the outcry with respect to the recent audit-packages change, I
would like to commit the following patch.  The changes are:

* Considerable cleanup and simplification of the make and shell code to
  perform the vulnerability checks.  It has all now been moved to the
  "check-vulnerable" target, which is also a prerequisite target for
  "do-fetch".  This causes the checks to be performed before fetching
  distfiles, which matches both the old and current behavior.

* Drop SKIP_AUDIT_PACKAGES, which is a "negative variable name", which
  we are actively trying to purge from pkgsrc.  A new yes/no variable
  "CHECK_VULNERABILITIES" has been added, which controls whether the
  vulnerability checks are performed or not.  Both SKIP_AUDIT_PACKAGES
  and ALLOW_VULNERABLE_PACKAGES may continue to be used, but are both
  marked for deprecation.  This should allow both old and current
  setups to behave as expected.

This last change is possibly controversial new behavior because it
matches neither the old nor the current behavior:

* "CHECK_VULNERABILITIES" defaults to "yes" only if the audit-packages
  script can be found; otherwise, it defaults to "no".  The purpose
  of this change is to not force audit-packages to be installed.

The old behavior was that vulnerability checks were performed regardless
of whether audit-packages was installed or not.  This was due to the
standalone implementation of the auditing code in the old
"check-vulnerable" target.  The current behavior is that audit-packages
must be installed for pkgsrc to work, which again forces the vulnerability
checks to be performed.  The proposed behavior is that we only perform
the checks if we can actually do so.  This can be considered a weakening
of the security of pkgsrc, so we may not want to do this.

One alternative to the last change is that CHECK_VULNERABILITIES should
default to "yes", but that specific packages should hardcode
"CHECK_VULNERABILITIES=no" in their package Makefiles.  This would
have the effect of making skipping the checks when building those
packages, but doesn't affect the vulnerability scan for those packages
when running audit-packages on the command line.  The candidate packages
are likely to be:

	pkgtools/digest
	pkgtools/pkg_install
	security/audit-packages

Please share your comments.

	Thanks,

	-- Johnny Lam <jlam@pkgsrc.org>

--AhhlLboLdkugWU4S
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="vuln.diff"

Index: bsd.pkg.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bsd.pkg.mk,v
retrieving revision 1.1767
diff -u -r1.1767 bsd.pkg.mk
--- bsd.pkg.mk	22 Nov 2005 03:41:20 -0000	1.1767
+++ bsd.pkg.mk	22 Nov 2005 07:33:19 -0000
@@ -1314,59 +1314,61 @@
 		${FALSE} ;;						\
 	esac
 
-# check for any vulnerabilities in the package
+# Check for any vulnerabilities in the package
 
-_AUDIT_PACKAGES_MIN_VERSION=1.40
-_AUDIT_PACKAGES_OK!=	${PKG_INFO} -qe 'audit-packages>=${_AUDIT_PACKAGES_MIN_VERSION}' ; echo $$?
+_AUDIT_PACKAGES_REQD=		1.40
 
-# Note: _any_ output from check-vulnerable is considered an error by do-fetch.
-.PHONY: check-vulnerable
-check-vulnerable:
-.if empty(_AUDIT_PACKAGES_OK:M0)
-	@${ECHO_MSG} "${_PKGSRC_IN}> *** The audit-packages package must be at least version ${_AUDIT_PACKAGES_MIN_VERSION}"
-	@${ECHO_MSG} "${_PKGSRC_IN}> *** Please install the security/audit-packages package and run";
-	@${ECHO_MSG} "${_PKGSRC_IN}> *** '${LOCALBASE}/sbin/download-vulnerability-list'.";
-	@false
-.else
-	@${AUDIT_PACKAGES} -i ""${ALLOW_VULNERABILITIES.${PKGBASE}:Q} -p ${PKGNAME:Q}
-.endif
-
-
-.if defined(ALLOW_VULNERABILITIES.${PKGBASE})
-_ALLOW_VULNERABILITIES=${ALLOW_VULNERABILITIES.${PKGBASE}}
+# We default to checking for package vulnerabilities only if the
+# audit-packages command exists.
+#
+.if exists(${AUDIT_PACKAGES_CMD})
+CHECK_VULNERABILITIES?=		yes
 .else
-_ALLOW_VULNERABILITIES=#none
+CHECK_VULNERABILITIES?=		no
 .endif
 
-.PHONY: do-fetch
-.if !target(do-fetch)
-do-fetch:
-.  if empty(SKIP_AUDIT_PACKAGES:M[Yy][Ee][Ss]) && empty(_ALLOW_VULNERABILITIES:M[Yy][Ee][Ss])
+.PHONY: check-vulnerable
+check-vulnerable:
+.if !empty(CHECK_VULNERABILITIES:M[yY][eE][sS])
 	${_PKG_SILENT}${_PKG_DEBUG}					\
-	if [ -f ${PKGVULNDIR}/pkg-vulnerabilities ]; then		\
-		${ECHO_MSG} "${_PKGSRC_IN}> Checking for vulnerabilities in ${PKGNAME}"; \
-		vul=`${MAKE} ${MAKEFLAGS} check-vulnerable || ${TRUE}`;		\
-		case "$$vul" in						\
-		"")	;;						\
-		*vulnid:*)	vulnids=`echo "$$vul" | ${GREP} vulnid: | ${SED} -e's/.*vulnid:\\([[:digit:]]*\\).*/\\1/'`; \
-			${ECHO} "$$vul";				\
-			${ECHO} "or if this package is absolutely essential, add this to mk.conf:"; \
-			for vulnid in $$vulnids ; do \
-				${ECHO} " ALLOW_VULNERABILITIES.${PKGBASE}+=$$vulnid"; \
-			done ; \
-			${FALSE} ;;					\
-		*) ${ECHO} "$$vul";				\
-			${FALSE} ;;                 \
-		esac;							\
+	vers=${_AUDIT_PACKAGES_REQD:Q};					\
+	if ${PKG_INFO} -qe "audit-packages>=$$vers"; then		\
+		: ;							\
 	else								\
-		${ECHO_MSG} "${_PKGSRC_IN}> *** No ${PKGVULNDIR}/pkg-vulnerabilities file found,"; \
-		${ECHO_MSG} "${_PKGSRC_IN}> *** skipping vulnerability checks. To fix, install"; \
-		${ECHO_MSG} "${_PKGSRC_IN}> *** the pkgsrc/security/audit-packages package and run"; \
+		${ECHO_MSG} "${_PKGSRC_IN}> *** The audit-packages package must be at least version $$vers."; \
+		${ECHO_MSG} "${_PKGSRC_IN}> *** Please install the security/audit-packages package and run"; \
 		${ECHO_MSG} "${_PKGSRC_IN}> *** '${LOCALBASE}/sbin/download-vulnerability-list'."; \
+		${FALSE};						\
 	fi
-.  else
-	@${ECHO_MSG} "${_PKGSRC_IN}> *** Skipping vulnerability checks for ${PKGNAME}"
-.  endif
+	${_PKG_SILENT}${_PKG_DEBUG}					\
+	vulndb=${PKGVULNDIR:Q}/pkg-vulnerabilities;			\
+	if [ ! -f "$$vulndb" ]; then					\
+		${ECHO_MSG} "${_PKGSRC_IN}> *** No $$vulndb file found;"; \
+		${ECHO_MSG} "${_PKGSRC_IN}> *** skipping vulnerability checks.  To fix, install"; \
+		${ECHO_MSG} "${_PKGSRC_IN}> *** the pkgsrc/security/audit-packages package and run"; \
+		${ECHO_MSG} "${_PKGSRC_IN}> *** '${LOCALBASE}/sbin/download-vulnerability-list'."; \
+		exit 0;							\
+	fi;								\
+	${ECHO_MSG} "${_PKGSRC_IN}> Checking for vulnerabilities in ${PKGNAME}"; \
+	${AUDIT_PACKAGES} -i ""${ALLOW_VULNERABILITIES.${PKGBASE}:Q}	\
+		-p ${PKGNAME:Q} |					\
+	${AWK} '{ print;						\
+		  if (match($$0, "vulnid:[0-9]*"))			\
+			a[n++] = substr($$0, RSTART, RLENGTH);		\
+		}							\
+		END {							\
+		  if (n == 0) exit;					\
+		  print "If this package is absolutely essential, "	\
+			"add the following to mk.conf:";		\
+		  for (i = 0; i < n; i++)				\
+			print "	ALLOW_VULNERABILITIES.${PKGBASE}+= " a[i]; \
+		  exit 1;						\
+		}'
+.endif	# CHECK_VULNERABILITIES
+
+.PHONY: do-fetch
+.if !target(do-fetch)
+do-fetch: check-vulnerable
 .  if !empty(_ALLFILES)
 	${_PKG_SILENT}${_PKG_DEBUG}					\
 	${TEST} -d ${_DISTDIR} || ${MKDIR} ${_DISTDIR}
Index: bulk/build
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bulk/build,v
retrieving revision 1.82
diff -u -r1.82 build
--- bulk/build	20 Nov 2005 11:18:45 -0000	1.82
+++ bulk/build	22 Nov 2005 07:33:19 -0000
@@ -216,7 +216,7 @@
 # Check that the package tools are up to date.
 #
 ( cd "${pkglint_dir}" \
-  && ${BMAKE} fetch SKIP_AUDIT_PACKAGES=yes >/dev/null 2>&1
+  && ${BMAKE} fetch >/dev/null 2>&1
 ) || {
 	echo "Updating pkgtools"
 	( cd "${pkgsrc_dir}/pkgtools/pkg_install" \
Index: defaults/mk.conf
===================================================================
RCS file: /cvsroot/pkgsrc/mk/defaults/mk.conf,v
retrieving revision 1.94
diff -u -r1.94 mk.conf
--- defaults/mk.conf	17 Nov 2005 00:28:48 -0000	1.94
+++ defaults/mk.conf	22 Nov 2005 07:33:19 -0000
@@ -22,12 +22,12 @@
 #           or the word "yes" to allow all. (not recommended)
 # Default: not defined
 
-SKIP_AUDIT_PACKAGES?=no
-# Completely skip running audit-packages to check for vulnerable packages.
-# Specifying individual vulnerabilities with
-# ALLOW_VULNERABILITIES.<pkgname>=<vulnid> is preferred to using this.
+#CHECK_VULNERABILITIES?= yes
+# Perform the checks to see if the package is ``vulnerable''.  Specifying
+# individual vulnerabilities with ALLOW_VULNERABILITIES.<pkgname> is
+# preferred to setting this to turning off this check.
 # Possible: yes, no
-# Default: no
+# Default: "yes" if security/audit-packages is installed; otherwise "no"
 
 MANINSTALL?= maninstall catinstall
 # Specify manpage installation types.
Index: defaults/obsolete.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/defaults/obsolete.mk,v
retrieving revision 1.16
diff -u -r1.16 obsolete.mk
--- defaults/obsolete.mk	3 Oct 2005 21:25:17 -0000	1.16
+++ defaults/obsolete.mk	22 Nov 2005 07:33:19 -0000
@@ -8,6 +8,20 @@
 PKG_SYSCONFDIR.priv?=	${PRIV_CONF_DIR}
 .endif
 
+# SKIP_AUDIT_PACKAGES is a "negative variable", which should no longer be
+# used in pkgsrc.  This should be removed after pkgsrc-2005Q4 has been
+# branched.
+#
+.if defined(SKIP_AUDIT_PACKAGES) && !empty(SKIP_AUDIT_PACKAGES:M[yY][eE][sS])
+CHECK_VULNERABILITIES=		no
+.endif
+
+# This should be removed after pkgsrc-2006Q1 has been branched.
+.if defined(ALLOW_VULNERABLE_PACKAGES)
+CHECK_VULNERABILITIES=		no
+.endif
+
+
 ###
 ### Set PKG_LEGACY_OPTIONS based on to-be-deprecated global variables.
 ###

--AhhlLboLdkugWU4S--