From cdf2b83d5b6921bc2e777fc32ec39db9dc0974cd Mon Sep 17 00:00:00 2001 From: Amal Thundiyil Date: Thu, 21 Apr 2022 18:41:02 +0530 Subject: [PATCH 1/4] fix: add configurations for `insecure` options --- cmd/reva/download.go | 6 ++---- cmd/reva/executor.go | 4 ++-- cmd/reva/main.go | 10 ++++----- pkg/cbox/group/rest/rest.go | 5 ++++- pkg/cbox/user/rest/rest.go | 5 ++++- pkg/cbox/utils/tokenmanagement.go | 32 +++++++++++++++-------------- pkg/datatx/manager/rclone/rclone.go | 4 ++-- 7 files changed, 35 insertions(+), 31 deletions(-) diff --git a/cmd/reva/download.go b/cmd/reva/download.go index 194f415c57..e06cf323f5 100644 --- a/cmd/reva/download.go +++ b/cmd/reva/download.go @@ -104,10 +104,8 @@ func downloadCommand() *command { httpReq.Header.Set(datagateway.TokenTransportHeader, p.Token) httpClient := rhttp.GetHTTPClient( rhttp.Context(ctx), - // TODO make insecure configurable - rhttp.Insecure(true), - // TODO make timeout configurable - rhttp.Timeout(time.Duration(24*int64(time.Hour))), + rhttp.Insecure(insecure), + rhttp.Timeout(time.Duration(timeout*int64(time.Hour))), ) httpRes, err := httpClient.Do(httpReq) diff --git a/cmd/reva/executor.go b/cmd/reva/executor.go index 07150735db..90558d00cf 100644 --- a/cmd/reva/executor.go +++ b/cmd/reva/executor.go @@ -30,7 +30,7 @@ import ( // Executor provides exec command handler type Executor struct { - Timeout int + Timeout int64 } // Execute provides execute commands @@ -79,7 +79,7 @@ func (e *Executor) Execute(s string) { select { case <-signalChan: cancel() - case <-time.After(time.Duration(e.Timeout * int(time.Second))): + case <-time.After(time.Duration(e.Timeout * int64(time.Second))): cancel() case <-ctx.Done(): } diff --git a/cmd/reva/main.go b/cmd/reva/main.go index abd30ba64a..b03e6efa22 100644 --- a/cmd/reva/main.go +++ b/cmd/reva/main.go @@ -34,7 +34,7 @@ var ( conf *config host string insecure, skipverify, disableargprompt bool - timeout int + timeout int64 helpCommandOutput string @@ -99,7 +99,7 @@ func init() { flag.BoolVar(&insecure, "insecure", false, "disables grpc transport security") flag.BoolVar(&skipverify, "skip-verify", false, "whether to skip verifying the server's certificate chain and host name") flag.BoolVar(&disableargprompt, "disable-arg-prompt", false, "whether to disable prompts for command arguments") - flag.IntVar(&timeout, "timout", -1, "the timeout in seconds for executing the commands, -1 means no timeout") + flag.Int64Var(&timeout, "timout", -1, "the timeout in seconds for executing the commands, -1 means no timeout") flag.Parse() } @@ -114,10 +114,8 @@ func main() { } client = rhttp.GetHTTPClient( - // TODO make insecure configurable - rhttp.Insecure(true), - // TODO make timeout configurable - rhttp.Timeout(time.Duration(24*int64(time.Hour))), + rhttp.Insecure(insecure), + rhttp.Timeout(time.Duration(timeout*int64(time.Hour))), ) generateMainUsage() diff --git a/pkg/cbox/group/rest/rest.go b/pkg/cbox/group/rest/rest.go index 9c14930fcb..7557b68571 100644 --- a/pkg/cbox/group/rest/rest.go +++ b/pkg/cbox/group/rest/rest.go @@ -116,7 +116,10 @@ func New(m map[string]interface{}) (group.Manager, error) { c.init() redisPool := initRedisPool(c.RedisAddress, c.RedisUsername, c.RedisPassword) - apiTokenManager := utils.InitAPITokenManager(c.TargetAPI, c.OIDCTokenEndpoint, c.ClientID, c.ClientSecret) + apiTokenManager, err := utils.InitAPITokenManager(m) + if err != nil { + return nil, err + } mgr := &manager{ conf: c, diff --git a/pkg/cbox/user/rest/rest.go b/pkg/cbox/user/rest/rest.go index 0c77c95c8a..47d14d1360 100644 --- a/pkg/cbox/user/rest/rest.go +++ b/pkg/cbox/user/rest/rest.go @@ -123,7 +123,10 @@ func (m *manager) Configure(ml map[string]interface{}) error { } c.init() redisPool := initRedisPool(c.RedisAddress, c.RedisUsername, c.RedisPassword) - apiTokenManager := utils.InitAPITokenManager(c.TargetAPI, c.OIDCTokenEndpoint, c.ClientID, c.ClientSecret) + apiTokenManager, err := utils.InitAPITokenManager(ml) + if err != nil { + return err + } m.conf = c m.redisPool = redisPool m.apiTokenManager = apiTokenManager diff --git a/pkg/cbox/utils/tokenmanagement.go b/pkg/cbox/utils/tokenmanagement.go index c8978f4929..8a995dc324 100644 --- a/pkg/cbox/utils/tokenmanagement.go +++ b/pkg/cbox/utils/tokenmanagement.go @@ -30,6 +30,7 @@ import ( "time" "github.com/cs3org/reva/pkg/rhttp" + "github.com/mitchellh/mapstructure" ) // APITokenManager stores config related to api management @@ -47,26 +48,27 @@ type OIDCToken struct { } type config struct { - TargetAPI string - OIDCTokenEndpoint string - ClientID string - ClientSecret string + TargetAPI string `mapstructure:"target_api"` + OIDCTokenEndpoint string `mapstructure:"oidc_token_endpoint"` + ClientID string `mapstructure:"client_id"` + ClientSecret string `mapstructure:"client_secret"` + Timeout int64 `mapstructure:"timeout"` + Insecure bool `mapstructure:"insecure"` } // InitAPITokenManager initializes a new APITokenManager -func InitAPITokenManager(targetAPI, oidcTokenEndpoint, clientID, clientSecret string) *APITokenManager { +func InitAPITokenManager(conf map[string]interface{}) (*APITokenManager, error) { + c := &config{} + if err := mapstructure.Decode(conf, c); err != nil { + return nil, err + } + return &APITokenManager{ - conf: &config{ - TargetAPI: targetAPI, - OIDCTokenEndpoint: oidcTokenEndpoint, - ClientID: clientID, - ClientSecret: clientSecret, - }, + conf: c, client: rhttp.GetHTTPClient( - rhttp.Timeout(10*time.Second), - rhttp.Insecure(true), - ), - } + rhttp.Timeout(time.Duration(c.Timeout*int64(time.Second))), + rhttp.Insecure(c.Insecure), + )}, nil } func (a *APITokenManager) renewAPIToken(ctx context.Context, forceRenewal bool) error { diff --git a/pkg/datatx/manager/rclone/rclone.go b/pkg/datatx/manager/rclone/rclone.go index 5e4008cbbf..7644df6658 100644 --- a/pkg/datatx/manager/rclone/rclone.go +++ b/pkg/datatx/manager/rclone/rclone.go @@ -67,6 +67,7 @@ type config struct { File string `mapstructure:"file"` JobStatusCheckInterval int `mapstructure:"job_status_check_interval"` JobTimeout int `mapstructure:"job_timeout"` + Insecure bool `mapstructure:"insecure"` } type rclone struct { @@ -125,8 +126,7 @@ func New(m map[string]interface{}) (txdriver.Manager, error) { } c.init(m) - // TODO insecure should be configurable - client := rhttp.GetHTTPClient(rhttp.Insecure(true)) + client := rhttp.GetHTTPClient(rhttp.Insecure(c.Insecure)) // The persistency driver // Load or create 'db' From d293425628ff24a10fba1c5a06a75537159885f9 Mon Sep 17 00:00:00 2001 From: Amal Thundiyil Date: Sat, 23 Apr 2022 18:24:21 +0530 Subject: [PATCH 2/4] changelog: add changelog --- changelog/unreleased/configure-http-insecure.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/configure-http-insecure.md diff --git a/changelog/unreleased/configure-http-insecure.md b/changelog/unreleased/configure-http-insecure.md new file mode 100644 index 0000000000..2fda6eed85 --- /dev/null +++ b/changelog/unreleased/configure-http-insecure.md @@ -0,0 +1,5 @@ +Bugfix: Make hardcoded HTTP "insecure" options configurable + +HTTP "insecure" options must be configurable and default to false. + +https://github.com/cs3org/reva/issues/2216 \ No newline at end of file From b77bc1a8480ac089f814da2dcfa0a6cf94b00f10 Mon Sep 17 00:00:00 2001 From: Amal Thundiyil Date: Thu, 28 Apr 2022 18:20:00 +0530 Subject: [PATCH 3/4] reva: use separate `insecure` flag for data gateway service --- cmd/reva/main.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/reva/main.go b/cmd/reva/main.go index b03e6efa22..d9eb75a7d7 100644 --- a/cmd/reva/main.go +++ b/cmd/reva/main.go @@ -31,10 +31,10 @@ import ( ) var ( - conf *config - host string - insecure, skipverify, disableargprompt bool - timeout int64 + conf *config + host string + insecure, skipverify, disableargprompt, insecuredatagateway bool + timeout int64 helpCommandOutput string @@ -97,9 +97,10 @@ var ( func init() { flag.StringVar(&host, "host", "", "address of the GRPC gateway host") flag.BoolVar(&insecure, "insecure", false, "disables grpc transport security") + flag.BoolVar(&insecuredatagateway, "insecure-data-gateway", false, "disables grpc transport security for data gateway service") flag.BoolVar(&skipverify, "skip-verify", false, "whether to skip verifying the server's certificate chain and host name") flag.BoolVar(&disableargprompt, "disable-arg-prompt", false, "whether to disable prompts for command arguments") - flag.Int64Var(&timeout, "timout", -1, "the timeout in seconds for executing the commands, -1 means no timeout") + flag.Int64Var(&timeout, "timeout", -1, "the timeout in seconds for executing the commands, -1 means no timeout") flag.Parse() } @@ -114,7 +115,7 @@ func main() { } client = rhttp.GetHTTPClient( - rhttp.Insecure(insecure), + rhttp.Insecure(insecuredatagateway), rhttp.Timeout(time.Duration(timeout*int64(time.Hour))), ) From 344613a0b6c44bb2568fa2d7b6c02300ec015076 Mon Sep 17 00:00:00 2001 From: Amal Thundiyil Date: Tue, 3 May 2022 16:13:56 +0530 Subject: [PATCH 4/4] reva: review comments --- cmd/reva/download.go | 19 ++++++++----------- cmd/reva/main.go | 21 ++++++++++++++++++--- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/cmd/reva/download.go b/cmd/reva/download.go index e06cf323f5..82da692842 100644 --- a/cmd/reva/download.go +++ b/cmd/reva/download.go @@ -23,7 +23,6 @@ import ( "io" "net/http" "os" - "time" "github.com/cheggaaa/pb" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -50,7 +49,7 @@ func downloadCommand() *command { remote := cmd.Args()[0] local := cmd.Args()[1] - client, err := getClient() + gatewayClient, err := getClient() if err != nil { return err } @@ -58,7 +57,7 @@ func downloadCommand() *command { ref := &provider.Reference{Path: remote} req1 := &provider.StatRequest{Ref: ref} ctx := getAuthContext() - res1, err := client.Stat(ctx, req1) + res1, err := gatewayClient.Stat(ctx, req1) if err != nil { return err } @@ -71,7 +70,7 @@ func downloadCommand() *command { req2 := &provider.InitiateFileDownloadRequest{ Ref: &provider.Reference{Path: remote}, } - res, err := client.InitiateFileDownload(ctx, req2) + res, err := gatewayClient.InitiateFileDownload(ctx, req2) if err != nil { return err } @@ -102,13 +101,8 @@ func downloadCommand() *command { } httpReq.Header.Set(datagateway.TokenTransportHeader, p.Token) - httpClient := rhttp.GetHTTPClient( - rhttp.Context(ctx), - rhttp.Insecure(insecure), - rhttp.Timeout(time.Duration(timeout*int64(time.Hour))), - ) - httpRes, err := httpClient.Do(httpReq) + httpRes, err := client.Do(httpReq) if err != nil { return err } @@ -142,7 +136,10 @@ func downloadCommand() *command { return cmd } -func getDownloadProtocolInfo(protocolInfos []*gateway.FileDownloadProtocol, protocol string) (*gateway.FileDownloadProtocol, error) { +func getDownloadProtocolInfo( + protocolInfos []*gateway.FileDownloadProtocol, + protocol string, +) (*gateway.FileDownloadProtocol, error) { for _, p := range protocolInfos { if p.Protocol == protocol { return p, nil diff --git a/cmd/reva/main.go b/cmd/reva/main.go index d9eb75a7d7..27e754d70a 100644 --- a/cmd/reva/main.go +++ b/cmd/reva/main.go @@ -97,8 +97,18 @@ var ( func init() { flag.StringVar(&host, "host", "", "address of the GRPC gateway host") flag.BoolVar(&insecure, "insecure", false, "disables grpc transport security") - flag.BoolVar(&insecuredatagateway, "insecure-data-gateway", false, "disables grpc transport security for data gateway service") - flag.BoolVar(&skipverify, "skip-verify", false, "whether to skip verifying the server's certificate chain and host name") + flag.BoolVar( + &insecuredatagateway, + "insecure-data-gateway", + false, + "disables grpc transport security for data gateway service", + ) + flag.BoolVar( + &skipverify, + "skip-verify", + false, + "whether to skip verifying the server's certificate chain and host name", + ) flag.BoolVar(&disableargprompt, "disable-arg-prompt", false, "whether to disable prompts for command arguments") flag.Int64Var(&timeout, "timeout", -1, "the timeout in seconds for executing the commands, -1 means no timeout") flag.Parse() @@ -152,6 +162,11 @@ func generateMainUsage() { helpCommandOutput = "Command line interface to REVA:\n" for _, cmd := range commands { - helpCommandOutput += fmt.Sprintf("%s%s%s\n", cmd.Name, strings.Repeat(" ", 4+(n-len(cmd.Name))), cmd.Description()) + helpCommandOutput += fmt.Sprintf( + "%s%s%s\n", + cmd.Name, + strings.Repeat(" ", 4+(n-len(cmd.Name))), + cmd.Description(), + ) } }