Skip to content

Commit

Permalink
check if credentials files are protectd on windows
Browse files Browse the repository at this point in the history
we have already been checking if permissions for credentials files are
appropreate or not on linux/macos, but haven't for windows.

windows doesn't support 0600 style permissions but rather it has ACL
mechanism which is slightly complicated.

now we are checking ACL for credential files and able to determine if
the files are protected properly.
  • Loading branch information
Takashi Oguma committed Apr 27, 2017
1 parent 132fdf7 commit 622dea3
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 32 deletions.
1 change: 1 addition & 0 deletions generators/assets/i18n/soracom-api.text.en.json
Original file line number Diff line number Diff line change
Expand Up @@ -2838,6 +2838,7 @@
"summary": "Show, create or update configurations.",
"description": "Show, create or update configurations.",
"profile": {
"permission_is_too_open": "Permissions for the file '%s' which contains your credentials are too open. It is required that your credential files are NOT accessible by others.",
"prompt": "--- SORACOM CLI setup ---\nThis will create a directory %s if it does not exist yet and place '%s.json' in it.",
"coverage_type": {
"prompt": "\n\nPlease select which coverage type to use.\n\n1. Global\n2. Japan\n\n",
Expand Down
1 change: 1 addition & 0 deletions generators/assets/i18n/soracom-api.text.ja.json
Original file line number Diff line number Diff line change
Expand Up @@ -2838,6 +2838,7 @@
"summary": "コマンド実行環境(プロファイル)の設定や表示を行います。",
"description": "コマンド実行環境(プロファイル)の設定や表示を行います。",
"profile": {
"permission_is_too_open": "認証情報ファイル %s へのアクセス権が十分に絞り込まれていません。\n認証情報ファイルへは、soracom コマンドを実行しているユーザーのみがアクセス可能なように設定する必要があります。",
"prompt": "--- SORACOM CLI セットアップ ---\n%s ディレクトリがなければ作成し、そこにファイル '%s.json' を作成します。",
"coverage_type": {
"prompt": "\n\nカバレッジタイプを選択してください。\n\n1. Global\n2. Japan\n\n",
Expand Down
1 change: 1 addition & 0 deletions generators/assets/i18n/soracom-api.text.zh.json
Original file line number Diff line number Diff line change
Expand Up @@ -2838,6 +2838,7 @@
"summary": "Show, create or update configurations.",
"description": "Show, create or update configurations.",
"profile": {
"permission_is_too_open": "Permissions for the file '%s' which contains your credentials are too open. It is required that your credential files are NOT accessible by others.",
"prompt": "--- SORACOM CLI setup ---\nThis will create a directory %s if it does not exist yet and place '%s.json' in it.",
"auth": {
"prompt": "\n\nPlease select which authentication method to use.\n\n1. Input AuthKeyId and AuthKey * Recommended * \n2. Input Operator credentials (Operator Email and Password)\n3. Input SAM credentials (OperatorId, User name and Password)\n\n",
Expand Down
21 changes: 8 additions & 13 deletions generators/cmd/predefined/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/kennygrant/sanitize"
"github.com/mitchellh/go-homedir"
"github.com/soracom/soracom-cli/generators/lib"
"golang.org/x/crypto/ssh/terminal"
)

Expand Down Expand Up @@ -91,18 +92,13 @@ func loadProfile(profileName string) (*profile, error) {

path := filepath.Join(dir, profileName+".json")

// check if permission is 0600
if runtime.GOOS != "windows" {
s, err := os.Stat(path)
if err != nil {
return nil, err
}

if s.Mode()&077 != 0 {
return nil, fmt.Errorf("permission for %s is too open", path)
}
} else {
// TODO: handle ACL on windows env
// check if permission is less than 0600
tooOpen, err := lib.IsFilePermissionTooOpen(path)
if err != nil {
return nil, err
}
if tooOpen {
return nil, fmt.Errorf(TR("configure.cli.profile.permission_is_too_open"), path)
}

b, err := ioutil.ReadFile(path)
Expand Down Expand Up @@ -132,7 +128,6 @@ func saveProfile(profileName string, prof *profile) error {

path := filepath.Join(dir, profileName+".json")

// check if profile dir exists
err = os.MkdirAll(dir, 0700)
if err != nil {
return err
Expand Down
18 changes: 18 additions & 0 deletions generators/lib/fileperm.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// +build !windows

package lib

import "os"

func IsFilePermissionTooOpen(path string) (bool, error) {
s, err := os.Stat(path)
if err != nil {
return false, err
}

if s.Mode()&077 != 0 {
return true, nil
}

return false, nil
}
79 changes: 79 additions & 0 deletions generators/lib/fileperm_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// +build windows

package lib

import (
"fmt"

"github.com/bearmini/go-acl/api"
"golang.org/x/sys/windows"
)

func IsFilePermissionTooOpen(path string) (bool, error) {
var (
ownerSID *windows.SID
dacl *api.ACL
secDesc windows.Handle
)
err := api.GetNamedSecurityInfo(
path,
api.SE_FILE_OBJECT,
api.OWNER_SECURITY_INFORMATION|api.DACL_SECURITY_INFORMATION,
&ownerSID,
nil,
&dacl,
nil,
&secDesc,
)
defer windows.LocalFree(secDesc)
if err != nil {
// This `err` always contains "The operation completed successfully"
// So we create a new error instance
return false, fmt.Errorf("unable to get security info for the file: %s", path)
}

currProcSID, err := GetCurrentProcessSID()
if err != nil {
return false, err
}
//fmt.Println(sidToString(currProcSID))

//fmt.Printf("dacl == %+v\n", dacl)
aces := dacl.GetACEList()
//fmt.Printf("ACEs == %+v\n", aces)
for _, ace := range aces {
switch ace.(type) {
case *api.AccessAllowedACE:
// ok to have this if it's sid == mine
default:
return true, nil
}
//fmt.Println(sidToString(ace.GetSID()))
if !windows.EqualSid(ace.GetSID(), currProcSID) {
return true, nil
}
}
return false, nil
}

func GetCurrentProcessSID() (*windows.SID, error) {
token, err := windows.OpenCurrentProcessToken()
if err != nil {
return nil, err
}
defer token.Close()

tu, err := token.GetTokenUser()
if err != nil {
return nil, err
}
return tu.User.Sid, nil
}

func sidToString(sid *windows.SID) string {
str, err := sid.String()
if err != nil {
return "<err: " + err.Error()
}
return str
}
12 changes: 6 additions & 6 deletions soracom/generated/cmd/i18n_data.go

Large diffs are not rendered by default.

21 changes: 8 additions & 13 deletions soracom/generated/cmd/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/kennygrant/sanitize"
"github.com/mitchellh/go-homedir"
"github.com/soracom/soracom-cli/generators/lib"
"golang.org/x/crypto/ssh/terminal"
)

Expand Down Expand Up @@ -91,18 +92,13 @@ func loadProfile(profileName string) (*profile, error) {

path := filepath.Join(dir, profileName+".json")

// check if permission is 0600
if runtime.GOOS != "windows" {
s, err := os.Stat(path)
if err != nil {
return nil, err
}

if s.Mode()&077 != 0 {
return nil, fmt.Errorf("permission for %s is too open", path)
}
} else {
// TODO: handle ACL on windows env
// check if permission is less than 0600
tooOpen, err := lib.IsFilePermissionTooOpen(path)
if err != nil {
return nil, err
}
if tooOpen {
return nil, fmt.Errorf(TR("configure.cli.profile.permission_is_too_open"), path)
}

b, err := ioutil.ReadFile(path)
Expand Down Expand Up @@ -132,7 +128,6 @@ func saveProfile(profileName string, prof *profile) error {

path := filepath.Join(dir, profileName+".json")

// check if profile dir exists
err = os.MkdirAll(dir, 0700)
if err != nil {
return err
Expand Down

0 comments on commit 622dea3

Please sign in to comment.