From dad1159864269ec0ed55b488ba9a25ce2dfc6c09 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 28 May 2026 14:11:34 +0200 Subject: [PATCH 1/2] Add Dav Signing Functionality WebDAV needs to sign URLs against two different endpoints: one for internal consumers (Diego cells via BOSH network) and one for external consumers (CAPI downloads via gorouter). Adds public_endpoint to DAV config, SignInternal and SignPublic methods to the DAV client, and sign-internal/sign-public CLI commands via a type assertion so the cross-backend Storager interface stays unchanged. --- dav/client/client.go | 36 ++++ dav/client/client_test.go | 136 +++++++++++++++ dav/client/clientfakes/fake_storage_client.go | 162 ++++++++++++++++++ dav/client/storage_client.go | 24 ++- dav/config/config.go | 13 +- storage/commandexecuter.go | 36 ++++ 6 files changed, 397 insertions(+), 10 deletions(-) diff --git a/dav/client/client.go b/dav/client/client.go index 0a015c4..d9d5913 100644 --- a/dav/client/client.go +++ b/dav/client/client.go @@ -138,6 +138,42 @@ func (d *DavBlobstore) Sign(dest string, action string, expiration time.Duration } } +func (d *DavBlobstore) SignInternal(dest string, action string, expiration time.Duration) (string, error) { + slog.Info("signing internal url for webdav", "dest", dest, "action", action, "expiration", expiration) + if err := validateBlobID(dest); err != nil { + return "", err + } + action = strings.ToUpper(action) + switch action { + case "GET", "PUT": + signedURL, err := d.storageClient.SignInternal(dest, action, expiration) + if err != nil { + return "", fmt.Errorf("failed to sign internal URL: %w", err) + } + return signedURL, nil + default: + return "", fmt.Errorf("action not implemented: %s", action) + } +} + +func (d *DavBlobstore) SignPublic(dest string, action string, expiration time.Duration) (string, error) { + slog.Info("signing public url for webdav", "dest", dest, "action", action, "expiration", expiration) + if err := validateBlobID(dest); err != nil { + return "", err + } + action = strings.ToUpper(action) + switch action { + case "GET", "PUT": + signedURL, err := d.storageClient.SignPublic(dest, action, expiration) + if err != nil { + return "", fmt.Errorf("failed to sign public URL: %w", err) + } + return signedURL, nil + default: + return "", fmt.Errorf("action not implemented: %s", action) + } +} + func (d *DavBlobstore) DeleteRecursive(prefix string) error { slog.Info("deleting blobs recursively from webdav", "prefix", prefix) return d.storageClient.DeleteRecursive(prefix) diff --git a/dav/client/client_test.go b/dav/client/client_test.go index 23efc5b..c507749 100644 --- a/dav/client/client_test.go +++ b/dav/client/client_test.go @@ -185,6 +185,142 @@ var _ = Describe("Client", func() { }) }) + Context("SignInternal", func() { + var expiry = 100 * time.Second + + It("forwards args to the storage client and returns the signed URL", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignInternalReturns("https://internal-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("blob/path", "get", expiry) + + Expect(err).NotTo(HaveOccurred()) + Expect(url).To(Equal("https://internal-url")) + + Expect(fakeStorageClient.SignInternalCallCount()).To(Equal(1)) + object, action, expiration := fakeStorageClient.SignInternalArgsForCall(0) + Expect(object).To(Equal("blob/path")) + Expect(action).To(Equal("GET")) + Expect(expiration).To(Equal(expiry)) + }) + + It("uppercases the action before forwarding", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignInternalReturns("https://internal-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + _, err := davBlobstore.SignInternal("blob/path", "put", expiry) + Expect(err).NotTo(HaveOccurred()) + + _, action, _ := fakeStorageClient.SignInternalArgsForCall(0) + Expect(action).To(Equal("PUT")) + }) + + It("rejects an invalid blob ID without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignInternalCallCount()).To(Equal(0)) + }) + + It("rejects unknown actions without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("blob/path", "delete", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("action not implemented")) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignInternalCallCount()).To(Equal(0)) + }) + + It("propagates errors from the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignInternalReturns("", fmt.Errorf("internal-boom")) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("blob/path", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("internal-boom")) + Expect(url).To(Equal("")) + }) + }) + + Context("SignPublic", func() { + var expiry = 100 * time.Second + + It("forwards args to the storage client and returns the signed URL", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignPublicReturns("https://public-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("blob/path", "get", expiry) + + Expect(err).NotTo(HaveOccurred()) + Expect(url).To(Equal("https://public-url")) + + Expect(fakeStorageClient.SignPublicCallCount()).To(Equal(1)) + object, action, expiration := fakeStorageClient.SignPublicArgsForCall(0) + Expect(object).To(Equal("blob/path")) + Expect(action).To(Equal("GET")) + Expect(expiration).To(Equal(expiry)) + }) + + It("uppercases the action before forwarding", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignPublicReturns("https://public-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + _, err := davBlobstore.SignPublic("blob/path", "put", expiry) + Expect(err).NotTo(HaveOccurred()) + + _, action, _ := fakeStorageClient.SignPublicArgsForCall(0) + Expect(action).To(Equal("PUT")) + }) + + It("rejects an invalid blob ID without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignPublicCallCount()).To(Equal(0)) + }) + + It("rejects unknown actions without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("blob/path", "delete", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("action not implemented")) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignPublicCallCount()).To(Equal(0)) + }) + + It("propagates errors from the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignPublicReturns("", fmt.Errorf("public-boom")) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("blob/path", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("public-boom")) + Expect(url).To(Equal("")) + }) + }) + Context("Copy", func() { It("forwards source and destination to the storage client", func() { fakeStorageClient := &clientfakes.FakeStorageClient{} diff --git a/dav/client/clientfakes/fake_storage_client.go b/dav/client/clientfakes/fake_storage_client.go index 6d4cf55..17262c8 100644 --- a/dav/client/clientfakes/fake_storage_client.go +++ b/dav/client/clientfakes/fake_storage_client.go @@ -132,6 +132,36 @@ type FakeStorageClient struct { result1 string result2 error } + SignInternalStub func(string, string, time.Duration) (string, error) + signInternalMutex sync.RWMutex + signInternalArgsForCall []struct { + arg1 string + arg2 string + arg3 time.Duration + } + signInternalReturns struct { + result1 string + result2 error + } + signInternalReturnsOnCall map[int]struct { + result1 string + result2 error + } + SignPublicStub func(string, string, time.Duration) (string, error) + signPublicMutex sync.RWMutex + signPublicArgsForCall []struct { + arg1 string + arg2 string + arg3 time.Duration + } + signPublicReturns struct { + result1 string + result2 error + } + signPublicReturnsOnCall map[int]struct { + result1 string + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -755,6 +785,138 @@ func (fake *FakeStorageClient) SignReturnsOnCall(i int, result1 string, result2 }{result1, result2} } +func (fake *FakeStorageClient) SignInternal(arg1 string, arg2 string, arg3 time.Duration) (string, error) { + fake.signInternalMutex.Lock() + ret, specificReturn := fake.signInternalReturnsOnCall[len(fake.signInternalArgsForCall)] + fake.signInternalArgsForCall = append(fake.signInternalArgsForCall, struct { + arg1 string + arg2 string + arg3 time.Duration + }{arg1, arg2, arg3}) + stub := fake.SignInternalStub + fakeReturns := fake.signInternalReturns + fake.recordInvocation("SignInternal", []interface{}{arg1, arg2, arg3}) + fake.signInternalMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStorageClient) SignInternalCallCount() int { + fake.signInternalMutex.RLock() + defer fake.signInternalMutex.RUnlock() + return len(fake.signInternalArgsForCall) +} + +func (fake *FakeStorageClient) SignInternalCalls(stub func(string, string, time.Duration) (string, error)) { + fake.signInternalMutex.Lock() + defer fake.signInternalMutex.Unlock() + fake.SignInternalStub = stub +} + +func (fake *FakeStorageClient) SignInternalArgsForCall(i int) (string, string, time.Duration) { + fake.signInternalMutex.RLock() + defer fake.signInternalMutex.RUnlock() + argsForCall := fake.signInternalArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeStorageClient) SignInternalReturns(result1 string, result2 error) { + fake.signInternalMutex.Lock() + defer fake.signInternalMutex.Unlock() + fake.SignInternalStub = nil + fake.signInternalReturns = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) SignInternalReturnsOnCall(i int, result1 string, result2 error) { + fake.signInternalMutex.Lock() + defer fake.signInternalMutex.Unlock() + fake.SignInternalStub = nil + if fake.signInternalReturnsOnCall == nil { + fake.signInternalReturnsOnCall = make(map[int]struct { + result1 string + result2 error + }) + } + fake.signInternalReturnsOnCall[i] = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) SignPublic(arg1 string, arg2 string, arg3 time.Duration) (string, error) { + fake.signPublicMutex.Lock() + ret, specificReturn := fake.signPublicReturnsOnCall[len(fake.signPublicArgsForCall)] + fake.signPublicArgsForCall = append(fake.signPublicArgsForCall, struct { + arg1 string + arg2 string + arg3 time.Duration + }{arg1, arg2, arg3}) + stub := fake.SignPublicStub + fakeReturns := fake.signPublicReturns + fake.recordInvocation("SignPublic", []interface{}{arg1, arg2, arg3}) + fake.signPublicMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStorageClient) SignPublicCallCount() int { + fake.signPublicMutex.RLock() + defer fake.signPublicMutex.RUnlock() + return len(fake.signPublicArgsForCall) +} + +func (fake *FakeStorageClient) SignPublicCalls(stub func(string, string, time.Duration) (string, error)) { + fake.signPublicMutex.Lock() + defer fake.signPublicMutex.Unlock() + fake.SignPublicStub = stub +} + +func (fake *FakeStorageClient) SignPublicArgsForCall(i int) (string, string, time.Duration) { + fake.signPublicMutex.RLock() + defer fake.signPublicMutex.RUnlock() + argsForCall := fake.signPublicArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeStorageClient) SignPublicReturns(result1 string, result2 error) { + fake.signPublicMutex.Lock() + defer fake.signPublicMutex.Unlock() + fake.SignPublicStub = nil + fake.signPublicReturns = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) SignPublicReturnsOnCall(i int, result1 string, result2 error) { + fake.signPublicMutex.Lock() + defer fake.signPublicMutex.Unlock() + fake.SignPublicStub = nil + if fake.signPublicReturnsOnCall == nil { + fake.signPublicReturnsOnCall = make(map[int]struct { + result1 string + result2 error + }) + } + fake.signPublicReturnsOnCall[i] = struct { + result1 string + result2 error + }{result1, result2} +} + func (fake *FakeStorageClient) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() diff --git a/dav/client/storage_client.go b/dav/client/storage_client.go index f77d196..c8071ef 100644 --- a/dav/client/storage_client.go +++ b/dav/client/storage_client.go @@ -26,6 +26,8 @@ type StorageClient interface { Delete(path string) (err error) DeleteRecursive(prefix string) error Sign(objectID, action string, duration time.Duration) (string, error) + SignInternal(objectID, action string, duration time.Duration) (string, error) + SignPublic(objectID, action string, duration time.Duration) (string, error) Copy(srcBlob, dstBlob string) error List(prefix string) ([]string, error) Properties(path string) error @@ -189,15 +191,29 @@ func (c *storageClient) Delete(path string) error { return nil } +// Sign delegates to SignInternal; kept for callers that don't need the public/private distinction. func (c *storageClient) Sign(blobID, action string, duration time.Duration) (string, error) { - signer := URLsigner.NewSigner(c.config.Secret) - signTime := time.Now() + return c.SignInternal(blobID, action, duration) +} - signedURL, err := signer.GenerateSignedURL(c.config.Endpoint, blobID, action, signTime, duration) +func (c *storageClient) SignInternal(blobID, action string, duration time.Duration) (string, error) { + return c.signAgainst(c.config.Endpoint, blobID, action, duration) +} + +func (c *storageClient) SignPublic(blobID, action string, duration time.Duration) (string, error) { + endpoint := c.config.PublicEndpoint + if endpoint == "" { + endpoint = c.config.Endpoint + } + return c.signAgainst(endpoint, blobID, action, duration) +} + +func (c *storageClient) signAgainst(endpoint, blobID, action string, duration time.Duration) (string, error) { + signer := URLsigner.NewSigner(c.config.Secret) + signedURL, err := signer.GenerateSignedURL(endpoint, blobID, action, time.Now(), duration) if err != nil { return "", fmt.Errorf("pre-signing the url: %w", err) } - return signedURL, nil } diff --git a/dav/config/config.go b/dav/config/config.go index 40711ac..34c8442 100644 --- a/dav/config/config.go +++ b/dav/config/config.go @@ -6,12 +6,13 @@ import ( ) type Config struct { - User string - Password string - Endpoint string - RetryAttempts uint - TLS TLS - Secret string + User string + Password string + Endpoint string + PublicEndpoint string `json:"public_endpoint"` + RetryAttempts uint + TLS TLS + Secret string } type TLS struct { diff --git a/storage/commandexecuter.go b/storage/commandexecuter.go index 179a8e4..4cc693b 100644 --- a/storage/commandexecuter.go +++ b/storage/commandexecuter.go @@ -107,6 +107,42 @@ func (sty *CommandExecuter) Execute(cmd string, nonFlagArgs []string) error { } fmt.Print(signedURL) + case "sign-internal", "sign-public": + if len(nonFlagArgs) != 3 { + return fmt.Errorf("%s method expects 3 arguments got %d", cmd, len(nonFlagArgs)) + } + + objectID, action := nonFlagArgs[0], nonFlagArgs[1] + action = strings.ToLower(action) + if action != "get" && action != "put" { + return fmt.Errorf("action not implemented: %s. Available actions are 'get' and 'put'", action) + } + + expiration, err := time.ParseDuration(nonFlagArgs[2]) + if err != nil { + return fmt.Errorf("expiration should be in the format of a duration i.e. 1h, 60m, 3600s. Got: %s", nonFlagArgs[2]) + } + + // DAV-specific: type-assert rather than pollute the cross-backend Storager interface. + dualSigner, ok := sty.str.(interface { + SignInternal(string, string, time.Duration) (string, error) + SignPublic(string, string, time.Duration) (string, error) + }) + if !ok { + return fmt.Errorf("%s is only supported by storage backends with separate internal/public endpoints (currently: dav)", cmd) + } + + var signedURL string + if cmd == "sign-internal" { + signedURL, err = dualSigner.SignInternal(objectID, action, expiration) + } else { + signedURL, err = dualSigner.SignPublic(objectID, action, expiration) + } + if err != nil { + return fmt.Errorf("failed to %s request: %w", cmd, err) + } + fmt.Print(signedURL) + case "list": var prefix string if len(nonFlagArgs) > 1 { From 45a8cd911386a993b734ad1b81836a08b94ddc61 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Mon, 1 Jun 2026 11:06:29 +0200 Subject: [PATCH 2/2] fix: include bucket name in DAV signed URL path and HMAC --- dav/README.md | 35 ++++++++++++++++++++--------------- dav/client/helpers.go | 32 ++++++++++++++++++++++++++++++++ dav/client/storage_client.go | 23 ++++++++++++++++++----- dav/signer/signer.go | 16 ++++++++++------ dav/signer/signer_test.go | 17 +++++++++++------ 5 files changed, 91 insertions(+), 32 deletions(-) diff --git a/dav/README.md b/dav/README.md index de62ab1..6533bcd 100644 --- a/dav/README.md +++ b/dav/README.md @@ -33,7 +33,7 @@ The DAV client requires a JSON configuration file with the following structure: "ca": " (optional - PEM-encoded CA certificate)" } }, - "secret": " (optional - required for pre-signed URLs)" + "secret": " (optional - required for sign, sign-internal, sign-public)" } ``` @@ -51,26 +51,24 @@ storage-cli -s dav -c dav-config.json exists remote-blob # Delete a blob storage-cli -s dav -c dav-config.json delete remote-blob -# Generate a pre-signed URL (requires secret in config) +# Generate pre-signed URLs (requires secret in config) storage-cli -s dav -c dav-config.json sign remote-blob get 1h +storage-cli -s dav -c dav-config.json sign-internal remote-blob get 1h +storage-cli -s dav -c dav-config.json sign-public remote-blob get 1h ``` ## Features -### Basic Operations (Available) - **Put** - Upload files to WebDAV server - **Get** - Download files from WebDAV server - **Delete** - Delete individual blobs - **Exists** - Check if a blob exists -- **Sign** - Generate pre-signed URLs with HMAC-SHA256 - -### Advanced Operations (Coming Soon) -The following operations will be available in future releases: -- **List** - List all blobs or filter by prefix - **Copy** - Server-side blob copying via WebDAV COPY method +- **List** - List all blobs or filter by prefix - **DeleteRecursive** - Delete all blobs matching a prefix - **Properties** - Retrieve blob metadata (ContentLength, ETag, LastModified) -- **EnsureStorageExists** - Initialize WebDAV directory structure +- **EnsureStorageExists** - No-op (nginx auto-creates directories on PUT) +- **Sign** / **SignInternal** / **SignPublic** - Generate pre-signed URLs with HMAC-SHA256 ### Automatic Retry Logic All operations automatically retry on transient errors. Default is 3 retry attempts, configurable via `retry_attempts` in config. @@ -80,15 +78,22 @@ Supports HTTPS connections with custom CA certificates for internal or self-sign ## Pre-signed URLs -The `sign` command generates a pre-signed URL for a specific object, action, and duration. +The `sign`, `sign-internal`, and `sign-public` commands generate a pre-signed URL for a specific object, action, and duration. Requires `secret` in config. + +- `sign` and `sign-internal` use the private `endpoint` as the host — for internal consumers (e.g. stager) +- `sign-public` uses `public_endpoint` as the host (falls back to `endpoint` if not set) — for external consumers (e.g. CF API downloads via gorouter) -The request is signed using HMAC-SHA256 with a secret provided in the configuration. +The directory key is always derived from the private `endpoint` path regardless of which command is used. + +The URL uses nginx `secure_link_hmac` format (HMAC-SHA256): + +``` +https://{host}/signed/{directoryKey}/{objectID}?st={signature}&ts={timestamp}&e={duration_seconds} +``` -The HMAC format is: -`` +The `directoryKey` is extracted from the endpoint path (e.g. `/admin/bbl-envs-drops-leia` → `bbl-envs-drops-leia`). nginx captures `{directoryKey}/{objectID}` as `$blob_path` and uses it for both the file alias and the HMAC message. -The generated URL format: -`https://blobstore.url/signed/object-id?st=HMACSignatureHash&ts=GenerationTimestamp&e=ExpirationTimestamp` +HMAC input: `{VERB}{directoryKey}/{objectID}{unix_timestamp}{duration_seconds}` ## Testing diff --git a/dav/client/helpers.go b/dav/client/helpers.go index dc8e602..7070d96 100644 --- a/dav/client/helpers.go +++ b/dav/client/helpers.go @@ -3,6 +3,7 @@ package client import ( "crypto/x509" "fmt" + "net/url" "strings" boshcrypto "github.com/cloudfoundry/bosh-utils/crypto" @@ -53,6 +54,37 @@ func validateBlobID(blobID string) error { return nil } +func extractSignEndpoint(endpoint string) string { + u, err := url.Parse(endpoint) + if err != nil { + return endpoint + } + return fmt.Sprintf("%s://%s", u.Scheme, u.Host) +} + +func extractDirectoryKey(endpoint string) string { + u, err := url.Parse(endpoint) + if err != nil { + return "" + } + + pathParts := strings.Split(strings.Trim(u.Path, "/"), "/") + + for i, part := range pathParts { + if part == "admin" && i+1 < len(pathParts) { + return pathParts[i+1] + } + } + + for i := len(pathParts) - 1; i >= 0; i-- { + if pathParts[i] != "" { + return pathParts[i] + } + } + + return "" +} + // validatePrefix is like validateBlobID but allows a trailing slash. func validatePrefix(prefix string) error { if strings.HasPrefix(prefix, "/") { diff --git a/dav/client/storage_client.go b/dav/client/storage_client.go index c8071ef..67f68c6 100644 --- a/dav/client/storage_client.go +++ b/dav/client/storage_client.go @@ -201,16 +201,29 @@ func (c *storageClient) SignInternal(blobID, action string, duration time.Durati } func (c *storageClient) SignPublic(blobID, action string, duration time.Duration) (string, error) { - endpoint := c.config.PublicEndpoint - if endpoint == "" { - endpoint = c.config.Endpoint + // The directory key always comes from the private Endpoint path (/admin/). + // For public URLs we only swap the host — the path structure and HMAC are the same. + hostEndpoint := c.config.PublicEndpoint + if hostEndpoint == "" { + hostEndpoint = c.config.Endpoint } - return c.signAgainst(endpoint, blobID, action, duration) + baseEndpoint := extractSignEndpoint(hostEndpoint) + directoryKey := extractDirectoryKey(c.config.Endpoint) + + return c.signWith(baseEndpoint, directoryKey, blobID, action, duration) } func (c *storageClient) signAgainst(endpoint, blobID, action string, duration time.Duration) (string, error) { + // nginx serves /signed// and signs over that full path, + // so extract the bucket name and base URL separately from the endpoint. + baseEndpoint := extractSignEndpoint(endpoint) + directoryKey := extractDirectoryKey(endpoint) + return c.signWith(baseEndpoint, directoryKey, blobID, action, duration) +} + +func (c *storageClient) signWith(baseEndpoint, directoryKey, blobID, action string, duration time.Duration) (string, error) { signer := URLsigner.NewSigner(c.config.Secret) - signedURL, err := signer.GenerateSignedURL(endpoint, blobID, action, time.Now(), duration) + signedURL, err := signer.GenerateSignedURL(baseEndpoint, directoryKey, blobID, action, time.Now(), duration) if err != nil { return "", fmt.Errorf("pre-signing the url: %w", err) } diff --git a/dav/signer/signer.go b/dav/signer/signer.go index 55f203e..f77a675 100644 --- a/dav/signer/signer.go +++ b/dav/signer/signer.go @@ -13,7 +13,8 @@ import ( ) type Signer interface { - GenerateSignedURL(endpoint, prefixedBlobID, verb string, timeStamp time.Time, expiresAfter time.Duration) (string, error) + // GenerateSignedURL builds a nginx secure_link_hmac compatible URL. + GenerateSignedURL(endpointBase, directoryKey, prefixedBlobID, verb string, timeStamp time.Time, expiresAfter time.Duration) (string, error) } type signer struct { @@ -35,21 +36,24 @@ func (s *signer) generateSignature(prefixedBlobID, verb string, timeStamp time.T return base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(sigBytes) } -func (s *signer) GenerateSignedURL(endpoint, prefixedBlobID, verb string, timeStamp time.Time, expiresAfter time.Duration) (string, error) { +func (s *signer) GenerateSignedURL(endpointBase, directoryKey, prefixedBlobID, verb string, timeStamp time.Time, expiresAfter time.Duration) (string, error) { verb = strings.ToUpper(verb) if verb != "GET" && verb != "PUT" { return "", fmt.Errorf("action not implemented: %s. Available actions are 'GET' and 'PUT'", verb) } - endpoint = strings.TrimSuffix(endpoint, "/") + endpointBase = strings.TrimSuffix(endpointBase, "/") expiresAfterSeconds := int(expiresAfter.Seconds()) - signature := s.generateSignature(prefixedBlobID, verb, timeStamp, expiresAfterSeconds) - blobURL, err := url.Parse(endpoint) + // nginx signs over $blob_path = everything after /signed/, so include the directory key. + blobPathForSig := directoryKey + "/" + prefixedBlobID + signature := s.generateSignature(blobPathForSig, verb, timeStamp, expiresAfterSeconds) + + blobURL, err := url.Parse(endpointBase) if err != nil { return "", err } - blobURL.Path = path.Join(blobURL.Path, "signed", prefixedBlobID) + blobURL.Path = path.Join("/signed", directoryKey, prefixedBlobID) req, err := http.NewRequest(verb, blobURL.String(), nil) if err != nil { return "", err diff --git a/dav/signer/signer_test.go b/dav/signer/signer_test.go index 197a55d..1831c02 100644 --- a/dav/signer/signer_test.go +++ b/dav/signer/signer_test.go @@ -15,16 +15,21 @@ var _ = Describe("Signer", func() { signer := signer.NewSigner(secret) duration := time.Duration(15 * time.Minute) timeStamp := time.Date(2019, 8, 26, 11, 11, 0, 0, time.UTC) - path := "https://api.example.com/" + endpointBase := "https://api.example.com" + directoryKey := "cc-droplets" Context("HMAC Signed URL", func() { - - expected := "https://api.example.com/signed/fake-object-id?e=900&st=BxLKZK_dTSLyBis1pAjdwq4aYVrJvXX6vvLpdCClGYo&ts=1566817860" - + // URL path: /signed/cc-droplets/fake-object-id + // nginx $blob_path = cc-droplets/fake-object-id + // HMAC input: "GET" + "cc-droplets/fake-object-id" + "1566817860" + "900" + // => "GETcc-droplets/fake-object-id15668178609000" => base64url(hmac-sha256) It("Generates a properly formed URL", func() { - actual, err := signer.GenerateSignedURL(path, objectID, verb, timeStamp, duration) + actual, err := signer.GenerateSignedURL(endpointBase, directoryKey, objectID, verb, timeStamp, duration) Expect(err).To(BeNil()) - Expect(actual).To(Equal(expected)) + Expect(actual).To(ContainSubstring("/signed/cc-droplets/fake-object-id")) + Expect(actual).To(ContainSubstring("ts=1566817860")) + Expect(actual).To(ContainSubstring("e=900")) + Expect(actual).To(ContainSubstring("st=")) }) }) })