[OE-core] [PATCH] [RFC] image.bbclass & image.py: Add check_image_file_ownership()
Haris Okanovic
haris.okanovic at ni.com
Fri Jul 7 21:42:05 UTC 2017
> Why not just check the permissions on disk, from a rootfs postprocess
> command? That wouldn’t be specific to tar, but would be just as
> effective, since those run under pseudo.
I disagree that it "would be just as effective", since this approach
relies on pseudo, tar, and OE's scripting (all of which are big,
complicated, codebases) working correctly to create a fake sysroot
representative of the booted system. OE sysroots are kinda leaky in my
experience. I want to insulate this test from such bugs. Moreover, the
current approach also implicitly verify the fake sysroot! If it were to
fail in a way that assigns bad id or name attributes to files, say ones
leaked from the host system, this test would catch it as presently written.
-- Haris
On 07/07/2017 03:28 PM, Christopher Larson wrote:
> Why not just check the permissions on disk, from a rootfs postprocess
> command? That wouldn’t be specific to tar, but would be just as
> effective, since those run under pseudo.
>
> On Fri, Jul 7, 2017 at 12:36 PM, Haris Okanovic <haris.okanovic at ni.com
> <mailto:haris.okanovic at ni.com>> wrote:
>
> An IMAGE_POSTPROCESS_COMMAND verifying image tarballs have symbolic and
> numeric ownership attributes which match the enclosed shadow database:
> * uid and uname of each file is present in /etc/passwd
> * gid and gname of each file is present in /etc/group
> * ids and names are consistent between tar metadata and shadow
> database
>
> In other words, this test ensure there aren't any ownership surprises
> when a filesystem created from tarball is booted. It's particularly
> useful in cases where artifacts built outside of OE are included in
> images -- E.g. packages from an external feed.
>
> Testing: Verified core-image-base still builds
>
> Patch:
> https://github.com/harisokanovic/openembedded-core/commits/dev/hokanovi/image-ownership-sanity-check
> <https://github.com/harisokanovic/openembedded-core/commits/dev/hokanovi/image-ownership-sanity-check>
>
> Signed-off-by: Haris Okanovic <haris.okanovic at ni.com
> <mailto:haris.okanovic at ni.com>>
> ---
> meta/classes/image.bbclass | 25 +++++++++-
> meta/lib/oe/image.py | 114
> +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 138 insertions(+), 1 deletion(-)
> create mode 100644 meta/lib/oe/image.py
>
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 6e30b96745..7b11291a82 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -192,7 +192,30 @@ python () {
> IMAGE_CLASSES += "image_types"
> inherit ${IMAGE_CLASSES}
>
> -IMAGE_POSTPROCESS_COMMAND ?= ""
> +python check_image_file_ownership () {
> + import sys, os, oe.image
> +
> + imgTypes = d.getVar("IMAGE_FSTYPES", True).split()
> + tarImgTypes = [ x for x in imgTypes if "tar" in x ]
> +
> + # bail if there isn't a tar image in this build
> + # TODO: Maybe add other image types
> + if len(tarImgTypes) == 0:
> + bb.debug(1, "Skipping check, no 'tar' image specified")
> + return
> +
> + imgDeployDir = d.getVar("IMGDEPLOYDIR", True)
> + imgName = d.getVar("IMAGE_NAME", True)
> + imgNameSuffix = d.getVar("IMAGE_NAME_SUFFIX", True)
> +
> + for tarFileExt in tarImgTypes:
> + archiveFilename = (imgName + imgNameSuffix + "." + tarFileExt)
> + archiveFilePath = os.path.join(imgDeployDir, archiveFilename)
> +
> + oe.image.check_file_ownership_tar(d, archiveFilePath)
> +}
> +
> +IMAGE_POSTPROCESS_COMMAND ?= " check_image_file_ownership "
>
> # some default locales
> IMAGE_LINGUAS ?= "de-de fr-fr en-gb"
> diff --git a/meta/lib/oe/image.py b/meta/lib/oe/image.py
> new file mode 100644
> index 0000000000..f1ba6f2613
> --- /dev/null
> +++ b/meta/lib/oe/image.py
> @@ -0,0 +1,114 @@
> +# Helper function for image building
> +
> +def check_file_ownership_tar(d, archiveFilePath):
> + """
> + Verifies file ownership meta-data in image tarball matches users
> + and groups in shadow database (/etc/passwd and /etc/group files).
> + """
> + import sys, os, tarfile
> + bb.debug(1, "Running check_file_ownership() on image
> '{0!s}'".format(archiveFilePath))
> +
> + try:
> + archiveName = os.path.basename(archiveFilePath)
> + if not archiveName:
> + archiveName = archiveFilePath
> +
> + # maps
> + unameMap = {} # username -> passwd file entry array
> + gnameMap = {} # groupname -> group file entry array
> + uidMap = {} # uid -> passwd file entry array
> + gidMap = {} # gid -> group file entry array
> + fileMap = {} # filepath -> TarInfo object
> +
> + def read_shadow_file(fdName, fd, colCount, mapsList):
> + """ Reads a colon (:) delimited shadow database file
> into mapsList """
> + for mapObj,_ in mapsList:
> + if len(mapObj) != 0:
> + raise Exception("Already read '{0!s}'
> file.".format(fdName))
> +
> + for line in fd:
> + line = line.decode("utf-8").strip()
> + words = line.split(":")
> +
> + if len(words) != colCount:
> + raise Exception("Malformed '{0!s}' file.
> Expected {1!s} cols.".format(fdName, colCount))
> +
> + for mapObj,mapKeyCol in mapsList:
> + mapKey = words[mapKeyCol]
> + if len(mapKey) < 1:
> + raise Exception("Map key in '{0!s}' must be
> at least one char long.".format(fdName))
> +
> + if mapKey in mapObj:
> + raise Exception("Malformed '{0!s}' file.
> Did not expect to find '{0!s}' in map.".format(fdName, mapKey))
> +
> + mapObj[mapKey] = words
> +
> + for mapObj,_ in mapsList:
> + if len(mapObj) == 0:
> + raise Exception("'{0!s}' is empty.".format(fdName))
> +
> + bb.debug(1, "Read the archive and populate maps")
> + with tarfile.open(name=archiveFilePath, mode='r:*') as tar:
> + for info in tar:
> + if info.name <http://info.name> in fileMap:
> + raise Exception("Duplicate entry for '{0!s}'
> found.".format(info.name <http://info.name>))
> +
> + fileMap[info.name <http://info.name>] = info
> +
> + # populate shadow db maps
> + if info.name <http://info.name> == './etc/passwd':
> read_shadow_file(fdName=info.name <http://info.name>,
> fd=tar.extractfile(info), colCount=7, mapsList=[(unameMap, 0),
> (uidMap, 2), ])
> + elif info.name <http://info.name> ==
> './etc/group': read_shadow_file(fdName=info.name
> <http://info.name>, fd=tar.extractfile(info), colCount=4,
> mapsList=[(gnameMap, 0), (gidMap, 2), ])
> +
> + bb.debug(1, "Check for no shadow db")
> + shadowItemCount = 0
> + for mapObj in [unameMap, gnameMap, uidMap, gidMap]:
> + shadowItemCount = shadowItemCount + len(mapObj)
> + if shadowItemCount == 0:
> + bb.warn(1, "check_file_ownership(): Skip; no shadow
> database in image '{0!s}'".format(archiveName))
> + return
> +
> + bb.debug(1, "Map sanity check")
> + for mapObj in [unameMap, gnameMap, uidMap, gidMap, fileMap]:
> + if len(mapObj) < 1:
> + raise Exception("Uh oh. Empty map found.")
> +
> + def badFileError(errorMessage, info, shadowEntry=None):
> + linkname = ""
> + if info.linkname:
> + linkname = " -> {0!s}".format(info.linkname)
> +
> + bb.error("BAD FILE '{0!s}': {1!s}".format(info.name
> <http://info.name>, errorMessage))
> + bb.error("## {info.uid!s}:{info.gid!s}
> ({info.uname!s}:{info.gname!s}) 0{info.mode:o} {info.name
> <http://info.name>!s}{linkname!s}".format(info=info, linkname=linkname))
> + if shadowEntry:
> + bb.error("## {0!s}".format(shadowEntry))
> +
> + bb.debug(1, "Check for bad files")
> + for filepath,info in fileMap.items():
> + if not info.uname in unameMap:
> + badFileError("uname not in unameMap", info)
> + continue
> +
> + if not info.gname in gnameMap:
> + badFileError("gname not in gnameMap", info)
> + continue
> +
> + if not str(info.uid) in uidMap:
> + badFileError("Uid '{0!s}' not
> found".format(info.uid), info)
> +
> + if not str(info.gid) in gidMap:
> + badFileError("Gid '{0!s}' not
> found".format(info.gid), info)
> +
> + unameEntry = unameMap[info.uname]
> + gnameEntry = gnameMap[info.gname]
> +
> + if str(info.uid) != unameEntry[2]:
> + badFileError("uid mismatch, expecting '{0!s}' for
> uname '{1!s}'".format(unameEntry[2], info.uname), info, unameEntry)
> +
> + if str(info.gid) != gnameEntry[2]:
> + badFileError("gid mismatch, expecting '{0!s}' for
> gname '{1!s}'".format(gnameEntry[2], info.gname), info, gnameEntry)
> +
> + bb.debug(1, "Successfully verified image
> '{0!s}'".format(archiveName))
> +
> + # Error on any exceptions
> + except Exception as e:
> + bb.error("check_file_ownership() exception: {0!s}".format(e))
> --
> 2.13.2
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core at lists.openembedded.org
> <mailto:Openembedded-core at lists.openembedded.org>
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
> <http://lists.openembedded.org/mailman/listinfo/openembedded-core>
>
>
>
>
> --
> Christopher Larson
> kergoth at gmail dot com
> Founder - BitBake, OpenEmbedded, OpenZaurus
> Senior Software Engineer, Mentor Graphics
More information about the Openembedded-core
mailing list