diff --git a/.gitignore b/.gitignore index d865eba..ed0320f 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ /_docs/ /_test/ node_modules/ + +*.so diff --git a/cmd/git-bundle-server/web-server.go b/cmd/git-bundle-server/web-server.go index 2683d26..4531ab7 100644 --- a/cmd/git-bundle-server/web-server.go +++ b/cmd/git-bundle-server/web-server.go @@ -77,7 +77,11 @@ func (w *webServerCmd) startServer(ctx context.Context, args []string) error { parser.Visit(func(f *flag.Flag) { if webServerFlags.Lookup(f.Name) != nil { value := f.Value.String() - if f.Name == "cert" || f.Name == "key" || f.Name == "client-ca" { + if f.Name == "cert" || + f.Name == "key" || + f.Name == "client-ca" || + f.Name == "auth-config" { + // Need the absolute value of the path value, err = filepath.Abs(value) if err != nil { diff --git a/cmd/git-bundle-web-server/bundle-server.go b/cmd/git-bundle-web-server/bundle-server.go index 0851aa1..8e03eed 100644 --- a/cmd/git-bundle-web-server/bundle-server.go +++ b/cmd/git-bundle-web-server/bundle-server.go @@ -20,13 +20,17 @@ import ( "github.com/git-ecosystem/git-bundle-server/internal/core" "github.com/git-ecosystem/git-bundle-server/internal/git" "github.com/git-ecosystem/git-bundle-server/internal/log" + "github.com/git-ecosystem/git-bundle-server/pkg/auth" ) +type authFunc func(*http.Request, string, string) auth.AuthResult + type bundleWebServer struct { logger log.TraceLogger server *http.Server serverWaitGroup *sync.WaitGroup listenAndServeFunc func() error + authorize authFunc } func NewBundleWebServer(logger log.TraceLogger, @@ -34,10 +38,12 @@ func NewBundleWebServer(logger log.TraceLogger, certFile string, keyFile string, tlsMinVersion uint16, clientCAFile string, + middlewareAuthorize authFunc, ) (*bundleWebServer, error) { bundleServer := &bundleWebServer{ logger: logger, serverWaitGroup: &sync.WaitGroup{}, + authorize: middlewareAuthorize, } // Configure the http.Server @@ -107,6 +113,13 @@ func (b *bundleWebServer) serve(w http.ResponseWriter, r *http.Request) { route := owner + "/" + repo + if b.authorize != nil { + authResult := b.authorize(r, owner, repo) + if authResult.ApplyResult(w) { + return + } + } + userProvider := common.NewUserProvider() fileSystem := common.NewFileSystem() commandExecutor := cmd.NewCommandExecutor(b.logger) @@ -172,6 +185,16 @@ func (b *bundleWebServer) StartServerAsync(ctx context.Context) { } }(ctx) + // Wait 0.1s before reporting that the server is started in case + // 'listenAndServeFunc' exits immediately. + // + // It's a hack, but a necessary one because 'ListenAndServe[TLS]()' doesn't + // have any mechanism of notifying if it starts successfully, only that it + // fails. We could get around that by copying/reimplementing those functions + // with a print statement inserted at the right place, but that's way more + // cumbersome than just adding a delay here (see: + // https://stackoverflow.com/questions/53332667/how-to-notify-when-http-server-starts-successfully). + time.Sleep(time.Millisecond * 100) fmt.Println("Server is running at address " + b.server.Addr) } diff --git a/cmd/git-bundle-web-server/main.go b/cmd/git-bundle-web-server/main.go index feb1255..19b621e 100644 --- a/cmd/git-bundle-web-server/main.go +++ b/cmd/git-bundle-web-server/main.go @@ -1,16 +1,122 @@ package main import ( + "bytes" "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" "flag" "fmt" + "hash" + "io" "os" + "plugin" + "strings" "github.com/git-ecosystem/git-bundle-server/cmd/utils" "github.com/git-ecosystem/git-bundle-server/internal/argparse" + auth_internal "github.com/git-ecosystem/git-bundle-server/internal/auth" "github.com/git-ecosystem/git-bundle-server/internal/log" + "github.com/git-ecosystem/git-bundle-server/pkg/auth" ) +func getPluginChecksum(pluginPath string) (hash.Hash, error) { + file, err := os.Open(pluginPath) + if err != nil { + return nil, err + } + defer file.Close() + + checksum := sha256.New() + if _, err := io.Copy(checksum, file); err != nil { + return nil, err + } + + return checksum, nil +} + +func parseAuthConfig(configPath string) (auth.AuthMiddleware, error) { + var config authConfig + fileBytes, err := os.ReadFile(configPath) + if err != nil { + return nil, err + } + + err = json.Unmarshal(fileBytes, &config) + if err != nil { + return nil, err + } + + switch strings.ToLower(config.AuthMode) { + case "fixed": + return auth_internal.NewFixedCredentialAuth(config.Parameters) + case "plugin": + if len(config.Path) == 0 { + return nil, fmt.Errorf("plugin .so is empty") + } + if len(config.Initializer) == 0 { + return nil, fmt.Errorf("plugin initializer symbol is empty") + } + if len(config.Checksum) == 0 { + return nil, fmt.Errorf("SHA256 checksum of plugin file is empty") + } + + // First, verify plugin checksum matches expected + // Note: time-of-check/time-of-use could be exploited here (anywhere + // between the checksum check and invoking the initializer). There's not + // much we can realistically do about that short of rewriting the plugin + // package, so we advise users to carefully control access to their + // system & limit write permissions on their plugin files as a + // mitigation (see docs/technical/auth-config.md). + expectedChecksum, err := hex.DecodeString(config.Checksum) + if err != nil { + return nil, fmt.Errorf("plugin checksum is invalid: %w", err) + } + checksum, err := getPluginChecksum(config.Path) + if err != nil { + return nil, fmt.Errorf("could not calculate plugin checksum: %w", err) + } + + if !bytes.Equal(expectedChecksum, checksum.Sum(nil)) { + return nil, fmt.Errorf("specified hash does not match plugin checksum") + } + + // Load the plugin and find the initializer function + p, err := plugin.Open(config.Path) + if err != nil { + return nil, fmt.Errorf("could not load auth plugin: %w", err) + } + + rawInit, err := p.Lookup(config.Initializer) + if err != nil { + return nil, fmt.Errorf("failed to load initializer: %w", err) + } + + initializer, ok := rawInit.(func(json.RawMessage) (auth.AuthMiddleware, error)) + if !ok { + return nil, fmt.Errorf("initializer function has incorrect signature") + } + + // Call the initializer + return initializer(config.Parameters) + default: + return nil, fmt.Errorf("unrecognized auth mode '%s'", config.AuthMode) + } +} + +type authConfig struct { + AuthMode string `json:"mode"` + + // Plugin-specific settings + Path string `json:"path,omitempty"` + Initializer string `json:"initializer,omitempty"` + Checksum string `json:"sha256,omitempty"` + + // Per-middleware custom config + Parameters json.RawMessage `json:"parameters,omitempty"` +} + func main() { log.WithTraceLogger(context.Background(), func(ctx context.Context, logger log.TraceLogger) { parser := argparse.NewArgParser(logger, "git-bundle-web-server [--port ] [--cert --key ]") @@ -28,6 +134,27 @@ func main() { key := utils.GetFlagValue[string](parser, "key") tlsMinVersion := utils.GetFlagValue[uint16](parser, "tls-version") clientCA := utils.GetFlagValue[string](parser, "client-ca") + authConfig := utils.GetFlagValue[string](parser, "auth-config") + + // Configure auth + var err error + middlewareAuthorize := authFunc(nil) + if authConfig != "" { + middleware, err := parseAuthConfig(authConfig) + if err != nil { + logger.Fatalf(ctx, "Invalid auth config: %w", err) + } + if middleware == nil { + // Up until this point, everything indicates that a user intends + // to use - and has properly configured - custom auth. However, + // despite there being no error from the initializer, the + // middleware was empty. This is almost certainly incorrect, so + // we exit. + logger.Fatalf(ctx, "Middleware is nil, but no error was returned from initializer. "+ + "If no middleware is desired, remove the --auth-config option.") + } + middlewareAuthorize = middleware.Authorize + } // Configure the server bundleServer, err := NewBundleWebServer(logger, @@ -35,6 +162,7 @@ func main() { cert, key, tlsMinVersion, clientCA, + middlewareAuthorize, ) if err != nil { logger.Fatal(ctx, err) diff --git a/cmd/utils/common-args.go b/cmd/utils/common-args.go index 863d00d..8848ff1 100644 --- a/cmd/utils/common-args.go +++ b/cmd/utils/common-args.go @@ -86,6 +86,7 @@ func WebServerFlags(parser argParser) (*flag.FlagSet, func(context.Context)) { tlsVersion := tlsVersionValue(tls.VersionTLS12) f.Var(&tlsVersion, "tls-version", "The minimum TLS version the server will accept") f.String("client-ca", "", "The path to the client authentication certificate authority PEM") + f.String("auth-config", "", "File containing the configuration for server auth middleware") // Function to call for additional arg validation (may exit with 'Usage()') validationFunc := func(ctx context.Context) { diff --git a/docs/man/git-bundle-web-server.adoc b/docs/man/git-bundle-web-server.adoc index 4610d6a..8174336 100644 --- a/docs/man/git-bundle-web-server.adoc +++ b/docs/man/git-bundle-web-server.adoc @@ -27,6 +27,93 @@ web-server* for managing the web server process on their systems. include::server-options.asc[] +== CONFIGURING AUTH + +The *--auth-config* option configures authentication middleware for the server, +either using a built-in mode or with a custom plugin. The auth config specified +by that option is a JSON file that identifies the type of access control +requested and information needed to configure it. + +=== Schema + +The auth config JSON contains the following fields: + +*mode* (string):: + The auth mode to use. Not case-sensitive. ++ +Available options: + + - _fixed_ + +*parameters* (object):: + A structure containing mode-specific key-value configuration fields, if + applicable. May be optional, depending on *mode*. + +*path* (string) - *plugin*-only:: + The absolute path to the auth plugin .so file. + +*initializer* (string) - *plugin*-only:: + The name of the symbol within the plugin binary that can invoked to create an + 'AuthMiddleware' instance. The initializer: + + - Must have the signature 'func(json.RawMessage) (AuthMiddleware, error)'. + - Must be exported in its package (i.e., UpperCamelCase name). + +*sha256* (string) - *plugin*-only:: + The SHA256 checksum of the plugin .so file, rendered as a hex string. If the + checksum does not match the calculated checksum of the plugin file, the web + server will refuse to start. ++ +The checksum can be determined using man:shasum[1]: ++ +[source,console] +---- +$ shasum -a 256 /path/to/your/plugin.so +---- + +=== Examples + +The following examples demonstrate typical usage of built-in and plugin modes. + +*** + +Static, server-wide username & password ("admin" & "bundle_server", +respectively): + +[source,json] +---- +{ + "mode": "fixed", + "parameters": { + "username": "admin", + "passwordHash": "c3c3520adf2f6e25672ba55dc70bcb3dd8b4ef3341bce1a5f38c5eca6571f372" + } +} +---- + +*** + +A custom auth plugin implementation: + + - The path to the Go plugin file is '/path/to/plugin.so' + - The file contains the symbol + 'func NewSimplePluginAuth(rawParams json.RawMessage) (AuthMiddleware, error)' + - The initializer ignores 'rawParams' + - The SHA256 checksum of '/path/to/plugin.so' is + '49db14bb838417a0292e293d0a6e90e82ed26fccb0d78670827c8c8516d2cca6' + +[source,json] +---- +{ + "mode": "plugin", + "path": "/path/to/plugin.so", + "initializer": "NewSimplePluginAuth", + "sha256": "49db14bb838417a0292e293d0a6e90e82ed26fccb0d78670827c8c8516d2cca6" +} +---- + +*** + == SEE ALSO man:git-bundle-server[1], man:git-bundle[1], man:git-fetch[1] diff --git a/docs/man/server-options.asc b/docs/man/server-options.asc index a586831..e94a087 100644 --- a/docs/man/server-options.asc +++ b/docs/man/server-options.asc @@ -29,3 +29,7 @@ configured for TLS, this option is a no-op. Require that requests to the bundle server include a client certificate that can be validated by the certificate authority file at the specified _path_. No-op if *--cert* and *--key* are not configured. + +*--auth-config* _path_::: + Use the JSON contents of the specified file to configure + authentication/authorization for requests to the web server. diff --git a/docs/technical/auth-config.md b/docs/technical/auth-config.md new file mode 100644 index 0000000..1edcf7d --- /dev/null +++ b/docs/technical/auth-config.md @@ -0,0 +1,264 @@ +# Configuring access control + +User access to web server endpoints on the bundle server is configured via the +`--auth-config` option to `git-bundle-web-server` and/or `git-bundle-server +web-server`. The auth config is a JSON file that identifies the type of access +control requested and information needed to configure it. + +## Schema + +The JSON file contains the following fields: + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ FieldTypeDescription
Common fieldsmodestring +

The auth mode to use. Not case-sensitive.

+ Available options: +
    +
  • fixed
    • +
  • plugin
    • +
+
parameters (optional; depends on mode)object + A structure containing mode-specific key-value configuration + fields, if applicable. +
plugin-onlypathstring + The absolute path to the auth plugin .so file. +
initializerstring + The name of the symbol within the plugin binary that can invoked + to create the AuthMiddleware. The initializer: +
    +
  • + Must have the signature + func(json.RawMessage) (AuthMiddleware, error). +
    • +
  • + Must be exported in its package (i.e., + UpperCamelCase name). +
    • +
+ See Plugin mode for more details. +
sha256string + The SHA256 checksum of the plugin .so file, + rendered as a hex string. If the checksum does not match the + calculated checksum of the plugin file, the web server will + refuse to start. +
+ +## Built-in modes + +### Fixed/single-user auth (server-wide) + +**Mode: `fixed`** + +This mode implements [Basic authentication][basic-rfc], authenticating each +request against a fixed username/password pair that is global to the web server. + +[basic-rfc]: https://datatracker.ietf.org/doc/html/rfc7617 + +#### Parameters + +The `parameters` object _must_ be specified for this mode, and both of the +fields below are required. + + + + + + + + + + + + + + + + + + + + + +
FieldTypeDescription
usernamestring + The username string for authentication. The username must + not contain a colon (":"). +
passwordHashstring +

+ The SHA256 hash of the password string. There are no + restrictions on characters used for the password. +

+

+ The hash of a string can be generated on the command line + with the command + echo -n '<your string>' | shasum -a 256. +

+
+ +#### Examples + +Valid (username `admin`, password `test`): + +```json +{ + "mode": "fixed", + "parameters": { + "username": "admin", + "passwordHash": "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" + } +} +``` + +Valid (empty username & password): + +```json +{ + "mode": "fixed", + "parameters": { + "usernameHash": "", + "passwordHash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + } +} +``` + +Invalid: + +```json +{ + "mode": "fixed", + "parameters": { + "username": "admin", + "passwordHash": "test123" + } +} +``` + +Invalid: + +```json +{ + "mode": "fixed", + "parameters": { + "username": "admin:MY_PASSWORD", + } +} +``` + +## Plugin mode + +**Mode: `plugin`** + +Plugin mode allows users to develop their custom auth middleware to serve a more +specific platform or need than the built-in modes (e.g., host-based federated +access). The bundle server makes use of Go's [`plugin`][plugin] package to load +the plugin and create an instance of the specified middleware. + +### The plugin + +The plugin is a `.so` shared library built using `go build`'s +`-buildmode=plugin` option. The custom auth middleware must implement the +`AuthMiddleware` interface defined in the exported `auth` package of this +repository. Additionally, the plugin must contain an initializer function that +creates and returns the custom `AuthMiddleware` interface. The function +signature of this initializer is: + +```go +func (json.RawMessage) (AuthMiddleware, error) +``` + +- The `json.RawMessage` input is the raw bytes of the `parameters` object (empty + if `parameters` is not in the auth config JSON). +- The `AuthMiddleware` is an instance of the plugin's custom `AuthMiddleware` + implementation. If this is `nil` and `error` is not `nil`, the web server will + fail to start. +- If the `AuthMiddleware` cannot be initialized, the `error` captures the + context of the failure. If `error` is not `nil`, the web server will fail to + start. + +> **Note** +> +> While this project is in a pre-release/alpha state, the `AuthMiddleware` +> and initializer interfaces may change, breaking older plugins. + +After the `AuthMiddleware` is loaded, its `Authorize()` function will be called +for each valid route request. The `AuthResult` returned must be created with one +of `Allow()` or `Deny()`; an accepted request will continue on to the logic for +serving bundle server content, a rejected one will return immediately with the +specified code and headers. + +Note that these requests may be processed in parallel, therefore **it is up to +the developer of the plugin to ensure their middleware's `Authorize()` function +is thread-safe**! Failure to do so could create race conditions and lead to +unexpected behavior. + +### The config + +When using `plugin` mode in the auth config, there are a few additional fields +that must be specified that are not required for built-in modes: `path`, +`initializer`, `sha256`. + +There are multiple ways to determine the SHA256 checksum of a file, but an +easy way to do so on the command line is: + +```bash +shasum -a 256 path/to/your/plugin.so +``` + +> **Warning** +> +> In the current plugin-loading implementation, the SHA256 checksum of the +> specified plugin is calculated and compared before loading its symbols. This +> opens up the possibility of a [time-of-check/time-of-use][toctou] attack +> wherein a malicious actor replaces a valid plugin file with their own plugin +> _after_ the checksum verification of the "good" file but before the plugin is +> loaded into memory. +> +> To mitigate this risk, ensure 'write' permissions are disabled on your plugin +> file. And, as always, practice caution when running third party code that +> interacts with credentials and other sensitive information. + +[plugin]: https://pkg.go.dev/plugin +[toctou]: https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use + +### Examples + +An example plugin and corresponding config can be found in the +[`examples/auth`][examples-dir] directory of this repository. + +[examples-dir]: ../../examples/auth diff --git a/examples/auth/README.md b/examples/auth/README.md new file mode 100644 index 0000000..a47b080 --- /dev/null +++ b/examples/auth/README.md @@ -0,0 +1,54 @@ +# Auth configuration examples + +This directory contains examples of auth configurations that may be used as a +reference for setting up auth for a bundle server. + +> **Warning** +> +> The examples contained within this directory should not be used directly in a +> production context due to publicly-visible (in this repo) credentials. + +## Built-in modes + +### Fixed credential/single-user auth + +The file [`config/fixed.json`][fixed-config] configures [Basic +authentication][basic] with username "admin" and password "bundle_server". + +[fixed-config]: ./config/fixed.json +[basic]: ../../docs/technical/auth-config.md#basic-auth-server-wide + +## Plugin mode + +The example plugin implemented in [`_plugins/simple-plugin.go`][simple-plugin] +can be built (from this directory) with: + +```bash +go build -buildmode=plugin -o ./plugins/ ./_plugins/simple-plugin.go +``` + +which will create `simple-plugin.so` - this is your plugin file. + +To use this plugin with `git-bundle-web-server`, the config in +[`config/plugin.json`][plugin-config] needs to be updated with the SHA256 +checksum of the plugin. This value can be determined by running (from this +directory): + +```bash +shasum -a 256 ./_plugins/simple-plugin.so +``` + +The configured `simple-plugin.so` auth middleware implements Basic +authentication with a hardcoded username "admin" and a password that is based on +the requested route (if the requested route is `test/repo` or +`test/repo/bundle-123456.bundle`, the password is "test_repo"). + +> **Note** +> +> The example `plugin.json` contains a relative, rather than absolute, path to +> the plugin file, relative to the root of this repository. This is meant to +> facilitate more portable testing and is _not_ recommended for typical use; +> please use an absolute path to identify your plugin file. + +[simple-plugin]: ./_plugins/simple-plugin.go +[plugin-config]: ./config/plugin.json diff --git a/examples/auth/_plugins/simple-plugin.go b/examples/auth/_plugins/simple-plugin.go new file mode 100644 index 0000000..b7120e6 --- /dev/null +++ b/examples/auth/_plugins/simple-plugin.go @@ -0,0 +1,45 @@ +package main + +import ( + "crypto/sha256" + "crypto/subtle" + "encoding/json" + "net/http" + + "github.com/git-ecosystem/git-bundle-server/pkg/auth" +) + +type simplePluginAuth struct { + usernameHash [32]byte +} + +// Example auth plugin: basic auth with username "admin" and password +// "{owner}_{repo}" (based on the owner & repo from the route). +// DO NOT USE THIS IN A PRODUCTION BUNDLE SERVER. +func NewSimplePluginAuth(_ json.RawMessage) (auth.AuthMiddleware, error) { + return &simplePluginAuth{ + usernameHash: sha256.Sum256([]byte("admin")), + }, nil +} + +// Nearly identical to Basic auth, but with a per-request password +func (a *simplePluginAuth) Authorize(r *http.Request, owner string, repo string) auth.AuthResult { + username, password, ok := r.BasicAuth() + if ok { + usernameHash := sha256.Sum256([]byte(username)) + passwordHash := sha256.Sum256([]byte(password)) + + perRoutePasswordHash := sha256.Sum256([]byte(owner + "_" + repo)) + + usernameMatch := (subtle.ConstantTimeCompare(usernameHash[:], a.usernameHash[:]) == 1) + passwordMatch := (subtle.ConstantTimeCompare(passwordHash[:], perRoutePasswordHash[:]) == 1) + + if usernameMatch && passwordMatch { + return auth.Allow() + } else { + return auth.Deny(404) + } + } + + return auth.Deny(401, auth.Header{"WWW-Authenticate", `Basic realm="restricted", charset="UTF-8"`}) +} diff --git a/examples/auth/config/fixed.json b/examples/auth/config/fixed.json new file mode 100644 index 0000000..53d0d72 --- /dev/null +++ b/examples/auth/config/fixed.json @@ -0,0 +1,7 @@ +{ + "mode": "fixed", + "parameters": { + "username": "admin", + "passwordHash": "c3c3520adf2f6e25672ba55dc70bcb3dd8b4ef3341bce1a5f38c5eca6571f372" + } +} diff --git a/examples/auth/config/plugin.json b/examples/auth/config/plugin.json new file mode 100644 index 0000000..f9c2927 --- /dev/null +++ b/examples/auth/config/plugin.json @@ -0,0 +1,6 @@ +{ + "mode": "plugin", + "path": "examples/auth/_plugins/simple-plugin.so", + "initializer": "NewSimplePluginAuth", + "sha256": "" +} diff --git a/go.mod b/go.mod index 02dd6ad..83544fe 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/git-ecosystem/git-bundle-server -go 1.19 +go 1.20 require ( github.com/google/uuid v1.3.0 diff --git a/internal/auth/middleware.go b/internal/auth/middleware.go new file mode 100644 index 0000000..ca92b61 --- /dev/null +++ b/internal/auth/middleware.go @@ -0,0 +1,78 @@ +package auth + +import ( + "crypto/sha256" + "crypto/subtle" + "encoding/hex" + "encoding/json" + "fmt" + "net/http" + "strings" + + "github.com/git-ecosystem/git-bundle-server/pkg/auth" +) + +/* Built-in auth modes */ +// Authorize users with credentials matching a static username/password pair +// that applies to the whole server. +type fixedCredentialAuth struct { + usernameHash [32]byte + passwordHash [32]byte +} + +type fixedCredentialAuthParams struct { + Username string `json:"username"` + PasswordHash string `json:"passwordHash"` +} + +func NewFixedCredentialAuth(rawParameters json.RawMessage) (auth.AuthMiddleware, error) { + if len(rawParameters) == 0 { + return nil, fmt.Errorf("parameters JSON must exist") + } + + var params fixedCredentialAuthParams + err := json.Unmarshal(rawParameters, ¶ms) + if err != nil { + return nil, err + } + + // Check for invalid username characters + if strings.Contains(params.Username, ":") { + return nil, fmt.Errorf("username contains a colon (\":\")") + } + + // Make sure password hash is a valid hash + passwordHashBytes, err := hex.DecodeString(params.PasswordHash) + if err != nil { + return nil, fmt.Errorf("passwordHash is invalid: %w", err) + } else if len(passwordHashBytes) != 32 { + return nil, fmt.Errorf("passwordHash is incorrect length (%d vs. expected 32)", len(passwordHashBytes)) + } + + return &fixedCredentialAuth{ + usernameHash: sha256.Sum256([]byte(params.Username)), + passwordHash: [32]byte(passwordHashBytes), + }, nil +} + +func (a *fixedCredentialAuth) Authorize(r *http.Request, _ string, _ string) auth.AuthResult { + username, password, ok := r.BasicAuth() + if ok { + usernameHash := sha256.Sum256([]byte(username)) + passwordHash := sha256.Sum256([]byte(password)) + + usernameMatch := (subtle.ConstantTimeCompare(usernameHash[:], a.usernameHash[:]) == 1) + passwordMatch := (subtle.ConstantTimeCompare(passwordHash[:], a.passwordHash[:]) == 1) + + if usernameMatch && passwordMatch { + return auth.Allow() + } else { + // Return a 404 status even though the issue is that the user is + // forbidden so we don't indirectly reveal which repositories are + // configured in the bundle server. + return auth.Deny(404) + } + } + + return auth.Deny(401, auth.Header{Key: "WWW-Authenticate", Value: `Basic realm="restricted", charset="UTF-8"`}) +} diff --git a/internal/auth/middleware_test.go b/internal/auth/middleware_test.go new file mode 100644 index 0000000..b2f7a39 --- /dev/null +++ b/internal/auth/middleware_test.go @@ -0,0 +1,164 @@ +package auth_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/git-ecosystem/git-bundle-server/internal/auth" + "github.com/stretchr/testify/assert" +) + +var basicAuthTests = []struct { + title string + + // Inputs + parameters string + authHeader string + + // Expected outputs + authInitializationError bool + expectedDoExit bool + expectedResponseCode int + expectedHeaders http.Header +}{ + { + "No auth with expected username, password returns 401", + `{ "username": "admin", "passwordHash": "ecd71870d1963316a97e3ac3408c9835ad8cf0f3c1bc703527c30265534f75ae" }`, // password: test123 + "", + false, + true, + 401, + map[string][]string{ + "Www-Authenticate": {`Basic realm="restricted", charset="UTF-8"`}, + }, + }, + { + "Garbage auth header returns 401", + `{ "username": "admin", "passwordHash": "ecd71870d1963316a97e3ac3408c9835ad8cf0f3c1bc703527c30265534f75ae" }`, // password: test123 + "Basic *asdf====", + false, + true, + 401, + map[string][]string{ + "Www-Authenticate": {`Basic realm="restricted", charset="UTF-8"`}, + }, + }, + { + "Incorrect username returns 404", + `{ "username": "admin", "passwordHash": "ecd71870d1963316a97e3ac3408c9835ad8cf0f3c1bc703527c30265534f75ae" }`, // password: test123 + "Basic aW52YWxpZDp0ZXN0MTIz", // Base64 encoded "invalid:test123" + false, + true, + 404, + map[string][]string{}, + }, + { + "Correct username and password returns Authorized", + `{ "username": "admin", "passwordHash": "ecd71870d1963316a97e3ac3408c9835ad8cf0f3c1bc703527c30265534f75ae" }`, // password: test123 + "Basic YWRtaW46dGVzdDEyMw==", // Base64 encoded "admin:test123" + false, + false, + 200, + nil, + }, + { + "Empty username and password with expected auth returns Forbidden", + `{ "username": "admin", "passwordHash": "ecd71870d1963316a97e3ac3408c9835ad8cf0f3c1bc703527c30265534f75ae" }`, // password: test123 + "Basic Og==", // Base64 encoded ":" + false, + true, + 404, + map[string][]string{}, + }, + { + "Empty username and password is valid, return Authorized", + `{ "username": "", "passwordHash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" }`, // password: + "Basic Og==", // Base64 encoded ":" + false, + false, + 200, + nil, + }, + { + "Extra JSON parameters are ignored", + `{ "username": "admin", "passwordHash": "ecd71870d1963316a97e3ac3408c9835ad8cf0f3c1bc703527c30265534f75ae", "extra": [true, false] }`, // password: test123 + "Basic YWRtaW46dGVzdDEyMw==", // Base64 encoded "admin:test123" + false, + false, + 200, + nil, + }, + { + "Empty parameter JSON throws error", + "{}", + "Basic Og==", // Base64 encoded ":" + true, + true, + -1, + nil, + }, + { + "Missing parameter JSON throws error", + "", + "", + true, + true, + -1, + nil, + }, + { + "Malformed parameter JSON throws error", + `{abc: "def"`, + "", // Base64 encoded ":" + true, + true, + -1, + nil, + }, + { + "Username with colon throws error", + `{ "username": "example:user", "passwordHash": "ecd71870d1963316a97e3ac3408c9835ad8cf0f3c1bc703527c30265534f75ae" }`, // password: test123 + "Basic ZXhhbXBsZTp1c2VyOnRlc3QxMjM=", // Base64 encoded "example:user:test123" + true, + true, + -1, + nil, + }, +} + +func Test_FixedCredentialAuth(t *testing.T) { + for _, tt := range basicAuthTests { + t.Run(tt.title, func(t *testing.T) { + // Construct the request + req, err := http.NewRequest("GET", "test/repo", nil) + assert.Nil(t, err) + + if len(tt.authHeader) > 0 { + req.Header.Set("Authorization", tt.authHeader) + } + + // Create the auth middleware + auth, err := auth.NewFixedCredentialAuth([]byte(tt.parameters)) + if tt.authInitializationError { + assert.NotNil(t, err) + return + } + assert.Nil(t, err) + + result := auth.Authorize(req, "test", "repo") + + wExpect := httptest.NewRecorder() + if tt.expectedDoExit { + wExpect.HeaderMap = tt.expectedHeaders //lint:ignore SA1019 set headers manually for test + wExpect.WriteHeader(tt.expectedResponseCode) + } + + wActual := httptest.NewRecorder() + actualDoExit := result.ApplyResult(wActual) + + assert.Equal(t, tt.expectedDoExit, actualDoExit) + assert.Equal(t, wExpect, wActual) + }) + } +} diff --git a/pkg/auth/auth-result.go b/pkg/auth/auth-result.go new file mode 100644 index 0000000..be6e7e1 --- /dev/null +++ b/pkg/auth/auth-result.go @@ -0,0 +1,75 @@ +package auth + +import ( + "fmt" + "net/http" +) + +// The Header type captures HTTP response header information. +type Header struct { + Key string + Value string +} + +// The AuthResult represents the result of authenticating/authorizing via an +// AuthMiddleware's Authorize function. +type AuthResult struct { + applyResultFunc func(http.ResponseWriter) bool +} + +// ApplyResult applies the AuthResult's configuration to the provided +// http.ResponseWriter w and returns whether the web server should immediately +// send the response (for an AuthResult created with Deny()) or continue on to +// get and serve bundle server content (for an AuthResult created with +// Accept()). If the AuthResult is invalid (e.g., created with AuthResult{}), +// ApplyResult will indicate an immediate 500 response. +func (a *AuthResult) ApplyResult(w http.ResponseWriter) bool { + if a.applyResultFunc == nil { + // AuthResult was initialized incorrectly - throw an ISE & exit + w.WriteHeader(http.StatusInternalServerError) + return true + } else { + return a.applyResultFunc(w) + } +} + +func writeCustomHeaders(w http.ResponseWriter, headers []Header) { + for _, h := range headers { + w.Header().Add(h.Key, h.Value) + } +} + +// Deny creates an AuthResult instance indicating that the bundle web server +// should not serve the requested content and instead return an error response. +// The response will have the status indicated by code (*must* be 4XX) and +// include HTTP headers specified by the headers arg(s). Repeated headers (e.g. +// multiple WWW-Authenticate headers) will be added to the response in the order +// they are provided to this function. +func Deny(code int, headers ...Header) AuthResult { + // Make sure the code is a 4XX + if code < 400 || code > 499 { + panic(fmt.Sprintf("invalid auth middleware response code (must be 4XX, got %d)", code)) + } + + // Configure ApplyResult to write the response & exit + return AuthResult{ + applyResultFunc: func(w http.ResponseWriter) bool { + writeCustomHeaders(w, headers) + w.WriteHeader(code) + return true + }, + } +} + +// Allow creates an AuthResult instance indicating that the bundle web server +// should serve the requested content. If headers are specified, they will be +// applied to the http.ResponseWriter and applied to the response. Repeated +// headers are applied in the order they are provided to this function. +func Allow(headers ...Header) AuthResult { + return AuthResult{ + applyResultFunc: func(w http.ResponseWriter) bool { + writeCustomHeaders(w, headers) + return false + }, + } +} diff --git a/pkg/auth/auth-result_test.go b/pkg/auth/auth-result_test.go new file mode 100644 index 0000000..e622f49 --- /dev/null +++ b/pkg/auth/auth-result_test.go @@ -0,0 +1,146 @@ +package auth_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/git-ecosystem/git-bundle-server/pkg/auth" + "github.com/stretchr/testify/assert" +) + +var denyTests = []struct { + title string + + code int + headers []auth.Header + expectedInitPanic bool + + expectedHeaders http.Header +}{ + { + "Invalid code causes panic", + 500, + []auth.Header{}, + true, + nil, + }, + { + "Valid code with no headers", + 404, + []auth.Header{}, + false, + map[string][]string{}, + }, + { + "Valid code with unique headers", + 401, + []auth.Header{{"WWW-Authenticate", `Basic realm="restricted", charset="UTF-8"`}}, + false, + map[string][]string{"Www-Authenticate": {`Basic realm="restricted", charset="UTF-8"`}}, + }, + { + "Valid code with repeated headers", + 401, + []auth.Header{ + {"www-authenticate", `Basic realm="example.com"`}, + {"WWW-Authenticate", `Bearer authorize="idp.example.com/oauth"`}, + }, + false, + map[string][]string{"Www-Authenticate": { + `Basic realm="example.com"`, + `Bearer authorize="idp.example.com/oauth"`, + }}, + }, +} + +func Test_Deny(t *testing.T) { + for _, tt := range denyTests { + t.Run(tt.title, func(t *testing.T) { + w := httptest.NewRecorder() + + // Create the AuthResult, call WriteResponse + if tt.expectedInitPanic { + assert.Panics(t, func() { auth.Deny(tt.code, tt.headers...) }) + return + } + result := auth.Deny(tt.code, tt.headers...) + wroteResponse := result.ApplyResult(w) + + // Response has been written; should exit + assert.True(t, wroteResponse) + + // Check code and content + assert.Equal(t, tt.code, w.Code) + assert.Equal(t, tt.expectedHeaders, w.Header()) + assert.Empty(t, w.Body) + }) + } +} + +var allowTests = []struct { + title string + + headers []auth.Header + expectedHeaders http.Header +}{ + { + "Allow with no headers", + []auth.Header{}, + map[string][]string{}, + }, + { + "Allow with headers", + []auth.Header{{"Cache-Control", "no-store"}}, + map[string][]string{"Cache-Control": {"no-store"}}, + }, + { + "Allow with repeated headers", + []auth.Header{ + {"FAKE-HEADER", "first value"}, + {"fake-header", "second value"}, + }, + map[string][]string{"Fake-Header": { + "first value", + "second value", + }}, + }, +} + +func Test_Allow(t *testing.T) { + for _, tt := range allowTests { + t.Run(tt.title, func(t *testing.T) { + w := httptest.NewRecorder() + + // Create the AuthResult, call WriteResponse + result := auth.Allow(tt.headers...) + wroteResponse := result.ApplyResult(w) + + // Make sure we aren't exiting + assert.False(t, wroteResponse) + + // Make sure no code or body was written, but headers are + assert.Equal(t, 200, w.Code) // default code + assert.Equal(t, tt.expectedHeaders, w.Header()) + assert.Empty(t, w.Body) + }) + } +} + +func Test_AuthResult(t *testing.T) { + t.Run("Default AuthResult writes 500 response", func(t *testing.T) { + w := httptest.NewRecorder() + + // Create the AuthResult, call WriteResponse + result := auth.AuthResult{} + wroteResponse := result.ApplyResult(w) + + // Response has been written; should exit + assert.True(t, wroteResponse) + + // Check code and content + assert.Equal(t, 500, w.Code) + assert.Empty(t, w.Header()) + assert.Empty(t, w.Body) + }) +} diff --git a/pkg/auth/middleware.go b/pkg/auth/middleware.go new file mode 100644 index 0000000..7fb91a0 --- /dev/null +++ b/pkg/auth/middleware.go @@ -0,0 +1,20 @@ +package auth + +import ( + "net/http" +) + +// AuthMiddleware provides custom authN/authZ functionality to validate requests +// to the bundle web server. +// +// BE CAREFUL! Accesses to the loaded AuthMiddleware instance will *not* be +// thread-safe. Custom implementations should ensure any writes to any common +// state are properly locked. +type AuthMiddleware interface { + // Authorize interprets the contents of a bundle server request of a valid + // format (i.e., //[/]) and returns an AuthResult + // indicating whether the request should be allowed or denied. If the + // AuthResult is invalid (not created with Allow() or Deny()), the server + // will respond with a 500 status. + Authorize(r *http.Request, owner string, repo string) AuthResult +} diff --git a/test/integration/features/auth.feature b/test/integration/features/auth.feature new file mode 100644 index 0000000..f38764b --- /dev/null +++ b/test/integration/features/auth.feature @@ -0,0 +1,35 @@ +Feature: Auth configuration on the web server + + Background: The bundle server has an initialized route + Given no bundle server repository exists at route 'integration/auth' + Given a new remote repository with main branch 'main' + Given the remote is cloned + Given 5 commits are pushed to the remote branch 'main' + Given a bundle server repository is created at route 'integration/auth' for the remote + + Scenario: With no auth config, bundle list can be accessed anonymously + Given the bundle web server was started at port 8080 + When I request the bundle list + Then the response code is 200 + Then the response is a valid bundle list + + Scenario: If basic auth is required but none is sent, get a 401 + Given an auth config with username 'my_username' and password 'p4sSW0rD!' + Given the bundle web server was started at port 8080 with auth config + When I request the bundle list + Then the response code is 401 + Then the response is empty + + Scenario: If basic auth is required and we send bad credentials, get a 404 + Given an auth config with username 'my_username' and password 'p4sSW0rD!' + Given the bundle web server was started at port 8080 with auth config + When I request the bundle list with username 'my_userName' and password 'password!' + Then the response code is 404 + Then the response is empty + + Scenario: If basic auth is required and we send the right credentials, can access the bundle list + Given an auth config with username 'my_username' and password 'p4sSW0rD!' + Given the bundle web server was started at port 8080 with auth config + When I request the bundle list with username 'my_username' and password 'p4sSW0rD!' + Then the response code is 200 + Then the response is a valid bundle list diff --git a/test/integration/features/step_definitions/auth.ts b/test/integration/features/step_definitions/auth.ts new file mode 100644 index 0000000..5285883 --- /dev/null +++ b/test/integration/features/step_definitions/auth.ts @@ -0,0 +1,24 @@ +import { IntegrationBundleServerWorld } from '../support/world' +import { Given } from '@cucumber/cucumber' +import * as fs from 'fs' +import * as path from 'path' +import * as crypto from 'crypto'; + +Given('an auth config with username {string} and password {string}', + async function (this: IntegrationBundleServerWorld, user: string, pass: string) { + const config = { + "mode": "fixed", + "parameters": { + "username": user, + "passwordHash": crypto.createHash('sha256').update(pass).digest('hex') + } + } + fs.writeFileSync(path.join(this.trashDirectory, "auth-config.json"), JSON.stringify(config)) + } +) + +Given('the bundle web server was started at port {int} with auth config', + async function (this: IntegrationBundleServerWorld, port: number) { + await this.bundleServer.startWebServer(port, path.join(this.trashDirectory, "auth-config.json")) + } +) diff --git a/test/integration/features/step_definitions/bundleServer.ts b/test/integration/features/step_definitions/bundleServer.ts index da330a0..1f5b545 100644 --- a/test/integration/features/step_definitions/bundleServer.ts +++ b/test/integration/features/step_definitions/bundleServer.ts @@ -1,6 +1,6 @@ import * as assert from 'assert' import { IntegrationBundleServerWorld } from '../support/world' -import { Given, Then } from '@cucumber/cucumber' +import { Given, When, Then } from '@cucumber/cucumber' import * as utils from '../../../shared/support/utils' import * as fs from 'fs' @@ -18,6 +18,27 @@ Given('no bundle server repository exists at route {string}', async function (th } }) +When('I request the bundle list', async function (this: IntegrationBundleServerWorld) { + this.requestResponse = await fetch(this.bundleServer.bundleUri(), { + method: 'GET', + headers: { + Accept: 'text/plain', + }, + }); +}) + +When('I request the bundle list with username {string} and password {string}', + async function (this: IntegrationBundleServerWorld, user: string, pass: string) { + this.requestResponse = await fetch(this.bundleServer.bundleUri(), { + method: 'GET', + headers: { + Accept: 'text/plain', + Authorization: 'Basic ' + Buffer.from(`${user}:${pass}`, 'utf8').toString('base64') + }, + }); + } +) + Then('a bundle server repository exists at route {string}', async function (this: IntegrationBundleServerWorld, route: string) { var repoRoot = utils.repoRoot(route) assert.equal(fs.existsSync(repoRoot), true) @@ -71,3 +92,28 @@ Then('the route exists in the routes file', async function (this: IntegrationBun throw new Error("Route not set") } }) + +Then('the response code is {int}', async function (this: IntegrationBundleServerWorld, code: number) { + if (!this.requestResponse) { + throw new Error("Request response not set") + } + assert.strictEqual(this.requestResponse.status, code) +}) + +Then('the response is a valid bundle list', async function (this: IntegrationBundleServerWorld) { + if (!this.requestResponse) { + throw new Error("Request response not set") + } + + const data = await this.requestResponse.text() + assert.notStrictEqual(data, "") +}) + +Then('the response is empty', async function (this: IntegrationBundleServerWorld) { + if (!this.requestResponse) { + throw new Error("Request response not set") + } + + const data = await this.requestResponse.text() + assert.strictEqual(data, "") +}) diff --git a/test/integration/features/support/world.ts b/test/integration/features/support/world.ts index 2525eed..cb4b227 100644 --- a/test/integration/features/support/world.ts +++ b/test/integration/features/support/world.ts @@ -10,6 +10,7 @@ export class IntegrationBundleServerWorld extends BundleServerWorldBase { local: ClonedRepository | undefined commandResult: child_process.SpawnSyncReturns | undefined + requestResponse: Response | undefined runCommand(commandArgs: string): void { this.commandResult = child_process.spawnSync(`${this.parameters.bundleServerCommand} ${commandArgs}`, [], { shell: true }) diff --git a/test/shared/classes/bundleServer.ts b/test/shared/classes/bundleServer.ts index 90aa36b..5a19b77 100644 --- a/test/shared/classes/bundleServer.ts +++ b/test/shared/classes/bundleServer.ts @@ -21,12 +21,42 @@ export class BundleServer { this.bundleWebServerCmd = bundleWebServerCmd } - startWebServer(port: number): void { + async startWebServer(port: number, authConfig: string = ""): Promise { if (this.webServerProcess) { throw new Error("Tried to start web server, but web server is already running") } - this.webServerProcess = child_process.spawn(this.bundleWebServerCmd, ["--port", String(port)]) + let args = ["--port", String(port)] + if (authConfig !== "") { + args = args.concat(["--auth-config", authConfig]) + } + const webServerProcess = child_process.spawn(this.bundleWebServerCmd, args) + this.webServerProcess = webServerProcess this.bundleUriBase = `http://localhost:${port}/` + + // Now, ensure the server is running + var timer: NodeJS.Timeout | undefined + var ok: boolean | undefined + + await Promise.race([ + new Promise(resolve => webServerProcess.stdout.on('data', (data: string) => { + if (data.includes("Server is running at address") && ok === undefined) { + ok = true + resolve() // server is running + } + })), + new Promise(resolve => webServerProcess.on('close', () => { + ok = false + resolve() // program failed to start/exited early + })), + new Promise(resolve => timer = setTimeout(resolve, 2000)) // fallback timeout + ]) + + // If it's still running, clear the timeout so it doesn't delay shutdown + clearTimeout(timer) + + if (!ok) { + throw new Error('Failed to start web server') + } } init(remote: RemoteRepo, routePrefix: string, route: string = ""): child_process.SpawnSyncReturns { diff --git a/test/shared/features/step_definitions/bundleServer.ts b/test/shared/features/step_definitions/bundleServer.ts index 5d1cc26..220e5b8 100644 --- a/test/shared/features/step_definitions/bundleServer.ts +++ b/test/shared/features/step_definitions/bundleServer.ts @@ -2,5 +2,5 @@ import { Given } from '@cucumber/cucumber' import { BundleServerWorldBase } from '../../support/world' Given('the bundle web server was started at port {int}', async function (this: BundleServerWorldBase, port: number) { - this.bundleServer.startWebServer(port) + await this.bundleServer.startWebServer(port) })