From b19b7cfd2eadf3dc7a1ef904756b435f5205a273 Mon Sep 17 00:00:00 2001 From: Dexter Chua Date: Mon, 22 Jun 2020 22:20:37 +0800 Subject: [PATCH] feat: allow modifying unix socket permissions (#1915) This allows the reverse proxy to actually read the unix socket, since - The default permissions are 0755 - Hydra is usually run as a user different than the reverse proxy - One needs read and write permissions to connect to the socket With the commit, one can set the group to be a group that contains the reverse proxy user and permissions to 0770 --- .schema/config.schema.json | 30 ++++++++++++ cmd/server/handler.go | 15 ++++-- docs/docs/production.md | 12 ++++- driver/configuration/helper.go | 53 +++++++++++++++++++++ driver/configuration/provider.go | 2 + driver/configuration/provider_viper.go | 23 +++++++++ driver/configuration/provider_viper_test.go | 36 ++++++++++++++ internal/.hydra.yaml | 9 ++++ internal/config/config.yaml | 8 ++++ 9 files changed, 182 insertions(+), 6 deletions(-) create mode 100644 driver/configuration/helper.go diff --git a/.schema/config.schema.json b/.schema/config.schema.json index e4f20d53143..ce2648968ef 100644 --- a/.schema/config.schema.json +++ b/.schema/config.schema.json @@ -25,6 +25,30 @@ "minimum": 1, "maximum": 65535 }, + "socket": { + "type": "object", + "additionalProperties": false, + "description": "Sets the permissions of the unix socket", + "properties": { + "owner": { + "type": "string", + "description": "Owner of unix socket. If empty, the owner will be the user running hydra.", + "default": "" + }, + "group": { + "type": "string", + "description": "Group of unix socket. If empty, the group will be the primary group of the user running hydra.", + "default": "" + }, + "mode": { + "type": "integer", + "description": "Mode of unix socket in numeric form", + "default": 493, + "minimum": 0, + "maximum": 511 + } + } + }, "cors": { "type": "object", "additionalProperties": false, @@ -246,6 +270,9 @@ "cors": { "$ref": "#/definitions/cors" }, + "socket": { + "$ref": "#/definitions/socket" + }, "access_log": { "type": "object", "additionalProperties": false, @@ -283,6 +310,9 @@ "cors": { "$ref": "#/definitions/cors" }, + "socket": { + "$ref": "#/definitions/socket" + }, "access_log": { "type": "object", "additionalProperties": false, diff --git a/cmd/server/handler.go b/cmd/server/handler.go index a9e1551fa7f..3484161dbed 100644 --- a/cmd/server/handler.go +++ b/cmd/server/handler.go @@ -40,6 +40,7 @@ import ( "github.com/ory/x/reqlog" "github.com/ory/hydra/driver" + "github.com/ory/hydra/driver/configuration" "github.com/ory/hydra/x" "github.com/julienschmidt/httprouter" @@ -111,7 +112,7 @@ func RunServeAdmin(version, build, date string) func(cmd *cobra.Command, args [] go serve(d, cmd, &wg, EnhanceMiddleware(d, adminmw, d.Configuration().AdminListenOn(), admin.Router, d.Configuration().CORSEnabled("admin"), "admin"), - d.Configuration().AdminListenOn(), cert, + d.Configuration().AdminListenOn(), d.Configuration().AdminSocketPermission(), cert, ) wg.Wait() @@ -139,7 +140,7 @@ func RunServePublic(version, build, date string) func(cmd *cobra.Command, args [ go serve(d, cmd, &wg, EnhanceMiddleware(d, publicmw, d.Configuration().PublicListenOn(), public.Router, false, "public"), - d.Configuration().PublicListenOn(), cert, + d.Configuration().PublicListenOn(), d.Configuration().PublicSocketPermission(), cert, ) wg.Wait() @@ -165,12 +166,12 @@ func RunServeAll(version, build, date string) func(cmd *cobra.Command, args []st go serve(d, cmd, &wg, EnhanceMiddleware(d, publicmw, d.Configuration().PublicListenOn(), public.Router, false, "public"), - d.Configuration().PublicListenOn(), cert, + d.Configuration().PublicListenOn(), d.Configuration().PublicSocketPermission(), cert, ) go serve(d, cmd, &wg, EnhanceMiddleware(d, adminmw, d.Configuration().AdminListenOn(), admin.Router, d.Configuration().CORSEnabled("admin"), "admin"), - d.Configuration().AdminListenOn(), cert, + d.Configuration().AdminListenOn(), d.Configuration().AdminSocketPermission(), cert, ) wg.Wait() @@ -285,7 +286,7 @@ func setup(d driver.Driver, cmd *cobra.Command) (admin *x.RouterAdmin, public *x return } -func serve(d driver.Driver, cmd *cobra.Command, wg *sync.WaitGroup, handler http.Handler, address string, cert []tls.Certificate) { +func serve(d driver.Driver, cmd *cobra.Command, wg *sync.WaitGroup, handler http.Handler, address string, permission *configuration.UnixPermission, cert []tls.Certificate) { defer wg.Done() var srv = graceful.WithDefaults(&http.Server{ @@ -309,6 +310,10 @@ func serve(d driver.Driver, cmd *cobra.Command, wg *sync.WaitGroup, handler http if e != nil { return e } + e = permission.SetPermission(addr) + if e != nil { + return e + } err = srv.Serve(unixListener) } else { if !d.Configuration().ServesHTTPS() { diff --git a/docs/docs/production.md b/docs/docs/production.md index 17b85fbc272..311aff3ca54 100644 --- a/docs/docs/production.md +++ b/docs/docs/production.md @@ -114,4 +114,14 @@ like: - `ADMIN_HOST="unix:/var/run/hydra/admin_socket"` ORY Hydra will try to create the socket file during startup and the socket will -be writeable by the user running ORY Hydra. +be writeable by the user running ORY Hydra. The owner, group and mode of the +socket can be modified: +```yaml +serve: + admin: + host: unix:/var/run/hydra/admin_socket + socket: + owner: hydra + group: hydra-admin-api + mode: 770 +``` diff --git a/driver/configuration/helper.go b/driver/configuration/helper.go new file mode 100644 index 00000000000..55c9f519834 --- /dev/null +++ b/driver/configuration/helper.go @@ -0,0 +1,53 @@ +package configuration + +import ( + "os" + "os/user" + "strconv" +) + +type UnixPermission struct { + Owner string + Group string + Mode os.FileMode +} + +func (p *UnixPermission) SetPermission(file string) error { + var e error + e = os.Chmod(file, p.Mode) + if e != nil { + return e + } + + gid := -1 + uid := -1 + + if p.Owner != "" { + var user_obj *user.User + user_obj, e = user.Lookup(p.Owner) + if e != nil { + return e + } + uid, e = strconv.Atoi(user_obj.Uid) + if e != nil { + return e + } + } + if p.Group != "" { + var group *user.Group + group, e := user.LookupGroup(p.Group) + if e != nil { + return e + } + gid, e = strconv.Atoi(group.Gid) + if e != nil { + return e + } + } + + e = os.Chown(file, uid, gid) + if e != nil { + return e + } + return nil +} diff --git a/driver/configuration/provider.go b/driver/configuration/provider.go index 7170ec80ffb..b05dd20d71f 100644 --- a/driver/configuration/provider.go +++ b/driver/configuration/provider.go @@ -42,8 +42,10 @@ type Provider interface { DataSourcePlugin() string DefaultClientScope() []string AdminListenOn() string + AdminSocketPermission() *UnixPermission AdminDisableHealthAccessLog() bool PublicListenOn() string + PublicSocketPermission() *UnixPermission PublicDisableHealthAccessLog() bool CookieSameSiteMode() http.SameSite CookieSameSiteLegacyWorkaround() bool diff --git a/driver/configuration/provider_viper.go b/driver/configuration/provider_viper.go index fc843b0465e..04172640d83 100644 --- a/driver/configuration/provider_viper.go +++ b/driver/configuration/provider_viper.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/url" + "os" "strings" "time" @@ -47,9 +48,15 @@ const ( ViperKeyEncryptSessionData = "oauth2.session.encrypt_at_rest" ViperKeyAdminListenOnHost = "serve.admin.host" ViperKeyAdminListenOnPort = "serve.admin.port" + ViperKeyAdminSocketOwner = "serve.admin.socket.owner" + ViperKeyAdminSocketGroup = "serve.admin.socket.group" + ViperKeyAdminSocketMode = "serve.admin.socket.mode" ViperKeyAdminDisableHealthAccessLog = "serve.admin.access_log.disable_for_health" ViperKeyPublicListenOnHost = "serve.public.host" ViperKeyPublicListenOnPort = "serve.public.port" + ViperKeyPublicSocketOwner = "serve.public.socket.owner" + ViperKeyPublicSocketGroup = "serve.public.socket.group" + ViperKeyPublicSocketMode = "serve.public.socket.mode" ViperKeyPublicDisableHealthAccessLog = "serve.public.access_log.disable_for_health" ViperKeyCookieSameSiteMode = "serve.cookies.same_site_mode" ViperKeyCookieSameSiteLegacyWorkaround = "serve.cookies.same_site_legacy_workaround" @@ -220,6 +227,14 @@ func (v *ViperProvider) publicPort() int { return viperx.GetInt(v.l, ViperKeyPublicListenOnPort, 4444, "PUBLIC_PORT") } +func (v *ViperProvider) PublicSocketPermission() *UnixPermission { + return &UnixPermission{ + Owner: viperx.GetString(v.l, ViperKeyPublicSocketOwner, ""), + Group: viperx.GetString(v.l, ViperKeyPublicSocketGroup, ""), + Mode: os.FileMode(viperx.GetInt(v.l, ViperKeyPublicSocketMode, 0755)), + } +} + func (v *ViperProvider) adminHost() string { return viperx.GetString(v.l, ViperKeyAdminListenOnHost, "", "ADMIN_HOST") } @@ -228,6 +243,14 @@ func (v *ViperProvider) adminPort() int { return viperx.GetInt(v.l, ViperKeyAdminListenOnPort, 4445, "ADMIN_PORT") } +func (v *ViperProvider) AdminSocketPermission() *UnixPermission { + return &UnixPermission{ + Owner: viperx.GetString(v.l, ViperKeyAdminSocketOwner, ""), + Group: viperx.GetString(v.l, ViperKeyAdminSocketGroup, ""), + Mode: os.FileMode(viperx.GetInt(v.l, ViperKeyAdminSocketMode, 0755)), + } +} + func (v *ViperProvider) CookieSameSiteMode() http.SameSite { sameSiteModeStr := viperx.GetString(v.l, ViperKeyCookieSameSiteMode, "default", "COOKIE_SAME_SITE_MODE") switch strings.ToLower(sameSiteModeStr) { diff --git a/driver/configuration/provider_viper_test.go b/driver/configuration/provider_viper_test.go index 50b9a78a2e5..6648bfde299 100644 --- a/driver/configuration/provider_viper_test.go +++ b/driver/configuration/provider_viper_test.go @@ -166,6 +166,20 @@ func TestViperProviderValidates(t *testing.T) { // serve assert.Equal(t, "localhost:1", c.PublicListenOn()) assert.Equal(t, "localhost:2", c.AdminListenOn()) + + expectedPublicPermission := &UnixPermission{ + Owner: "hydra", + Group: "hydra-public-api", + Mode: 0775, + } + expectedAdminPermission := &UnixPermission{ + Owner: "hydra", + Group: "hydra-admin-api", + Mode: 0770, + } + assert.Equal(t, expectedPublicPermission, c.PublicSocketPermission()) + assert.Equal(t, expectedAdminPermission, c.AdminSocketPermission()) + assert.Equal(t, false, c.CORSEnabled("public")) assert.Equal(t, false, c.CORSEnabled("admin")) expectedCors := cors.Options{ @@ -248,3 +262,25 @@ func TestViperProviderValidates(t *testing.T) { ServerURL: "http://zipkin/api/v2/spans", }, c.TracingZipkinConfig()) } + +func TestSetPerm(t *testing.T) { + f, e := ioutil.TempFile("", "test") + require.NoError(t, e) + path := f.Name() + + // We cannot test setting owner and group, because we don't know what the + // tester has access to. + (&UnixPermission{ + Owner: "", + Group: "", + Mode: 0654, + }).SetPermission(path) + + stat, err := f.Stat() + require.NoError(t, err) + + assert.Equal(t, os.FileMode(0654), stat.Mode()) + + require.NoError(t, f.Close()) + require.NoError(t, os.Remove(path)) +} diff --git a/internal/.hydra.yaml b/internal/.hydra.yaml index eae25993874..ad915903965 100644 --- a/internal/.hydra.yaml +++ b/internal/.hydra.yaml @@ -7,6 +7,10 @@ serve: public: port: 1 host: localhost + socket: + owner: hydra + group: hydra-public-api + mode: 0775 cors: enabled: false allowed_origins: @@ -26,6 +30,11 @@ serve: admin: port: 2 host: localhost + socket: + owner: hydra + group: hydra-admin-api + mode: 0770 + cors: enabled: false allowed_origins: diff --git a/internal/config/config.yaml b/internal/config/config.yaml index fad51f1244c..ec6e45ca3c4 100644 --- a/internal/config/config.yaml +++ b/internal/config/config.yaml @@ -91,6 +91,10 @@ serve: # Leave empty to listen on all interfaces. host: localhost # leave this out or empty to listen on all devices which is the default # host: unix:/path/to/socket + # socket: + # owner: hydra + # group: hydra + # mode: 0775 # cors configures Cross Origin Resource Sharing for public endpoints. cors: @@ -147,6 +151,10 @@ serve: # Leave empty to listen on all interfaces. host: localhost # leave this out or empty to listen on all devices which is the default # host: unix:/path/to/socket + # socket: + # owner: hydra + # group: hydra + # mode: 0775 # cors configures Cross Origin Resource Sharing for admin endpoints. cors: