Merge pull request #9 from ripienaar/8

(#8) do not confuse certname and identity

filesec/file_security.go

@@ -30,6 +30,12 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// used by tests to stub out uids etc, should probably be a class and use dependency injection, meh
+var useFakeUID = false
+var fakeUID = 0
+var useFakeOS = false
+var fakeOS = "fake"
+
 // FileSecurity impliments SecurityProvider using files on disk
 type FileSecurity struct {
 	conf *Config
@@ -63,9 +69,6 @@ type Config struct {
 
 	// DisableTLSVerify disables TLS verify in HTTP clients etc
 	DisableTLSVerify bool
-
-	useFakeUID bool
-	fakeUID    int
 }
 
 // Option is a function that can configure the File Security Provider
@@ -81,12 +84,19 @@ func WithChoriaConfig(c *config.Config) Option {
 		DisableTLSVerify: c.DisableTLSVerify,
 		Key:              c.Choria.FileSecurityKey,
 		PrivilegedUsers:  c.Choria.PrivilegedUsers,
+		Identity:         c.Identity,
 	}
 
-	cfg.Identity = c.Identity
+	if cn, ok := os.LookupEnv("MCOLLECTIVE_CERTNAME"); ok {
+		c.OverrideCertname = cn
+	}
 
 	if c.OverrideCertname != "" {
 		cfg.Identity = c.OverrideCertname
+	} else if !(runtimeOs() == "windows" || uid() == 0) {
+		if u, ok := os.LookupEnv("USER"); ok {
+			cfg.Identity = fmt.Sprintf("%s.mcollective", u)
+		}
 	}
 
 	return WithConfig(&cfg)
@@ -131,6 +141,10 @@ func New(opts ...Option) (*FileSecurity, error) {
 		return nil, errors.New("logger not given")
 	}
 
+	if f.conf.Identity == "" {
+		return nil, errors.New("identity could not be determine automatically via Choria or was not supplied")
+	}
+
 	return f, nil
 }
 
@@ -354,27 +368,7 @@ func (s *FileSecurity) CachedPublicData(identity string) ([]byte, error) {
 
 // Identity determines the choria certname
 func (s *FileSecurity) Identity() string {
-	if s.conf.Identity != "" {
-		return s.conf.Identity
-	}
-
-	certname := s.conf.Identity
-
-	if s.uid() != 0 {
-		if u, ok := os.LookupEnv("USER"); ok {
-			certname = fmt.Sprintf("%s.mcollective", u)
-		}
-	}
-
-	return certname
-}
-
-func (s *FileSecurity) uid() int {
-	if s.conf.useFakeUID {
-		return s.conf.fakeUID
-	}
-
-	return os.Geteuid()
+	return s.conf.Identity
 }
 
 func (s *FileSecurity) privilegedCerts() []string {

filesec/file_security_test.go

@@ -31,8 +31,9 @@ func setSSL(c *Config, parent string, id string) {
 	c.PrivilegedUsers = []string{"\\.privileged.mcollective$"}
 	c.DisableTLSVerify = false
 	c.Identity = id
-	c.useFakeUID = true
-	c.fakeUID = 500
+
+	useFakeUID = true
+	fakeUID = 500
 }
 
 var _ = Describe("FileSSL", func() {
@@ -68,6 +69,10 @@ var _ = Describe("FileSSL", func() {
 	})
 
 	Describe("WithChoriaConfig", func() {
+		BeforeEach(func() {
+			os.Unsetenv("MCOLLECTIVE_CERTNAME")
+		})
+
 		It("Should support OverrideCertname", func() {
 			c, err := config.NewDefaultConfig()
 			Expect(err).ToNot(HaveOccurred())
@@ -78,15 +83,27 @@ var _ = Describe("FileSSL", func() {
 			Expect(prov.conf.Identity).To(Equal("override.choria"))
 		})
 
+		It("Should support MCOLLECTIVE_CERTNAME", func() {
+			os.Setenv("MCOLLECTIVE_CERTNAME", "bob.mcollective")
+			c, err := config.NewDefaultConfig()
+			Expect(err).ToNot(HaveOccurred())
+			prov, err := New(WithChoriaConfig(c), WithLog(l.WithFields(logrus.Fields{})))
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(prov.conf.Identity).To(Equal("bob.mcollective"))
+		})
+
 		It("Should copy all the relevant settings", func() {
 			c, err := config.NewDefaultConfig()
 			Expect(err).ToNot(HaveOccurred())
 
+			fakeUID = 0
 			c.Choria.FileSecurityCA = "stub/ca.pem"
 			c.Choria.FileSecurityCache = "stub/cache"
 			c.Choria.FileSecurityCertificate = "stub/cert.pem"
 			c.Choria.FileSecurityKey = "stub/key.pem"
 			c.DisableTLSVerify = true
+			c.Identity = "test.identity"
 
 			prov, err := New(WithChoriaConfig(c), WithLog(l.WithFields(logrus.Fields{})))
 			Expect(err).ToNot(HaveOccurred())
@@ -98,6 +115,47 @@ var _ = Describe("FileSSL", func() {
 			Expect(prov.conf.Certificate).To(Equal("stub/cert.pem"))
 			Expect(prov.conf.Key).To(Equal("stub/key.pem"))
 			Expect(prov.conf.DisableTLSVerify).To(BeTrue())
+			Expect(prov.conf.Identity).To(Equal("test.identity"))
+		})
+
+		It("Should support override certname", func() {
+			c, err := config.NewDefaultConfig()
+			Expect(err).ToNot(HaveOccurred())
+
+			c.Choria.FileSecurityCA = "stub/ca.pem"
+			c.Choria.FileSecurityCache = "stub/cache"
+			c.Choria.FileSecurityCertificate = "stub/cert.pem"
+			c.Choria.FileSecurityKey = "stub/key.pem"
+			c.DisableTLSVerify = true
+			c.Identity = "test.identity"
+			c.OverrideCertname = "bob.identity"
+
+			prov, err := New(WithChoriaConfig(c), WithLog(l.WithFields(logrus.Fields{})))
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(prov.conf.Identity).To(Equal("bob.identity"))
+		})
+
+		It("Should support root and windows", func() {
+			c, err := config.NewDefaultConfig()
+			Expect(err).ToNot(HaveOccurred())
+
+			c.Choria.FileSecurityCA = "stub/ca.pem"
+			c.Choria.FileSecurityCache = "stub/cache"
+			c.Choria.FileSecurityCertificate = "stub/cert.pem"
+			c.Choria.FileSecurityKey = "stub/key.pem"
+			c.DisableTLSVerify = true
+			c.Identity = "test.identity"
+
+			useFakeOS = true
+			defer func() { useFakeOS = false }()
+			fakeOS = "windows"
+			Expect(runtimeOs()).To(Equal("windows"))
+
+			prov, err = New(WithChoriaConfig(c), WithLog(l.WithFields(logrus.Fields{})))
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(prov.conf.Identity).To(Equal("test.identity"))
 		})
 	})
 
@@ -126,23 +184,10 @@ var _ = Describe("FileSSL", func() {
 	})
 
 	Describe("Identity", func() {
-		It("Should support OverrideCertname", func() {
+		It("Should return the identity", func() {
 			cfg.Identity = "bob.choria"
-
 			Expect(prov.Identity()).To(Equal("bob.choria"))
 		})
-
-		It("Should support non root users", func() {
-			cfg.Identity = ""
-			os.Setenv("USER", "bob")
-			Expect(prov.Identity()).To(Equal("bob.mcollective"))
-		})
-
-		It("Should support root users", func() {
-			cfg.fakeUID = 0
-			cfg.Identity = "node.example.net"
-			Expect(prov.Identity()).To(Equal("node.example.net"))
-		})
 	})
 
 	Describe("CallerName", func() {

filesec/util.go

@@ -1,6 +1,10 @@
 package filesec
 
-import "regexp"
+import (
+	"os"
+	"regexp"
+	"runtime"
+)
 
 // MatchAnyRegex checks str against a list of possible regex, if any match true is returned
 func MatchAnyRegex(str []byte, regex []string) bool {
@@ -12,3 +16,19 @@ func MatchAnyRegex(str []byte, regex []string) bool {
 
 	return false
 }
+
+func uid() int {
+	if useFakeUID {
+		return fakeUID
+	}
+
+	return os.Geteuid()
+}
+
+func runtimeOs() string {
+	if useFakeOS {
+		return fakeOS
+	}
+
+	return runtime.GOOS
+}

puppetsec/puppet_security.go

@@ -21,6 +21,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"runtime"
 	"time"
 
 	"github.com/choria-io/go-choria/config"
@@ -88,16 +89,25 @@ func WithChoriaConfig(c *config.Config) Option {
 			SSLDir:           c.Choria.SSLDir,
 			PuppetCAHost:     c.Choria.PuppetCAHost,
 			PuppetCAPort:     c.Choria.PuppetCAPort,
+			Identity:         c.Identity,
 		}
 
 		if c.HasOption("plugin.choria.puppetca_host") || c.HasOption("plugin.choria.puppetca_port") {
 			cfg.DisableSRV = true
 		}
 
-		cfg.Identity = c.Identity
+		if c.OverrideCertname == "" {
+			if cn, ok := os.LookupEnv("MCOLLECTIVE_CERTNAME"); ok {
+				c.OverrideCertname = cn
+			}
+		}
 
 		if c.OverrideCertname != "" {
 			cfg.Identity = c.OverrideCertname
+		} else if !(runtime.GOOS == "windows" || os.Getuid() == 0) {
+			if u, ok := os.LookupEnv("USER"); ok {
+				cfg.Identity = fmt.Sprintf("%s.mcollective", u)
+			}
 		}
 
 		if cfg.SSLDir == "" {
@@ -161,6 +171,10 @@ func New(opts ...Option) (*PuppetSecurity, error) {
 		return nil, errors.New("logger not given")
 	}
 
+	if p.conf.Identity == "" {
+		return nil, errors.New("identity could not be determine automatically via Choria or was not supplied")
+	}
+
 	return p, p.reinit()
 }
 
@@ -175,6 +189,7 @@ func (s *PuppetSecurity) reinit() error {
 		Cache:            s.certCacheDir(),
 		Certificate:      s.publicCertPath(),
 		Key:              s.privateKeyPath(),
+		Identity:         s.conf.Identity,
 	}
 
 	s.fsec, err = filesec.New(filesec.WithConfig(&fc), filesec.WithLog(s.log))
@@ -386,19 +401,7 @@ func (s *PuppetSecurity) PublicCertTXT() ([]byte, error) {
 
 // Identity determines the choria certname
 func (s *PuppetSecurity) Identity() string {
-	if s.conf.Identity != "" {
-		return s.conf.Identity
-	}
-
-	certname := s.conf.Identity
-
-	if s.uid() != 0 {
-		if u, ok := os.LookupEnv("USER"); ok {
-			certname = fmt.Sprintf("%s.mcollective", u)
-		}
-	}
-
-	return certname
+	return s.conf.Identity
 }
 
 func (s *PuppetSecurity) uid() int {

puppetsec/puppet_security_test.go

@@ -153,18 +153,6 @@ var _ = Describe("PuppetSSL", func() {
 
 			Expect(prov.Identity()).To(Equal("bob.choria"))
 		})
-
-		It("Should support non root users", func() {
-			cfg.Identity = ""
-			os.Setenv("USER", "bob")
-			Expect(prov.Identity()).To(Equal("bob.mcollective"))
-		})
-
-		It("Should support root users", func() {
-			cfg.fakeUID = 0
-			cfg.Identity = "node.example.net"
-			Expect(prov.Identity()).To(Equal("node.example.net"))
-		})
 	})
 
 	Describe("cachePath", func() {

puppetsec/util.go

@@ -10,7 +10,7 @@ import (
 )
 
 func userSSlDir() (string, error) {
-	if os.Getuid() == 0 {
+	if os.Geteuid() == 0 {
 		path, err := puppet.Setting("ssldir")
 		if err != nil {
 			return "", err