Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add binary shims #553

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
34 changes: 34 additions & 0 deletions data/libexec/host-runner
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/sh

#
# Copyright © 2019 – 2020 Red Hat Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

executable=$(basename $0)
priviledged=false

if [ $# -ge 1 ]; then
case "$1" in
--host-runner-sudo )
shift
priviledged=true
esac
fi

if $priviledged; then
executable="sudo $executable"
fi

exec flatpak-spawn --host --watch-bus --forward-fd=1 --forward-fd=2 --directory=$(pwd) --env=TERM=xterm-256color $executable "$@"
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to explicitly forward file descriptors 1 and 2? What about 0?

flatpak-spawn(1) has always forwarded file descriptors 0, 1 and 2. We don't do it explicitly elsewhere either.

Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't hard code TERM. Instead: --env=TERM=$TERM :)

Also, shouldn't we forward all the other environment variables that we usually do?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need the --directory=$(pwd) because flatpak-spawn(1) has been retaining the current working directory since before the --directory option became a thing.

Copy link
Member

Choose a reason for hiding this comment

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

I am curious about the use of --watch-bus. It's not wrong, but given that flatpak-spawn(1) always forwards signals, I am curious if you came across a scenario where the container-side shim would cleanly exit without signals leaving the host-side running. I am trying to understand if we should use --watch-bus more widely.

12 changes: 12 additions & 0 deletions data/libexec/meson.build
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
host_runner = files('host-runner')
sudo_wrapper = files('sudo-wrapper')

if shellcheck.found()
test('shellcheck', shellcheck, args: [host_runner, sudo_wrapper])
endif

install_data(
host_runner,
sudo_wrapper,
install_dir: get_option('libexecdir'),
)
47 changes: 47 additions & 0 deletions data/libexec/sudo-wrapper
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/bin/sh

#
# Copyright © 2019 – 2020 Red Hat Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

# sudo-wrapper is meant to be used before /bin/sudo to make it possible to run
# commands on the host from inside of a container with "sudo" prefixed. It
# checks first if the command, that is to be run with sudo, is to be run on the
# host by checking if it is a symlink to /usr/libexec/toolbox/host-runner. If it
# is not, /bin/bash is executed instead.

# Name of the executable that should be ran using sudo
executable=""

# This is a wrapper but the cli user experience should remain the same.
# TODO: Parse sudo's command line options
if [ $# -ge 1 ]; then

executable=$(basename $1)
# Move the executable's name out of the argument list
shift
fi

# Shims are located under /usr/libexec/toolbox; the list should not include
# "host-runner" and "sudo"
shims=$(ls /usr/libexec/toolbox | grep -v "host-runner" | grep -v "sudo")
Copy link
Member

Choose a reason for hiding this comment

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

How about using a separate sub-directory for the shims? It would remove the need for this filtering. Moreover, if we do want users to place their own shims, then we can't trust them not to overwrite things like host-runner and sudo.


if [ -n "$executable" ] && echo "$shims" | grep "^$executable$" >/dev/null; then
set -x
exec $executable --host-runner-sudo $@
fi

# The called command is not a shim-binary -> run 'sudo' as usually
exec /bin/sudo $executable $@
1 change: 1 addition & 0 deletions data/meson.build
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
subdir('config')
subdir('tmpfiles.d')
subdir('libexec')
4 changes: 4 additions & 0 deletions profile.d/toolbox.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# shellcheck shell=sh

# Toolbox provides shim binaries for executing commands on the host from inside
# of a toolbox container
export PATH="/usr/libexec/toolbox:$PATH"
Copy link
Member

Choose a reason for hiding this comment

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

Should we really do this unconditionally for both hosts and containers? I think this should ideally happen only for Toolbx containers, not even generic Podman containers. Or did I misunderstand something?


# shellcheck disable=SC2153
[ "${BASH_VERSION:-}" != "" ] || [ "${ZSH_VERSION:-}" != "" ] || return 0
[ "$PS1" != "" ] || return 0
Expand Down
137 changes: 137 additions & 0 deletions src/cmd/createShim.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
* Copyright © 2019 – 2020 Red Hat Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package cmd

/*
import (
"errors"
"fmt"
"os"
"strings"

"github.com/containers/toolbox/pkg/shell"
"github.com/containers/toolbox/pkg/utils"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)

var (
forbiddenBinaryShims = []string{
"toolbox",
}
)

var createShimCmd = &cobra.Command{
Use: "create-shim",
Short: "Create a binary shim for a command to be run on the host",
Hidden: true,
Example: "asdasdasd",
Args: cobra.ExactArgs(1),
RunE: createShim,
}

func init() {
// flags := initContainerCmd.Flags()

createShimCmd.SetHelpFunc(createShimHelp)
rootCmd.AddCommand(createShimCmd)
}

func createShim(cmd *cobra.Command, args []string) error {
if !utils.IsInsideContainer() {
var builder strings.Builder
fmt.Fprintf(&builder, "the 'create-shim' command can only be used inside containers\n")
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

shimBinary := fmt.Sprintf("/usr/libexec/toolbox/%s", args[0])

if !utils.PathExists(hostRunnerShim.containerPath) || !utils.PathExists(sudoShim.containerPath) {
var builder strings.Builder
fmt.Fprintf(&builder, "This toolbox container is not set up for creating binary shims\n")
fmt.Fprintf(&builder, "You're possibly trying to use a newer Toolbox in a container created with an older version\n")
fmt.Fprintf(&builder, "Try to update the Toolbox binary used by this container")

errMsg := builder.String()
return errors.New(errMsg)
}

if utils.PathExists(shimBinary) {
fmt.Printf("The requested shim binary already exists.\n")
return nil
}

err := redirectPath(shimBinary, hostRunnerShim.containerPath, false)
if err != nil {
return fmt.Errorf("Failed to create shim binary %s: %w", shimBinary, err)
}

return nil
}

func createShimHelp(cmd *cobra.Command, args []string) {
if utils.IsInsideContainer() {
if !utils.IsInsideToolboxContainer() {
fmt.Fprintf(os.Stderr, "Error: this is not a toolbox container\n")
return
}

if _, err := utils.ForwardToHost(); err != nil {
fmt.Fprintf(os.Stderr, "Error: %s\n", err)
return
}

return
}

if err := utils.ShowManual("toolbox-create-shim"); err != nil {
fmt.Fprintf(os.Stderr, "Error: %s\n", err)
return
}
}

func runOnHost(command string, commandLineArgs []string) (int, error) {
envOptions := utils.GetEnvOptionsForPreservedVariables()

var flatpakSpawnArgs []string

flatpakSpawnArgs = append(flatpakSpawnArgs, envOptions...)

flatpakSpawnArgs = append(flatpakSpawnArgs, []string{
"--host",
command,
}...)

flatpakSpawnArgs = append(flatpakSpawnArgs, commandLineArgs...)

logrus.Debug("Forwarding to host:")
logrus.Debugf("%s", command)
for _, arg := range commandLineArgs {
logrus.Debugf("%s", arg)
}

exitCode, err := shell.RunWithExitCode("flatpak-spawn", os.Stdin, os.Stdout, nil, flatpakSpawnArgs...)
if err != nil {
return exitCode, err
}

return exitCode, nil
}
*/
40 changes: 40 additions & 0 deletions src/cmd/initContainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ var (
user string
}

toolboxLibexecDirectory = "/usr/libexec/toolbox"

initContainerMounts = []struct {
containerPath string
source string
Expand All @@ -68,6 +70,19 @@ var (
{"/var/log/journal", "/run/host/var/log/journal", "ro"},
{"/var/mnt", "/run/host/var/mnt", "rslave"},
}
hostRunnerShim = struct {
containerPath string
source string
}{
"/usr/libexec/toolbox/host-runner", "/run/host/usr/libexec/toolbox/host-runner",
}

sudoShim = struct {
containerPath string
source string
}{
"/usr/libexec/toolbox/sudo", "/run/host/usr/libexec/toolbox/sudo",
}
)

var initContainerCmd = &cobra.Command{
Expand Down Expand Up @@ -294,6 +309,31 @@ func initContainer(cmd *cobra.Command, args []string) error {
}
}

logrus.Debugf("Creating directory libexec directory %s", toolboxLibexecDirectory)
if err := os.MkdirAll(toolboxLibexecDirectory, 0755); err != nil {
return fmt.Errorf("failed to create toolbox libexec directory %s: %w", toolboxLibexecDirectory, err)
}

// '/usr/libexec/toolbox/host-runner' is a script that makes use of 'flatpak-spawn
// --host' to run run commands from inside of a container on the host
if utils.PathExists(hostRunnerShim.source) {
logrus.Debugf("Seting up %s", hostRunnerShim.containerPath)
err = redirectPath(hostRunnerShim.containerPath, hostRunnerShim.source, false)
if err != nil {
return fmt.Errorf("failed to setup host-runner shim: %w", err)
}
}

// '/usr/libexec/toolbox/sudo' is a script that wraps around 'sudo' to make it
// possible to run commands from inside of a container on the host with sudo
if utils.PathExists(sudoShim.source) {
logrus.Debugf("Seting up %s", sudoShim.containerPath)
err = redirectPath(sudoShim.containerPath, sudoShim.source, false)
if err != nil {
return fmt.Errorf("failed to setup sudo shim: %w", err)
}
}

logrus.Debug("Setting up daily ticker")

daily, err := time.ParseDuration("24h")
Expand Down
1 change: 1 addition & 0 deletions src/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ sources = files(
'toolbox.go',
'cmd/completion.go',
'cmd/create.go',
'cmd/createShim.go',
'cmd/enter.go',
'cmd/help.go',
'cmd/initContainer.go',
Expand Down