From 881bed56c8e228d0d61e7e850654cc27527f225a Mon Sep 17 00:00:00 2001 From: Nishad Mathur Date: Thu, 8 Oct 2026 16:32:16 -0700 Subject: [PATCH 1/4] Accept salted HMAC-SHA256 verifiers for HTTPS mbus auth bosh create-env passes the Director agent's mbus URL, including its Basic Auth password, to the CPI. The CPI writes it into IaaS VM metadata (vSphere guestinfo, AWS/GCP/Azure user data), where anyone with read access to that metadata can see it. That password is never rotated. Let the configured mbus password be a verifier instead: bosh-hmac-sha256$$ salt is 16 random bytes, and hmac is HMAC-SHA256(key=salt, msg=password). Both are unpadded URL-safe base64, so the verifier needs no escaping in URL userinfo. HMAC-SHA256 is FIPS-approved, and it is enough here because the passwords are machine-generated with at least ~100 bits of entropy. The hashing and verification code lives in a new stdlib-only package, agentpassword, so bosh-cli can import it to produce verifiers without pulling in agent dependencies. When the configured password has the verifier prefix, the agent parses it at startup and fails to start if it is malformed. Requests must then carry Basic Auth with a matching username and a password that verifies. Plaintext passwords keep the existing whole-header constant-time comparison, so older CLIs and existing environments are unaffected. ai-assisted=yes TNZ-129825 Co-Authored-By: Chris Selzo Co-Authored-By: Claude Opus 5.5 --- agentpassword/agentpassword.go | 93 +++++++++ agentpassword/agentpassword_suite_test.go | 13 ++ agentpassword/agentpassword_test.go | 221 ++++++++++++++++++++++ mbus/handler_provider.go | 6 +- mbus/handler_provider_test.go | 6 +- mbus/https_dispatcher.go | 69 ++++--- mbus/https_dispatcher_auth_test.go | 145 ++++++++++++++ mbus/https_dispatcher_test.go | 3 +- mbus/https_handler.go | 10 +- mbus/https_handler_test.go | 3 +- 10 files changed, 534 insertions(+), 35 deletions(-) create mode 100644 agentpassword/agentpassword.go create mode 100644 agentpassword/agentpassword_suite_test.go create mode 100644 agentpassword/agentpassword_test.go create mode 100644 mbus/https_dispatcher_auth_test.go diff --git a/agentpassword/agentpassword.go b/agentpassword/agentpassword.go new file mode 100644 index 000000000..c8b25c2b0 --- /dev/null +++ b/agentpassword/agentpassword.go @@ -0,0 +1,93 @@ +package agentpassword + +import ( + "crypto/hmac" + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "errors" + "net/url" + "strings" +) + +const ( + Feature = "http-password-hmac-sha256" + + prefix = "bosh-hmac-sha256$" + saltLength = 16 + keyLength = 32 +) + +type Verifier struct { + salt []byte + key []byte +} + +func IsVerifier(password string) bool { + return strings.HasPrefix(password, prefix) +} + +func ParseVerifier(s string) (*Verifier, error) { + if !strings.HasPrefix(s, prefix) { + return nil, errors.New("malformed HTTP password verifier: missing prefix") + } + parts := strings.Split(s, "$") + if len(parts) != 3 { + return nil, errors.New("malformed HTTP password verifier: invalid part count") + } + salt, err := base64.RawURLEncoding.Strict().DecodeString(parts[1]) + if err != nil || len(salt) != saltLength { + return nil, errors.New("malformed HTTP password verifier: invalid salt") + } + key, err := base64.RawURLEncoding.Strict().DecodeString(parts[2]) + if err != nil || len(key) != keyLength { + return nil, errors.New("malformed HTTP password verifier: invalid key") + } + return &Verifier{salt: salt, key: key}, nil +} + +func (v *Verifier) Matches(password string) bool { + if v == nil { + return false + } + mac := hmac.New(sha256.New, v.salt) + _, _ = mac.Write([]byte(password)) + return subtle.ConstantTimeCompare(v.key, mac.Sum(nil)) == 1 +} + +func HashURL(raw string) (string, error) { + u, err := url.Parse(raw) + if err != nil { + return "", errors.New("invalid URL") + } + if u.Scheme != "https" { + return raw, nil + } + if u.User == nil { + return "", errors.New("URL missing userinfo") + } + password, hasPassword := u.User.Password() + if !hasPassword || password == "" { + return "", errors.New("URL missing password") + } + if IsVerifier(password) { + return "", errors.New("URL password is already a verifier") + } + + salt := make([]byte, saltLength) + if _, err := rand.Read(salt); err != nil { + return "", errors.New("failed to generate random salt") + } + + mac := hmac.New(sha256.New, salt) + _, _ = mac.Write([]byte(password)) + key := mac.Sum(nil) + + encodedSalt := base64.RawURLEncoding.EncodeToString(salt) + encodedKey := base64.RawURLEncoding.EncodeToString(key) + verifier := prefix + encodedSalt + "$" + encodedKey + + u.User = url.UserPassword(u.User.Username(), verifier) + return u.String(), nil +} diff --git a/agentpassword/agentpassword_suite_test.go b/agentpassword/agentpassword_suite_test.go new file mode 100644 index 000000000..860c339da --- /dev/null +++ b/agentpassword/agentpassword_suite_test.go @@ -0,0 +1,13 @@ +package agentpassword_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestAgentPassword(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Agent Password Suite") +} diff --git a/agentpassword/agentpassword_test.go b/agentpassword/agentpassword_test.go new file mode 100644 index 000000000..94acbeb0a --- /dev/null +++ b/agentpassword/agentpassword_test.go @@ -0,0 +1,221 @@ +package agentpassword_test + +import ( + "encoding/base64" + "net/url" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/cloudfoundry/bosh-agent/v2/agentpassword" +) + +var _ = Describe("Agentpassword", func() { + Describe("Feature constant", func() { + It("exposes the expected feature identifier", func() { + Expect(agentpassword.Feature).To(Equal("http-password-hmac-sha256")) + }) + }) + + Describe("IsVerifier", func() { + It("returns true for verifiers and malformed verifiers with the prefix", func() { + Expect(agentpassword.IsVerifier("bosh-hmac-sha256$bad")).To(BeTrue()) + Expect(agentpassword.IsVerifier("bosh-hmac-sha256$AAECAwQFBgcICQoLDA0ODw$EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw")).To(BeTrue()) + }) + + It("returns false when the prefix is missing or incomplete", func() { + Expect(agentpassword.IsVerifier("plain-password")).To(BeFalse()) + Expect(agentpassword.IsVerifier("")).To(BeFalse()) + Expect(agentpassword.IsVerifier("bosh-hmac-sha256")).To(BeFalse()) + Expect(agentpassword.IsVerifier("other-prefix$secret")).To(BeFalse()) + }) + }) + + Describe("ParseVerifier and Matches", func() { + It("matches a fixed known-answer vector guarding the format", func() { + // salt: bytes 0..15 -> base64url "AAECAwQFBgcICQoLDA0ODw" + // password: "correct horse battery staple" + // HMAC-SHA256(key=salt, msg=password) -> base64url "EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw" + const knownVector = "bosh-hmac-sha256$AAECAwQFBgcICQoLDA0ODw$EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw" + v, err := agentpassword.ParseVerifier(knownVector) + Expect(err).NotTo(HaveOccurred()) + Expect(v).NotTo(BeNil()) + Expect(v.Matches("correct horse battery staple")).To(BeTrue()) + Expect(v.Matches("wrong-password")).To(BeFalse()) + }) + + It("rejects malformed verifiers", func() { + validSalt := base64.RawURLEncoding.EncodeToString(make([]byte, 16)) + validKey := base64.RawURLEncoding.EncodeToString(make([]byte, 32)) + + // Missing prefix + for _, missingPrefix := range []string{ + "", + "invalid-prefix$" + validSalt + "$" + validKey, + "bosh-hmac-sha256", + } { + v, err := agentpassword.ParseVerifier(missingPrefix) + Expect(err).To(HaveOccurred(), "expected error for missing prefix: %s", missingPrefix) + Expect(v).To(BeNil()) + } + + // Bad part count + for _, badParts := range []string{ + "bosh-hmac-sha256$", + "bosh-hmac-sha256$" + validSalt, + "bosh-hmac-sha256$" + validSalt + "$" + validKey + "$extra", + "bosh-hmac-sha256$" + validSalt + "$" + validKey + "$extra$more", + } { + v, err := agentpassword.ParseVerifier(badParts) + Expect(err).To(HaveOccurred(), "expected error for bad part count: %s", badParts) + Expect(v).To(BeNil()) + } + + // Bad base64 + for _, badB64 := range []string{ + "bosh-hmac-sha256$invalid!salt$" + validKey, + "bosh-hmac-sha256$" + validSalt + "$invalid!key", + } { + v, err := agentpassword.ParseVerifier(badB64) + Expect(err).To(HaveOccurred(), "expected error for bad base64: %s", badB64) + Expect(v).To(BeNil()) + } + + // Std base64 alphabet (+ and /) rejected by RawURLEncoding.Strict() + for _, stdAlphabet := range []string{ + // PR #487 vector used std base64 '+' in the key + "bosh-hmac-sha256$AAECAwQFBgcICQoLDA0ODw$EEqYVo55l7MGfPvwVPSRCq7RN6JXw+TBbKyW9De5Grw", + // Salt containing '/' + "bosh-hmac-sha256$AAECAwQFBgcICQoLDA0OD/$EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw", + } { + v, err := agentpassword.ParseVerifier(stdAlphabet) + Expect(err).To(HaveOccurred(), "expected error for std base64 alphabet: %s", stdAlphabet) + Expect(v).To(BeNil()) + } + + // Wrong salt length (must be 16 bytes) + shortSalt := base64.RawURLEncoding.EncodeToString(make([]byte, 15)) + longSalt := base64.RawURLEncoding.EncodeToString(make([]byte, 17)) + for _, badSalt := range []string{ + "bosh-hmac-sha256$" + shortSalt + "$" + validKey, + "bosh-hmac-sha256$" + longSalt + "$" + validKey, + } { + v, err := agentpassword.ParseVerifier(badSalt) + Expect(err).To(HaveOccurred(), "expected error for wrong salt length: %s", badSalt) + Expect(v).To(BeNil()) + } + + // Wrong key length (must be 32 bytes) + shortKey := base64.RawURLEncoding.EncodeToString(make([]byte, 31)) + longKey := base64.RawURLEncoding.EncodeToString(make([]byte, 33)) + for _, badKey := range []string{ + "bosh-hmac-sha256$" + validSalt + "$" + shortKey, + "bosh-hmac-sha256$" + validSalt + "$" + longKey, + } { + v, err := agentpassword.ParseVerifier(badKey) + Expect(err).To(HaveOccurred(), "expected error for wrong key length: %s", badKey) + Expect(v).To(BeNil()) + } + }) + }) + + Describe("HashURL", func() { + It("performs a complete HashURL -> ParseVerifier -> Matches round trip", func() { + raw := "https://vcap:mypassword@127.0.0.1:6868/path?foo=bar#frag" + hashed, err := agentpassword.HashURL(raw) + Expect(err).NotTo(HaveOccurred()) + + parsedURL, err := url.Parse(hashed) + Expect(err).NotTo(HaveOccurred()) + Expect(parsedURL.Scheme).To(Equal("https")) + Expect(parsedURL.Host).To(Equal("127.0.0.1:6868")) + Expect(parsedURL.Path).To(Equal("/path")) + Expect(parsedURL.User.Username()).To(Equal("vcap")) + + pw, ok := parsedURL.User.Password() + Expect(ok).To(BeTrue()) + + v, err := agentpassword.ParseVerifier(pw) + Expect(err).NotTo(HaveOccurred()) + Expect(v.Matches("mypassword")).To(BeTrue()) + Expect(v.Matches("wrong-password")).To(BeFalse()) + }) + + It("produces different verifiers on two HashURL calls on the same input due to fresh salts", func() { + raw := "https://vcap:mypassword@127.0.0.1:6868" + h1, err := agentpassword.HashURL(raw) + Expect(err).NotTo(HaveOccurred()) + h2, err := agentpassword.HashURL(raw) + Expect(err).NotTo(HaveOccurred()) + + Expect(h1).NotTo(Equal(h2)) + }) + + It("ensures the password component contains only [A-Za-z0-9_$-] and appears unescaped in the returned string", func() { + raw := "https://vcap:plain-password-with-chars@127.0.0.1:6868" + hashed, err := agentpassword.HashURL(raw) + Expect(err).NotTo(HaveOccurred()) + + parsedURL, err := url.Parse(hashed) + Expect(err).NotTo(HaveOccurred()) + pw, ok := parsedURL.User.Password() + Expect(ok).To(BeTrue()) + + Expect(pw).To(MatchRegexp(`^[A-Za-z0-9_$-]+$`)) + // Verifier appears unescaped in the returned string + Expect(hashed).To(ContainSubstring(":" + pw + "@")) + }) + + It("passes non-https URLs through unchanged with nil error", func() { + for _, nonHTTPS := range []string{ + "http://vcap:secret@127.0.0.1:6868", + "nats://user:pass@127.0.0.1:4222", + "custom://user:pass@127.0.0.1:8080/path", + "", + } { + result, err := agentpassword.HashURL(nonHTTPS) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(nonHTTPS)) + } + }) + + It("returns an error and does not echo the password for every HashURL error case", func() { + const secret = "very-secret-password-12345" + + testCases := []struct { + name string + raw string + }{ + { + name: "unparseable URL with control character", + raw: "https://vcap:" + secret + "@127.0.0.1:6868\n/invalid", + }, + { + name: "missing userinfo", + raw: "https://127.0.0.1:6868", + }, + { + name: "absent password", + raw: "https://vcap@127.0.0.1:6868", + }, + { + name: "empty password", + raw: "https://vcap:@127.0.0.1:6868", + }, + { + name: "password already has verifier prefix", + raw: "https://vcap:bosh-hmac-sha256$" + secret + "@127.0.0.1:6868", + }, + } + + for _, tc := range testCases { + By(tc.name) + hashed, err := agentpassword.HashURL(tc.raw) + Expect(err).To(HaveOccurred()) + Expect(hashed).To(BeEmpty()) + Expect(err.Error()).NotTo(ContainSubstring(secret)) + Expect(err.Error()).NotTo(ContainSubstring("bosh-hmac-sha256$")) + } + }) + }) +}) diff --git a/mbus/handler_provider.go b/mbus/handler_provider.go index 42e7b9bcb..be5844049 100644 --- a/mbus/handler_provider.go +++ b/mbus/handler_provider.go @@ -53,7 +53,11 @@ func (p HandlerProvider) Get( return NewNatsHandler(p.settingsService, f, p.logger, platform), nil case "https": mbusKeyPair := p.settingsService.GetSettings().GetMbusCerts() - return NewHTTPSHandler(mbusURL, mbusKeyPair, blobManager, p.logger, p.auditLogger), nil + handler, err := NewHTTPSHandler(mbusURL, mbusKeyPair, blobManager, p.logger, p.auditLogger) + if err != nil { + return nil, err + } + return handler, nil default: return nil, bosherr.Errorf("Message Bus Handler with scheme %s could not be found", mbusURL.Scheme) } diff --git a/mbus/handler_provider_test.go b/mbus/handler_provider_test.go index 863bb0fe0..b55d9dc45 100644 --- a/mbus/handler_provider_test.go +++ b/mbus/handler_provider_test.go @@ -58,7 +58,8 @@ var _ = Describe("HandlerProvider", func() { settingsService.Settings.Mbus = "https://foo:bar@lol" handler, err := provider.Get(platform, blobManager) Expect(err).ToNot(HaveOccurred()) - expectedHandler := mbus.NewHTTPSHandler(mbusURL, settings.CertKeyPair{}, blobManager, logger, auditLogger) + expectedHandler, constructorErr := mbus.NewHTTPSHandler(mbusURL, settings.CertKeyPair{}, blobManager, logger, auditLogger) + Expect(constructorErr).NotTo(HaveOccurred()) httpsHandler, ok := handler.(mbus.HTTPSHandler) Expect(ok).To(BeTrue()) Expect(httpsHandler).To(Equal(expectedHandler)) @@ -73,13 +74,14 @@ var _ = Describe("HandlerProvider", func() { settingsService.Settings.Env.Bosh.Mbus.Cert.PrivateKey = "private-key-pem-block" handler, err := provider.Get(platform, blobManager) - expectedHandler := mbus.NewHTTPSHandler( + expectedHandler, constructorErr := mbus.NewHTTPSHandler( mbusURL, settingsService.Settings.Env.Bosh.Mbus.Cert, blobManager, logger, auditLogger, ) + Expect(constructorErr).NotTo(HaveOccurred()) Expect(err).NotTo(HaveOccurred()) httpsHandler, ok := handler.(mbus.HTTPSHandler) Expect(ok).To(BeTrue()) diff --git a/mbus/https_dispatcher.go b/mbus/https_dispatcher.go index 6f152da2e..60d62ea78 100644 --- a/mbus/https_dispatcher.go +++ b/mbus/https_dispatcher.go @@ -12,26 +12,41 @@ import ( bosherr "github.com/cloudfoundry/bosh-utils/errors" boshlog "github.com/cloudfoundry/bosh-utils/logger" + "github.com/cloudfoundry/bosh-agent/v2/agentpassword" "github.com/cloudfoundry/bosh-agent/v2/settings" tlsconfig "code.cloudfoundry.org/tlsconfig" ) -const httpsDispatcherLogTag = "HTTPS Dispatcher" +const ( + httpsDispatcherLogTag = "HTTPS Dispatcher" + maxHTTPPasswordLength = 1024 +) type HTTPSDispatcher struct { - httpServer *http.Server - mux *http.ServeMux - keyPair settings.CertKeyPair - listener net.Listener - logger boshlog.Logger - baseURL *url.URL - expectedAuthorizationHeader string + httpServer *http.Server + mux *http.ServeMux + keyPair settings.CertKeyPair + listener net.Listener + logger boshlog.Logger + baseURL *url.URL + passwordVerifier *agentpassword.Verifier + expectedAuthorization string } type HTTPHandlerFunc func(writer http.ResponseWriter, request *http.Request) -func NewHTTPSDispatcher(baseURL *url.URL, keyPair settings.CertKeyPair, logger boshlog.Logger) *HTTPSDispatcher { +func NewHTTPSDispatcher(baseURL *url.URL, keyPair settings.CertKeyPair, logger boshlog.Logger) (*HTTPSDispatcher, error) { + password, _ := baseURL.User.Password() + var verifier *agentpassword.Verifier + var err error + if agentpassword.IsVerifier(password) { + verifier, err = agentpassword.ParseVerifier(password) + if err != nil { + return nil, bosherr.WrapError(err, "Configuring HTTP authentication") + } + } + tlsConfig, _ := tlsconfig.Build(tlsconfig.WithInternalServiceDefaults()).Server() //nolint:errcheck httpServer := &http.Server{ @@ -40,20 +55,18 @@ func NewHTTPSDispatcher(baseURL *url.URL, keyPair settings.CertKeyPair, logger b mux := http.NewServeMux() httpServer.Handler = mux - expectedUsername := baseURL.User.Username() - expectedPassword, _ := baseURL.User.Password() - auth := fmt.Sprintf("%s:%s", expectedUsername, expectedPassword) + auth := fmt.Sprintf("%s:%s", baseURL.User.Username(), password) encodedAuth := base64.StdEncoding.EncodeToString([]byte(auth)) - expectedAuthorizationHeader := fmt.Sprintf("Basic %s", encodedAuth) return &HTTPSDispatcher{ - httpServer: httpServer, - mux: mux, - keyPair: keyPair, - logger: logger, - baseURL: baseURL, - expectedAuthorizationHeader: expectedAuthorizationHeader, - } + httpServer: httpServer, + mux: mux, + keyPair: keyPair, + logger: logger, + baseURL: baseURL, + passwordVerifier: verifier, + expectedAuthorization: fmt.Sprintf("Basic %s", encodedAuth), + }, nil } func (h *HTTPSDispatcher) Start() error { @@ -86,12 +99,15 @@ func (h *HTTPSDispatcher) Stop() { } } +// requestNotAuthorized checks HTTP credentials; cryptographic verification lives +// in agentpassword.Verifier and is safe for simultaneous requests. func (h *HTTPSDispatcher) requestNotAuthorized(request *http.Request) bool { - return h.constantTimeEquals(h.expectedAuthorizationHeader, request.Header.Get("Authorization")) -} - -func (h *HTTPSDispatcher) constantTimeEquals(a, b string) bool { - return subtle.ConstantTimeCompare([]byte(a), []byte(b)) != 1 + if h.passwordVerifier == nil { + return subtle.ConstantTimeCompare([]byte(h.expectedAuthorization), []byte(request.Header.Get("Authorization"))) != 1 + } + username, password, ok := request.BasicAuth() + return !ok || subtle.ConstantTimeCompare([]byte(h.baseURL.User.Username()), []byte(username)) != 1 || + len(password) == 0 || len(password) > maxHTTPPasswordLength || !h.passwordVerifier.Matches(password) } func (h *HTTPSDispatcher) AddRoute(route string, handler HTTPHandlerFunc) { @@ -100,10 +116,9 @@ func (h *HTTPSDispatcher) AddRoute(route string, handler HTTPHandlerFunc) { if h.requestNotAuthorized(r) { w.Header().Add("WWW-Authenticate", `Basic realm=""`) - w.WriteHeader(401) + w.WriteHeader(http.StatusUnauthorized) return } - handler(w, r) } diff --git a/mbus/https_dispatcher_auth_test.go b/mbus/https_dispatcher_auth_test.go new file mode 100644 index 000000000..9f8d2aaf8 --- /dev/null +++ b/mbus/https_dispatcher_auth_test.go @@ -0,0 +1,145 @@ +package mbus + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/cloudfoundry/bosh-agent/v2/agentpassword" + "github.com/cloudfoundry/bosh-agent/v2/settings" + boshlog "github.com/cloudfoundry/bosh-utils/logger" +) + +var _ = Describe("HTTPS dispatcher authentication", func() { + const password = "correct-horse-battery-staple" + + Context("when configured with an agentpassword verifier URL", func() { + var dispatcher *HTTPSDispatcher + + BeforeEach(func() { + rawURL := "https://vcap:" + password + "@127.0.0.1:6868" + hashedURL, err := agentpassword.HashURL(rawURL) + Expect(err).NotTo(HaveOccurred()) + + u, err := url.Parse(hashedURL) + Expect(err).NotTo(HaveOccurred()) + + dispatcher, err = NewHTTPSDispatcher(u, settings.CertKeyPair{}, boshlog.NewLogger(boshlog.LevelNone)) + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns 200/ok on /agent and /blobs/x with the correct password", func() { + for _, path := range []string{"/agent", "/blobs/test-blob"} { + dispatcher.AddRoute(path, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + req := httptest.NewRequest(http.MethodPost, "https://localhost"+path, nil) + req.SetBasicAuth("vcap", password) + rec := httptest.NewRecorder() + dispatcher.mux.ServeHTTP(rec, req) + + Expect(rec.Code).To(Equal(http.StatusOK)) + } + }) + + It("returns 401 for wrong password, empty password, wrong username, and password longer than 1024", func() { + dispatcher.AddRoute("/agent", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + testCases := []struct { + name string + username string + password string + }{ + {"wrong password", "vcap", "incorrect-password"}, + {"empty password", "vcap", ""}, + {"wrong username", "wrong-user", password}, + {"password longer than 1024", "vcap", strings.Repeat("x", 1025)}, + } + + for _, tc := range testCases { + By(tc.name) + req := httptest.NewRequest(http.MethodPost, "https://localhost/agent", nil) + req.SetBasicAuth(tc.username, tc.password) + rec := httptest.NewRecorder() + dispatcher.mux.ServeHTTP(rec, req) + + Expect(rec.Code).To(Equal(http.StatusUnauthorized)) + } + }) + + It("returns 401 and WWW-Authenticate for requests with missing or non-Basic Authorization header", func() { + dispatcher.AddRoute("/agent", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + // Request with no Authorization header + reqNoAuth := httptest.NewRequest(http.MethodPost, "https://localhost/agent", nil) + recNoAuth := httptest.NewRecorder() + dispatcher.mux.ServeHTTP(recNoAuth, reqNoAuth) + + Expect(recNoAuth.Code).To(Equal(http.StatusUnauthorized)) + Expect(recNoAuth.Header().Get("WWW-Authenticate")).To(Equal(`Basic realm=""`)) + + // Request with non-Basic Authorization header + reqBearer := httptest.NewRequest(http.MethodPost, "https://localhost/agent", nil) + reqBearer.Header.Set("Authorization", "Bearer x") + recBearer := httptest.NewRecorder() + dispatcher.mux.ServeHTTP(recBearer, reqBearer) + + Expect(recBearer.Code).To(Equal(http.StatusUnauthorized)) + Expect(recBearer.Header().Get("WWW-Authenticate")).To(Equal(`Basic realm=""`)) + }) + }) + + Context("when configured with a legacy plaintext password", func() { + It("still works and authenticates valid requests", func() { + u, err := url.Parse("https://vcap:legacy-password@127.0.0.1:6868") + Expect(err).NotTo(HaveOccurred()) + + dispatcher, err := NewHTTPSDispatcher(u, settings.CertKeyPair{}, boshlog.NewLogger(boshlog.LevelNone)) + Expect(err).NotTo(HaveOccurred()) + + dispatcher.AddRoute("/agent", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + // Valid credentials + req := httptest.NewRequest(http.MethodPost, "https://localhost/agent", nil) + req.SetBasicAuth("vcap", "legacy-password") + rec := httptest.NewRecorder() + dispatcher.mux.ServeHTTP(rec, req) + Expect(rec.Code).To(Equal(http.StatusOK)) + + // Invalid credentials + reqBad := httptest.NewRequest(http.MethodPost, "https://localhost/agent", nil) + reqBad.SetBasicAuth("vcap", "wrong-password") + recBad := httptest.NewRecorder() + dispatcher.mux.ServeHTTP(recBad, reqBad) + Expect(recBad.Code).To(Equal(http.StatusUnauthorized)) + + // Missing Authorization header + reqNoAuth := httptest.NewRequest(http.MethodPost, "https://localhost/agent", nil) + recNoAuth := httptest.NewRecorder() + dispatcher.mux.ServeHTTP(recNoAuth, reqNoAuth) + Expect(recNoAuth.Code).To(Equal(http.StatusUnauthorized)) + }) + }) + + Context("when configured with a malformed verifier", func() { + It("fails closed and returns an error from NewHTTPSDispatcher", func() { + u, err := url.Parse("https://vcap:bosh-hmac-sha256$bad@127.0.0.1:6868") + Expect(err).NotTo(HaveOccurred()) + + dispatcher, err := NewHTTPSDispatcher(u, settings.CertKeyPair{}, boshlog.NewLogger(boshlog.LevelNone)) + Expect(err).To(HaveOccurred()) + Expect(dispatcher).To(BeNil()) + }) + }) +}) diff --git a/mbus/https_dispatcher_test.go b/mbus/https_dispatcher_test.go index 850821ca9..8114446e0 100644 --- a/mbus/https_dispatcher_test.go +++ b/mbus/https_dispatcher_test.go @@ -89,11 +89,12 @@ var _ = Describe("HTTPSDispatcher", func() { logger = &fakelogger.FakeLogger{} serverURL, err := url.Parse(targetURL) Expect(err).ToNot(HaveOccurred()) - dispatcher = mbus.NewHTTPSDispatcher(serverURL, settings.CertKeyPair{ + dispatcher, err = mbus.NewHTTPSDispatcher(serverURL, settings.CertKeyPair{ Certificate: agentCert, PrivateKey: agentKey, CA: "", }, logger) + Expect(err).ToNot(HaveOccurred()) go func() { defer GinkgoRecover() diff --git a/mbus/https_handler.go b/mbus/https_handler.go index 5d29d60cc..bb41275f6 100644 --- a/mbus/https_handler.go +++ b/mbus/https_handler.go @@ -33,14 +33,18 @@ func NewHTTPSHandler( blobManager boshagentblobstore.BlobManagerInterface, logger boshlog.Logger, auditLogger platform.AuditLogger, -) HTTPSHandler { +) (HTTPSHandler, error) { + dispatcher, err := NewHTTPSDispatcher(parsedURL, keyPair, logger) + if err != nil { + return HTTPSHandler{}, bosherr.WrapError(err, "Creating HTTPS dispatcher") + } return HTTPSHandler{ parsedURL: parsedURL, logger: logger, blobManager: blobManager, - dispatcher: NewHTTPSDispatcher(parsedURL, keyPair, logger), + dispatcher: dispatcher, auditLogger: auditLogger, - } + }, nil } func (h HTTPSHandler) Run(handlerFunc boshhandler.Func) error { diff --git a/mbus/https_handler_test.go b/mbus/https_handler_test.go index bd617d3c5..36e8213d2 100644 --- a/mbus/https_handler_test.go +++ b/mbus/https_handler_test.go @@ -66,7 +66,8 @@ var _ = Describe("HTTPSHandler", func() { mbusURL, _ := url.Parse(serverURL) //nolint:errcheck logger := boshlog.NewWriterLogger(boshlog.LevelDebug, GinkgoWriter) - handler = mbus.NewHTTPSHandler(mbusURL, mbusKeyPair, blobManager, logger, fakes.NewFakeAuditLogger()) + handler, err = mbus.NewHTTPSHandler(mbusURL, mbusKeyPair, blobManager, logger, fakes.NewFakeAuditLogger()) + Expect(err).NotTo(HaveOccurred()) go handler.Start(func(req boshhandler.Request) (resp boshhandler.Response) { //nolint:errcheck receivedRequest = req From 9d03e8527d5a5af42f3f5f6d2306c250e0f6fdd9 Mon Sep 17 00:00:00 2001 From: Nishad Mathur Date: Fri, 9 Oct 2026 11:24:26 -0700 Subject: [PATCH 2/4] Name the shared password API around hashed passwords Use HashedPassword, IsHashedPassword and ParseHashedPassword to describe the credential representation. Prefix detection deliberately includes malformed reserved-format strings so parsing fails closed instead of treating them as plaintext. Explain that already-hashed input requires the original client password. --- agentpassword/agentpassword.go | 15 +++++++------ agentpassword/agentpassword_test.go | 34 ++++++++++++++--------------- mbus/https_dispatcher.go | 10 ++++----- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/agentpassword/agentpassword.go b/agentpassword/agentpassword.go index c8b25c2b0..e6f30f7bd 100644 --- a/agentpassword/agentpassword.go +++ b/agentpassword/agentpassword.go @@ -19,16 +19,17 @@ const ( keyLength = 32 ) -type Verifier struct { +type HashedPassword struct { salt []byte key []byte } -func IsVerifier(password string) bool { +// IsHashedPassword identifies the reserved prefix; ParseHashedPassword validates its contents. +func IsHashedPassword(password string) bool { return strings.HasPrefix(password, prefix) } -func ParseVerifier(s string) (*Verifier, error) { +func ParseHashedPassword(s string) (*HashedPassword, error) { if !strings.HasPrefix(s, prefix) { return nil, errors.New("malformed HTTP password verifier: missing prefix") } @@ -44,10 +45,10 @@ func ParseVerifier(s string) (*Verifier, error) { if err != nil || len(key) != keyLength { return nil, errors.New("malformed HTTP password verifier: invalid key") } - return &Verifier{salt: salt, key: key}, nil + return &HashedPassword{salt: salt, key: key}, nil } -func (v *Verifier) Matches(password string) bool { +func (v *HashedPassword) Matches(password string) bool { if v == nil { return false } @@ -71,8 +72,8 @@ func HashURL(raw string) (string, error) { if !hasPassword || password == "" { return "", errors.New("URL missing password") } - if IsVerifier(password) { - return "", errors.New("URL password is already a verifier") + if IsHashedPassword(password) { + return "", errors.New("URL password is already hashed; provide the original password") } salt := make([]byte, saltLength) diff --git a/agentpassword/agentpassword_test.go b/agentpassword/agentpassword_test.go index 94acbeb0a..c8b21bb6b 100644 --- a/agentpassword/agentpassword_test.go +++ b/agentpassword/agentpassword_test.go @@ -17,27 +17,27 @@ var _ = Describe("Agentpassword", func() { }) }) - Describe("IsVerifier", func() { + Describe("IsHashedPassword", func() { It("returns true for verifiers and malformed verifiers with the prefix", func() { - Expect(agentpassword.IsVerifier("bosh-hmac-sha256$bad")).To(BeTrue()) - Expect(agentpassword.IsVerifier("bosh-hmac-sha256$AAECAwQFBgcICQoLDA0ODw$EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw")).To(BeTrue()) + Expect(agentpassword.IsHashedPassword("bosh-hmac-sha256$bad")).To(BeTrue()) + Expect(agentpassword.IsHashedPassword("bosh-hmac-sha256$AAECAwQFBgcICQoLDA0ODw$EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw")).To(BeTrue()) }) It("returns false when the prefix is missing or incomplete", func() { - Expect(agentpassword.IsVerifier("plain-password")).To(BeFalse()) - Expect(agentpassword.IsVerifier("")).To(BeFalse()) - Expect(agentpassword.IsVerifier("bosh-hmac-sha256")).To(BeFalse()) - Expect(agentpassword.IsVerifier("other-prefix$secret")).To(BeFalse()) + Expect(agentpassword.IsHashedPassword("plain-password")).To(BeFalse()) + Expect(agentpassword.IsHashedPassword("")).To(BeFalse()) + Expect(agentpassword.IsHashedPassword("bosh-hmac-sha256")).To(BeFalse()) + Expect(agentpassword.IsHashedPassword("other-prefix$secret")).To(BeFalse()) }) }) - Describe("ParseVerifier and Matches", func() { + Describe("ParseHashedPassword and Matches", func() { It("matches a fixed known-answer vector guarding the format", func() { // salt: bytes 0..15 -> base64url "AAECAwQFBgcICQoLDA0ODw" // password: "correct horse battery staple" // HMAC-SHA256(key=salt, msg=password) -> base64url "EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw" const knownVector = "bosh-hmac-sha256$AAECAwQFBgcICQoLDA0ODw$EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw" - v, err := agentpassword.ParseVerifier(knownVector) + v, err := agentpassword.ParseHashedPassword(knownVector) Expect(err).NotTo(HaveOccurred()) Expect(v).NotTo(BeNil()) Expect(v.Matches("correct horse battery staple")).To(BeTrue()) @@ -54,7 +54,7 @@ var _ = Describe("Agentpassword", func() { "invalid-prefix$" + validSalt + "$" + validKey, "bosh-hmac-sha256", } { - v, err := agentpassword.ParseVerifier(missingPrefix) + v, err := agentpassword.ParseHashedPassword(missingPrefix) Expect(err).To(HaveOccurred(), "expected error for missing prefix: %s", missingPrefix) Expect(v).To(BeNil()) } @@ -66,7 +66,7 @@ var _ = Describe("Agentpassword", func() { "bosh-hmac-sha256$" + validSalt + "$" + validKey + "$extra", "bosh-hmac-sha256$" + validSalt + "$" + validKey + "$extra$more", } { - v, err := agentpassword.ParseVerifier(badParts) + v, err := agentpassword.ParseHashedPassword(badParts) Expect(err).To(HaveOccurred(), "expected error for bad part count: %s", badParts) Expect(v).To(BeNil()) } @@ -76,7 +76,7 @@ var _ = Describe("Agentpassword", func() { "bosh-hmac-sha256$invalid!salt$" + validKey, "bosh-hmac-sha256$" + validSalt + "$invalid!key", } { - v, err := agentpassword.ParseVerifier(badB64) + v, err := agentpassword.ParseHashedPassword(badB64) Expect(err).To(HaveOccurred(), "expected error for bad base64: %s", badB64) Expect(v).To(BeNil()) } @@ -88,7 +88,7 @@ var _ = Describe("Agentpassword", func() { // Salt containing '/' "bosh-hmac-sha256$AAECAwQFBgcICQoLDA0OD/$EEqYVo55l7MGfPvwVPSRCq7RN6JXw-TBbKyW9De5Grw", } { - v, err := agentpassword.ParseVerifier(stdAlphabet) + v, err := agentpassword.ParseHashedPassword(stdAlphabet) Expect(err).To(HaveOccurred(), "expected error for std base64 alphabet: %s", stdAlphabet) Expect(v).To(BeNil()) } @@ -100,7 +100,7 @@ var _ = Describe("Agentpassword", func() { "bosh-hmac-sha256$" + shortSalt + "$" + validKey, "bosh-hmac-sha256$" + longSalt + "$" + validKey, } { - v, err := agentpassword.ParseVerifier(badSalt) + v, err := agentpassword.ParseHashedPassword(badSalt) Expect(err).To(HaveOccurred(), "expected error for wrong salt length: %s", badSalt) Expect(v).To(BeNil()) } @@ -112,7 +112,7 @@ var _ = Describe("Agentpassword", func() { "bosh-hmac-sha256$" + validSalt + "$" + shortKey, "bosh-hmac-sha256$" + validSalt + "$" + longKey, } { - v, err := agentpassword.ParseVerifier(badKey) + v, err := agentpassword.ParseHashedPassword(badKey) Expect(err).To(HaveOccurred(), "expected error for wrong key length: %s", badKey) Expect(v).To(BeNil()) } @@ -120,7 +120,7 @@ var _ = Describe("Agentpassword", func() { }) Describe("HashURL", func() { - It("performs a complete HashURL -> ParseVerifier -> Matches round trip", func() { + It("performs a complete HashURL -> ParseHashedPassword -> Matches round trip", func() { raw := "https://vcap:mypassword@127.0.0.1:6868/path?foo=bar#frag" hashed, err := agentpassword.HashURL(raw) Expect(err).NotTo(HaveOccurred()) @@ -135,7 +135,7 @@ var _ = Describe("Agentpassword", func() { pw, ok := parsedURL.User.Password() Expect(ok).To(BeTrue()) - v, err := agentpassword.ParseVerifier(pw) + v, err := agentpassword.ParseHashedPassword(pw) Expect(err).NotTo(HaveOccurred()) Expect(v.Matches("mypassword")).To(BeTrue()) Expect(v.Matches("wrong-password")).To(BeFalse()) diff --git a/mbus/https_dispatcher.go b/mbus/https_dispatcher.go index 60d62ea78..dcfdb1e21 100644 --- a/mbus/https_dispatcher.go +++ b/mbus/https_dispatcher.go @@ -30,7 +30,7 @@ type HTTPSDispatcher struct { listener net.Listener logger boshlog.Logger baseURL *url.URL - passwordVerifier *agentpassword.Verifier + passwordVerifier *agentpassword.HashedPassword expectedAuthorization string } @@ -38,10 +38,10 @@ type HTTPHandlerFunc func(writer http.ResponseWriter, request *http.Request) func NewHTTPSDispatcher(baseURL *url.URL, keyPair settings.CertKeyPair, logger boshlog.Logger) (*HTTPSDispatcher, error) { password, _ := baseURL.User.Password() - var verifier *agentpassword.Verifier + var verifier *agentpassword.HashedPassword var err error - if agentpassword.IsVerifier(password) { - verifier, err = agentpassword.ParseVerifier(password) + if agentpassword.IsHashedPassword(password) { + verifier, err = agentpassword.ParseHashedPassword(password) if err != nil { return nil, bosherr.WrapError(err, "Configuring HTTP authentication") } @@ -100,7 +100,7 @@ func (h *HTTPSDispatcher) Stop() { } // requestNotAuthorized checks HTTP credentials; cryptographic verification lives -// in agentpassword.Verifier and is safe for simultaneous requests. +// in agentpassword.HashedPassword and is safe for simultaneous requests. func (h *HTTPSDispatcher) requestNotAuthorized(request *http.Request) bool { if h.passwordVerifier == nil { return subtle.ConstantTimeCompare([]byte(h.expectedAuthorization), []byte(request.Header.Get("Authorization"))) != 1 From 414c6a0a206c38a2baa613a3b1db246302ebd0bf Mon Sep 17 00:00:00 2001 From: Nishad Mathur Date: Fri, 9 Oct 2026 11:28:40 -0700 Subject: [PATCH 3/4] Put each HTTP authentication rejection condition on its own line --- mbus/https_dispatcher.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mbus/https_dispatcher.go b/mbus/https_dispatcher.go index dcfdb1e21..411ededf5 100644 --- a/mbus/https_dispatcher.go +++ b/mbus/https_dispatcher.go @@ -106,8 +106,11 @@ func (h *HTTPSDispatcher) requestNotAuthorized(request *http.Request) bool { return subtle.ConstantTimeCompare([]byte(h.expectedAuthorization), []byte(request.Header.Get("Authorization"))) != 1 } username, password, ok := request.BasicAuth() - return !ok || subtle.ConstantTimeCompare([]byte(h.baseURL.User.Username()), []byte(username)) != 1 || - len(password) == 0 || len(password) > maxHTTPPasswordLength || !h.passwordVerifier.Matches(password) + return !ok || + subtle.ConstantTimeCompare([]byte(h.baseURL.User.Username()), []byte(username)) != 1 || + len(password) == 0 || + len(password) > maxHTTPPasswordLength || + !h.passwordVerifier.Matches(password) } func (h *HTTPSDispatcher) AddRoute(route string, handler HTTPHandlerFunc) { From 50adbab21696333e372a71b8a494b08733776ad3 Mon Sep 17 00:00:00 2001 From: Nishad Mathur Date: Fri, 9 Oct 2026 11:32:47 -0700 Subject: [PATCH 4/4] Reject passwords that exceed the shared HTTP authentication limit Use the same maximum plaintext password length when generating metadata hashes and authenticating requests. Oversized credentials must fail in the CLI before VM creation rather than producing metadata for an agent that its original credentials cannot reach. --- agentpassword/agentpassword.go | 7 +++++++ mbus/https_dispatcher.go | 7 ++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/agentpassword/agentpassword.go b/agentpassword/agentpassword.go index e6f30f7bd..03fc7b55a 100644 --- a/agentpassword/agentpassword.go +++ b/agentpassword/agentpassword.go @@ -7,6 +7,7 @@ import ( "crypto/subtle" "encoding/base64" "errors" + "fmt" "net/url" "strings" ) @@ -14,6 +15,9 @@ import ( const ( Feature = "http-password-hmac-sha256" + // MaxPasswordLength is the maximum plaintext password length in bytes. + MaxPasswordLength = 1024 + prefix = "bosh-hmac-sha256$" saltLength = 16 keyLength = 32 @@ -72,6 +76,9 @@ func HashURL(raw string) (string, error) { if !hasPassword || password == "" { return "", errors.New("URL missing password") } + if len(password) > MaxPasswordLength { + return "", fmt.Errorf("URL password exceeds %d bytes", MaxPasswordLength) + } if IsHashedPassword(password) { return "", errors.New("URL password is already hashed; provide the original password") } diff --git a/mbus/https_dispatcher.go b/mbus/https_dispatcher.go index 411ededf5..832417aa7 100644 --- a/mbus/https_dispatcher.go +++ b/mbus/https_dispatcher.go @@ -18,10 +18,7 @@ import ( tlsconfig "code.cloudfoundry.org/tlsconfig" ) -const ( - httpsDispatcherLogTag = "HTTPS Dispatcher" - maxHTTPPasswordLength = 1024 -) +const httpsDispatcherLogTag = "HTTPS Dispatcher" type HTTPSDispatcher struct { httpServer *http.Server @@ -109,7 +106,7 @@ func (h *HTTPSDispatcher) requestNotAuthorized(request *http.Request) bool { return !ok || subtle.ConstantTimeCompare([]byte(h.baseURL.User.Username()), []byte(username)) != 1 || len(password) == 0 || - len(password) > maxHTTPPasswordLength || + len(password) > agentpassword.MaxPasswordLength || !h.passwordVerifier.Matches(password) }