From owner-p4-projects@FreeBSD.ORG Wed May 4 05:42:16 2005 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 0F86D16A4D0; Wed, 4 May 2005 05:42:16 +0000 (GMT) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id AD18116A4CE for ; Wed, 4 May 2005 05:42:15 +0000 (GMT) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7BF8543D1D for ; Wed, 4 May 2005 05:42:15 +0000 (GMT) (envelope-from csjp@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.1/8.13.1) with ESMTP id j445fUde018034 for ; Wed, 4 May 2005 05:41:30 GMT (envelope-from csjp@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.1/8.13.1/Submit) id j445fUJf018031 for perforce@freebsd.org; Wed, 4 May 2005 05:41:30 GMT (envelope-from csjp@freebsd.org) Date: Wed, 4 May 2005 05:41:30 GMT Message-Id: <200505040541.j445fUJf018031@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to csjp@freebsd.org using -f From: "Christian S.J. Peron" To: Perforce Change Reviews Subject: PERFORCE change 76475 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 May 2005 05:42:17 -0000 http://perforce.freebsd.org/chv.cgi?CH=76475 Change 76475 by csjp@csjp_xor on 2005/05/04 05:41:26 -define constant for the dependency namespace -s/error/error != 0 for logic clarity -add various debug printfs to assist with debugging -move dependency check. This accomplishes two things, first there is no point in wasting the overhead associated with processing dependencies if the initial binary will not pass integrity tests, secondly the return value of the dependency test can clobber the error associated with retrieving checksums. -update userspace bits to use the constant instead of the string Affected files ... .. //depot/projects/trustedbsd/mac/sys/security/mac_chkexec/mac_chkexec.c#4 edit .. //depot/projects/trustedbsd/mac/sys/security/mac_chkexec/mac_chkexec.h#2 edit .. //depot/projects/trustedbsd/mac/usr.sbin/getfhash/getfhash.c#2 edit Differences ... ==== //depot/projects/trustedbsd/mac/sys/security/mac_chkexec/mac_chkexec.c#4 (text+ko) ==== @@ -509,15 +509,15 @@ size_t alen; ASSERT_VOP_LOCKED(vp, "no vlock held"); - error = VOP_GETEXTATTR(vp, MAC_CHKEXEC_ATTRN, "chkexec_depend", + error = VOP_GETEXTATTR(vp, MAC_CHKEXEC_ATTRN, MAC_CHKEXEC_DEP, NULL, &alen, NOCRED, curthread); - if (error && error == ENOATTR) + if (error != 0 && error == ENOATTR) return (0); else if (error) return (error); depends = malloc(alen + 1, M_CHKEXEC, M_WAITOK | M_ZERO); error = vn_extattr_get(vp, IO_NODELOCKED, MAC_CHKEXEC_ATTRN, - "chkexec_depend", &alen, depends, curthread); + MAC_CHKEXEC_DEP, &alen, depends, curthread); for (npaths = 0, ap = paths; (*ap = strsep(&depends, ":")) != NULL; npaths++) if (**ap != '\0') @@ -564,8 +564,12 @@ if ((mp->mnt_flag & MNT_RDONLY) == 1 && !mac_chkexec_enforce) return (0); /* We are only interested in the execution of regular files */ - if (vp->v_type != VREG) + if (vp->v_type != VREG) { +#ifdef DEBUG + printf("mac_chkexec_check: File is not VREG, skipping\n"); +#endif return (0); + } /* * Retrieve the algorithm specified in the sysctl OID. By default * we leave this as SHA1. If the algorithm is invalid, deny access @@ -584,10 +588,7 @@ * Thus we will deny access to this file, erroring on the side of security. */ error = mac_chkexec_get_vcsum(vp, &vcsum); - if (error && error != ENOATTR) - return (error); - error = mac_chkexec_check_depends(vp, cred); - if (error && mac_chkexec_enforce) + if (error != 0 && error != ENOATTR) return (error); /* * If no checksum is present in the file, and we are ignoring @@ -638,6 +639,14 @@ error = mac_chkexec_set_vcsum(vp, &vcsum); return (error); } + /* If the binary itself checks out, then we should check for any + * dependencies it may have. + */ + if (match) { + error = mac_chkexec_check_depends(vp, cred); + if (error && mac_chkexec_enforce) + return (error); + } #ifdef DEBUG if (!match) printf("mac_chkexec: checksum mismatch, denying\n"); ==== //depot/projects/trustedbsd/mac/sys/security/mac_chkexec/mac_chkexec.h#2 (text+ko) ==== @@ -30,6 +30,7 @@ #define MAC_VCSUM_SHA1 0x00000002 #define MAC_CHKEXEC_ATTRN EXTATTR_NAMESPACE_SYSTEM #define MAC_CHKEXEC "chkexec" +#define MAC_CHKEXEC_DEP "chkexec_depend" #define MAXCSUMSIZE 32 #ifdef _KERNEL ==== //depot/projects/trustedbsd/mac/usr.sbin/getfhash/getfhash.c#2 (text+ko) ==== @@ -58,7 +58,7 @@ ssize_t nbytes; nbytes = extattr_get_file(pathname, MAC_CHKEXEC_ATTRN, - "chkexec_depend", NULL, 0); + MAC_CHKEXEC_DEP, NULL, 0); if (nbytes < 0 && errno == ENOATTR) return; else if (nbytes < 0) { @@ -71,7 +71,7 @@ return; } error = extattr_get_file(pathname, MAC_CHKEXEC_ATTRN, - "chkexec_depend", dependlist, nbytes); + MAC_CHKEXEC_DEP, dependlist, nbytes); dependlist[nbytes] = '\0'; for (ndeps = 0, av = depends; (*av = strsep(&dependlist, ":")) != NULL; ndeps++) @@ -95,7 +95,7 @@ if (rflag) { error = extattr_delete_file(pathname, MAC_CHKEXEC_ATTRN, - "chkexec_depend"); + MAC_CHKEXEC_DEP); if (error < 0) warn("extattr_delete_file failed"); } @@ -105,7 +105,7 @@ return; slen = strlen(mflag); error = extattr_set_file(pathname, MAC_CHKEXEC_ATTRN, - "chkexec_depend", mflag, slen); + MAC_CHKEXEC_DEP, mflag, slen); if (error < 0) warn("extattr_set_file failed"); }