diff --git a/auth/iam.go b/auth/iam.go index 9d642e54..d8ec96dc 100644 --- a/auth/iam.go +++ b/auth/iam.go @@ -220,6 +220,7 @@ func New(o *Opts) (IAMService, error) { switch { case o.StandaloneIAMEndpoint != "": + fmt.Printf("initializing standalone IAM with %q\n", o.StandaloneIAMEndpoint) svc, err = NewIAMServiceStandalone(o.RootAccount, IAMServiceStandaloneConfig{ Endpoint: o.StandaloneIAMEndpoint, Access: o.StandaloneIAMAccess, @@ -231,7 +232,6 @@ func New(o *Opts) (IAMService, error) { DefaultGroupID: o.StandaloneDefaultGroupID, DefaultProjectID: o.StandaloneDefaultProjectID, }) - fmt.Printf("initializing standalone IAM with %q\n", o.StandaloneIAMEndpoint) if err != nil { return nil, err } diff --git a/auth/iam_standalone.go b/auth/iam_standalone.go index 76407917..62245b82 100644 --- a/auth/iam_standalone.go +++ b/auth/iam_standalone.go @@ -18,10 +18,14 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" + "log" "net" "net/http" + "net/url" + "strconv" "time" "github.com/versity/versitygw/iamapi/private" @@ -39,6 +43,34 @@ const ( standaloneRequestTimeout = 10 * time.Second ) +// standaloneProbeWindow bounds how long NewIAMServiceStandalone waits for the +// IAM service to answer compatibly before it gives up, and +// standaloneProbeInterval how often it retries within that window. Both +// failures are retried, for different reasons: an unreachable service is the +// ordinary case of the two processes starting in parallel, and an +// incompatible one is what a gateway sees while the IAM service it is paired +// with is still rolling. Failing immediately on either would turn a routine +// deployment into a crash loop whose backoff long outlives the condition. +// +// Variables rather than constants so tests can shorten the window; nothing +// else writes them. +var ( + standaloneProbeWindow = 30 * time.Second + standaloneProbeInterval = 2 * time.Second +) + +// protocolMismatchError reports that whatever answered the private endpoints +// is not a standalone IAM service this gateway can use. It is a distinct type +// so the startup probe can tell an incompatible peer, which is fatal, from an +// unreachable one, which is not. +type protocolMismatchError struct { + detail string +} + +func (e *protocolMismatchError) Error() string { + return "iam standalone: " + e.detail +} + // IAMServiceStandaloneConfig configures IAMServiceStandalone. type IAMServiceStandaloneConfig struct { // Endpoint is either a "host:port" TCP address (mTLS required - @@ -46,8 +78,8 @@ type IAMServiceStandaloneConfig struct { // the standalone IAM service's own --private-ports address shape. Endpoint string // Access/Secret are this client's own SigV4 identity — the credential - // it signs its private requests with. Defaults both to the - // gateway's root account when unset. + // it signs its private requests with. Both must be set together, or + // both left empty to sign with the gateway's root account. Access string Secret string // ClientCert/ClientCertKey/ServerCA configure outbound mTLS. Required @@ -96,13 +128,13 @@ func NewIAMServiceStandalone(rootAcc Account, cfg IAMServiceStandaloneConfig) (* return nil, fmt.Errorf("iam standalone: endpoint is required") } - access := cfg.Access - if access == "" { - access = rootAcc.Access + if (cfg.Access == "") != (cfg.Secret == "") { + return nil, fmt.Errorf("iam standalone: access and secret must both be set, or both left empty to sign with the root account") } - secret := cfg.Secret - if secret == "" { - secret = rootAcc.Secret + + access, secret := cfg.Access, cfg.Secret + if access == "" { + access, secret = rootAcc.Access, rootAcc.Secret } client, baseURL, err := newStandaloneHTTPClient(cfg) @@ -110,14 +142,93 @@ func NewIAMServiceStandalone(rootAcc Account, cfg IAMServiceStandaloneConfig) (* return nil, err } - return &IAMServiceStandalone{ + svc := &IAMServiceStandalone{ client: client, baseURL: baseURL, access: access, secret: secret, rootAcc: rootAcc, cfg: cfg, - }, nil + } + + if err := svc.probeProtocol(); err != nil { + return nil, err + } + + return svc, nil +} + +// probeProtocol verifies at startup what every request verifies anyway, so a +// version skew is diagnosed once, here, instead of once per S3 request as an +// opaque 500. Because it is a signed request to a root-authenticated +// endpoint, reaching a compatible answer also proves the transport, the mTLS +// material, and this gateway's own IAM credential. +// +// An incompatible service is fatal: a gateway that cannot authorize a single +// request is more useful refusing to start, with the reason in its log, than +// running and serving errors. An unreachable one is only a warning — the two +// processes legitimately start in parallel, and every request checks the +// version regardless. +func (s *IAMServiceStandalone) probeProtocol() error { + deadline := time.Now().Add(standaloneProbeWindow) + + for { + var resp private.VersionResponse + // The endpoint takes no arguments; an empty object is the request. + err := s.doPrivateRequest(private.VersionPath, struct{}{}, &resp) + + // The version endpoint answers even a gateway the service will not + // serve — that is the whole point of exempting it from the service's + // own check — so the probe has to draw that conclusion itself from + // the minimum the service reports. Without this the one direction a + // gateway cannot detect from a response header would pass startup and + // fail on every request afterwards. + if err == nil && private.ProtocolVersion < resp.MinClient { + err = &protocolMismatchError{fmt.Sprintf( + "IAM service at %q serves private protocol %d and newer, this gateway speaks %d: upgrade the gateway", + s.cfg.Endpoint, resp.MinClient, private.ProtocolVersion)} + } + + if err == nil { + serverVersion := resp.ServerVersion + if serverVersion == "" { + serverVersion = "unknown" + } + fmt.Printf("standalone IAM service %q: version %s, private protocol %d\n", + s.cfg.Endpoint, serverVersion, resp.Protocol) + return nil + } + + if probeRetryable(err) && time.Now().Before(deadline) { + time.Sleep(standaloneProbeInterval) + continue + } + + var mismatch *protocolMismatchError + if errors.As(err, &mismatch) { + return fmt.Errorf("%w (still incompatible after %v, so this is a version skew rather than a rollout in progress)", + err, standaloneProbeWindow) + } + + log.Printf("WARNING: iam standalone: could not verify the IAM service at %q: %v; "+ + "the private protocol version is still checked on every request", + s.cfg.Endpoint, err) + return nil + } +} + +// probeRetryable reports whether a failed probe could resolve on its own. +// Only two can: the service not being up yet, and it being mid-rollout at an +// incompatible version. Anything it answered definitively — a rejected +// gateway credential above all — will answer the same way in thirty seconds, +// so retrying only delays the warning that says so. +func probeRetryable(err error) bool { + var mismatch *protocolMismatchError + if errors.As(err, &mismatch) { + return true + } + var transport *url.Error + return errors.As(err, &transport) } func newStandaloneHTTPClient(cfg IAMServiceStandaloneConfig) (*http.Client, string, error) { @@ -180,6 +291,11 @@ func (s *IAMServiceStandalone) doPrivateRequest(path string, reqBody, respBody a return fmt.Errorf("iam standalone: build request: %w", err) } req.Header.Set("Content-Type", "application/json") + // Set before signing, so it is covered by the signature: SigningInput + // FromRequest leaves SignedHeaders nil, and sigv4auth's default policy + // excludes only Authorization, User-Agent, X-Amzn-Trace-Id, Expect and + // Transfer-Encoding. + req.Header.Set(private.ProtocolHeader, strconv.Itoa(private.ProtocolVersion)) payloadHash := sigv4auth.PayloadSHA256Hex(bodyBytes) req.Header.Set("X-Amz-Content-Sha256", payloadHash) @@ -203,6 +319,12 @@ func (s *IAMServiceStandalone) doPrivateRequest(path string, reqBody, respBody a } defer resp.Body.Close() + // Checked before the status and before the body: a peer whose protocol + // this gateway cannot read is not one whose response it should interpret. + if err := s.checkServerProtocol(path, resp); err != nil { + return err + } + respBytes, err := io.ReadAll(resp.Body) if err != nil { return fmt.Errorf("iam standalone: read response from %s: %w", path, err) @@ -240,11 +362,47 @@ func standaloneResponseError(path string, status int, body []byte) error { return ErrNoSuchUser case private.CodeInvalidToken: return ErrInvalidSessionToken + case private.CodeProtocolMismatch: + // The other direction of the same check: this gateway is too old for + // the IAM service to serve safely, which only that service can know. + return &protocolMismatchError{fmt.Sprintf( + "IAM service refused this gateway's private protocol version %d at %s: %s", + private.ProtocolVersion, path, errBody.Error)} } return fmt.Errorf("iam standalone: %s returned %d: %s", path, status, string(body)) } +// checkServerProtocol verifies the private protocol version the IAM service +// declared on a response. +// +// A missing version fails just as hard as an incompatible one. No build of +// this protocol omits the header, so a response without one did not come from +// a compatible IAM service — it came from something else answering on that +// address, such as a proxy returning its own error page. The message says +// what was observed rather than naming a cause, since both look identical from here. +func (s *IAMServiceStandalone) checkServerProtocol(path string, resp *http.Response) error { + value := resp.Header.Get(private.ProtocolHeader) + if value == "" { + return &protocolMismatchError{fmt.Sprintf( + "no %s header on the %d response from %s at %q: not a versioned IAM service", + private.ProtocolHeader, resp.StatusCode, path, s.cfg.Endpoint)} + } + + server, err := private.ParseProtocolVersion(value) + if err != nil { + return &protocolMismatchError{fmt.Sprintf("response from %s at %q: %v", path, s.cfg.Endpoint, err)} + } + + if server < private.ProtocolVersion { + return &protocolMismatchError{fmt.Sprintf( + "IAM service at %q speaks private protocol %d, this gateway requires %d or newer: upgrade the IAM service before the gateway", + s.cfg.Endpoint, server, private.ProtocolVersion)} + } + + return nil +} + // DeriveSigningKey implements SigningKeyProvider. Root is special-cased // locally: its secret is already known to this process either way, so // there's no reason to round-trip it through the IAM service. diff --git a/auth/iam_standalone_test.go b/auth/iam_standalone_test.go index 8f055888..57167711 100644 --- a/auth/iam_standalone_test.go +++ b/auth/iam_standalone_test.go @@ -16,9 +16,12 @@ package auth import ( "context" "errors" + "fmt" "net" + "net/http" "os" "path/filepath" + "strconv" "testing" "time" @@ -354,3 +357,265 @@ func TestNewIAMServiceStandaloneRespectsExplicitCredentials(t *testing.T) { t.Errorf("secret = %q, want %q", client.secret, "CUSTOMSECRET") } } + +// TestNewIAMServiceStandalonePartialCredentialsRejected covers a half +// configured signing identity: pairing one supplied key with the other half +// of the root credential would silently sign with a mismatched identity, so +// it must fail at construction instead. +func TestNewIAMServiceStandalonePartialCredentialsRejected(t *testing.T) { + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + _, sock := standaloneTestServer(t) + + for _, tc := range []struct { + name string + access string + secret string + }{ + {name: "access only", access: "AKIDCUSTOM"}, + {name: "secret only", secret: "CUSTOMSECRET"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{ + Endpoint: sock, + Access: tc.access, + Secret: tc.secret, + }) + if err == nil { + t.Fatal("expected an error when only one of access/secret is configured") + } + }) + } +} + +// TestIAMServiceStandaloneRejectsIncompatibleService covers every response a +// peer can give that this gateway must not interpret: no protocol header at +// all (a pre-versioning build, or something else answering on the address), +// one it cannot read, and one older than the protocol this gateway speaks. +// None of them may yield a working client. +func TestIAMServiceStandaloneRejectsIncompatibleService(t *testing.T) { + shortenProbeWindow(t) + + for _, tc := range []struct { + name string + protocol string + }{ + {"no header", ""}, + {"unreadable", "one"}, + {"older service", strconv.Itoa(private.ProtocolVersion - 1)}, + } { + t.Run(tc.name, func(t *testing.T) { + sock := serveFakePrivate(t, tc.protocol, http.StatusOK, `{"protocol":0}`) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + _, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{Endpoint: sock}) + if err == nil { + t.Fatal("expected the gateway to refuse to start against an incompatible IAM service") + } + var mismatch *protocolMismatchError + if !errors.As(err, &mismatch) { + t.Fatalf("error = %v, want a protocolMismatchError", err) + } + }) + } +} + +// TestIAMServiceStandaloneAcceptsNewerService confirms the rule is +// "not older", not "equal": an IAM service upgraded ahead of its gateways is +// the supported deployment order, so it must keep serving them. +func TestIAMServiceStandaloneAcceptsNewerService(t *testing.T) { + shortenProbeWindow(t) + + newer := strconv.Itoa(private.ProtocolVersion + 1) + sock := serveFakePrivate(t, newer, http.StatusOK, + `{"protocol":`+newer+`,"minClient":1,"serverVersion":"v9.9.9"}`) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + client, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{Endpoint: sock}) + if err != nil { + t.Fatalf("NewIAMServiceStandalone against a newer IAM service: %v", err) + } + defer client.Shutdown() +} + +// TestIAMServiceStandaloneRefusedByNewerService is the other direction of the +// same check: an IAM service that has raised its minimum turns this gateway +// away, and the gateway must recognise that as a version problem rather than +// as a generic server error. +func TestIAMServiceStandaloneRefusedByNewerService(t *testing.T) { + shortenProbeWindow(t) + + sock := serveFakePrivate(t, strconv.Itoa(private.ProtocolVersion+1), http.StatusBadRequest, + `{"error":"gateway speaks private protocol 1, this IAM service requires 2 or newer","code":"`+private.CodeProtocolMismatch+`"}`) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + _, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{Endpoint: sock}) + if err == nil { + t.Fatal("expected the gateway to refuse to start when the IAM service refuses it") + } + var mismatch *protocolMismatchError + if !errors.As(err, &mismatch) { + t.Fatalf("error = %v, want a protocolMismatchError", err) + } +} + +// TestIAMServiceStandaloneRefusedByServiceMinimum covers the one direction a +// response header cannot express. The version endpoint is exempt from the +// service's own client-version check, so it answers 200 even to a gateway the +// service will not serve; the gateway has to reach that conclusion from the +// minimum the endpoint reports, or it would start cleanly and then fail every +// real request. +func TestIAMServiceStandaloneRefusedByServiceMinimum(t *testing.T) { + shortenProbeWindow(t) + + current := strconv.Itoa(private.ProtocolVersion) + sock := serveFakePrivate(t, current, http.StatusOK, + `{"protocol":`+current+`,"minClient":`+strconv.Itoa(private.ProtocolVersion+1)+`}`) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + _, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{Endpoint: sock}) + if err == nil { + t.Fatal("expected the gateway to refuse to start below the IAM service's minimum") + } + var mismatch *protocolMismatchError + if !errors.As(err, &mismatch) { + t.Fatalf("error = %v, want a protocolMismatchError", err) + } +} + +// TestIAMServiceStandaloneUnreachableIsNotFatal confirms an unreachable IAM +// service only warns. The two processes legitimately start in parallel, and +// every request checks the version anyway, so refusing to start here would +// invent an ordering dependency without buying any safety. +func TestIAMServiceStandaloneUnreachableIsNotFatal(t *testing.T) { + shortenProbeWindow(t) + + sockDir, err := os.MkdirTemp("", "vgw-priv") + if err != nil { + t.Fatalf("MkdirTemp: %v", err) + } + t.Cleanup(func() { os.RemoveAll(sockDir) }) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + client, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{ + Endpoint: filepath.Join(sockDir, "nothing-here.sock"), + }) + if err != nil { + t.Fatalf("an unreachable IAM service must not be fatal, got: %v", err) + } + defer client.Shutdown() +} + +// TestIAMServiceStandaloneDoesNotRetryDefinitiveRejection confirms the probe's +// retry window applies only to failures that can resolve on their own. A +// rejected gateway credential is answered by a service that is up and +// compatible, so it must warn at once rather than hold startup for the full +// window. Deliberately run against the real, unshortened window. +func TestIAMServiceStandaloneDoesNotRetryDefinitiveRejection(t *testing.T) { + sock := serveFakePrivate(t, strconv.Itoa(private.ProtocolVersion), http.StatusForbidden, + `{"error":"The security token included in the request is invalid","code":"InvalidClientTokenId"}`) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + + start := time.Now() + client, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{Endpoint: sock}) + if err != nil { + t.Fatalf("a rejected credential must warn, not fail startup: %v", err) + } + defer client.Shutdown() + + if elapsed := time.Since(start); elapsed > standaloneProbeInterval { + t.Errorf("probe took %v; a definitive rejection must not be retried", elapsed) + } +} + +// TestIAMServiceStandaloneSendsProtocolHeader confirms the gateway advertises +// its own version, and that it does so inside the signature: the real server +// verifies the signature over that header, so an unsigned or absent one would +// fail before reaching a handler. +func TestIAMServiceStandaloneSendsProtocolHeader(t *testing.T) { + store, sock := standaloneTestServer(t) + createStandaloneTestUser(t, store, "alice", "AKIAALICE", "alicesecret", "") + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + client, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{Endpoint: sock}) + if err != nil { + t.Fatalf("NewIAMServiceStandalone: %v", err) + } + defer client.Shutdown() + + if _, err := client.GetUserAccount("AKIAALICE"); err != nil { + t.Fatalf("GetUserAccount: %v", err) + } +} + +// TestIAMServiceStandaloneShapeChecksSurviveMatchingProtocol confirms the +// version header did not replace the response-shape checks. A peer can declare +// a compatible version and still send a matrix that disagrees — a forgotten +// bump, a locally patched build — and that must still fail closed. +func TestIAMServiceStandaloneShapeChecksSurviveMatchingProtocol(t *testing.T) { + shortenProbeWindow(t) + + // Compatible on the wire version, but one action decision short of the two + // actions asked for below. + current := strconv.Itoa(private.ProtocolVersion) + sock := serveFakePrivate(t, current, http.StatusOK, + `{"protocol":`+current+`,"decisions":[["allow"]]}`) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret} + client, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{Endpoint: sock}) + if err != nil { + t.Fatalf("NewIAMServiceStandalone: %v", err) + } + defer client.Shutdown() + + _, err = client.EvaluatePolicy("AKIAALICE", "", []Action{GetObjectAction, PutObjectAction}, []string{"arn:aws:s3:::b/o"}, nil) + if err == nil { + t.Fatal("expected a short decision row to fail closed even at a matching protocol version") + } +} + +// shortenProbeWindow collapses the startup probe's retry window for tests that +// deliberately point the client at an incompatible or absent service, which +// would otherwise sit through the full production window. +func shortenProbeWindow(t *testing.T) { + t.Helper() + + window, interval := standaloneProbeWindow, standaloneProbeInterval + standaloneProbeWindow, standaloneProbeInterval = 0, time.Millisecond + t.Cleanup(func() { standaloneProbeWindow, standaloneProbeInterval = window, interval }) +} + +// serveFakePrivate serves a stand-in for the standalone IAM service on a unix +// socket, answering every request with the given protocol header (omitted when +// empty), status, and body. It exists because the cases worth testing — a +// build older or newer than this one, or one predating versioning altogether — +// cannot be produced by the real server, which only ever speaks its own +// version. +func serveFakePrivate(t *testing.T, protocol string, status int, body string) string { + t.Helper() + + sockDir, err := os.MkdirTemp("", "vgw-priv") + if err != nil { + t.Fatalf("MkdirTemp: %v", err) + } + t.Cleanup(func() { os.RemoveAll(sockDir) }) + sockPath := filepath.Join(sockDir, "p.sock") + + ln, err := net.Listen("unix", sockPath) + if err != nil { + t.Fatalf("listen: %v", err) + } + + srv := &http.Server{Handler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if protocol != "" { + w.Header().Set(private.ProtocolHeader, protocol) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + fmt.Fprint(w, body) + })} + go srv.Serve(ln) + t.Cleanup(func() { srv.Close() }) + + return sockPath +} diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index 970a820b..21750ad2 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -812,13 +812,13 @@ func initFlags() []cli.Flag { }, &cli.StringFlag{ Name: "iam-standalone-access", - Usage: "access key this gateway signs its own calls to the standalone IAM service with (defaults to --access/root)", + Usage: "access key this gateway signs its own calls to the standalone IAM service with (must be set together with --iam-standalone-secret; both default to --access/--secret root)", EnvVars: []string{"VGW_IAM_STANDALONE_ACCESS"}, Destination: &standaloneIAMAccess, }, &cli.StringFlag{ Name: "iam-standalone-secret", - Usage: "secret key this gateway signs its own calls to the standalone IAM service with (defaults to --secret/root)", + Usage: "secret key this gateway signs its own calls to the standalone IAM service with (must be set together with --iam-standalone-access; both default to --access/--secret root)", EnvVars: []string{"VGW_IAM_STANDALONE_SECRET"}, Destination: &standaloneIAMSecret, }, diff --git a/embedgw/embedgw.go b/embedgw/embedgw.go index 847a6b2a..b4100a8e 100644 --- a/embedgw/embedgw.go +++ b/embedgw/embedgw.go @@ -298,8 +298,8 @@ type Config struct { StandaloneIAMEndpoint string // StandaloneIAMAccess/StandaloneIAMSecret are this gateway's own // signing identity for its calls to the private endpoints — not a - // fetched account. Both default to RootUserAccess/RootUserSecret when - // unset. + // fetched account. Both must be set together, or both left empty to + // default to RootUserAccess/RootUserSecret. StandaloneIAMAccess string StandaloneIAMSecret string // StandaloneClientCert/ClientCertKey are this gateway's client diff --git a/embedgw/iam.go b/embedgw/iam.go index d8180ade..d94e05bb 100644 --- a/embedgw/iam.go +++ b/embedgw/iam.go @@ -155,28 +155,39 @@ type IAMConfig struct { DisableOIDCThumbprintAutoFetch bool } +// privateAPIServer is the standalone IAM service's private endpoint set +// together with everything RunIAMAPI needs to serve and maintain it: the +// TLS options ServeMultiPort will enforce, and the cert storage backing +// them so a SIGHUP can swap in a rotated certificate. +type privateAPIServer struct { + api *private.PrivateAPI + tlsOpts netutil.TLSOptions + certStorage *netutil.CertStorage +} + // newPrivateAPI builds the standalone IAM service's private endpoint set // and the TLS options ServeMultiPort will enforce (mTLS, or nothing at all // for a unix-socket-only deployment — see netutil.RequireSecureTransport). -func newPrivateAPI(store storage.Storer, cfg *IAMConfig) (*private.PrivateAPI, netutil.TLSOptions, error) { +func newPrivateAPI(store storage.Storer, cfg *IAMConfig) (*privateAPIServer, error) { allSet := cfg.PrivateCertFile != "" && cfg.PrivateKeyFile != "" && cfg.PrivateClientCAFile != "" noneSet := cfg.PrivateCertFile == "" && cfg.PrivateKeyFile == "" && cfg.PrivateClientCAFile == "" if !allSet && !noneSet { - return nil, netutil.TLSOptions{}, fmt.Errorf("--private-cert, --private-cert-key, and --private-client-ca must all be set together, or all left empty for a unix-socket-only private listener") + return nil, fmt.Errorf("--private-cert, --private-cert-key, and --private-client-ca must all be set together, or all left empty for a unix-socket-only private listener") } var tlsOpts netutil.TLSOptions + var certStorage *netutil.CertStorage if allSet { - cs := netutil.NewCertStorage() - if err := cs.SetCertificate(cfg.PrivateCertFile, cfg.PrivateKeyFile); err != nil { - return nil, netutil.TLSOptions{}, fmt.Errorf("private listener: load certs: %w", err) + certStorage = netutil.NewCertStorage() + if err := certStorage.SetCertificate(cfg.PrivateCertFile, cfg.PrivateKeyFile); err != nil { + return nil, fmt.Errorf("private listener: load certs: %w", err) } pool, err := netutil.LoadCACertPool(cfg.PrivateClientCAFile) if err != nil { - return nil, netutil.TLSOptions{}, fmt.Errorf("private listener: %w", err) + return nil, fmt.Errorf("private listener: %w", err) } tlsOpts = netutil.TLSOptions{ - GetCertificate: cs.GetCertificate, + GetCertificate: certStorage.GetCertificate, ClientCAs: pool, RequireClientCert: true, } @@ -186,23 +197,26 @@ func newPrivateAPI(store storage.Storer, cfg *IAMConfig) (*private.PrivateAPI, n if cfg.PrivateSocketPerm != "" { perm, err := strconv.ParseUint(cfg.PrivateSocketPerm, 8, 32) if err != nil { - return nil, netutil.TLSOptions{}, fmt.Errorf("invalid PrivateSocketPerm value %q: must be an octal integer (e.g. '0660'): %w", cfg.PrivateSocketPerm, err) + return nil, fmt.Errorf("invalid PrivateSocketPerm value %q: must be an octal integer (e.g. '0660'): %w", cfg.PrivateSocketPerm, err) } privOpts = append(privOpts, private.WithPrivateSocketPerm(os.FileMode(perm))) } if cfg.Quiet { privOpts = append(privOpts, private.WithPrivateQuiet()) } + if cfg.Version != "" { + privOpts = append(privOpts, private.WithPrivateServerVersion(cfg.Version)) + } p, err := private.New(store, iamapi.RootCredentials{ Access: cfg.RootUserAccess, Secret: cfg.RootUserSecret, }, privOpts...) if err != nil { - return nil, netutil.TLSOptions{}, fmt.Errorf("init private IAM API: %w", err) + return nil, fmt.Errorf("init private IAM API: %w", err) } - return p, tlsOpts, nil + return &privateAPIServer{api: p, tlsOpts: tlsOpts, certStorage: certStorage}, nil } var iamAPIRunning atomic.Bool @@ -310,10 +324,9 @@ func RunIAMAPI(ctx context.Context, cfg *IAMConfig) error { return fmt.Errorf("init IAM API server: %w", err) } - var privateAPI *private.PrivateAPI - var privateTLSOpts netutil.TLSOptions + var privateAPI *privateAPIServer if len(cfg.PrivatePorts) > 0 { - privateAPI, privateTLSOpts, err = newPrivateAPI(store, cfg) + privateAPI, err = newPrivateAPI(store, cfg) if err != nil { return err } @@ -330,7 +343,7 @@ func RunIAMAPI(ctx context.Context, cfg *IAMConfig) error { if privateAPI != nil { go func() { - errCh <- privateAPI.ServeMultiPort(cfg.PrivatePorts, privateTLSOpts) + errCh <- privateAPI.api.ServeMultiPort(cfg.PrivatePorts, privateAPI.tlsOpts) }() } @@ -357,6 +370,18 @@ Loop: fmt.Printf("iam api cert reloaded (cert: %s, key: %s)\n", cfg.CertFile, cfg.KeyFile) } } + // the private listener has its own certificate, so it needs + // its own reload: without this, new gateway-to-IAM TLS + // connections would keep getting the pre-rotation cert until + // the IAM service restarts. + if privateAPI != nil && privateAPI.certStorage != nil { + reloadErr := privateAPI.certStorage.SetCertificate(cfg.PrivateCertFile, cfg.PrivateKeyFile) + if reloadErr != nil { + debuglogger.InternalError(fmt.Errorf("private iam api cert reload failed: %w", reloadErr)) + } else { + fmt.Printf("private iam api cert reloaded (cert: %s, key: %s)\n", cfg.PrivateCertFile, cfg.PrivateKeyFile) + } + } } } saveErr := err @@ -365,7 +390,7 @@ Loop: fmt.Fprintf(os.Stderr, "shutdown IAM API server: %v\n", err) } if privateAPI != nil { - if err := privateAPI.Shutdown(); err != nil { + if err := privateAPI.api.Shutdown(); err != nil { fmt.Fprintf(os.Stderr, "shutdown private IAM API server: %v\n", err) } } diff --git a/iamapi/private/errors.go b/iamapi/private/errors.go index 1cceb2c8..97932675 100644 --- a/iamapi/private/errors.go +++ b/iamapi/private/errors.go @@ -30,9 +30,10 @@ import ( // credential was rotated would tell the *user* their access key doesn't // exist. const ( - CodeNoSuchIdentity = "NoSuchIdentity" - CodeInvalidToken = "InvalidToken" - CodeBadRequest = "BadRequest" + CodeNoSuchIdentity = "NoSuchIdentity" + CodeInvalidToken = "InvalidToken" + CodeBadRequest = "BadRequest" + CodeProtocolMismatch = "ProtocolMismatch" ) // privateAPIError is a minimal local error for failures (like a malformed @@ -68,6 +69,18 @@ var ( } ) +// errProtocolMismatch reports that the calling gateway speaks a private +// protocol this build will not serve. Unlike the sentinels above it carries +// a message built at the call site, since which versions disagreed is the +// whole diagnosis. +func errProtocolMismatch(message string) *privateAPIError { + return &privateAPIError{ + status: http.StatusBadRequest, + code: CodeProtocolMismatch, + message: message, + } +} + // mapResolveError translates iamutil's identity-resolution sentinels into // the wire errors this protocol reports. Anything unrecognized falls through // unchanged and renders as a 500, which is the correct signal: it is a fault diff --git a/iamapi/private/handlers.go b/iamapi/private/handlers.go index adf3ecdc..d4273f5c 100644 --- a/iamapi/private/handlers.go +++ b/iamapi/private/handlers.go @@ -25,6 +25,20 @@ import ( "github.com/versity/versitygw/internal/sigv4auth" ) +// handleVersion reports what this build speaks. It is root-signed like every +// other endpoint here, which is what lets the gateway's startup probe verify +// its own credential and its mTLS transport in the same round trip that +// verifies the protocol — a rotated gateway credential is a far more common +// misconfiguration than a version skew, and an unauthenticated probe would +// report success right through one. +func (p *PrivateAPI) handleVersion(ctx fiber.Ctx) error { + return ctx.JSON(VersionResponse{ + Protocol: ProtocolVersion, + MinClient: MinClientProtocol, + ServerVersion: p.serverVersion, + }) +} + func (p *PrivateAPI) handleDeriveSigningKey(ctx fiber.Ctx) error { var req DeriveSigningKeyRequest if err := json.Unmarshal(ctx.Body(), &req); err != nil { diff --git a/iamapi/private/listener.go b/iamapi/private/listener.go index 5e56d559..437dd04c 100644 --- a/iamapi/private/listener.go +++ b/iamapi/private/listener.go @@ -19,6 +19,7 @@ import ( "time" "github.com/gofiber/fiber/v3" + "github.com/versity/versitygw/debuglogger" "github.com/versity/versitygw/internal/netutil" ) @@ -53,6 +54,7 @@ func (p *PrivateAPI) ServeMultiPort(addrs []string, tlsOpts netutil.TLSOptions) ln, err = netutil.NewMultiAddrTLSListenerWithOptions(fiber.NetworkTCP, addr, tlsOpts, netutil.ListenerOptions{SocketPerm: p.socketPerm}) } if err != nil { + closeListeners(listeners) return fmt.Errorf("failed to bind private iam listener %s: %w", addr, err) } listeners = append(listeners, ln) @@ -62,6 +64,17 @@ func (p *PrivateAPI) ServeMultiPort(addrs []string, tlsOpts netutil.TLSOptions) return p.app.Listener(finalListener, fiber.ListenConfig{DisableStartupMessage: true}) } +// closeListeners closes already bound listeners so a failed bind part way +// through ServeMultiPort does not leave the earlier addresses (and unix +// socket files) held open. +func closeListeners(listeners []net.Listener) { + for _, ln := range listeners { + if err := ln.Close(); err != nil { + debuglogger.InternalError(fmt.Errorf("close private iam listener %v: %w", ln.Addr(), err)) + } + } +} + // Shutdown gracefully stops the private endpoint listeners. func (p *PrivateAPI) Shutdown() error { return p.app.ShutdownWithTimeout(shutDownDuration) diff --git a/iamapi/private/private_test.go b/iamapi/private/private_test.go index bf424251..96ce399a 100644 --- a/iamapi/private/private_test.go +++ b/iamapi/private/private_test.go @@ -20,6 +20,8 @@ import ( "io" "net/http" "net/http/httptest" + "strconv" + "strings" "testing" "time" @@ -115,6 +117,7 @@ func doPrivateRequest(t *testing.T, p *PrivateAPI, method, target, access, secre req := httptest.NewRequest(method, target, bytes.NewReader(body)) req.Header.Set("Content-Type", "application/json") + req.Header.Set(ProtocolHeader, strconv.Itoa(ProtocolVersion)) req.ContentLength = int64(len(body)) hash := sigv4auth.PayloadSHA256Hex(body) @@ -127,6 +130,28 @@ func doPrivateRequest(t *testing.T, p *PrivateAPI, method, target, access, secre return resp } +// doPrivateRequestWithProtocol is doPrivateRequest with the protocol header +// set to an arbitrary value — including "" for a gateway build that predates +// versioning and sends none at all. +func doPrivateRequestWithProtocol(t *testing.T, p *PrivateAPI, target, protocol string, body []byte) *http.Response { + t.Helper() + + req := httptest.NewRequest(http.MethodPost, target, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + if protocol != "" { + req.Header.Set(ProtocolHeader, protocol) + } + req.ContentLength = int64(len(body)) + + signPrivateRequest(t, req, testRoot.Access, testRoot.Secret, sigv4auth.PayloadSHA256Hex(body)) + + resp, err := p.app.Test(req) + if err != nil { + t.Fatalf("app.Test: %v", err) + } + return resp +} + func readBody(t *testing.T, resp *http.Response) string { t.Helper() @@ -636,6 +661,157 @@ func TestPrivateAPIRejectsUnsignedRequest(t *testing.T) { } } +func TestPrivateAPIVersion(t *testing.T) { + store, err := storage.New(storage.Config{Dir: t.TempDir()}) + if err != nil { + t.Fatalf("storage.New: %v", err) + } + p, err := New(store, testRoot, WithPrivateServerVersion("v1.2.3")) + if err != nil { + t.Fatalf("New: %v", err) + } + + resp := doPrivateRequest(t, p, http.MethodPost, VersionPath, testRoot.Access, testRoot.Secret, []byte("{}")) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body = %s", resp.StatusCode, readBody(t, resp)) + } + + var got VersionResponse + if err := json.Unmarshal([]byte(readBody(t, resp)), &got); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got.Protocol != ProtocolVersion || got.MinClient != MinClientProtocol { + t.Errorf("VersionResponse = %+v, want protocol %d minClient %d", got, ProtocolVersion, MinClientProtocol) + } + if got.ServerVersion != "v1.2.3" { + t.Errorf("ServerVersion = %q, want %q", got.ServerVersion, "v1.2.3") + } +} + +// TestPrivateAPIVersionRequiresRootCredential confirms the version endpoint is +// authenticated like every other one here — that is what lets the gateway's +// startup probe verify its own credential in the same round trip. +func TestPrivateAPIVersionRequiresRootCredential(t *testing.T) { + p, store := newTestServer(t) + createTestUser(t, store, "alice", "AKIAALICE", "alicesecret", "") + + resp := doPrivateRequest(t, p, http.MethodPost, VersionPath, "AKIAALICE", "alicesecret", []byte("{}")) + if resp.StatusCode == http.StatusOK { + t.Fatalf("version endpoint served a non-root credential: %s", readBody(t, resp)) + } +} + +// TestPrivateAPIProtocolHeaderOnEveryResponse covers the success path, an +// application error, and an unknown route. The last two go through +// errorHandler, which must not drop the header — a mismatch response that +// carries no version is the one response an operator most needs it on. +func TestPrivateAPIProtocolHeaderOnEveryResponse(t *testing.T) { + p, store := newTestServer(t) + createTestUser(t, store, "alice", "AKIAALICE", "alicesecret", "") + + for _, tc := range []struct { + name string + path string + body string + }{ + {"success", ResolveIdentityPath, `{"accessKeyIds":["AKIAALICE"]}`}, + {"application error", DerivePath, "not json"}, + {"unknown route", "/private/nope", "{}"}, + } { + t.Run(tc.name, func(t *testing.T) { + resp := doPrivateRequest(t, p, http.MethodPost, tc.path, testRoot.Access, testRoot.Secret, []byte(tc.body)) + if got := resp.Header.Get(ProtocolHeader); got != strconv.Itoa(ProtocolVersion) { + t.Errorf("%s = %q, want %q", ProtocolHeader, got, strconv.Itoa(ProtocolVersion)) + } + }) + } +} + +// TestPrivateAPIRejectsIncompatibleClientProtocol covers every request-header +// value this build refuses. A gateway too old to be served safely, and one +// whose version cannot be read at all, are both refused with a code the +// gateway dispatches on — never served on an assumed version. +func TestPrivateAPIRejectsIncompatibleClientProtocol(t *testing.T) { + p, store := newTestServer(t) + createTestUser(t, store, "alice", "AKIAALICE", "alicesecret", "") + + for _, tc := range []struct { + name string + protocol string + }{ + {"absent", ""}, + {"empty", " "}, + {"not a number", "one"}, + {"signed", "+1"}, + {"zero", "0"}, + {"absurdly long", "11111111111111111111"}, + } { + t.Run(tc.name, func(t *testing.T) { + resp := doPrivateRequestWithProtocol(t, p, ResolveIdentityPath, tc.protocol, []byte(`{"accessKeyIds":["AKIAALICE"]}`)) + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status = %d, want %d; body = %s", resp.StatusCode, http.StatusBadRequest, readBody(t, resp)) + } + body := readBody(t, resp) + if !strings.Contains(body, CodeProtocolMismatch) { + t.Errorf("body = %s, want code %s", body, CodeProtocolMismatch) + } + if got := resp.Header.Get(ProtocolHeader); got != strconv.Itoa(ProtocolVersion) { + t.Errorf("%s = %q, want the refusing build's own version", ProtocolHeader, got) + } + }) + } +} + +// TestPrivateAPIVersionExemptFromClientProtocolCheck confirms the version +// endpoint answers a gateway this build would otherwise refuse. Without it, a +// future service that raised MinClientProtocol could not tell an older gateway +// why it was being turned away. +func TestPrivateAPIVersionExemptFromClientProtocolCheck(t *testing.T) { + p, _ := newTestServer(t) + + resp := doPrivateRequestWithProtocol(t, p, VersionPath, "", []byte("{}")) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", resp.StatusCode, readBody(t, resp)) + } + if got := resp.Header.Get(ProtocolHeader); got != strconv.Itoa(ProtocolVersion) { + t.Errorf("%s = %q, want %q", ProtocolHeader, got, strconv.Itoa(ProtocolVersion)) + } +} + +func TestParseProtocolVersion(t *testing.T) { + for _, tc := range []struct { + value string + want int + }{ + {"1", 1}, + {"2", 2}, + {"1000", 1000}, + {"", 0}, + {" 1", 0}, + {"1 ", 0}, + {"+1", 0}, + {"-1", 0}, + {"0", 0}, + {"1.0", 0}, + {"v1", 0}, + {"99999", 0}, + } { + got, err := ParseProtocolVersion(tc.value) + if tc.want == 0 { + if err == nil { + t.Errorf("ParseProtocolVersion(%q) = %d, want an error", tc.value, got) + } + continue + } + if err != nil { + t.Errorf("ParseProtocolVersion(%q): %v", tc.value, err) + } + if got != tc.want { + t.Errorf("ParseProtocolVersion(%q) = %d, want %d", tc.value, got, tc.want) + } + } +} + // createTestRole creates a role with an optional inline permission policy // directly against store, the same way createTestUser bypasses the // control-plane API. Arn and RoleID are set explicitly because diff --git a/iamapi/private/server.go b/iamapi/private/server.go index 6b465007..b035c4a7 100644 --- a/iamapi/private/server.go +++ b/iamapi/private/server.go @@ -29,6 +29,7 @@ package private import ( "fmt" "os" + "strconv" "github.com/gofiber/fiber/v3" "github.com/gofiber/fiber/v3/middleware/logger" @@ -44,6 +45,39 @@ const ( DerivePath = "/private/derive-signing-key" EvaluatePath = "/private/evaluate-policy" ResolveIdentityPath = "/private/resolve-identity" + VersionPath = "/private/version" + + // ProtocolHeader carries the private protocol version each peer speaks. + // Both send it: the S3 gateway on every request, this service on every + // response, including error responses. + ProtocolHeader = "X-Vgw-Private-Protocol" + + // ProtocolVersion is the private protocol version this build speaks, and + // MinClientProtocol the oldest S3 gateway it will serve. Together they + // express compatibility in both directions, since a skew can be unsafe + // from either side: + // + // - Bump ProtocolVersion when the gateway starts relying on something + // an older service would silently ignore — a new request field, or a + // new endpoint. An unrecognized field is dropped by json.Unmarshal, + // so an older service evaluating without one (Condition being the + // worked example) is fail-open. The gateway catches this itself by + // refusing a service older than the version it speaks. + // + // - Bump both when this service starts returning something an older + // gateway must understand to stay fail-closed — a new deny dimension, + // or a decision matrix that narrows an Allow, as HasSessionPolicy/ + // SessionDecisions would have been had they landed later. An older + // gateway cannot detect this on its own: it does not know the field + // exists. This service refuses it instead. + // + // - Bump neither for an addition an older gateway can safely ignore. + // + // Changing an existing field's meaning in place is not a bump; it is a + // new route. The operational rule that falls out of all this: upgrade + // the IAM service before the gateways. + ProtocolVersion = 1 + MinClientProtocol = 1 // privateService is the SigV4 credential-scope service name the S3 // gateway signs its own requests to these endpoints with. It's an @@ -55,10 +89,11 @@ const ( // PrivateAPI is the standalone IAM service's private endpoint set type PrivateAPI struct { - app *fiber.App - store storage.Storer - socketPerm os.FileMode - quiet bool + app *fiber.App + store storage.Storer + socketPerm os.FileMode + quiet bool + serverVersion string } type PrivateAPIOption func(*PrivateAPI) @@ -77,6 +112,14 @@ func WithPrivateQuiet() PrivateAPIOption { return func(p *PrivateAPI) { p.quiet = true } } +// WithPrivateServerVersion sets the build version the version endpoint +// reports. It is what lets an operator map a protocol number back to an +// image, so it is worth passing even though nothing decides compatibility +// on it. +func WithPrivateServerVersion(version string) PrivateAPIOption { + return func(p *PrivateAPI) { p.serverVersion = version } +} + // New constructs the private endpoint set. root is the identity these // endpoints authenticate every request against. func New(store storage.Storer, root iammiddleware.RootCredentials, opts ...PrivateAPIOption) (*PrivateAPI, error) { @@ -102,7 +145,10 @@ func New(store storage.Storer, root iammiddleware.RootCredentials, opts ...Priva })) } + app.Use("*", p.checkProtocolVersion) + rootAuth := iammiddleware.VerifyRootOnlySigV4(privateService, &root) + app.Post(VersionPath, chainHandlers(rootAuth, p.handleVersion)) app.Post(DerivePath, chainHandlers(rootAuth, p.handleDeriveSigningKey)) app.Post(EvaluatePath, chainHandlers(rootAuth, p.handleEvaluatePolicy)) app.Post(ResolveIdentityPath, chainHandlers(rootAuth, p.handleResolveIdentity)) @@ -122,3 +168,68 @@ func chainHandlers(handlers ...fiber.Handler) fiber.Handler { return nil } } + +// checkProtocolVersion answers every response with this build's protocol +// version and refuses a gateway too old for it to serve safely. +// +// It runs before root authentication so a version refusal is reported and +// logged as exactly that, rather than as a misleading 403 about the +// gateway's credential. The only thing that discloses to an unauthenticated +// peer is the protocol version, which the response header carries either +// way, on a listener that is already mTLS- or unix-socket-only. +func (p *PrivateAPI) checkProtocolVersion(ctx fiber.Ctx) error { + ctx.Set(ProtocolHeader, strconv.Itoa(ProtocolVersion)) + + // The version endpoint answers even a gateway this build refuses to + // serve: it is how that gateway finds out what it is talking to. Keyed + // on the path rather than on registration order, since a route + // registered ahead of this middleware would not get the header set + // above either. + if ctx.Path() == VersionPath { + return ctx.Next() + } + + client, err := ParseProtocolVersion(ctx.Get(ProtocolHeader)) + if err != nil { + return errProtocolMismatch(err.Error()) + } + if client < MinClientProtocol { + return errProtocolMismatch(fmt.Sprintf( + "gateway speaks private protocol %d, this IAM service requires %d or newer: upgrade the gateway", + client, MinClientProtocol)) + } + + return ctx.Next() +} + +// ParseProtocolVersion reads a ProtocolHeader value. It is shared by both +// peers so they agree on what the header means, and is deliberately strict: +// a value it cannot read is a mismatch, never a default. Treating an absent +// or unreadable version as some assumed one is the fail-open direction for +// the check everything else is gated on. +func ParseProtocolVersion(value string) (int, error) { + if value == "" { + return 0, fmt.Errorf("no %s header", ProtocolHeader) + } + // Bounded before it is echoed into an error: this value is attacker- + // controlled and ends up in a log line. + if len(value) > maxProtocolDigits { + return 0, fmt.Errorf("malformed %s header", ProtocolHeader) + } + // Digits only. Atoi by itself would also accept a leading sign, which is + // not a version. + for i := 0; i < len(value); i++ { + if value[i] < '0' || value[i] > '9' { + return 0, fmt.Errorf("malformed %s header %q", ProtocolHeader, value) + } + } + version, err := strconv.Atoi(value) + if err != nil || version < 1 { + return 0, fmt.Errorf("malformed %s header %q", ProtocolHeader, value) + } + return version, nil +} + +// maxProtocolDigits bounds a protocol version's wire length, so an +// arbitrarily long header value never reaches a log line. +const maxProtocolDigits = 4 diff --git a/iamapi/private/types.go b/iamapi/private/types.go index 2b438ec5..4a1cf298 100644 --- a/iamapi/private/types.go +++ b/iamapi/private/types.go @@ -112,3 +112,17 @@ type EvaluatePolicyResponse struct { HasSessionPolicy bool `json:"hasSessionPolicy,omitempty"` PrincipalArn string `json:"principalArn,omitempty"` } + +// VersionResponse is the version endpoint's body. +// +// MinClient is load-bearing, and this is the only place it appears: the +// version endpoint is exempt from the service's own client-version check so +// that it can answer a gateway the service will not serve, which leaves the +// gateway to draw that conclusion itself from this field. Protocol duplicates +// the ProtocolHeader every response carries, and ServerVersion is the build +// tag — what maps a protocol number back to an image during a rollout. +type VersionResponse struct { + Protocol int `json:"protocol"` + MinClient int `json:"minClient"` + ServerVersion string `json:"serverVersion,omitempty"` +} diff --git a/iamapi/server.go b/iamapi/server.go index a2dabad0..c89c6c2f 100644 --- a/iamapi/server.go +++ b/iamapi/server.go @@ -188,6 +188,7 @@ func (s *IAMApiServer) ServeMultiPort(ports []string) error { ln, err = netutil.NewMultiAddrListener(fiber.NetworkTCP, portSpec, netutil.ListenerOptions{SocketPerm: s.socketPerm}) } if err != nil { + closeListeners(listeners) return fmt.Errorf("failed to bind iam listener %s: %w", portSpec, err) } @@ -213,6 +214,17 @@ func (s *IAMApiServer) ServeMultiPort(ports []string) error { }) } +// closeListeners closes already bound listeners so a failed bind part way +// through ServeMultiPort does not leave the earlier ports (and unix socket +// files) held open. +func closeListeners(listeners []net.Listener) { + for _, ln := range listeners { + if err := ln.Close(); err != nil { + debuglogger.InternalError(fmt.Errorf("close iam listener %v: %w", ln.Addr(), err)) + } + } +} + func (s *IAMApiServer) Shutdown() error { return s.app.ShutdownWithTimeout(shutDownDuration) } diff --git a/tests/integration/presigned_urls.go b/tests/integration/presigned_urls.go index 03791918..ae59a005 100644 --- a/tests/integration/presigned_urls.go +++ b/tests/integration/presigned_urls.go @@ -825,6 +825,8 @@ func PresignedAuth_Put_GetObject_with_data(s *S3Conf) error { return err } + req.Header = v4GetReq.SignedHeader + resp, err = s.httpClient.Do(req) if err != nil { return err @@ -892,6 +894,8 @@ func PresignedAuth_Put_GetObject_with_UTF8_chars(s *S3Conf) error { return err } + req.Header = v4GetReq.SignedHeader + resp, err = s.httpClient.Do(req) if err != nil { return err diff --git a/tests/integration/s3_iam_session_access_control.go b/tests/integration/s3_iam_session_access_control.go index 2e670229..1b2a5da6 100644 --- a/tests/integration/s3_iam_session_access_control.go +++ b/tests/integration/s3_iam_session_access_control.go @@ -17,6 +17,7 @@ package integration import ( "context" "fmt" + "net/http" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -497,7 +498,13 @@ func S3IAMSession_presigned_url_with_session_credentials(s *S3Conf) error { return fmt.Errorf("expected the presigned URL to carry X-Amz-Security-Token") } - resp, err := s.httpClient.Get(presigned.URL) + req, err := http.NewRequest(presigned.Method, presigned.URL, nil) + if err != nil { + return err + } + req.Header = presigned.SignedHeader + + resp, err := s.httpClient.Do(req) if err != nil { return err }