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

[Feature Request] Add Sudo Support on Windows #3541

Open
lunatic-gh opened this issue Nov 21, 2024 · 21 comments · May be fixed by #3578
Open

[Feature Request] Add Sudo Support on Windows #3541

lunatic-gh opened this issue Nov 21, 2024 · 21 comments · May be fixed by #3578

Comments

@lunatic-gh
Copy link

Description of the problem or steps to reproduce

Since Sudo wasn't really a thing on Windows, micro has no support for this, and will happily tell this to the user when trying to use:
image

However, since 24H2, Windows added sudo as an option. It behaves pretty similar, eg by running sudo <my-command> it would show a UAC prompt, and on success run my-command in an administrator instance of that commandline.

Additionally, even if 24H2 is not being used, GSudo is basically the exact same.

It would be a great addition to support sudo when either 24H2's Sudo or Gsudo is present.

Specifications

Commit hash: /
OS: Windows 11
Terminal: /

@niten94
Copy link
Contributor

niten94 commented Nov 21, 2024

I am not available to look much into adding support, but I tried checking what had to be specifically done to add support months ago. Micro runs dd when saving files using sudo but dd is not preinstalled. I wrote about it in a comment in a different issue: #2660 (comment)

Edit: I was thinking about running a PowerShell command instead of dd sometime after I posted the comment, but I have not checked anything about it.

@lunatic-gh
Copy link
Author

What exactly is micro internally using on windows to save files? Couldn't it just do the exact same it is already doing, but instead with this sudo/gsudo command as a prefix? Sadly i don't know anything about the go language or windows internals itself, otherwise i'd gladly help however i can.

@niten94
Copy link
Contributor

niten94 commented Nov 21, 2024

Micro writes to files using Go standard library functions in all platforms when saving without sudo, so the same thing cannot be done as a command using sudo. The actual command run with sudo in all platforms except Windows is like echo "new file content" | sudo dd bs=4k of=file.txt, but micro uses functions to write input that sudo passes to dd instead of echo "new file content" |.

dd is a program used like a command that is usually preinstalled in Linux distributions. It's not available by default in Windows so it may be inconvenient if micro uses dd in Windows. Micro may just use a different command instead of dd ... in Windows to add support, but I don't know what command can be used.

@JoeKar
Copy link
Collaborator

JoeKar commented Nov 21, 2024

As @niten94 already wrote, it's unfortunately not done by allowing/enabling sudo on Windows, because it would require additionally dd (coreutils) for Windows. The latter one isn't impossible, because they already have been ported (e.g. gnuwin32 coreutils, rust coreutils), but need to be installed too.
Or it requires a fully different approach for storing files with extended privileges like ShellExecute (see here: https://blog.hadenes.io/post/how-to-request-admin-permissions-in-windows-through-uac-with-golang/).

The easiest way for us would be option one, since the mechanism doesn't need to be rewritten or extended and we just need to enable the sudo support and give hints that additional tools are required.

@lunatic-gh
Copy link
Author

Micro may just use a different command instead of dd ... in Windows to add support, but I don't know what command can be used.

If all that is needed is an alternative native command, can't we just use powershell for that?
I googled a bit, and tested a bit of powershell script in a freshly installed windows-10 vm (only gsudo installed), and could observe the following:
image

Maybe this could be used on windows?

@JoeKar
Copy link
Collaborator

JoeKar commented Nov 21, 2024

Maybe this could be used on windows?

That's like running micro itself with super user rights and using Go's standard functionalities to open and write, but this isn't what we want...at least not in this form. The extended privileges shall only be used for the file save process, not for the rest of micro.
This is the current approach to store the file:

cmd = exec.Command(config.GlobalSettings["sucmd"].(string), "dd", "bs=4k", "of="+name)

Currently I don't know if there is a different alternative other than dd under Windows to use the stdin as input and write to a file.

BTW:

Or it requires a fully different approach for storing files with extended privileges like ShellExecute (see here: https://blog.hadenes.io/post/how-to-request-admin-permissions-in-windows-through-uac-with-golang/).

Currently realized, that we don't even need sudo, because Windows already offers runas, which can be used via the native exec.Command() without the need of ShellExecute. And the runas can be stored into sucmd.

@lunatic-gh
Copy link
Author

lunatic-gh commented Nov 21, 2024

in this case, my basic "idea" wasn't to run the entirety of micro as sudo, but rather something like

cmd = exec.Command(config.GlobalSettings["sucmd"].(string), "powershell -command", "& {...the-command...}")

(assuming that syntax is correct)
since the saving-process is done via a command anyway.

But if the runas method is more convenient that is probably better than this awfully long powershell-script.

@JoeKar
Copy link
Collaborator

JoeKar commented Nov 22, 2024

But the PowerShell isn't available by default either or am I wrong? micro can be executed under Windows without the PowerShell. So we end up in a situation where something additionally needs to be installed anyway. Then I would recommend to go with a coreutils alternative (mentioned here: #3541 (comment)) and the dd path should most probably be configurable.

@lunatic-gh
Copy link
Author

lunatic-gh commented Nov 22, 2024

both windows 10 and 11 come with powershell 5.1 pre-installed. it should therefore be available on every pc that isn't stuck 200 years ago. I think it might even be available out of the box since windows 7, but i am not 100% sure on that.

the screenshot i gave above was indeed done on a vm with windows 10, where i really only installed gsudo (well and updated the app installer through ms-store, so i can actually install gsudo)

@JoeKar
Copy link
Collaborator

JoeKar commented Nov 22, 2024

Ok, if this is the case then...

cmd = exec.Command(config.GlobalSettings["sucmd"].(string), "powershell", "-command", "Get-Content -Path '-' | Set-Content -Path "+name)

...could do the trick, right?

@lunatic-gh
Copy link
Author

lunatic-gh commented Nov 22, 2024

That command does not seem to work from what i can see. Dunno what the '-' means either.

Does the 4k block size matter? If not, the simplest way would probably be the command:

powershell -command "string-content" | Out-File -FilePath "path/to/file"

or in go:

cmd = exec.Command(config.GlobalSettings["sucmd"].(string), "powershell", "-command", content-string, "| Out-File -FilePath", name)

If i got the syntax right.

This would create the given file (if it does not exist), and afterwards write the given content to it.

@niten94
Copy link
Contributor

niten94 commented Nov 23, 2024

I think @JoeKar included Get-Content -Path - in the command to read the data written to PowerShell standard input. Each string passed except the first argument are passed as one argument to the program in the first argument (exec.Command: sudo, sudo: powershell), so arguments have to be passed like in the syntax that @JoeKar wrote. The file content is written to the standard input of cmd in line 39, 63 and 64 below, so it does not have to be included in arguments:

if writeCloser, err = cmd.StdinPipe(); err != nil {
return
}
c = make(chan os.Signal, 1)
signal.Reset(os.Interrupt)
signal.Notify(c, os.Interrupt)
screenb = screen.TempFini()
// need to start the process now, otherwise when we flush the file
// contents to its stdin it might hang because the kernel's pipe size
// is too small to handle the full file contents all at once
if err = cmd.Start(); err != nil {
screen.TempStart(screenb)
signal.Notify(util.Sigterm, os.Interrupt)
signal.Stop(c)
return
}
} else if writeCloser, err = os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666); err != nil {
return
}
w := bufio.NewWriter(transform.NewWriter(writeCloser, enc.NewEncoder()))
err = fn(w)

I do not know much about .NET and PowerShell but I tried reading documentation and I do not think cmdlets can be used. The output encoding is handled in line 63 but encoding seems to be converted when using pipes and Out-File1 in PowerShell. Set-Content -AsByteStream cannot be used in PowerShell 5.1.2 I do not know if writing with 4k block size is needed.

I did not check anything with error handling but I think this can be done. The path would be passed as a different argument like -command "code" file.txt to avoid escaping. The code can also be passed in Base64 but I don't know if that would be worse.

using namespace System.IO
$f = [File]::Open($args[0],
    [FileMode]::OpenOrCreate -bor [FileMode]::Truncate, [FileAccess]::Write)
$i = [System.Console]::OpenStandardInput()
$i.CopyTo($f)
$i.Dispose()
$f.Dispose()

Footnotes

  1. https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/out-file?view=powershell-5.1#-encoding

  2. https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.management/set-content?view=powershell-7.4#-asbytestream

@lunatic-gh
Copy link
Author

lunatic-gh commented Nov 23, 2024

I am not sure if Go would do that different, but whenever i run this inside a powershell terminal, it just fails because it takes that '-' literally, but expects an actual filesystem path it seems:
image

The file content is written to the standard input of cmd in line 39, 63 and 64 below, so it does not have to be included in arguments:

Wouldn't that require both of those processes (A: Creating the file, and B: Writing to it) require admin permissions? I am pretty sure just running the creation process as sudo won't automatically give the process access to write to it. And since the 2nd Process (writing to the file) is not done via shell-commands, there's no easy way to give it those needed permissions

@JoeKar
Copy link
Collaborator

JoeKar commented Nov 23, 2024

I am not sure if Go would do that different, but whenever i run this inside a powershell terminal, it just fails because it takes that '-' literally, but expects an actual filesystem path it seems:

Yep, the AI fooled me, since "-" can't be used as placeholder for the stdin there. So the PS script must be more complex.

Maybe then something like this:
cmd = exec.Command(config.GlobalSettings["sucmd"].(string), "powershell", "-command", "$input = [System.Console]::In.ReadToEnd(); $input | Set-Content -Path", name)

But to be honest, I never used PS before.
If you find a nice way to create a short replacement for the dd-approach without dd itself but with Windows internals...why not.

@lunatic-gh
Copy link
Author

Honestly, i think i totally lack the options to test something like this. I have no idea what [System.Console]::In.ReadToEnd() does, or specifically i cannot test it, since in a console it just... freezes, probably since it does something entirely else in the Go Program context.

If someone else with go & ps knowledge and the ability to build micro could test this all, it would probably be way easier than me trying to fiddle my way into this without knowing anything about... anything really.

@JoeKar
Copy link
Collaborator

JoeKar commented Nov 24, 2024

I have no idea what [System.Console]::In.ReadToEnd() does, or specifically i cannot test it, since in a console it just... freezes, probably since it does something entirely else in the Go Program context.

It has nothing to do with Go and it doesn't freeze in the first place, it just waits for input, since we try to read from the stdin.
Anyway, I created my own Win10 VM for a faster test and was able via...
echo test | powershell -command "[System.Console]::In.ReadToEnd() | Set-Content -Path test.txt
...to forward the echoed string into the file, but most probably we will run into trouble, since it just takes the string and doesn't interpret it as byte stream. With runas additional user information must be provided as parameter and can't be set within the sucmd...otherwise the whole string will be searched as command, so it will increase complexity especially when it shall execute a command like this as one-liner.

@niten94
Copy link
Contributor

niten94 commented Nov 30, 2024

Sorry, I think my previous comment had unnecessary information and was confusing. I have Windows 10 installed on my personal machine so I can build micro and test, but replacing dd ... only with PowerShell code is more complicated than I thought. It may be better to use dd on Windows like initally suggested at this point, but I still wrote about using long PowerShell code in this commment.

But to be honest, I never used PS before. If you find a nice way to create a short replacement for the dd-approach without dd itself but with Windows internals...why not.

Anyway, I created my own Win10 VM for a faster test and was able via...
echo test | powershell -command "[System.Console]::In.ReadToEnd() | Set-Content -Path test.txt
...to forward the echoed string into the file, but most probably we will run into trouble, since it just takes the string and doesn't interpret it as byte stream.

I do not think there is anything else available on Windows that can be used by default. I tried to somewhat search and learn about .NET and PowerShell, but I do not think there is a short command that can be used. I used Console.OpenStandardInput in the code I wrote because Console.In is a TextReader, but the type of the object returned does not have a method like ReadToEnd. There may be no better way to read bytes in standard input.

I have only tested the code with $i.CopyTo($f) using gsudo on micro (it worked), but there are 3 ways the file can be written. The commands can be written in one line, but I think the first and third one look the least worst:

trap { break }
$f = [IO.File]::Open($env:OUTFILE, [IO.FileMode]::OpenOrCreate, [IO.FileAccess]::Write)
$i = [Console]::OpenStandardInput()
$i.CopyTo($f)
$i.Dispose()
$f.SetLength($f.Position)
$f.Dispose()
trap { break }
$i = [Console]::OpenStandardInput()
& {
    $b = [byte[]]::new(4096)
    while(($c = $i.Read($b, 0, $b.Length)) -gt 0) {
        [ArraySegment[byte]]::new($b, 0, $c)
    }
} | Set-Content -Encoding byte -LiteralPath $env:OUTFILE
$i.Dispose()
trap { break }
$i = [Console]::OpenStandardInput()
$ms = [IO.MemoryStream]::new()
$i.CopyTo($ms)
Set-Content -Encoding byte -Value $ms.ToArray() -LiteralPath $env:OUTFILE
$ms.Dispose()
$i.Dispose()

I have no experience with measuring performance so something wrong is done, but these results came out when I ran this script using busybox-w32:

set -u -e

if [ $# -ne 2 ]; then
    prog=`basename -- "$0"`
    printf '%s\n' "usage: $prog dest src" >&2
    exit 1
fi

d=`dirname -- "$0"`

export OUTFILE="$1"
for s in "$d"/*.ps1; do
    printf '%s\n' "${s#$d/}"
    cmd=`cat "$s"`
    time powershell -command "$cmd" < "$2"
    cmp -- "$2" "$1"
    read _
done
> busybox sh bench.sh samples\micro-README.md out.txt
copyto.ps1
real    0m 0.16s
user    0m 0.12s
sys     0m 0.07s

scbuf.ps1
real    0m 0.31s
user    0m 0.28s
sys     0m 0.07s

scval.ps1
real    0m 0.20s
user    0m 0.06s
sys     0m 0.15s

@niten94
Copy link
Contributor

niten94 commented Nov 30, 2024

With runas additional user information must be provided as parameter and can't be set within the sucmd...otherwise the whole string will be searched as command, so it will increase complexity especially when it shall execute a command like this as one-liner.

I realized this when testing but I think additional parameters have to provided when using gsudo too. I do not know how the command is escaped, but gsudo runs it in a shell that is somehow detected automatically. cmd.exe is used as the shell when gsudo -d is run. The path has to be set in an environment variable if using PowerShell because arguments after -command "cmd" are included in cmd, so there is nothing to mind about escaping paths in arguments.

@JoeKar
Copy link
Collaborator

JoeKar commented Nov 30, 2024

It may be better to use dd on Windows like initally suggested at this point

I've the same feeling, since the PS script becomes quite big and would need to be bundled (for better maintainability) with micro (maybe inside the runtime).

@niten94
Copy link
Contributor

niten94 commented Dec 16, 2024

Sorry, I am probably the one who has searched the most in the thread, but I did not update on details I have found and what I was going to do. I think I should have at least said something even if short but I tried creating a pull request and adding support.

@niten94
Copy link
Contributor

niten94 commented Dec 16, 2024

I do not know why I just realized this now, but is there any reason to not run micro itself with sudo and pass an internal flag like this?

sucmd := config.GlobalSettings["sucmd"].(string)
if runtime.GOOS == "windows" {
    exe, err := os.Executable()
    if err == nil {
        cmd = exec.Command(sucmd, exe, "-write", name)
    } else {
        return err
    }
}

Using ShellExecute and runas is mentioned in this thread but a prompt will always be displayed unlike other editors on Windows, so other programs like gsudo will still have to be used.

Edit: Sorry for the noise. I tried reading the thread again but I do not think using a PowerShell script would go well or be accepted, so I went ahead and changed the pull request to just running micro itself with an internal flag on Windows. I think there was a bug when I was testing with gsudo so I do not think gsudo -d has to be used. However, I don't know where notes about the programs tested can be included in documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants