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.okano...@ni.com <mailto:haris.okano...@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.okano...@ni.com
    <mailto:haris.okano...@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@lists.openembedded.org
    <mailto:Openembedded-core@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
--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Reply via email to