Skip to content

Commit d5e50cb

Browse files
authored
Merge pull request #1942 from mxmauro/win_fixes
This PR aims to fix different Windows issues that appeared while attempted to use wallets. 1. Windows uses a totally different security model for directories than *nix like OSes. On this release, the check will be bypassed but the installer will be changed to add the proper access control list. 2. Windows 10 build 16257 added the ANSI compatible terminal which is not enabled by default. Sending escape codes will simply add garbage to the output. This behavior was only seen when an account is created and the mnemonic displayed. The colorization was removed in Windows builds. 3. `Signal(0)` handling. *nix OSes can use signal(0) to check if a process is valid and if the caller can send signals to it. Code was modified to enforce a similar behavior on Windows.
2 parents b0abee6 + 2bbf815 commit d5e50cb

File tree

9 files changed

+250
-31
lines changed

9 files changed

+250
-31
lines changed

cmd/goal/messages.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ const (
164164
infoCreatedWallet = "Created wallet '%s'"
165165
infoBackupExplanation = "Your new wallet has a backup phrase that can be used for recovery.\nKeeping this backup phrase safe is extremely important.\nWould you like to see it now? (Y/n): "
166166
infoPrintedBackupPhrase = "Your backup phrase is printed below.\nKeep this information safe -- never share it with anyone!"
167-
infoBackupPhrase = "\n\x1B[32m%s\033[0m"
168167
infoNoWallets = "No wallets found. You can create a wallet with `goal wallet new`"
169168
errorCouldntCreateWallet = "Couldn't create wallet: %s"
170169
errorCouldntInitializeWallet = "Couldn't initialize wallet: %s"

cmd/goal/messages_common.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright (C) 2019-2021 Algorand, Inc.
2+
// This file is part of go-algorand
3+
//
4+
// go-algorand is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as
6+
// published by the Free Software Foundation, either version 3 of the
7+
// License, or (at your option) any later version.
8+
//
9+
// go-algorand is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Affero General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Affero General Public License
15+
// along with go-algorand. If not, see <https://www.gnu.org/licenses/>.
16+
17+
// +build !windows
18+
19+
package main
20+
21+
const (
22+
// Wallet
23+
infoBackupPhrase = "\n\x1B[32m%s\033[0m"
24+
)

cmd/goal/messages_windows.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright (C) 2019-2021 Algorand, Inc.
2+
// This file is part of go-algorand
3+
//
4+
// go-algorand is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as
6+
// published by the Free Software Foundation, either version 3 of the
7+
// License, or (at your option) any later version.
8+
//
9+
// go-algorand is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Affero General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Affero General Public License
15+
// along with go-algorand. If not, see <https://www.gnu.org/licenses/>.
16+
17+
package main
18+
19+
const (
20+
// Wallet
21+
infoBackupPhrase = "\n%s"
22+
)

nodecontrol/NodeController.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package nodecontrol
1818

1919
import (
20-
"os"
2120
"path/filepath"
2221
"syscall"
2322
"time"
@@ -113,7 +112,7 @@ func (nc NodeController) stopProcesses() (kmdAlreadyStopped bool, err error) {
113112
}
114113

115114
func killPID(pid int) error {
116-
process, err := os.FindProcess(pid)
115+
process, err := util.FindProcess(pid)
117116
if process == nil || err != nil {
118117
return err
119118
}

nodecontrol/kmdControl.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,7 @@ func (kc *KMDController) StartKMD(args KMDStartArgs) (alreadyRunning bool, err e
200200
logging.Base().Errorf("%s: kmd data dir exists but is not a directory", kc.kmdDataDir)
201201
return false, errors.New("bad kmd data dir")
202202
}
203-
if (dataDirStat.Mode() & 0077) != 0 {
204-
logging.Base().Errorf("%s: kmd data dir exists but is too permissive (%o), change to (%o)", kc.kmdDataDir, dataDirStat.Mode()&0777, DefaultKMDDataDirPerms)
203+
if !kc.isDirectorySafe(dataDirStat) {
205204
return false, errors.New("kmd data dir not secure")
206205
}
207206
} else {

nodecontrol/kmdControl_common.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (C) 2019-2021 Algorand, Inc.
2+
// This file is part of go-algorand
3+
//
4+
// go-algorand is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as
6+
// published by the Free Software Foundation, either version 3 of the
7+
// License, or (at your option) any later version.
8+
//
9+
// go-algorand is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Affero General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Affero General Public License
15+
// along with go-algorand. If not, see <https://www.gnu.org/licenses/>.
16+
17+
// +build !windows
18+
19+
package nodecontrol
20+
21+
import (
22+
"os"
23+
24+
"github.com/algorand/go-algorand/logging"
25+
)
26+
27+
func (kc *KMDController) isDirectorySafe(dirStats os.FileInfo) bool {
28+
if (dirStats.Mode() & 0077) != 0 {
29+
logging.Base().Errorf("%s: kmd data dir exists but is too permissive (%o), change to (%o)", kc.kmdDataDir, dirStats.Mode()&0777, DefaultKMDDataDirPerms)
30+
return false
31+
}
32+
return true
33+
}

nodecontrol/kmdControl_windows.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright (C) 2019-2021 Algorand, Inc.
2+
// This file is part of go-algorand
3+
//
4+
// go-algorand is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as
6+
// published by the Free Software Foundation, either version 3 of the
7+
// License, or (at your option) any later version.
8+
//
9+
// go-algorand is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Affero General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Affero General Public License
15+
// along with go-algorand. If not, see <https://www.gnu.org/licenses/>.
16+
17+
package nodecontrol
18+
19+
import (
20+
"os"
21+
)
22+
23+
func (kc *KMDController) isDirectorySafe(_ os.FileInfo) bool {
24+
return true
25+
}

util/process_common.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,15 @@
1919
package util
2020

2121
import (
22+
"os"
2223
"syscall"
2324
)
2425

26+
// FindProcess looks for a running process by its pid
27+
func FindProcess(pid int) (*os.Process, error) {
28+
return os.FindProcess(pid)
29+
}
30+
2531
// KillProcess kills a running OS process
2632
func KillProcess(pid int, sig syscall.Signal) error {
2733
return syscall.Kill(pid, sig)

util/process_windows.go

Lines changed: 138 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,49 +19,161 @@
1919
package util
2020

2121
import (
22+
"errors"
2223
"os"
24+
"syscall"
2325
"unsafe"
2426

2527
"golang.org/x/sys/windows"
2628
)
2729

28-
// KillProcess kills a running OS process
29-
func KillProcess(pid int, _ os.Signal) error {
30+
const (
31+
ERROR_INVALID_PARAMETER = syscall.Errno(87)
32+
33+
STATUS_CANCELLED = uint32(0xC0000120)
34+
35+
processTerminateWaitInMs = 1000
36+
37+
killChildsPassCount = 4
38+
)
39+
40+
var (
41+
errFinishedProcess = errors.New("os: process already finished")
42+
)
3043

31-
p, err := os.FindProcess(pid)
44+
// FindProcess looks for a running process by its pid
45+
func FindProcess(pid int) (*os.Process, error) {
46+
var h syscall.Handle
47+
48+
process, err := os.FindProcess(pid)
49+
if err != nil {
50+
if isInvalidParameterError(err) { // NOTE: See function definition for details
51+
return nil, nil
52+
}
53+
return nil, err
54+
}
55+
56+
// If we have a process, check if it is terminated
57+
h, err = syscall.OpenProcess(syscall.SYNCHRONIZE, false, uint32(pid))
3258
if err == nil {
59+
defer func() {
60+
_ = syscall.CloseHandle(h)
61+
}()
3362

34-
for _, v := range getChildrenProcesses(pid) {
35-
_ = v.Kill()
63+
ret, e2 := syscall.WaitForSingleObject(h, 0)
64+
if e2 == nil && ret == syscall.WAIT_OBJECT_0 {
65+
return nil, nil
3666
}
67+
} else {
68+
if isInvalidParameterError(err) { // NOTE: See function definition for details
69+
return nil, nil
70+
}
71+
}
3772

38-
err = p.Kill()
73+
return process, nil
74+
}
75+
76+
// KillProcess kills a running OS process
77+
func KillProcess(pid int, signal os.Signal) error {
78+
// Signal(0) only checks if we have access to kill a process and if it is really dead
79+
if signal == syscall.Signal(0) {
80+
return isProcessAlive(pid)
3981
}
82+
83+
return killProcessTree(pid)
84+
}
85+
86+
func isProcessAlive(pid int) error {
87+
var ret uint32
88+
89+
h, err := syscall.OpenProcess(syscall.SYNCHRONIZE|syscall.PROCESS_TERMINATE, false, uint32(pid))
90+
if err != nil {
91+
if isInvalidParameterError(err) { // NOTE: See function definition for details
92+
return errFinishedProcess
93+
}
94+
return err
95+
}
96+
ret, err = syscall.WaitForSingleObject(h, 0)
97+
if err == nil && ret == syscall.WAIT_OBJECT_0 {
98+
err = errFinishedProcess
99+
}
100+
101+
_ = syscall.CloseHandle(h)
40102
return err
41103
}
42104

43-
func getChildrenProcesses(parentPid int) []*os.Process {
44-
out := []*os.Process{}
105+
func killProcessTree(pid int) error {
106+
err := killProcess(pid)
107+
if err != nil {
108+
return err
109+
}
110+
111+
// We do several passes just in case the process being killed spawns a new one
112+
for pass := 1; pass <= killChildsPassCount; pass++ {
113+
childProcessList := getChildProcesses(pid)
114+
if len(childProcessList) == 0 {
115+
break
116+
}
117+
for _, childPid := range childProcessList {
118+
killProcessTree(childPid)
119+
}
120+
}
121+
122+
return nil
123+
}
124+
125+
func getChildProcesses(pid int) []int {
126+
var pe32 windows.ProcessEntry32
127+
128+
out := make([]int, 0)
129+
45130
snap, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPPROCESS, uint32(0))
46-
if err == nil {
47-
var pe32 windows.ProcessEntry32
48-
49-
defer windows.CloseHandle(snap)
50-
51-
pe32.Size = uint32(unsafe.Sizeof(pe32))
52-
if err := windows.Process32First(snap, &pe32); err == nil {
53-
for {
54-
if pe32.ParentProcessID == uint32(parentPid) {
55-
p, err := os.FindProcess(int(pe32.ProcessID))
56-
if err == nil {
57-
out = append(out, p)
58-
}
59-
}
60-
if err = windows.Process32Next(snap, &pe32); err != nil {
61-
break
62-
}
63-
}
131+
if err != nil {
132+
return out
133+
}
134+
135+
defer func() {
136+
_ = windows.CloseHandle(snap)
137+
}()
138+
139+
pe32.Size = uint32(unsafe.Sizeof(pe32))
140+
err = windows.Process32First(snap, &pe32)
141+
for err != nil {
142+
if pe32.ParentProcessID == uint32(pid) {
143+
// Add to list
144+
out = append(out, int(pe32.ProcessID))
64145
}
146+
147+
err = windows.Process32Next(snap, &pe32)
65148
}
149+
66150
return out
67151
}
152+
153+
func killProcess(pid int) error {
154+
h, err := syscall.OpenProcess(syscall.SYNCHRONIZE | syscall.PROCESS_TERMINATE, false, uint32(pid))
155+
if err == nil {
156+
err = syscall.TerminateProcess(h, STATUS_CANCELLED)
157+
if err == nil {
158+
_, _ = syscall.WaitForSingleObject(h, processTerminateWaitInMs)
159+
}
160+
161+
_ = syscall.CloseHandle(h)
162+
}
163+
164+
return err
165+
}
166+
167+
// NOTE: Unlike Unix, Windows tries to open the target process in order to kill it.
168+
// ERROR_INVALID_PARAMETER is returned if the process does not exists.
169+
// To mimic other OS behavior, if the process does not exist, don't return an error
170+
func isInvalidParameterError(err error) bool {
171+
var syscallError syscall.Errno
172+
173+
if errors.As(err, &syscallError) {
174+
if syscallError == ERROR_INVALID_PARAMETER {
175+
return true
176+
}
177+
}
178+
return false
179+
}

0 commit comments

Comments
 (0)