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.

Sign up for GitHub

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

Jump to bottom

Cpu namespace over ssh #279

Merged
merged 18 commits into from
Sep 25, 2024
Merged

Cpu namespace over ssh #279

merged 18 commits into from
Sep 25, 2024

Conversation

rminnich
Copy link
Member

@rminnich rminnich commented Jul 10, 2024 •
edited
Loading

Did you want to run cpu, but don't want to set up cpud?
If you have an sshd, you're good to gol

This is cpu talking to an sshd.

rminnich@pop-os:~/go/src/github.com/u-root/cpu/cmds/cpu$ cpu -nfs=true -9p=false  q date
Wed Jul 10 03:08:16 PM PDT 2024

This is cpu talking to an sshd, but setting up the namespace.

rminnich@pop-os:~/go/src/github.com/u-root/cpu/cmds/cpu$ cpu -nfs=true -9p=false  q ~/bin/ns
[sudo] password for rminnich:
rminnich@pop-os:~$ mount
...
127.0.0.1:9973ecb0-b836-4f7b-997d-d7bc7dc08456 on /tmp/cpu type nfs (rw,relatime,vers=3,rsize=1048576,wsize=1048576,namlen=255,hard,nolock,proto=tcp,port=44957,timeo=600,retrans=2,sec=sys,mountaddr=127.0.0.1,mountvers=3,mountport=44957,mountproto=tcp,local_lock=all,addr=127.0.0.1)
127.0.0.1:9973ecb0-b836-4f7b-997d-d7bc7dc08456/usr/lib on /usr/lib type nfs (rw,relatime,vers=3,rsize=1048576,wsize=1048576,namlen=255,hard,nolock,proto=tcp,port=44957,timeo=600,retrans=2,sec=sys,mountaddr=127.0.0.1,mountvers=3,mountport=44957,mountproto=tcp,local_lock=all,addr=127.0.0.1)

...

ns is a script:

#!/bin/sh

sudo -E unshare -m ~rminnich/go/bin/cpuns

So the key new command here is cpuns.

So, if you want cpu, with just a plain old sshd, you've got it now.

There are a few issues left, but if you want early access, here it is.

I don't even know how to debug it ...

@rminnich rminnich requested a review from brho July 10, 2024 22:15
@rminnich
Copy link
Member Author

This may the point at which cpu defaults to nfs instead of 9p.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 10, 2024
View reviewed changes
cmds/cpuns/main_linux.go Fixed Show fixed Hide fixed
cmds/cpuns/main_linux.go Fixed Show fixed Hide fixed
@rminnich rminnich force-pushed the cpu-namespace-over-ssh branch 2 times, most recently from 3cf6146 to eafb8c1 Compare July 12, 2024 00:25
@rminnich
Copy link
Member Author

I wonder ... certainly another way to get the environment intact to the remote program is this:
cpu remote somecommand -env export [args ...]
So the remote command would see, for the env switch, the desired environment for the child process. Then, ...
cpu remote cpuns -env export [command [switches [args]]]
cpuns would consume the -env just enough to pass it through; and it would process CPU_FSTAB.

Then the hack with LC_GLENDA_CPU_FSTAB would not be needed at all.

comments?

@rminnich
Copy link
Member Author

I am going to redesign this a bit, b/c the ssh restriction on environment variables is truly a nightmare. I think I'll set up if so that cpuns has an arg:
cpuns -env environment-variables-seperated-by-null-chars
where env will be created by the cpu command.

option to is just coopt LC_TERM and add more names to it. But there's always the change we'll hit more restrictions, as we did with sudo, so that seems a neverending story.

Look for the rework next week.

@rminnich
Copy link
Member Author

I think this is ready for review. Anyone?

binjip978
binjip978 reviewed Sep 24, 2024
View reviewed changes
client/fns.go Show resolved Hide resolved
cmds/cpuns/main_linux.go Outdated Show resolved Hide resolved
session/session.go Show resolved Hide resolved
session/session.go Show resolved Hide resolved
client/client.go Show resolved Hide resolved
@rminnich
Add a cpuns command
d4fdad1 
This command lets you ssh to a system with the cpu command.
You can then run this command, with or without arguments,
and it will give you a cpu environment.

A typical way to run it:
sudo -E unshare -m cpuns

The -E is important to get the SUDO_UID and SUDO_GID

Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
@rminnich
client: do not disable echoing
7f8a3ec 
Turns out we should not ask the remote end to disable echoing.
Discovered 7 years later when we start really connecting to ssh ...

Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
@rminnich
cpu and packages: pass and use LC_GLENDA_CPU_FSTAB
c3f20aa 
sshd everywhere are configured to limit environment
variables. We can not ask people everywhere to change that.

Pass the CPU_FSTAB as LC_GLENDA_CPU_FSTAB as well, and have
code check for it when it checks CPU_FSTAB.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
gofmt -w -s and update .gitignore and update comments
d51456a 
remove comments about non-empty namespace enabling 9p

In earlier times, if the namespace was nonempty, 9p
was enabled even if not requested.

In our new era of NFS support, this is no longer true.
Remove those comments.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
client: use GetHostUser for host and user
5a00ff8 
At this point, we can do this:
~/go/bin/cpu -nfs=true -9p=false -namespace=/Users s76 bin/cpuns
[sudo] password for rminnich:
2024/07/12 17:36:33 [ERROR] No handler for 100227.0
bash: /.cargo/env: No such file or directory
rminnich@pop-os:/home/rminnich$ ls /Users
nancyminnich  rminnich  Shared

Remaining to be done: Need to add a handler to get rid of that message;
need to have cpuns take args so it can just start a shell;
and it would sure be nice if we could pass environment variables
but that's unlikely to ever get fixed.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cmds/cpu: remove call to GetHost
4a6942e 
It is no longer needed, it is handled in the client.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cmds/cpu: set *ninep and *srvnfs in main() for port 22
9fa19db 
If the port is 22, then set ninep and srvnfs
for that case.

Because, going back 8 years, with ninep, we send the Nonce
on the ssh channel, we can not use ninep with sshd.

Sorry about that, but 9p has never gotten to the point that
it is worth fixing this and breaking all current users out there.

Just set things for nfs if we are on port 22.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cmds/cpuns: accept args, and pass them on
36d01c6 
For the part of cpuns that runs as root, the flags are
not evaluated, save for the debug option. That can be turned off
it if is a concern.

This now works:
~/go/bin/cpu -namespace=/Users s76 bin/cpuns ls /Users
[sudo] password for rminnich:
2024/07/12 17:58:52 [ERROR] No handler for 100227.0
nancyminnich  rminnich	Shared

For .bashrc, one could test the existence of LC_GLENDA_CPU_FSTAB,
and run cpuns automatically.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cmds/cpu: -ssh switch; pass environment in command line
c1d74ac 
if -ssh is true then assume ssh; this is in addition
to the port 22 test.

If on ssh, insert a cpuns command, with a -env switch,
to pass environment variables through all the filters
(sshd and sudo) that are stripping them out.

The only environment variable that is processed is CPU_FSTAB;
the rest are ignored.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cpuns: add -env for environment handling
8b9653a 
the -env is for cpuns to pass environment to the
eventual spawned process.

It is handled as follows:
If cpuns is not running as root, pass the switch on as -env=

If  cpuns is running as root, break the switch value up with
strings.Split(..., "\n") and append that to the environment.

Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
@rminnich
session: remove LC_GLENDA_CPU_FSTAB usage
a52d434 
Turns out we can firewall it in the first step of cpuns.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cpuns: set pwd from $PWD
fa89e2a 
Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
@rminnich
cpu: add CPU_PWD environment variable
633d4b5 
sshd simulates a login and starts the process in $HOME.

Add CPU_PWD, derived from PWD on the client, which cpuns
can use to set the correct directory.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cmds/cpu: do not try to start cpunfs if *srvnfs is false
9bbeae5 
Sometimes we just have a plain ssh connection.
We do not always want to start cpuns.

Do not prepend 'cpuns' to the shell command if *srvnfs is not set.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cpu: change ssh option to sshd
ef722ed 
Discussions with users indicate we need to have an 'ssh'
(or similar name) option which means 'do only what an ssh
would do'.

Change the ssh option to sshd.

The exact naming of the ssh option is TBD.
-plainssh?
-ssh?
-ssh-no-mounts?

The initial plan is -ssh.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cpu: add -ssh flag which disables servers and namespace
da4ba2e 
In certain cases, even if a namespace and mounts are possible,
users want to run a basic ssh session.

Add a -ssh switch which disables *srvfs, *ninep, enables
*sshd, and sets *namespace to ""

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
cpuns: work if login is root; remove /bin/unshare usage
76f3c6b 
The previous version of cpuns did not work if the login
was root. Further, it had a dependency on unshare.

Redesign cpuns so even if the login is root it works.
Remove the unshare dependency. cpuns now only depends no
sudo being present.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
@rminnich
change usages of %v to %w
908af04 
Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
rjkroege
rjkroege approved these changes Sep 25, 2024
View reviewed changes
Copy link
Collaborator

@rjkroege rjkroege left a comment

Choose a reason for hiding this comment

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

Some thoughts added but you can probably ignore me. 😊

cmds/cpu/cpu.go
@@ -136,7 +131,23 @@ func getPort(host, port string) string {
}

func newCPU(host string, args ...string) (retErr error) {
// note that 9P is enabled if namespace is not empty OR if ninep is true
// When running over ssh, many environment variables
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code for building the port and environment seems twisty and exactly the kind of thing that would benefit from being pulled out into utility functions that have some tests to make sure that the environment is sane? Maybe just add a TODO in this PR.

cmds/cpu/doc.go Show resolved Hide resolved
cmds/cpu/doc.go Show resolved Hide resolved
cmds/cpu/doc.go Show resolved Hide resolved
@rminnich rminnich merged commit d03915d into u-root:main Sep 25, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@binjip978 binjip978 binjip978 approved these changes

@rjkroege rjkroege rjkroege approved these changes

@brho brho Awaiting requested review from brho

@ericvh ericvh Awaiting requested review from ericvh

@orangecms orangecms Awaiting requested review from orangecms

@Lencerf Lencerf Awaiting requested review from Lencerf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rminnich @rjkroege @binjip978