Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,18 @@ func createContainer(container, image, release, authFile string, showCommandToEn
}
}

isImageCompatible, warningMessage, err := podman.DoesImageFulfillRequirements(imageFull)
if err != nil {
return fmt.Errorf("%w", err)
}

if !isImageCompatible {
fmt.Fprintf(os.Stderr, "%s\n", warningMessage)
if !rootFlags.assumeYes && !askForConfirmation("One or more of the image's requirements are not met. Continue anyway? [y/N]:") {
return nil
}
}

var toolbxDelayEntryPointEnv []string

if toolbxDelayEntryPoint, ok := os.LookupEnv("TOOLBX_DELAY_ENTRY_POINT"); ok {
Expand Down
5 changes: 4 additions & 1 deletion src/cmd/rmi.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,12 @@ func rmi(cmd *cobra.Command, args []string) error {
}

for _, image := range args {
if _, err := podman.IsToolboxImage(image); err != nil {
if isToolboxImage, err := podman.IsToolboxImage(image); err != nil {
fmt.Fprintf(os.Stderr, "Error: %s\n", err)
continue
} else if !isToolboxImage {
fmt.Fprintf(os.Stderr, "Error: %s is not a Toolbx image\n", image)
continue
}

if err := podman.RemoveImage(image, rmiFlags.forceDelete); err != nil {
Expand Down
18 changes: 18 additions & 0 deletions src/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ func runCommand(container string,
}
}

checkImageCompatibility := true

logrus.Debugf("Checking if container %s exists", container)

if _, err := podman.ContainerExists(container); err != nil {
Expand Down Expand Up @@ -225,6 +227,10 @@ func runCommand(container string,
if err := createContainer(container, image, release, "", false); err != nil {
return err
}

// set to false -> check was already made when creating container during toolbx enter
checkImageCompatibility = false

} else if containersCount == 1 && defaultContainer {
fmt.Fprintf(os.Stderr, "Error: container %s not found\n", container)

Expand All @@ -249,6 +255,18 @@ func runCommand(container string,
return fmt.Errorf("failed to inspect container %s", container)
}

isImageCompatible, warningMessage, err := podman.DoesImageFulfillRequirements(containerObj.Image())
if err != nil {
return fmt.Errorf("%w", err)
}

if !isImageCompatible && checkImageCompatibility {
fmt.Fprintf(os.Stderr, "%s\n", warningMessage)
if !rootFlags.assumeYes && !askForConfirmation("One or more of the image's requirements are not met. Continue anyway? [y/N]:") {
return nil
}
}

entryPoint := containerObj.EntryPoint()
entryPointPID := containerObj.EntryPointPID()
logrus.Debugf("Entry point of container %s is %s (PID=%d)", container, entryPoint, entryPointPID)
Expand Down
107 changes: 105 additions & 2 deletions src/pkg/podman/podman.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"fmt"
"io"
"strconv"
"strings"
"time"

"github.com/HarryMichal/go-version"
Expand Down Expand Up @@ -352,17 +353,84 @@ func IsToolboxImage(image string) (bool, error) {
}

if info["Labels"] == nil {
return false, fmt.Errorf("%s is not a Toolbx image", image)
return false, nil
}

labels := info["Labels"].(map[string]interface{})
if labels["com.github.containers.toolbox"] != "true" && labels["com.github.debarshiray.toolbox"] != "true" {
return false, fmt.Errorf("%s is not a Toolbx image", image)
return false, nil
}

return true, nil
}

func IsLDPRELOADEnvSet(image string) (bool, error) {
info, err := InspectImage(image)
if err != nil {
return false, fmt.Errorf("failed to inspect image %s: %s", image, err)
}

if info["Config"] == nil {
return false, nil
}

config := info["Config"].(map[string]interface{})
if config["Env"] == nil {
return false, nil
}

env := config["Env"]
switch envVars := env.(type) {
case []interface{}:
for _, envVar := range envVars {
if envVarStr, ok := envVar.(string); ok {
envVarStrTrimmed := strings.TrimSpace(envVarStr)
if strings.HasPrefix(envVarStrTrimmed, "LD_PRELOAD=") {
return true, nil
}
}
}
case []string:
for _, envVar := range envVars {
envVarTrimmed := strings.TrimSpace(envVar)
if strings.HasPrefix(envVarTrimmed, "LD_PRELOAD=") {
return true, nil
}
}
default:
return false, fmt.Errorf("unexpected type '%T' of environment variables in image %s", env, image)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decoding the JSON in this way can be fragile if the structure of the JSON changes, and over time Podman has sometimes done that. :(

See the comments in src/pkg/podman/container.go, and the UnmarshalJSON() method of the Image object.

Fortunately, Go can decode JSON on a best effort basis, which can cover-up many of the superficial changes and needs a lot less code. Here's an introductory article about how it works. Sometimes, Go can't automatically handle the changes and we need to handle it ourselves, like in the above comments.

To take advantage of this, and to avoid invoking podman inspect repeatedly, we can decode the JSON into an Image object that implements the json.Unmarshaler interface. It's implemented by the containerInspect, containerPS and Image objects above. The interface will limit all manual interventions during decoding in one place.

Unfortunately, the JSON used by Podman to represent images differs between podman images and podman inspect --type image. The Image object only handles the former, but not the latter.

We have the same problem for containers. That's why we have a Container interface to hide the details, and it's implemented by the private containerInspect and containerPS objects that actually decode the JSON.

So, our first step would be to have a commit that introduces an Image interface, and renames the existing Image object to make it private and makes it implement the interface. I don't know what would be the best name for the object, because imageImages looks awkward, but it's also a private type so it won't be seen in too many places.

Then, we have to add a second implementation of the Image interface that decodes the JSON from podman inspect --type image.

Here are some commits that did the same thing for containers that you can use for help:

  • commit e611969726ed4260
    cmd, pkg/podman: Turn Container into an interface
  • commit ec7eb59bb058c12d
    cmd/run, pkg/podman: Make podman.InspectContainer() return a Container

Then, the IsToolboxImage() function can turn into the IsToolbx() method of the Image interface:

  • commit d363eb26d10cd93c
    cmd, pkg/podman: Turn IsToolboxContainer() into Container.IsToolbx()


return false, nil
}

func HasImageEntrypoint(image string) (bool, error) {
info, err := InspectImage(image)
if err != nil {
return false, fmt.Errorf("failed to inspect image %s: %s", image, err)
}

if info["Config"] == nil {
return false, nil
}

config := info["Config"].(map[string]interface{})
if config["Entrypoint"] == nil {
return false, nil
}

entrypoint := config["Entrypoint"]

switch ep := entrypoint.(type) {
case []interface{}:
return len(ep) > 0, nil
case []string:
return len(ep) > 0, nil
default:
return false, fmt.Errorf("unexpected type '%T' of entrypoint of image %s", entrypoint, image)
}
}

func Logs(container string, since time.Time, stderr io.Writer) error {
ctx := context.Background()
err := LogsContext(ctx, container, false, since, stderr)
Expand Down Expand Up @@ -506,3 +574,38 @@ func SystemMigrate(ociRuntimeRequired string) error {

return nil
}

func DoesImageFulfillRequirements(image string) (bool, string, error) {
var warnings []string

isToolboxImage, err := IsToolboxImage(image)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are invoking podman inspect --type image several times while doing the checks. First, to check the labels; second, to check LD_PRELOAD; and third, to check the EntryPoint. Generally, each invocation of podman(1) is sufficiently expensive, because it does a lot of heavy initialization when it starts. So, it's good to avoid them, if possible. This is especially true for Toolbx commands like enter and run that are used very often, and a delay of one extra second can start to annoy the user over time.

For example, here are some past commits where we optimized enter and run by reducing the number of podman(1) invocations:

The create command is not that frequently used and already does enough disk and network I/O that people expect it to be slow, but we also shouldn't make it slower than it needs to be. :)

We can reduce the number of podman inspect invocations if we parse the JSON into an Image object and then keep using the object repeatedly.

if err != nil {
return false, "", fmt.Errorf("failed to verify image compatibility: %w", err)
}
if !isToolboxImage {
warnings = append(warnings, fmt.Sprintf("Warning: Image '%s' does not contain either of the labels 'com.github.containers.toolbox=true' and 'com.github.debarshiray.toolbox=true'", image))
}

isLDPRELOADEnvSet, err := IsLDPRELOADEnvSet(image)
if err != nil {
return false, "", fmt.Errorf("failed to validate LD_PRELOAD variable settings: %w", err)
}
if isLDPRELOADEnvSet {
warnings = append(warnings, fmt.Sprintf("Warning: Image '%s' has environment variable LD_PRELOAD set, which may cause container vulnerability (Container Escape)", image))
}

hasEntrypoint, err := HasImageEntrypoint(image)
if err != nil {
return false, "", fmt.Errorf("failed to check image entrypoint: %w", err)
}
if hasEntrypoint {
warnings = append(warnings, fmt.Sprintf("Warning: Image '%s' has an entrypoint defined", image))
}

if len(warnings) > 0 {
warningMessage := strings.Join(warnings, "\n")
return false, warningMessage, nil
}

return true, "", nil
}
Loading