mirror/opencloud
1
0
Fork 0
mirror of https://github.com/opencloud-eu/opencloud.git synced 2026-01-05 11:51:16 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix: properly check expiry and verify signature of signed urls (#8383)

Browse Source 
fix: signed url expiry validation only checks for expiry and not for used before
This commit is contained in:
Thomas Müller
2024-02-07 17:13:28 +01:00
committed by GitHub
parent 60a1ec700c
commit 2b667003b5
4 changed files with 221 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
changelog/unreleased/fix-signed-url-expiry.md Normal file
View File

@@ -0,0 +1,5 @@
Bugfix: signed url verification
Signed urls now expire properly
https://github.com/owncloud/ocis/pull/8385

1
services/proxy/pkg/command/server.go
View File

@@ -374,6 +374,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config,
UserProvider: userProvider,
UserRoleAssigner: roleAssigner,
Store: storeClient,
Now: time.Now,
})
return alice.New(

97
services/proxy/pkg/middleware/signed_url_auth.go
View File

@@ -27,6 +27,7 @@ const (
_paramOCDate = "OC-Date"
_paramOCExpires = "OC-Expires"
_paramOCVerb = "OC-Verb"
_paramOCAlgo = "OC-Algo"
)
var (
@@ -46,6 +47,7 @@ type SignedURLAuthenticator struct {
UserProvider backend.UserBackend
UserRoleAssigner userroles.UserRoleAssigner
Store storesvc.StoreService
Now func() time.Time
}
func (m SignedURLAuthenticator) shouldServe(req *http.Request) bool {
@@ -58,30 +60,30 @@ func (m SignedURLAuthenticator) shouldServe(req *http.Request) bool {
func (m SignedURLAuthenticator) validate(req *http.Request) (err error) {
query := req.URL.Query()
if ok, err := m.allRequiredParametersArePresent(query); !ok {
if err := m.allRequiredParametersArePresent(query); err != nil {
return err
}
if ok, err := m.requestMethodMatches(req.Method, query); !ok {
if err := m.requestMethodMatches(req.Method, query); err != nil {
return err
}
if ok, err := m.requestMethodIsAllowed(req.Method); !ok {
if err := m.requestMethodIsAllowed(req.Method); err != nil {
return err
}
if expired, err := m.urlIsExpired(query, time.Now); expired {
if err = m.urlIsExpired(query); err != nil {
return err
}
if ok, err := m.signatureIsValid(req); !ok {
if err := m.signatureIsValid(req); err != nil {
return err
}
return nil
}
func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values) (ok bool, err error) {
func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values) (err error) {
// check if required query parameters exist in given request query parameters
// OC-Signature - the computed signature - server will verify the request upon this REQUIRED
// OC-Credential - defines the user scope (shall we use the owncloud user id here - this might leak internal data ....) REQUIRED
@@ -90,23 +92,23 @@ func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values
// TODO OC-Verb - defines for which http verb the request is valid - defaults to GET OPTIONAL
for _, p := range _requiredParams {
if query.Get(p) == "" {
return false, fmt.Errorf("required %s parameter not found", p)
return fmt.Errorf("required %s parameter not found", p)
}
}
return true, nil
return nil
}
func (m SignedURLAuthenticator) requestMethodMatches(meth string, query url.Values) (ok bool, err error) {
func (m SignedURLAuthenticator) requestMethodMatches(meth string, query url.Values) (err error) {
// check if given url query parameter OC-Verb matches given request method
if !strings.EqualFold(meth, query.Get(_paramOCVerb)) {
return false, errors.New("required OC-Verb parameter did not match request method")
return errors.New("required OC-Verb parameter did not match request method")
}
return true, nil
return nil
}
func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (ok bool, err error) {
func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (err error) {
// check if given request method is allowed
methodIsAllowed := false
for _, am := range m.PreSignedURLConfig.AllowedHTTPMethods {
@@ -117,50 +119,81 @@ func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (ok bool, er
}
if !methodIsAllowed {
return false, errors.New("request method is not listed in PreSignedURLConfig AllowedHTTPMethods")
return errors.New("request method is not listed in PreSignedURLConfig AllowedHTTPMethods")
}
return true, nil
return nil
}
func (m SignedURLAuthenticator) urlIsExpired(query url.Values, now func() time.Time) (expired bool, err error) {
func (m SignedURLAuthenticator) urlIsExpired(query url.Values) (err error) {
// check if url is expired by checking if given date (OC-Date) + expires in seconds (OC-Expires) is after now
validFrom, err := time.Parse(time.RFC3339, query.Get(_paramOCDate))
if err != nil {
return true, err
return err
}
requestExpiry, err := time.ParseDuration(query.Get(_paramOCExpires) + "s")
if err != nil {
return true, err
return err
}
validTo := validFrom.Add(requestExpiry)
if !(m.Now().Before(validTo)) {
return errors.New("URL is expired")
}
return !(now().After(validFrom) && now().Before(validTo)), nil
return nil
}
func (m SignedURLAuthenticator) signatureIsValid(req *http.Request) (ok bool, err error) {
u := revactx.ContextMustGetUser(req.Context())
signingKey, err := m.getSigningKey(req.Context(), u.Id.OpaqueId)
func (m SignedURLAuthenticator) signatureIsValid(req *http.Request) (err error) {
c := revactx.ContextMustGetUser(req.Context())
signingKey, err := m.getSigningKey(req.Context(), c.Id.OpaqueId)
if err != nil {
m.Logger.Error().Err(err).Msg("could not retrieve signing key")
return false, err
return err
}
if len(signingKey) == 0 {
m.Logger.Error().Err(err).Msg("signing key empty")
return false, err
return err
}
q := req.URL.Query()
signature := q.Get(_paramOCSignature)
q.Del(_paramOCSignature)
req.URL.RawQuery = q.Encode()
url := req.URL.String()
if !req.URL.IsAbs() {
url = "https://" + req.Host + url // TODO where do we get the scheme from
u := m.buildUrlToSign(req)
computedSignature := m.createSignature(u, signingKey)
signatureInURL := req.URL.Query().Get(_paramOCSignature)
if computedSignature == signatureInURL {
return nil
}
return fmt.Errorf("signature mismatch: expected %s != actual %s", computedSignature, signatureInURL)
}
return m.createSignature(url, signingKey) == signature, nil
func (m SignedURLAuthenticator) buildUrlToSign(req *http.Request) string {
q := req.URL.Query()
// only params required for signing
signParameters := make(url.Values)
signParameters.Add(_paramOCCredential, q.Get(_paramOCCredential))
signParameters.Add(_paramOCDate, q.Get(_paramOCDate))
signParameters.Add(_paramOCExpires, q.Get(_paramOCExpires))
signParameters.Add(_paramOCVerb, q.Get(_paramOCVerb))
// remaining query params
q.Del(_paramOCAlgo)
q.Del(_paramOCCredential)
q.Del(_paramOCDate)
q.Del(_paramOCExpires)
q.Del(_paramOCSignature)
q.Del(_paramOCVerb)
url := *req.URL
if len(q) == 0 {
url.RawQuery = signParameters.Encode()
} else {
url.RawQuery = strings.Join([]string{q.Encode(), signParameters.Encode()}, "&")
}
u := url.String()
if !url.IsAbs() {
u = "https://" + req.Host + u // TODO where do we get the scheme
}
return u
}
func (m SignedURLAuthenticator) createSignature(url string, signingKey []byte) string {
@@ -223,10 +256,12 @@ func (m SignedURLAuthenticator) Authenticate(r *http.Request) (*http.Request, bo
Err(err).
Str("authenticator", "signed_url").
Str("path", r.URL.Path).
Str("url", r.URL.String()).
Msg("Could not get user by claim")
return nil, false
}
// TODO: set user in context
m.Logger.Debug().
Str("authenticator", "signed_url").
Str("path", r.URL.Path).

192
services/proxy/pkg/middleware/signed_url_auth_test.go
View File

@@ -1,6 +1,14 @@
package middleware
import (
"context"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
revactx "github.com/cs3org/reva/v2/pkg/ctx"
storemsg "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/store/v0"
v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/store/v0"
"github.com/owncloud/ocis/v2/services/proxy/pkg/config"
"github.com/stretchr/testify/assert"
"go-micro.dev/v4/client"
"net/http/httptest"
"testing"
"time"
@@ -34,21 +42,23 @@ func TestSignedURLAuth_allRequiredParametersPresent(t *testing.T) {
pua := SignedURLAuthenticator{}
baseURL := "https://example.com/example.jpg?"
tests := []struct {
params string
expected bool
params string
errorMessage string
}{
{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", true},
{"OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", false},
{"OC-Signature=something&OC-Date=something&OC-Expires=something&OC-Verb=something", false},
{"OC-Signature=something&OC-Credential=something&OC-Expires=something&OC-Verb=something", false},
{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Verb=something", false},
{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something", false},
{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", ""},
{"OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", "required OC-Signature parameter not found"},
{"OC-Signature=something&OC-Date=something&OC-Expires=something&OC-Verb=something", "required OC-Credential parameter not found"},
{"OC-Signature=something&OC-Credential=something&OC-Expires=something&OC-Verb=something", "required OC-Date parameter not found"},
{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Verb=something", "required OC-Expires parameter not found"},
{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something", "required OC-Verb parameter not found"},
}
for _, tt := range tests {
r := httptest.NewRequest("", baseURL+tt.params, nil)
ok, _ := pua.allRequiredParametersArePresent(r.URL.Query())
if ok != tt.expected {
t.Errorf("with %s expected %t got %t", tt.params, tt.expected, ok)
err := pua.allRequiredParametersArePresent(r.URL.Query())
if tt.errorMessage != "" {
assert.EqualError(t, err, tt.errorMessage, tt.params)
} else {
assert.Nil(t, err)
}
}
}
@@ -56,20 +66,22 @@ func TestSignedURLAuth_allRequiredParametersPresent(t *testing.T) {
func TestSignedURLAuth_requestMethodMatches(t *testing.T) {
pua := SignedURLAuthenticator{}
tests := []struct {
method string
url string
expected bool
method string
url string
errorMessage string
}{
{"GET", "https://example.com/example.jpg?OC-Verb=GET", true},
{"GET", "https://example.com/example.jpg?OC-Verb=get", true},
{"POST", "https://example.com/example.jpg?OC-Verb=GET", false},
{"GET", "https://example.com/example.jpg?OC-Verb=GET", ""},
{"GET", "https://example.com/example.jpg?OC-Verb=get", ""},
{"POST", "https://example.com/example.jpg?OC-Verb=GET", "required OC-Verb parameter did not match request method"},
}
for _, tt := range tests {
r := httptest.NewRequest(tt.method, tt.url, nil)
ok, _ := pua.requestMethodMatches(r.Method, r.URL.Query())
if ok != tt.expected {
t.Errorf("with method %s and url %s expected %t got %t", tt.method, tt.url, tt.expected, ok)
err := pua.requestMethodMatches(r.Method, r.URL.Query())
if tt.errorMessage != "" {
assert.EqualError(t, err, tt.errorMessage, tt.url)
} else {
assert.Nil(t, err)
}
}
}
@@ -77,50 +89,62 @@ func TestSignedURLAuth_requestMethodMatches(t *testing.T) {
func TestSignedURLAuth_requestMethodIsAllowed(t *testing.T) {
pua := SignedURLAuthenticator{}
tests := []struct {
method string
allowed []string
expected bool
method string
allowed []string
errorMessage string
}{
{"GET", []string{}, false},
{"GET", []string{"POST"}, false},
{"GET", []string{"GET"}, true},
{"GET", []string{"get"}, true},
{"GET", []string{"POST", "GET"}, true},
{"GET", []string{}, "request method is not listed in PreSignedURLConfig AllowedHTTPMethods"},
{"GET", []string{"POST"}, "request method is not listed in PreSignedURLConfig AllowedHTTPMethods"},
{"GET", []string{"GET"}, ""},
{"GET", []string{"get"}, ""},
{"GET", []string{"POST", "GET"}, ""},
}
for _, tt := range tests {
pua.PreSignedURLConfig.AllowedHTTPMethods = tt.allowed
ok, _ := pua.requestMethodIsAllowed(tt.method)
if ok != tt.expected {
t.Errorf("with method %s and allowed methods %s expected %t got %t", tt.method, tt.allowed, tt.expected, ok)
err := pua.requestMethodIsAllowed(tt.method)
if tt.errorMessage != "" {
assert.EqualError(t, err, tt.errorMessage)
} else {
assert.Nil(t, err)
}
}
}
func TestSignedURLAuth_urlIsExpired(t *testing.T) {
pua := SignedURLAuthenticator{}
nowFunc := func() time.Time {
t, _ := time.Parse(time.RFC3339, "2020-02-02T12:30:00.000Z")
return t
}
pua := SignedURLAuthenticator{
Now: nowFunc,
}
tests := []struct {
url string
isExpired bool
url string
errorMessage string
}{
{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=61", false},
{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=invalid", true},
{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=59", true},
{"http://example.com/example.jpg?OC-Date=2020-02-03T12:29:00.000Z&OC-Expires=59", true},
{"http://example.com/example.jpg?OC-Date=2020-02-01T12:29:00.000Z&OC-Expires=59", true},
// a valid signed url
{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=61", ""},
// invalid expiry
{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=invalid", "time: invalid duration \"invalids\""},
// wrong date format on OC-Date
{"http://example.com/example.jpg?OC-Date=2020-02-02TTT12:29:00.000Z&OC-Expires=5", "parsing time \"2020-02-02TTT12:29:00.000Z\" as \"2006-01-02T15:04:05Z07:00\": cannot parse \"TT12:29:00.000Z\" as \"15\""},
// expired - 12:29:00 + 59s < 12:30
{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=59", "URL is expired"},
// expired - basically url was created yesterday
{"http://example.com/example.jpg?OC-Date=2020-02-01T12:29:00.000Z&OC-Expires=59", "URL is expired"},
// future OC-Date - also valid now
{"http://example.com/example.jpg?OC-Date=2020-02-03T12:29:00.000Z&OC-Expires=59", ""},
}
for _, tt := range tests {
r := httptest.NewRequest("", tt.url, nil)
expired, _ := pua.urlIsExpired(r.URL.Query(), nowFunc)
if expired != tt.isExpired {
t.Errorf("with %s expected %t got %t", tt.url, tt.isExpired, expired)
err := pua.urlIsExpired(r.URL.Query())
if tt.errorMessage != "" {
assert.EqualError(t, err, tt.errorMessage, tt.url)
} else {
assert.Nil(t, err)
}
}
}
@@ -134,3 +158,85 @@ func TestSignedURLAuth_createSignature(t *testing.T) {
t.Fail()
}
}
type MyStoreService struct {
}
func (s MyStoreService) Read(_ context.Context, _ *v0.ReadRequest, _ ...client.CallOption) (*v0.ReadResponse, error) {
r := v0.ReadResponse{
Records: []*storemsg.Record{{
Key: "foo",
Value: []byte("1234567890"),
Expiry: 0,
Metadata: nil,
}},
}
return &r, nil
}
func (s MyStoreService) Write(_ context.Context, _ *v0.WriteRequest, _ ...client.CallOption) (*v0.WriteResponse, error) {
return nil, nil
}
func (s MyStoreService) Delete(_ context.Context, _ *v0.DeleteRequest, _ ...client.CallOption) (*v0.DeleteResponse, error) {
return nil, nil
}
func (s MyStoreService) List(_ context.Context, _ *v0.ListRequest, _ ...client.CallOption) (v0.Store_ListService, error) {
return nil, nil
}
func (s MyStoreService) Databases(_ context.Context, _ *v0.DatabasesRequest, _ ...client.CallOption) (*v0.DatabasesResponse, error) {
return nil, nil
}
func (s MyStoreService) Tables(_ context.Context, _ *v0.TablesRequest, _ ...client.CallOption) (*v0.TablesResponse, error) {
return nil, nil
}
func TestSignedURLAuth_validate(t *testing.T) {
nowFunc := func() time.Time {
t, _ := time.Parse(time.RFC3339, "2020-02-02T12:30:00.000Z")
return t
}
cfg := config.PreSignedURL{
AllowedHTTPMethods: []string{"get"},
Enabled: true,
}
store := MyStoreService{}
pua := SignedURLAuthenticator{
PreSignedURLConfig: cfg,
Store: store,
Now: nowFunc,
}
tests := []struct {
now string
url string
errorMessage string
}{
{"2020-02-02T12:30:00.000Z", "http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=invalid", "required OC-Signature parameter not found"},
{"2020-02-02T12:30:00.000Z", "http://cloud.example.net/?OC-Credential=alice&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", "URL is expired"},
{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Credential=alice&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b", "signature mismatch: expected f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6 != actual f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b"},
{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Credential=alice&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", ""},
{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Credential=alice&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", ""},
{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Algo=PBKDF2%2F10000-SHA512&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Credential=alice&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", ""},
{"2024-02-07T12:03:11.966Z", "http://localhost:33001/try?id=1&id=2&OC-Credential=user&OC-Date=2024-02-07T12%3A03%3A11.966Z&OC-Expires=2&OC-Verb=GET&OC-Algo=PBKDF2%2F10000-SHA512&OC-Signature=86e21a1efbf0be989a206109cfedf70a22f338dc8995e849ce002032bc6741c5", ""},
}
for _, tt := range tests {
u := userpb.User{
Id: &userpb.UserId{OpaqueId: "useri"},
DisplayName: "Test User",
}
ctx := revactx.ContextSetUser(context.Background(), &u)
pua.Now = func() time.Time {
t, _ := time.Parse(time.RFC3339, tt.now)
return t
}
r := httptest.NewRequest("", tt.url, nil).WithContext(ctx)
err := pua.validate(r)
if tt.errorMessage == "" {
assert.Nil(t, err)
} else {
assert.EqualError(t, err, tt.errorMessage)
}
}
}