Skip to content

Commit c3d261b

Browse files
authored
Merge pull request #1335 from jerrynbc/fix/eliminate-scan-findings
fix: close remaining CE scan findings
2 parents 7f024ce + 8f0a7d9 commit c3d261b

25 files changed

Lines changed: 540 additions & 149 deletions

File tree

‎compose.yaml‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,10 @@ services:
4848
# ${SAFELINE_DIR}/resources/sock/mgt_grpc_token, which this container
4949
# creates and the tengine container reads.
5050
- MGT_GRPC_TOKEN=${MGT_GRPC_TOKEN:-}
51-
# Optional out of band token for the anonymous TFA bootstrap. As long as
52-
# it is empty, GET /api/OTPUrl hands the TFA secret of the account to
53-
# whoever reaches the port first, and the login is then bound to that
54-
# address; the daemon warns about it at startup. Set it, and pass the
55-
# same value to the request as X-Bootstrap-Token, to keep the binding for
56-
# an administrator who has the token.
51+
# Token required on GET /api/OTPUrl as X-Bootstrap-Token. Leave empty
52+
# to let the process generate one into
53+
# ${SAFELINE_DIR}/resources/sock/tfa_bootstrap_token (0600) and log the
54+
# path. The console must send that value; until it does, bind with curl.
5755
- MGT_BOOTSTRAP_TOKEN=${MGT_BOOTSTRAP_TOKEN:-}
5856
# How long the account may be bound without a credential, measured from
5957
# the moment the installation was set up (or from `mgt -reset_user

‎management/tcontrollerd/controller/controller.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"strings"
77

88
"google.golang.org/grpc"
9-
"google.golang.org/grpc/credentials/insecure"
109
"google.golang.org/grpc/metadata"
1110

1211
"chaitin.cn/dev/go/errors"
@@ -48,12 +47,7 @@ func Handle() error {
4847
if err != nil {
4948
return err
5049
}
51-
if creds != nil {
52-
dialOptions = append(dialOptions, grpc.WithTransportCredentials(creds))
53-
} else {
54-
// nil creds only when TCD_MGT_TLS is an explicit off value.
55-
dialOptions = append(dialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
56-
}
50+
dialOptions = append(dialOptions, grpc.WithTransportCredentials(creds))
5751

5852
gRPCConn, err := grpc.Dial(config.GlobalConfig.MgtWebserver, dialOptions...)
5953
if err != nil {

‎management/tcontrollerd/controller/creds.go‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,9 @@ import (
1414
)
1515

1616
const (
17-
// TLSEnv names the environment variable that decides how the control
18-
// channel is reached.
19-
//
20-
// - "0" / off / false / no: dial in the clear (warned).
21-
// - anything else, including unset: TLS is required. Credentials are
22-
// those published by `mgt -gen_certs`. Missing files are an error;
23-
// there is no silent fallback to plaintext.
17+
// TLSEnv names the environment variable that used to allow a plaintext
18+
// control channel. Plaintext is no longer supported: a value of "0" /
19+
// off / false / no is an error.
2420
TLSEnv = "TCD_MGT_TLS"
2521

2622
// ServerCertFile is the certificate the management server is expected to
@@ -35,24 +31,15 @@ const (
3531

3632
// transportCreds returns the credentials of the control channel.
3733
//
38-
// A nil result means the channel is reached in the clear, and that only happens
39-
// when TLSEnv is an explicit off value. Unset TLSEnv still requires TLS: the
40-
// management side is expected to have published client credentials via
41-
// `mgt -gen_certs`; missing files are returned as an error instead of a
42-
// plaintext dial.
34+
// The channel is always reached over TLS. Missing credentials or an explicit
35+
// request for plaintext are errors.
4336
func transportCreds() (credentials.TransportCredentials, error) {
4437
switch strings.ToLower(strings.TrimSpace(os.Getenv(TLSEnv))) {
4538
case "0", "off", "false", "no":
46-
logger.Warnf("%s is disabled: the control channel is reached in the clear, and the subscription token is readable by anything that shares the network of this container.", TLSEnv)
47-
return nil, nil
39+
return nil, errors.New("plaintext is not supported: unset " + TLSEnv + " and use credentials published by mgt -gen_certs")
4840
}
4941

50-
creds, err := loadTransportCreds()
51-
if err != nil {
52-
return nil, err
53-
}
54-
55-
return creds, nil
42+
return loadTransportCreds()
5643
}
5744

5845
// loadTransportCreds reads the credentials the management server published for

‎management/webserver/api/auth.go‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -257,16 +257,15 @@ func BootstrapWindow() time.Duration {
257257
return window
258258
}
259259

260-
// GetOTPUrl hands the TFA secret to the first installer. It stays on the
261-
// public /api group so the stock console login page can show a QR code without
262-
// a session; requiring MGT_BOOTSTRAP_TOKEN by default would leave that page
263-
// unable to bind a fresh CE install.
260+
// GetOTPUrl hands the TFA secret during first-boot binding.
264261
//
265-
// Existing gates, not a missing auth middleware:
262+
// The request must carry X-Bootstrap-Token. Remaining gates:
266263
// - LastLoginTime > 0: no more secret after the first successful login
267-
// - secret is bound to the first ClientIP (TFABootstrapIP); other IPs are refused
264+
// - secret is bound to the first ClientIP (TFABootstrapIP)
268265
// - MGT_BOOTSTRAP_WINDOW, bootstrapThrottle, claimBootstrapSecret / adoptBootstrapSecret
269-
// Public management ports should still set MGT_BOOTSTRAP_TOKEN.
266+
//
267+
// The stock console must send the header. Until that image is updated, bind
268+
// with curl using the token written at startup.
270269
func GetOTPUrl(c *gin.Context) {
271270
db := database.GetDB()
272271

@@ -521,22 +520,12 @@ func otpURL(secret string) string {
521520
return bootstrapURL.String()
522521
}
523522

524-
// checkBootstrapToken enforces the optional out of band token of the anonymous
525-
// TFA bootstrap.
526-
//
527-
// Installations that do not want the account to be bindable by whoever reaches
528-
// the API first set MGT_BOOTSTRAP_TOKEN and deliver it to the administrator
529-
// through a channel of their choice (the installation output, for example).
530-
//
531-
// The empty value keeps the bootstrap open on purpose: the console that ships
532-
// in the management image asks this endpoint for the secret without knowing it,
533-
// so requiring a token here would leave a fresh installation without any way to
534-
// bind the account. That trade off is announced at startup (see main.go) so
535-
// that it is a visible choice rather than a silent default.
523+
// checkBootstrapToken requires X-Bootstrap-Token to match the configured
524+
// bootstrap token. An empty expected value is a server misconfiguration.
536525
func checkBootstrapToken(c *gin.Context) error {
537-
expected := os.Getenv(BootstrapTokenEnv)
526+
expected := bootstrapToken
538527
if expected == "" {
539-
return nil
528+
return fmt.Errorf("bootstrap token is not configured")
540529
}
541530

542531
if subtle.ConstantTimeCompare([]byte(c.GetHeader(BootstrapTokenHeader)), []byte(expected)) == 1 {
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package api
2+
3+
import (
4+
"crypto/rand"
5+
"encoding/hex"
6+
"fmt"
7+
"os"
8+
"path/filepath"
9+
"strings"
10+
)
11+
12+
// BootstrapTokenFileEnv names the file the bootstrap token is read from when
13+
// BootstrapTokenEnv is empty.
14+
const BootstrapTokenFileEnv = "MGT_BOOTSTRAP_TOKEN_FILE"
15+
16+
// DefaultBootstrapTokenFile is next to the control-channel token on the
17+
// shared sock volume.
18+
const DefaultBootstrapTokenFile = "/app/sock/tfa_bootstrap_token"
19+
20+
const bootstrapTokenBytes = 32
21+
const bootstrapTokenFileMode os.FileMode = 0600
22+
23+
// bootstrapToken is the value GET /api/OTPUrl must present. It is never empty
24+
// after a successful EnsureBootstrapToken.
25+
var bootstrapToken string
26+
27+
// BootstrapTokenFile returns the path of the persisted bootstrap token.
28+
func BootstrapTokenFile() string {
29+
if path := strings.TrimSpace(os.Getenv(BootstrapTokenFileEnv)); path != "" {
30+
return path
31+
}
32+
return DefaultBootstrapTokenFile
33+
}
34+
35+
// EnsureBootstrapToken loads MGT_BOOTSTRAP_TOKEN or a persisted file, creating
36+
// the file once when neither is set. The console must send the value as
37+
// X-Bootstrap-Token; an empty token is never accepted.
38+
func EnsureBootstrapToken() (created bool, path string, err error) {
39+
if token := strings.TrimSpace(os.Getenv(BootstrapTokenEnv)); token != "" {
40+
bootstrapToken = token
41+
return false, "", nil
42+
}
43+
44+
path = BootstrapTokenFile()
45+
token, created, err := loadOrCreateBootstrapToken(path)
46+
if err != nil {
47+
return false, path, err
48+
}
49+
if token == "" {
50+
return false, path, fmt.Errorf("bootstrap token is empty")
51+
}
52+
bootstrapToken = token
53+
return created, path, nil
54+
}
55+
56+
func loadOrCreateBootstrapToken(path string) (string, bool, error) {
57+
if content, err := os.ReadFile(path); err == nil {
58+
if token := strings.TrimSpace(string(content)); token != "" {
59+
return token, false, nil
60+
}
61+
}
62+
63+
raw := make([]byte, bootstrapTokenBytes)
64+
if _, err := rand.Read(raw); err != nil {
65+
return "", false, err
66+
}
67+
value := hex.EncodeToString(raw)
68+
69+
if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil {
70+
return "", false, err
71+
}
72+
73+
file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, bootstrapTokenFileMode)
74+
if err != nil {
75+
if !os.IsExist(err) {
76+
return "", false, err
77+
}
78+
content, readErr := os.ReadFile(path)
79+
if readErr != nil {
80+
return "", false, readErr
81+
}
82+
if token := strings.TrimSpace(string(content)); token != "" {
83+
return token, false, nil
84+
}
85+
// Empty leftover file: replace it. Do not truncate a non-empty secret.
86+
if err := os.WriteFile(path, []byte(value+"\n"), bootstrapTokenFileMode); err != nil {
87+
return "", false, err
88+
}
89+
return value, true, nil
90+
}
91+
if _, err = file.WriteString(value + "\n"); err != nil {
92+
_ = file.Close()
93+
return "", false, err
94+
}
95+
if err = file.Close(); err != nil {
96+
return "", false, err
97+
}
98+
return value, true, nil
99+
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package api
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
)
9+
10+
func TestEnsureBootstrapTokenCreatesAndReusesFile(t *testing.T) {
11+
dir := t.TempDir()
12+
path := filepath.Join(dir, "tfa_bootstrap_token")
13+
t.Setenv(BootstrapTokenEnv, "")
14+
t.Setenv(BootstrapTokenFileEnv, path)
15+
16+
created, gotPath, err := EnsureBootstrapToken()
17+
if err != nil {
18+
t.Fatal(err)
19+
}
20+
if !created || gotPath != path || bootstrapToken == "" {
21+
t.Fatalf("created=%v path=%q token empty=%v", created, gotPath, bootstrapToken == "")
22+
}
23+
first := bootstrapToken
24+
25+
created, _, err = EnsureBootstrapToken()
26+
if err != nil {
27+
t.Fatal(err)
28+
}
29+
if created {
30+
t.Fatal("second call rotated the token")
31+
}
32+
if bootstrapToken != first {
33+
t.Fatal("token changed")
34+
}
35+
content, err := os.ReadFile(path)
36+
if err != nil {
37+
t.Fatal(err)
38+
}
39+
if string(content) == "" {
40+
t.Fatal("file empty")
41+
}
42+
}
43+
44+
func TestEnsureBootstrapTokenPrefersEnv(t *testing.T) {
45+
t.Setenv(BootstrapTokenEnv, "from-env")
46+
t.Setenv(BootstrapTokenFileEnv, filepath.Join(t.TempDir(), "unused"))
47+
created, path, err := EnsureBootstrapToken()
48+
if err != nil {
49+
t.Fatal(err)
50+
}
51+
if created || path != "" || bootstrapToken != "from-env" {
52+
t.Fatalf("created=%v path=%q token=%q", created, path, bootstrapToken)
53+
}
54+
}
55+
56+
func TestEnsureBootstrapTokenReplacesEmptyFile(t *testing.T) {
57+
path := filepath.Join(t.TempDir(), "tfa_bootstrap_token")
58+
if err := os.WriteFile(path, []byte(" \n"), 0o600); err != nil {
59+
t.Fatal(err)
60+
}
61+
t.Setenv(BootstrapTokenEnv, "")
62+
t.Setenv(BootstrapTokenFileEnv, path)
63+
created, _, err := EnsureBootstrapToken()
64+
if err != nil {
65+
t.Fatal(err)
66+
}
67+
if !created || bootstrapToken == "" {
68+
t.Fatalf("created=%v token empty", created)
69+
}
70+
content, err := os.ReadFile(path)
71+
if err != nil {
72+
t.Fatal(err)
73+
}
74+
if strings.TrimSpace(string(content)) != bootstrapToken {
75+
t.Fatalf("file=%q token=%q", content, bootstrapToken)
76+
}
77+
}

‎management/webserver/api/response/error.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ const (
99
ErrWrongFileType = "wrong-filetype"
1010
ErrReadOnly = "read-only"
1111
ErrTooManyAttempts = "too-many-attempts"
12+
ErrForbidden = "forbidden"
1213
)

‎management/webserver/main.go‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -155,13 +155,16 @@ func main() {
155155
store := cookie.NewStore([]byte(option.Value))
156156
r.Use(sessions.Sessions("session", store))
157157

158-
// The bootstrap token is the only gate in front of the TFA secret of the
159-
// account. It is optional so that a fresh installation can be bound from
160-
// the console, and what limits the exposure when it is empty is the window
161-
// the account may be bound in: announce both.
162-
if os.Getenv(api.BootstrapTokenEnv) == "" {
163-
logger.Warnf("%s is not set: the TFA secret of the account is handed to whoever reaches GET %s%s first, for %s after the bootstrap was opened. Set it to require the %s header on that request.",
164-
api.BootstrapTokenEnv, "/api", api.OTPUrl, api.BootstrapWindow(), api.BootstrapTokenHeader)
158+
created, tokenPath, err := api.EnsureBootstrapToken()
159+
if err != nil {
160+
logger.Fatalln("Failed to initialize the TFA bootstrap token: ", err)
161+
}
162+
if os.Getenv(api.BootstrapTokenEnv) != "" {
163+
logger.Infof("GET /api%s requires the %s header from %s.", api.OTPUrl, api.BootstrapTokenHeader, api.BootstrapTokenEnv)
164+
} else if created {
165+
logger.Infof("Wrote the TFA bootstrap token to %s (mode 0600). GET /api%s requires the %s header. The stock console must send it; until then bind with curl.", tokenPath, api.OTPUrl, api.BootstrapTokenHeader)
166+
} else {
167+
logger.Infof("GET /api%s requires the %s header; token loaded from %s.", api.OTPUrl, api.BootstrapTokenHeader, tokenPath)
165168
}
166169
if window := api.BootstrapWindow(); window <= 0 {
167170
logger.Warnf("%s is off: the account can be bound from the network until somebody logs in. A console that is left unbound stays claimable.",
@@ -172,12 +175,10 @@ func main() {
172175
}
173176

174177
publicRouters := r.Group("/api")
178+
publicRouters.Use(middleware.SameOrigin)
175179
publicRouters.POST(api.Login, api.PostLogin)
176180
publicRouters.POST(api.Logout, api.PostLogout)
177-
// Public on purpose for first-boot QR on the login page. Empty
178-
// MGT_BOOTSTRAP_TOKEN is the CE installer path; GetOTPUrl still
179-
// binds the secret to the first IP, closes after LastLoginTime,
180-
// and honours MGT_BOOTSTRAP_WINDOW. See api.GetOTPUrl.
181+
// First-boot QR still has no session, but requires X-Bootstrap-Token.
181182
publicRouters.GET(api.OTPUrl, api.GetOTPUrl)
182183
publicRouters.GET(api.Version, api.GetVersion)
183184
publicRouters.GET(api.UpgradeTips, api.GetUpgradeTips)
@@ -189,6 +190,7 @@ func main() {
189190
})
190191

191192
limitedRouters := r.Group("/api")
193+
limitedRouters.Use(middleware.SameOrigin)
192194
if envEnabled(os.Getenv("NO_AUTH")) {
193195
logger.Warn("Authentication is disabled because NO_AUTH is enabled")
194196
} else {

0 commit comments

Comments
 (0)