Skip to content

Commit

Permalink
Add require-oslogin-certificates logic to disable keys (#368)
Browse files Browse the repository at this point in the history 
* Add require-oslogin-certificates logic to disable keys

* gofmt changes
  • Loading branch information
@bpl4vv
bpl4vv authored Mar 14, 2024
1 parent f54037a commit f7edd55
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 39 deletions.
6 changes: 3 additions & 3 deletions google_guest_agent/non_windows_accounts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ func (a *accountsMgr) Diff(ctx context.Context) (bool, error) {
}
}
// If we've just disabled OS Login.
oldOslogin, _, _ := getOSLoginEnabled(oldMetadata)
newOslogin, _, _ := getOSLoginEnabled(newMetadata)
oldOslogin, _, _, _ := getOSLoginEnabled(oldMetadata)
newOslogin, _, _, _ := getOSLoginEnabled(newMetadata)
if oldOslogin && !newOslogin {
return true, nil
}
Expand All @@ -106,7 +106,7 @@ func (a *accountsMgr) Timeout(ctx context.Context) (bool, error) {

func (a *accountsMgr) Disabled(ctx context.Context) (bool, error) {
config := cfg.Get()
oslogin, _, _ := getOSLoginEnabled(newMetadata)
oslogin, _, _, _ := getOSLoginEnabled(newMetadata)
return false || runtime.GOOS == "windows" || oslogin || !config.Daemons.AccountsDaemon, nil
}

Expand Down
43 changes: 27 additions & 16 deletions google_guest_agent/oslogin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type osloginMgr struct{}

// We also read project keys first, letting instance-level keys take
// precedence.
func getOSLoginEnabled(md *metadata.Descriptor) (bool, bool, bool) {
func getOSLoginEnabled(md *metadata.Descriptor) (bool, bool, bool, bool) {
var enable bool
if md.Project.Attributes.EnableOSLogin != nil {
enable = *md.Project.Attributes.EnableOSLogin
Expand All @@ -73,7 +73,14 @@ func getOSLoginEnabled(md *metadata.Descriptor) (bool, bool, bool) {
if md.Instance.Attributes.SecurityKey != nil {
skey = *md.Instance.Attributes.SecurityKey
}
return enable, twofactor, skey
var reqCerts bool
if md.Project.Attributes.RequireCerts != nil {
reqCerts = *md.Project.Attributes.RequireCerts
}
if md.Instance.Attributes.RequireCerts != nil {
reqCerts = *md.Instance.Attributes.RequireCerts
}
return enable, twofactor, skey, reqCerts
}

func enableDisableOSLoginCertAuth(ctx context.Context) error {
Expand All @@ -83,7 +90,7 @@ func enableDisableOSLoginCertAuth(ctx context.Context) error {
}

eventManager := events.Get()
osLoginEnabled, _, _ := getOSLoginEnabled(newMetadata)
osLoginEnabled, _, _, _ := getOSLoginEnabled(newMetadata)
if osLoginEnabled {
if trustedCAWatcher == nil {
trustedCAWatcher = sshtrustedca.New(sshtrustedca.DefaultPipePath)
Expand All @@ -106,13 +113,14 @@ func enableDisableOSLoginCertAuth(ctx context.Context) error {
}

func (o *osloginMgr) Diff(ctx context.Context) (bool, error) {
oldEnable, oldTwoFactor, oldSkey := getOSLoginEnabled(oldMetadata)
enable, twofactor, skey := getOSLoginEnabled(newMetadata)
oldEnable, oldTwoFactor, oldSkey, oldReqCerts := getOSLoginEnabled(oldMetadata)
enable, twofactor, skey, reqCerts := getOSLoginEnabled(newMetadata)
return oldMetadata.Project.ProjectID == "" ||
// True on first run or if any value has changed.
(oldTwoFactor != twofactor) ||
(oldEnable != enable) ||
(oldSkey != skey), nil
(oldSkey != skey) ||
(oldReqCerts != reqCerts), nil
}

func (o *osloginMgr) Timeout(ctx context.Context) (bool, error) {
Expand All @@ -126,8 +134,8 @@ func (o *osloginMgr) Disabled(ctx context.Context) (bool, error) {
func (o *osloginMgr) Set(ctx context.Context) error {
// We need to know if it was previously enabled for the clearing of
// metadata-based SSH keys.
oldEnable, _, _ := getOSLoginEnabled(oldMetadata)
enable, twofactor, skey := getOSLoginEnabled(newMetadata)
oldEnable, _, _, _ := getOSLoginEnabled(oldMetadata)
enable, twofactor, skey, reqCerts := getOSLoginEnabled(newMetadata)

cleanupDeprecatedDirectives()

Expand All @@ -142,7 +150,7 @@ func (o *osloginMgr) Set(ctx context.Context) error {
logger.Infof("Disabling OS Login")
}

if err := writeSSHConfig(enable, twofactor, skey); err != nil {
if err := writeSSHConfig(enable, twofactor, skey, reqCerts); err != nil {
logger.Errorf("Error updating SSH config: %v.", err)
}

Expand Down Expand Up @@ -279,7 +287,7 @@ func writeConfigFile(path, contents string) error {
return nil
}

func updateSSHConfig(sshConfig string, enable, twofactor, skey bool) string {
func updateSSHConfig(sshConfig string, enable, twofactor, skey, reqCerts bool) string {
// TODO: this feels like a case for a text/template
challengeResponseEnable := "ChallengeResponseAuthentication yes"
authorizedKeysCommand := "AuthorizedKeysCommand /usr/bin/google_authorized_keys"
Expand Down Expand Up @@ -312,12 +320,15 @@ func updateSSHConfig(sshConfig string, enable, twofactor, skey bool) string {
if enable {
osLoginBlock := []string{googleBlockStart}

if cfg.Get().OSLogin.CertAuthentication {
// Metadata overrides the config file.
if reqCerts {
osLoginBlock = append(osLoginBlock, trustedUserCAKeys, authorizedPrincipalsCommand, authorizedPrincipalsUser)
} else {
if cfg.Get().OSLogin.CertAuthentication {
osLoginBlock = append(osLoginBlock, trustedUserCAKeys, authorizedPrincipalsCommand, authorizedPrincipalsUser)
}
osLoginBlock = append(osLoginBlock, authorizedKeysCommand, authorizedKeysUser)
}

osLoginBlock = append(osLoginBlock, authorizedKeysCommand, authorizedKeysUser)

if twofactor {
osLoginBlock = append(osLoginBlock, twoFactorAuthMethods, challengeResponseEnable)
}
Expand All @@ -331,12 +342,12 @@ func updateSSHConfig(sshConfig string, enable, twofactor, skey bool) string {
return strings.Join(filtered, "\n")
}

func writeSSHConfig(enable, twofactor, skey bool) error {
func writeSSHConfig(enable, twofactor, skey, reqCerts bool) error {
sshConfig, err := os.ReadFile("/etc/ssh/sshd_config")
if err != nil {
return err
}
proposed := updateSSHConfig(string(sshConfig), enable, twofactor, skey)
proposed := updateSSHConfig(string(sshConfig), enable, twofactor, skey, reqCerts)
if proposed == string(sshConfig) {
return nil
}
Expand Down
113 changes: 93 additions & 20 deletions google_guest_agent/oslogin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ func TestUpdateSSHConfig(t *testing.T) {
matchblock2 := ` AuthenticationMethods publickey`

var tests = []struct {
contents, want []string
enable, twofactor, skey, cert bool
contents, want []string
enable, twofactor, skey, reqCerts, cfgCert bool
}{
{
// Full block is created, any others removed.
Expand Down Expand Up @@ -227,7 +227,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: true,
skey: false,
cert: true,
reqCerts: false,
cfgCert: true,
},
{
// Full block is created, any others removed.
Expand All @@ -253,7 +254,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: true,
skey: false,
cert: false,
reqCerts: false,
cfgCert: false,
},
{
// Full block is created, google comments removed.
Expand Down Expand Up @@ -283,7 +285,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: true,
skey: false,
cert: true,
reqCerts: false,
cfgCert: true,
},
{
// Full block is created, google comments removed.
Expand All @@ -310,7 +313,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: true,
skey: false,
cert: false,
reqCerts: false,
cfgCert: false,
},
{
// Block is created without two-factor options.
Expand All @@ -332,7 +336,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: false,
skey: false,
cert: true,
reqCerts: false,
cfgCert: true,
},
{
// Block is created without two-factor options.
Expand All @@ -351,7 +356,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: false,
skey: false,
cert: false,
reqCerts: false,
cfgCert: false,
},
{
// Existing block is removed.
Expand All @@ -369,7 +375,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: false,
twofactor: true,
skey: false,
cert: true,
reqCerts: true,
cfgCert: true,
},
{
// Existing block is removed.
Expand All @@ -387,7 +394,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: false,
twofactor: true,
skey: false,
cert: false,
reqCerts: false,
cfgCert: false,
},
{
// Skey binary is chosen instead.
Expand All @@ -412,7 +420,8 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: false,
skey: true,
cert: true,
reqCerts: false,
cfgCert: true,
},
{
// Skey binary is chosen instead.
Expand All @@ -434,7 +443,56 @@ func TestUpdateSSHConfig(t *testing.T) {
enable: true,
twofactor: false,
skey: true,
cert: false,
reqCerts: false,
cfgCert: false,
},
{
// Keys are disabled by metadata.
contents: []string{
"line1",
"line2",
googleBlockStart,
"line3",
googleBlockEnd,
},
want: []string{
googleBlockStart,
trustedUserCAKeys,
authorizedPrincipalsCommand,
authorizedPrincipalsUser,
googleBlockEnd,
"line1",
"line2",
},
enable: true,
twofactor: false,
skey: false,
reqCerts: true,
cfgCert: true,
},
{
// Metadata overrides config.
contents: []string{
"line1",
"line2",
googleBlockStart,
"line3",
googleBlockEnd,
},
want: []string{
googleBlockStart,
trustedUserCAKeys,
authorizedPrincipalsCommand,
authorizedPrincipalsUser,
googleBlockEnd,
"line1",
"line2",
},
enable: true,
twofactor: false,
skey: false,
reqCerts: true,
cfgCert: false,
},
}

Expand All @@ -448,9 +506,9 @@ func TestUpdateSSHConfig(t *testing.T) {
for idx, tt := range tests {
contents := strings.Join(tt.contents, "\n")
want := strings.Join(tt.want, "\n")
config.OSLogin.CertAuthentication = tt.cert
config.OSLogin.CertAuthentication = tt.cfgCert

if res := updateSSHConfig(contents, tt.enable, tt.twofactor, tt.skey); res != want {
if res := updateSSHConfig(contents, tt.enable, tt.twofactor, tt.skey, tt.reqCerts); res != want {
t.Errorf("test %v\nwant:\n%v\ngot:\n%v\n", idx, want, res)
}
}
Expand Down Expand Up @@ -607,55 +665,70 @@ func TestUpdateGroupConf(t *testing.T) {

func TestGetOSLoginEnabled(t *testing.T) {
var tests = []struct {
md string
enable, twofactor, skey bool
md string
enable, twofactor, skey, reqCerts bool
}{
{
md: `{"instance": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true"}}}`,
enable: true,
twofactor: true,
skey: false,
reqCerts: false,
},
{
md: `{"project": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true"}}}`,
md: `{"project": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true", "require-oslogin-certificates": "true"}}}`,
enable: true,
twofactor: true,
skey: false,
reqCerts: true,
},
{
// Instance keys take precedence
md: `{"project": {"attributes": {"enable-oslogin": "false", "enable-oslogin-2fa": "false"}}, "instance": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true"}}}`,
enable: true,
twofactor: true,
skey: false,
reqCerts: false,
},
{
// Instance keys take precedence
md: `{"project": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true"}}, "instance": {"attributes": {"enable-oslogin": "false", "enable-oslogin-2fa": "false"}}}`,
enable: false,
twofactor: false,
skey: false,
reqCerts: false,
},
{
// Handle weird values
md: `{"instance": {"attributes": {"enable-oslogin": "TRUE", "enable-oslogin-2fa": "foobar"}}}`,
enable: true,
twofactor: false,
skey: false,
reqCerts: false,
},
{
// Mixed test
md: `{"project": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true"}}, "instance": {"attributes": {"enable-oslogin-2fa": "false"}}}`,
enable: true,
twofactor: false,
skey: false,
reqCerts: false,
},
{
// Skey test
md: `{"instance": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true", "enable-oslogin-sk": "true"}}}`,
enable: true,
twofactor: true,
skey: true,
reqCerts: false,
},
{
// ReqCerts test
md: `{"instance": {"attributes": {"enable-oslogin": "true", "enable-oslogin-2fa": "true", "require-oslogin-certificates": "true"}}}`,
enable: true,
twofactor: true,
skey: false,
reqCerts: true,
},
}

Expand All @@ -664,9 +737,9 @@ func TestGetOSLoginEnabled(t *testing.T) {
if err := json.Unmarshal([]byte(tt.md), &md); err != nil {
t.Errorf("Failed to unmarshal metadata JSON for test %v: %v", idx, err)
}
enable, twofactor, skey := getOSLoginEnabled(&md)
if enable != tt.enable || twofactor != tt.twofactor || skey != tt.skey {
t.Errorf("Test %v failed. Expected: %v/%v/%v Got: %v/%v/%v", idx, tt.enable, tt.twofactor, tt.skey, enable, twofactor, skey)
enable, twofactor, skey, reqCerts := getOSLoginEnabled(&md)
if enable != tt.enable || twofactor != tt.twofactor || skey != tt.skey || reqCerts != tt.reqCerts {
t.Errorf("Test %v failed. Expected: %v/%v/%v/%v Got: %v/%v/%v/%v", idx, tt.enable, tt.twofactor, tt.skey, tt.reqCerts, enable, twofactor, skey, reqCerts)
}
}
}
Loading

0 comments on commit f7edd55

Please sign in to comment.