Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to bind to a Unix socket instead of a TCP port #703

Merged
merged 6 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ name: build
on: [push, pull_request]
jobs:
build:
env:
# The special value "local" tells Go to use the bundled Go
# version rather than trying to fetch one according to a
# `toolchain` value in `go.mod`. This ensures that we're
# really running the Go version in the CI matrix rather than
# one that the Go command has upgraded to automatically.
GOTOOLCHAIN: local
strategy:
matrix:
go-version: [1.21.x, 1.22.x]
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ In either case, the given file part will be parsed as JSON and added to the `pay

TLS version and cipher suite selection flags are available from the command line. To list available cipher suites, use the `-list-cipher-suites` flag. The `-tls-min-version` flag can be used with `-list-cipher-suites`.

## Running behind a reverse proxy
[webhook][w] may be run behind a "reverse proxy" - another web-facing server such as [Apache httpd](https://httpd.apache.org) or [Nginx](https://nginx.org) that accepts requests from clients and forwards them on to [webhook][h]. You can have [webhook][w] listen on a regular TCP port or on a Unix domain socket (with the `-socket` flag), then configure your proxy to send requests for a specific host name or sub-path over that port or socket to [webhook][w].

Note that when running in this mode the [`ip-whitelist`](docs/Hook-Rules.md#match-whitelisted-ip-range) trigger rule will not work as expected, since it will be checking the address of the _proxy_, not the _client_. Client IP restrictions will need to be enforced within the proxy, before it decides whether to forward the request to [webhook][w].

## CORS Headers
If you want to set CORS headers, you can use the `-header name=value` flag while starting [webhook][w] to set the appropriate CORS headers that will be returned with each response.

Expand Down
2 changes: 2 additions & 0 deletions docs/Hook-Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ The IP can be IPv4- or IPv6-formatted, using [CIDR notation](https://en.wikipedi
}
```

Note this does not work if webhook is running behind a reverse proxy, as the "client IP" will either not be available at all (if webhook is using a Unix socket or named pipe) or it will be the address of the _proxy_, not of the real client. You will probably need to enforce client IP restrictions in the reverse proxy itself, before forwarding the requests to webhook.

### Match scalr-signature

The trigger rule checks the scalr signature and also checks that the request was signed less than 5 minutes before it was received.
Expand Down
6 changes: 5 additions & 1 deletion docs/Webhook-Parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Usage of webhook:
-hotreload
watch hooks file for changes and reload them automatically
-http-methods string
globally restrict allowed HTTP methods; separate methods with comma
set default allowed HTTP methods (ie. "POST"); separate methods with comma
Comment on lines 16 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't something I've changed, just something where the snapshot of the -help message in this file had drifted out of date with respect to the code.

-ip string
ip the webhook should serve hooks on (default "0.0.0.0")
-key string
Expand All @@ -23,6 +23,8 @@ Usage of webhook:
list available TLS cipher suites
-logfile string
send log output to a file; implicitly enables verbose logging
-max-multipart-mem int
maximum memory in bytes for parsing multipart form data before disk caching (default 1048576)
-nopanic
do not panic if hooks cannot be loaded when webhook is not running in verbose mode
-pidfile string
Expand All @@ -35,6 +37,8 @@ Usage of webhook:
set group ID after opening listening port; must be used with setuid
-setuid int
set user ID after opening listening port; must be used with setgid
-socket string
path to a Unix socket (e.g. /tmp/webhook.sock) or Windows named pipe (e.g. \\.\pipe\webhook) to use instead of listening on an ip and port; if specified, the ip and port options are ignored
Comment on lines +40 to +41
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual -help message is different on different platforms, I manually merged the two for the documentation.

-template
parse hooks file as a Go template
-tls-min-version string
Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
module github.com/adnanh/webhook

go 1.17
go 1.21

toolchain go1.22.0

require (
github.com/Microsoft/go-winio v0.6.2
github.com/clbanning/mxj/v2 v2.7.0
github.com/dustin/go-humanize v1.0.1
github.com/fsnotify/fsnotify v1.7.0
Expand Down
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
github.com/clbanning/mxj/v2 v2.7.0 h1:WA/La7UGCanFe5NpHF0Q3DNtnCsVoxbPKuyBNHWRyME=
github.com/clbanning/mxj/v2 v2.7.0/go.mod h1:hNiWqW14h+kc+MdF9C6/YoRfjEJoR3ou6tn/Qo+ve2s=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
Expand All @@ -19,7 +21,6 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
22 changes: 22 additions & 0 deletions platform_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//go:build !windows
// +build !windows

package main

import (
"flag"
"fmt"
"net"
)

func platformFlags() {
flag.StringVar(&socket, "socket", "", "path to a Unix socket (e.g. /tmp/webhook.sock) to use instead of listening on an ip and port; if specified, the ip and port options are ignored")
}

func trySocketListener() (net.Listener, error) {
if socket != "" {
addr = fmt.Sprintf("{unix:%s}", socket)
return net.Listen("unix", socket)
}
return nil, nil
}
23 changes: 23 additions & 0 deletions platform_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//go:build windows
// +build windows

package main

import (
"flag"
"fmt"
"github.com/Microsoft/go-winio"
"net"
)

func platformFlags() {
flag.StringVar(&socket, "socket", "", "path to a Windows named pipe (e.g. \\\\.\\pipe\\webhook) to use instead of listening on an ip and port; if specified, the ip and port options are ignored")
}

func trySocketListener() (net.Listener, error) {
if socket != "" {
addr = fmt.Sprintf("{pipe:%s}", socket)
return winio.ListenPipe(socket, nil)
}
return nil, nil
}
11 changes: 11 additions & 0 deletions signals.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build !windows
// +build !windows

package main
Expand All @@ -6,6 +7,7 @@ import (
"log"
"os"
"os/signal"
"strings"
"syscall"
)

Expand Down Expand Up @@ -43,6 +45,15 @@ func watchForSignals() {
log.Print(err)
}
}
if socket != "" && !strings.HasPrefix(socket, "@") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you give a socket path starting with @ to net.Listen("unix", path) then it is treated as an "abstract" socket, a separate namespace that doesn't have a direct representation as an object in the filesystem, in which case there's no socket file to remove.

// we've been listening on a named Unix socket, delete it
// before we exit so subsequent runs can re-bind the same
// socket path
err := os.Remove(socket)
Comment on lines +49 to +52
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will almost certainly error out in combination with -setuid, since the socket is created before dropping privileges. But you don't need to be root to bind a Unix socket (as long as you have write access to the parent directory) so this situation should rarely arise.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should do it after dropping the privileges then?

Copy link
Contributor Author

@ianroberts ianroberts Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did wonder about either delaying opening the socket until after setuid or just forbidding the use of both -socket and -setuid at the same time. Like I say the only valid use case for combining -socket with -setuid would be if you wanted to bind to a socket file in a directory to which the target uid doesn't have write access, in which case moving the bind to happen after privileges have been dropped will fail the same way as this cleanup would in the current design.

My gut feeling is that the cleanest option is to make it an error if both -socket and -setuid are set. If you need to run webhook as a regular unprivileged user but listen on a socket as root, then I have another PR ready to go that stacks on top of this one that would make webhook compatible with systemd socket activation - that would mean that systemd itself manages the socket (as root, meaning it can be a Unix socket in a locked down directory or a TCP socket on a privileged port) and it can spawn webhook as an unprivileged user with the socket already open.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok, I was assuming if you agreed with my analysis then I’d implement that change before you merged… I’ll prepare the systemd PR and roll this fix into that.

if err != nil {
log.Printf("Failed to remove socket file %s: %v", socket, err)
}
}
os.Exit(0)

default:
Expand Down
30 changes: 30 additions & 0 deletions testutils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//go:build !windows
// +build !windows

package main

import (
"context"
"io/ioutil"
"net"
"net/http"
"os"
"path"
)

func prepareTestSocket(_ string) (socketPath string, transport *http.Transport, cleanup func(), err error) {
tmp, err := ioutil.TempDir("", "webhook-socket-")
if err != nil {
return "", nil, nil, err
}
cleanup = func() { os.RemoveAll(tmp) }
socketPath = path.Join(tmp, "webhook.sock")
socketDialer := &net.Dialer{}
transport = &http.Transport{
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
return socketDialer.DialContext(ctx, "unix", socketPath)
},
}

return socketPath, transport, cleanup, nil
}
22 changes: 22 additions & 0 deletions testutils_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//go:build windows
// +build windows

package main

import (
"context"
"github.com/Microsoft/go-winio"
"net"
"net/http"
)

func prepareTestSocket(hookTmpl string) (socketPath string, transport *http.Transport, cleanup func(), err error) {
socketPath = "\\\\.\\pipe\\webhook-" + hookTmpl
transport = &http.Transport{
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
return winio.DialPipeContext(ctx, socketPath)
},
}

return socketPath, transport, nil, nil
}
1 change: 1 addition & 0 deletions vendor/github.com/Microsoft/go-winio/.gitattributes

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions vendor/github.com/Microsoft/go-winio/.gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

147 changes: 147 additions & 0 deletions vendor/github.com/Microsoft/go-winio/.golangci.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions vendor/github.com/Microsoft/go-winio/CODEOWNERS

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading