From ff2abea064245eeebbee30189b2e7dc49be8b076 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Sun, 25 Sep 2022 13:48:55 +0300 Subject: [PATCH 01/30] guard-gate --- pkg/guard-gate/gate.go | 217 +++++++++++++++++ pkg/guard-gate/gate_test.go | 412 +++++++++++++++++++++++++++++++++ pkg/guard-gate/session.go | 177 ++++++++++++++ pkg/guard-gate/session_test.go | 193 +++++++++++++++ pkg/guard-utils/ticker_test.go | 2 +- 5 files changed, 1000 insertions(+), 1 deletion(-) create mode 100644 pkg/guard-gate/gate.go create mode 100644 pkg/guard-gate/gate_test.go create mode 100644 pkg/guard-gate/session.go create mode 100644 pkg/guard-gate/session_test.go diff --git a/pkg/guard-gate/gate.go b/pkg/guard-gate/gate.go new file mode 100644 index 00000000..23dcc07a --- /dev/null +++ b/pkg/guard-gate/gate.go @@ -0,0 +1,217 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package guardgate + +import ( + "context" + "errors" + "net/http" + "strings" + "time" + + pi "knative.dev/security-guard/pkg/pluginterfaces" + + utils "knative.dev/security-guard/pkg/guard-utils" +) + +const plugVersion string = "0.0.1" +const plugName string = "guard" + +const ( + guardianLoadIntervalDefault = 5 * time.Minute + reportPileIntervalDefault = 4 * time.Second + podMonitorIntervalDefault = 5 * time.Second +) + +var errSecurity error = errors.New("security blocked by guard") + +type ctxKey string + +type plug struct { + name string + version string + + // guard gate plug specifics + gateState *gateState // maintainer of the criteria and ctrl, include pod profile, gate stats and gate level alert + guardianLoadTicker utils.Ticker // tick to gateState.loadConfig() gateState + reportPileTicker utils.Ticker // tick to gateState.flushPile() + podMonitorTicker utils.Ticker // tick to gateState.profileAndDecidePod() +} + +func (p *plug) Shutdown() { + pi.Log.Infof("%s: Shutdown", p.name) + p.gateState.flushPile() +} + +func (p *plug) PlugName() string { + return p.name +} + +func (p *plug) PlugVersion() string { + return p.version +} + +func (p *plug) ApproveRequest(req *http.Request) (*http.Request, error) { + ctx, cancelFunction := context.WithCancel(req.Context()) + + s := newSession(p.gateState, cancelFunction) // maintainer of the profile + + // Req + s.screenEnvelop() + s.screenRequest(req) + s.screenRequestBody(req) + + if p.gateState.shouldBlock() && (s.hasAlert() || p.gateState.hasAlert()) { + p.gateState.addStat("BlockOnRequest") + cancelFunction() + return nil, errSecurity + } + + // Request not blocked + ctx = s.addSessionToContext(ctx) + ctx = context.WithValue(ctx, ctxKey("GuardSession"), s) + + req = req.WithContext(ctx) + + //goroutine to accompany the request + go s.sessionEventLoop(ctx) + + return req, nil +} + +func (p *plug) ApproveResponse(req *http.Request, resp *http.Response) (*http.Response, error) { + s := getSessionFromContext(req.Context()) + if s == nil { // This should never happen! + pi.Log.Warnf("%s ........... Blocked During Response! Missing context!", p.name) + return nil, errors.New("missing context") + } + + s.gotResponse = true + + s.screenResponse(resp) + s.screenResponseBody(resp) + s.screenEnvelop() + if p.gateState.shouldBlock() && (s.hasAlert() || p.gateState.hasAlert()) { + s.cancel() + p.gateState.addStat("BlockOnResponse") + return nil, errSecurity + } + + return resp, nil +} + +func (p *plug) guardMainEventLoop(ctx context.Context) { + p.guardianLoadTicker.Start() + p.reportPileTicker.Start() + p.podMonitorTicker.Start() + defer func() { + p.guardianLoadTicker.Stop() + p.reportPileTicker.Stop() + p.podMonitorTicker.Stop() + p.gateState.flushPile() + pi.Log.Infof("Statistics: %s", p.gateState.stat.Log()) + pi.Log.Infof("%s Done!", plugName) + }() + + for { + select { + // Always finish guard here! + case <-ctx.Done(): + return + + // Periodically get an updated Guardian + case <-p.guardianLoadTicker.Ch(): + pi.Log.Debugf("Load Guardian Ticker") + p.gateState.loadConfig() + + // Periodically send pile to the guard-service + case <-p.reportPileTicker.Ch(): + pi.Log.Debugf("Report Pile Ticker") + p.gateState.flushPile() + + // Periodically profile of the pod + case <-p.podMonitorTicker.Ch(): + p.gateState.profileAndDecidePod() + } + } +} +func (p *plug) preInit(ctx context.Context, c map[string]string, sid string, ns string, logger pi.Logger) (context.Context, context.CancelFunc) { + var ok bool + var v string + + ctx, cancelFunction := context.WithCancel(ctx) + + // Defaults used without config when used as a sidecar + guardServiceUrl := "http://guard-service.knative-guard" + useCm := false + monitorPod := true + + if c != nil { + if v, ok = c["guard-url"]; ok && v != "" { + // use default + guardServiceUrl = v + } + + if v, ok = c["use-cm"]; ok && strings.EqualFold(v, "true") { + useCm = true + } + + if v, ok = c["monitor-pod"]; ok && !strings.EqualFold(v, "true") { + monitorPod = false + } + p.guardianLoadTicker.Parse(c["guardian-load-interval"], guardianLoadIntervalDefault) + p.reportPileTicker.Parse(c["report-pile-interval"], reportPileIntervalDefault) + p.podMonitorTicker.Parse(c["pod-monitor-interval"], podMonitorIntervalDefault) + + pi.Log.Debugf("guard-gate configuration: sid=%s, ns=%s, useCm=%t, guardUrl=%s, p.monitorPod=%t, guardian-load-interval %v, report-pile-interval %v, pod-monitor-interval %v", + sid, ns, useCm, guardServiceUrl, monitorPod, c["guardian-load-interval"], c["report-pile-interval"], c["pod-monitor-interval"]) + } else { + p.guardianLoadTicker.Parse("", guardianLoadIntervalDefault) + p.reportPileTicker.Parse("", reportPileIntervalDefault) + p.podMonitorTicker.Parse("", podMonitorIntervalDefault) + } + + // serviceName should never be "ns.{namespace}" as this is a reserved name + if strings.HasPrefix(sid, "ns.") { + // mandatory + panic("Ileal serviceName - ns.{Namespace} is reserved") + } + + p.gateState = new(gateState) + p.gateState.init(cancelFunction, monitorPod, guardServiceUrl, sid, ns, useCm) + pi.Log.Infof("Loading Guardian") + return ctx, cancelFunction +} + +func (p *plug) Init(ctx context.Context, c map[string]string, sid string, ns string, logger pi.Logger) context.Context { + newCtx, _ := p.preInit(ctx, c, sid, ns, logger) + + // cant be tested as depend on KubeMgr + p.gateState.start() + p.gateState.loadConfig() + p.gateState.profileAndDecidePod() + + //goroutine for Guard instance + go p.guardMainEventLoop(newCtx) + + return newCtx +} + +func init() { + gate := &plug{version: plugVersion, name: plugName} + pi.RegisterPlug(gate) +} diff --git a/pkg/guard-gate/gate_test.go b/pkg/guard-gate/gate_test.go new file mode 100644 index 00000000..3ed918ef --- /dev/null +++ b/pkg/guard-gate/gate_test.go @@ -0,0 +1,412 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package guardgate + +import ( + "context" + "fmt" + "net/http/httptest" + "testing" + "time" + + utils "knative.dev/security-guard/pkg/guard-utils" + pi "knative.dev/security-guard/pkg/pluginterfaces" +) + +type dLog struct{} + +func (d dLog) Debugf(format string, args ...interface{}) {} +func (d dLog) Infof(format string, args ...interface{}) {} +func (d dLog) Warnf(format string, args ...interface{}) {} +func (d dLog) Errorf(format string, args ...interface{}) {} +func (d dLog) Sync() error { return nil } + +var defaultLog dLog + +func testInit(c map[string]string) *plug { + p := new(plug) + p.version = plugVersion + p.name = plugName + + if c == nil { + c = make(map[string]string) + c["guard-url"] = "url" + c["use-cm"] = "true" + c["monitor-pod"] = "x" + } + + pi.RegisterPlug(p) + p.preInit(context.Background(), c, "svcName", "myns", defaultLog) + p.gateState = fakeGateState() + p.gateState.loadConfig() + return p +} + +//func TestMain(m *testing.M) { +// code := m.Run() +// os.Exit(code) +//} + +func Test_plug_guardMainEventLoop(t *testing.T) { + t.Run("guardianLoadTicker", func(t *testing.T) { + p := new(plug) + p.version = plugVersion + p.name = plugName + c := make(map[string]string) + c["guard-url"] = "url" + c["use-cm"] = "true" + c["monitor-pod"] = "x" + + pi.RegisterPlug(p) + ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) + p.gateState = fakeGateState() + p.gateState.loadConfig() + p.gateState.srv.pile.Count = 7 + utils.MinimumInterval = 100000 + td, _ := time.ParseDuration("10ms") + + p.gateState.stat.Init() + p.guardianLoadTicker.Parse("", 300000) + // lets rely on timeout + go p.guardMainEventLoop(ctx) + time.Sleep(td) + cancelFunction() + if ret := p.gateState.stat.Log(); ret != "map[]" { + t.Errorf("expected stat %s received %s", "map[]", ret) + } + if p.gateState.srv.pile.Count != 7 { + t.Errorf("expected no pile flush") + } + }) + t.Run("podMonitorTicker", func(t *testing.T) { + p := new(plug) + p.version = plugVersion + p.name = plugName + + c := make(map[string]string) + c["guard-url"] = "url" + c["use-cm"] = "true" + c["monitor-pod"] = "x" + + pi.RegisterPlug(p) + ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) + p.gateState = fakeGateState() + p.gateState.loadConfig() + p.gateState.srv.pile.Count = 7 + utils.MinimumInterval = 100000 + td, _ := time.ParseDuration("10ms") + + p.gateState.stat.Init() + p.podMonitorTicker.Parse("", 300000) + // lets rely on timeout + go p.guardMainEventLoop(ctx) + time.Sleep(td) + cancelFunction() + if ret := p.gateState.stat.Log(); ret != "map[]" { + t.Errorf("expected stat %s received %s", "map[]", ret) + } + if p.gateState.srv.pile.Count != 7 { + t.Errorf("expected no pile flush") + } + }) + t.Run("reportPileTicker", func(t *testing.T) { + p := new(plug) + p.version = plugVersion + p.name = plugName + + c := make(map[string]string) + c["guard-url"] = "url" + c["use-cm"] = "true" + c["monitor-pod"] = "x" + + pi.RegisterPlug(p) + ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) + p.gateState = fakeGateState() + p.gateState.loadConfig() + + utils.MinimumInterval = 100000 + td, _ := time.ParseDuration("10ms") + + p.gateState.stat.Init() + p.reportPileTicker.Parse("", 300000) + // lets rely on timeout + go p.guardMainEventLoop(ctx) + time.Sleep(td) + cancelFunction() + if ret := p.gateState.stat.Log(); ret != "map[]" { + t.Errorf("expected stat %s received %s", "map[]", ret) + } + if p.gateState.srv.pile.Count != 0 { + t.Errorf("expected pile flush") + } + }) +} + +func Test_plug_guardMainEventLoop_1(t *testing.T) { + p := new(plug) + p.version = plugVersion + p.name = plugName + + c := make(map[string]string) + c["guard-url"] = "url" + c["use-cm"] = "true" + c["monitor-pod"] = "x" + + pi.RegisterPlug(p) + ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) + p.gateState = fakeGateState() + p.gateState.loadConfig() + + t.Run("simple", func(t *testing.T) { + utils.MinimumInterval = 100000 + td, _ := time.ParseDuration("10ms") + + p.gateState.stat.Init() + p.guardianLoadTicker.Parse("", 300000) + // lets rely on timeout + go p.guardMainEventLoop(ctx) + time.Sleep(td) + cancelFunction() + if ret := p.gateState.stat.Log(); ret != "map[]" { + t.Errorf("expected stat %s received %s", "map[]", ret) + } + + p.gateState.stat.Init() + p.podMonitorTicker.Parse("", 200000) + // lets rely on timeout + go p.guardMainEventLoop(ctx) + td, _ = time.ParseDuration("10ms") + time.Sleep(td) + cancelFunction() + if ret := p.gateState.stat.Log(); ret != "map[]" { + t.Errorf("expected stat %s received %s", "map[]", ret) + } + + p.gateState.stat.Init() + p.reportPileTicker.Parse("", 100000) + // lets rely on timeout + go p.guardMainEventLoop(ctx) + td, _ = time.ParseDuration("10ms") + time.Sleep(td) + cancelFunction() + if ret := p.gateState.stat.Log(); ret != "map[]" { + t.Errorf("expected stat %s received %s", "map[]", ret) + } + + cancelFunction() + p.guardMainEventLoop(ctx) + if ret := p.gateState.stat.Log(); ret != "map[]" { + t.Errorf("expected stat %s received %s", "map[]", ret) + } + + }) + +} + +func Test_plug_Initialize(t *testing.T) { + + tests := []struct { + name string + c map[string]string + monitorPod bool + guardServiceUrl string + useCm bool + }{ + // TODO: Add test cases. + { + name: "default", + c: map[string]string{ + "guard-url": "url", + "use-cm": "x", + "monitor-pod": "x", + }, + monitorPod: false, + guardServiceUrl: "url", + useCm: false, + }, { + name: "alternative", + c: map[string]string{ + "guard-url": "url1", + "use-cm": "true", + "monitor-pod": "true", + }, + monitorPod: true, + guardServiceUrl: "url1", + useCm: true, + }, { + name: "no c", + c: nil, + monitorPod: true, + guardServiceUrl: "http://guard-service.knative-guard", + useCm: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := new(plug) + p.version = plugVersion + p.name = plugName + + pi.RegisterPlug(p) + ctx, cancelFunction := p.preInit(context.Background(), tt.c, "svcName", "myns", defaultLog) + if ctx == context.Background() { + t.Error("extected a derived ctx") + } + if cancelFunction == nil { + t.Error("extected a cancelFunction") + } + if tt.monitorPod != p.gateState.monitorPod { + t.Errorf("extected monitorPod %t got %t", tt.monitorPod, p.gateState.monitorPod) + } + if tt.guardServiceUrl != p.gateState.srv.guardServiceUrl { + t.Errorf("extected guardServiceUrl %s got %s", tt.guardServiceUrl, p.gateState.srv.guardServiceUrl) + } + if tt.useCm != p.gateState.srv.useCm { + t.Errorf("extected useCm %t got %t", tt.useCm, p.gateState.srv.useCm) + } + }) + + } +} + +func Test_plug_initPanic(t *testing.T) { + t.Run("panic on sid", func(t *testing.T) { + defer func() { _ = recover() }() + p := new(plug) + p.preInit(context.Background(), nil, "ns.svcName", "myns", defaultLog) + t.Error("extected to panic") + }) +} + +func Test_plug_Shutdown(t *testing.T) { + tests := []struct { + name string + }{ + // TODO: Add test cases. + {""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := testInit(nil) + p.Shutdown() + }) + } +} + +func Test_plug_PlugName(t *testing.T) { + tests := []struct { + name string + want string + }{ + // TODO: Add test cases. + {"", plugName}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := testInit(nil) + if got := p.PlugName(); got != tt.want { + t.Errorf("plug.PlugName() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_plug_PlugVersion(t *testing.T) { + tests := []struct { + name string + want string + }{ + // TODO: Add test cases. + {"", plugVersion}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := testInit(nil) + if got := p.PlugVersion(); got != tt.want { + t.Errorf("plug.PlugVersion() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_plug_ApproveResponse(t *testing.T) { + t.Run("", func(t *testing.T) { + p := testInit(nil) + + req := httptest.NewRequest("GET", "/some/path", nil) + respRecorder := httptest.NewRecorder() + fmt.Fprintf(respRecorder, "Hi there!") + resp := respRecorder.Result() + resp.Request = req + resp.Header.Set("name", "val") + + _, err1 := p.ApproveResponse(req, resp) + if err1 == nil { + t.Errorf("ApproveResponse expected error ! ") + } + + req1, _ := p.ApproveRequest(req) + resp.Request = req1 + + _, err2 := p.ApproveResponse(req1, resp) + if err2 != nil { + t.Errorf("ApproveResponse error %v! ", err2) + } + + p.gateState.alert = "x" + p.gateState.ctrl.Block = true + + _, err3 := p.ApproveResponse(req1, resp) + + if err3 == nil { + t.Errorf("ApproveRequest returned error = %v", err1) + } + + }) + +} + +func Test_plug_ApproveRequest(t *testing.T) { + t.Run("", func(t *testing.T) { + p := testInit(nil) + req := httptest.NewRequest("GET", "/some/path", nil) + req.Header.Set("name", "value") + + req1, err1 := p.ApproveRequest(req) + + if err1 != nil { + t.Errorf("ApproveRequest returned error = %v", err1) + } + if req1 == nil { + t.Errorf("ApproveRequest did not return a req ") + } + + p.gateState.alert = "x" + p.gateState.ctrl.Block = true + + req2, err2 := p.ApproveRequest(req) + + if err2 == nil { + t.Errorf("ApproveRequest returned error = %v", err1) + } + if req2 != nil { + t.Errorf("ApproveRequest did not return a req ") + } + + }) + +} diff --git a/pkg/guard-gate/session.go b/pkg/guard-gate/session.go new file mode 100644 index 00000000..dafea45e --- /dev/null +++ b/pkg/guard-gate/session.go @@ -0,0 +1,177 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package guardgate + +import ( + "context" + "fmt" + "net" + "net/http" + "time" + + spec "knative.dev/security-guard/pkg/apis/guard/v1alpha1" + utils "knative.dev/security-guard/pkg/guard-utils" + pi "knative.dev/security-guard/pkg/pluginterfaces" +) + +const sessionKey = "GuardSession" + +type session struct { + sessionTicker utils.Ticker + gotResponse bool + alert string // session alert + reqTime time.Time // time when session was started + respTime time.Time // time when session response came + cancelFunc context.CancelFunc // cancel the session + profile spec.SessionDataProfile // maintainer of the session profile + gateState *gateState // maintainer of the criteria and ctrl, include pod profile, gate stats and gate level alert +} + +func newSession(state *gateState, cancel context.CancelFunc) *session { + s := new(session) + s.reqTime = time.Now() + s.respTime = s.reqTime // indicates that we do not know the response time + s.gateState = state + s.cancelFunc = cancel + + if err := s.sessionTicker.Parse("", podMonitorIntervalDefault); err != nil { + pi.Log.Debugf("Error on Ticker Parse: %v", err) + } + state.addStat("Total") + return s +} + +func getSessionFromContext(ctx context.Context) *session { + defer func() { + // This should never happen! + if r := recover(); r != nil { + pi.Log.Warnf("getSessionFromContext Recovered %s", r) + } + }() + + s, sExists := ctx.Value(ctxKey(sessionKey)).(*session) + if !sExists { + // This should never happen! + return nil + } + return s +} + +func (s *session) addSessionToContext(ctxIn context.Context) context.Context { + return context.WithValue(ctxIn, ctxKey(sessionKey), s) +} + +func (s *session) hasAlert() bool { + return s.alert != "" +} + +func (s *session) cancel() { + s.cancelFunc() +} + +func (s *session) sessionEventLoop(ctx context.Context) { + s.sessionTicker.Start() + + defer func() { + s.sessionTicker.Stop() + + // Should we learn? + if s.gateState.shouldLearn(s.hasAlert()) && s.gotResponse { + s.gateState.addProfile(&s.profile) + } + + // Should we alert? + if s.gateState.hasAlert() { + logAlert("Pod has an alert") + s.gateState.addStat("BlockOnPod") + return + } + if s.hasAlert() { + logAlert(s.alert) + s.gateState.addStat("SessionLevelAlert") + return + } + if !s.gotResponse { + pi.Log.Debugf("No Alert but completed before receiving a response!") + s.gateState.addStat("NoResponse") + return + } + pi.Log.Debugf("No Alert!") + s.gateState.addStat("NoAlert") + }() + + for { + select { + case <-ctx.Done(): + return + case <-s.sessionTicker.Ch(): + s.screenEnvelop() + s.screenPod() + if s.gateState.shouldBlock() && (s.hasAlert() || s.gateState.hasAlert()) { + s.cancel() + return + } + } + } +} + +func (s *session) screenResponseBody(req *http.Response) { + // TBD add screenResponseBody in a future PR + s.alert += s.gateState.decideRespBody(&s.profile.RespBody) +} + +func (s *session) screenRequestBody(req *http.Request) { + // TBD add screenRequestBody in a future PR + s.alert += s.gateState.decideReqBody(&s.profile.ReqBody) +} + +func (s *session) screenEnvelop() { + now := time.Now() + respTime := s.respTime + if !s.respTime.After(s.reqTime) { + // we do not know the response time, lets assume it is now + respTime = now + } + s.profile.Envelop.Profile(s.reqTime, respTime, now) + + s.alert += s.gateState.decideEnvelop(&s.profile.Envelop) +} + +func (s *session) screenPod() { + s.gateState.copyPodProfile(&s.profile.Pod) +} + +func (s *session) screenRequest(req *http.Request) { + // Request client and server identities + cip, _, err := net.SplitHostPort(req.RemoteAddr) + if err != nil { + s.alert += fmt.Sprintf("illegal req.RemoteAddr %s", err.Error()) + s.gateState.addStat("ReqCipFault") + } + + ip := net.ParseIP(cip) + s.profile.Req.Profile(req, ip) + + s.alert += s.gateState.decideReq(&s.profile.Req) +} + +func (s *session) screenResponse(resp *http.Response) { + s.respTime = time.Now() + s.profile.Resp.Profile(resp) + + s.alert += s.gateState.decideResp(&s.profile.Resp) +} diff --git a/pkg/guard-gate/session_test.go b/pkg/guard-gate/session_test.go new file mode 100644 index 00000000..8ddd7ebe --- /dev/null +++ b/pkg/guard-gate/session_test.go @@ -0,0 +1,193 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package guardgate + +import ( + "context" + "net/http" + "reflect" + "strings" + "testing" + "time" + + "github.com/emicklei/go-restful" + utils "knative.dev/security-guard/pkg/guard-utils" +) + +var sessionCanceled int + +func fakeSessionCancel() { + sessionCanceled++ +} + +func Test_SessionInContext(t *testing.T) { + ctx1 := context.Background() + gs := fakeGateState() + gs.loadConfig() + + t.Run("simple", func(t *testing.T) { + s1 := newSession(gs, nil) + s1.cancelFunc = fakeSessionCancel + ctx2 := s1.addSessionToContext(ctx1) + s2 := getSessionFromContext(ctx2) + + if !reflect.DeepEqual(s1, s2) { + t.Errorf("received a different session %v, want %v", s2, s1) + } + if s2.hasAlert() { + t.Errorf("expected no alert") + } + + sessionCanceled = 0 + if s2.cancel(); sessionCanceled != 1 { + t.Errorf("expected canceled") + } + + bodyReader := strings.NewReader(`{"Username": "12124", "Password": "testinasg", "Channel": "M"}`) + + req, _ := http.NewRequest("POST", "/config", bodyReader) + req.Header.Set("Content-Type", restful.MIME_JSON) + s2.screenRequest(req) + if !s2.hasAlert() { + t.Errorf("expected alert") + } + s2.alert = "" + req.RemoteAddr = "1.2.3.4:80" + s2.screenRequest(req) + if s2.hasAlert() { + t.Errorf("expected no alert") + } + s2.gateState.criteria.Active = true + s2.screenRequest(req) + if !s2.hasAlert() { + t.Errorf("expected alert") + } + s2.alert = "" + s2.gateState.criteria.Active = false + + td, _ := time.ParseDuration("1s") + time.Sleep(td) + s2.screenEnvelop() + if s2.hasAlert() { + t.Errorf("expected no alert") + } + s2.gateState.criteria.Active = true + s2.screenEnvelop() + if !s2.hasAlert() { + t.Errorf("expected alert") + } + s2.alert = "" + s2.gateState.criteria.Active = false + + resp := &http.Response{ContentLength: 20} + s2.screenResponse(resp) + if s2.hasAlert() { + t.Errorf("expected no alert") + } + s2.gateState.criteria.Active = true + s2.screenResponse(resp) + if !s2.hasAlert() { + t.Errorf("expected alert") + } + s2.alert = "" + s2.gateState.criteria.Active = false + + s2.screenPod() + if s2.hasAlert() { + t.Errorf("expected no alert") + } + + }) + +} + +func Test_session_sessionEventLoop(t *testing.T) { + gs := fakeGateState() + gs.loadConfig() + gs.stat.Init() + ctx, cancelFunction := context.WithCancel(context.Background()) + t.Run("simple", func(t *testing.T) { + s := newSession(gs, nil) + s.cancelFunc = cancelFunction + gs.stat.Init() + s.cancel() + s.sessionEventLoop(ctx) + if ret := gs.stat.Log(); ret != "map[NoResponse:1]" { + t.Errorf("expected stat %s received %s", "map[NoResponse:1]", ret) + } + gs.stat.Init() + s.gotResponse = true + s.cancel() + s.sessionEventLoop(ctx) + if ret := gs.stat.Log(); ret != "map[NoAlert:1]" { + t.Errorf("expected stat %s received %s", "map[NoAlert:1]", ret) + } + gs.stat.Init() + gs.ctrl.Force = true + gs.ctrl.Learn = true + s.gotResponse = true + s.cancel() + s.sessionEventLoop(ctx) + if ret := gs.stat.Log(); ret != "map[NoAlert:1]" { + t.Errorf("expected stat %s received %s", "map[NoAlert:1]", ret) + } + + gs.stat.Init() + s.alert = "x" + s.cancel() + s.sessionEventLoop(ctx) + if ret := gs.stat.Log(); ret != "map[SessionLevelAlert:1]" { + t.Errorf("expected stat %s received %s", "map[SessionLevelAlert:1]", ret) + } + gs.stat.Init() + gs.alert = "x" + s.cancel() + s.sessionEventLoop(ctx) + if ret := gs.stat.Log(); ret != "map[BlockOnPod:1]" { + t.Errorf("expected stat %s received %s", "map[BlockOnPod:1]", ret) + } + }) + +} + +func Test_session_sessionEventLoopTicker(t *testing.T) { + gs := fakeGateState() + gs.loadConfig() + gs.stat.Init() + t.Run("simple", func(t *testing.T) { + ctx, cancelFunction := context.WithCancel(context.Background()) + s := newSession(gs, nil) + s.cancelFunc = cancelFunction + gs.stat.Init() + + s.alert = "x" + + // lets rely on timeout + utils.MinimumInterval = 100000 + s.sessionTicker.Parse("", 100000) + gs.stat.Init() + gs.ctrl.Block = true + go s.sessionEventLoop(ctx) + <-ctx.Done() + td, _ := time.ParseDuration("1ms") + <-time.After(td) + if ret := gs.stat.Log(); ret != "map[SessionLevelAlert:1]" { + t.Errorf("expected stat %s received %s", "map[SessionLevelAlert:1]", ret) + } + }) + +} diff --git a/pkg/guard-utils/ticker_test.go b/pkg/guard-utils/ticker_test.go index 471b9cbc..e9407631 100644 --- a/pkg/guard-utils/ticker_test.go +++ b/pkg/guard-utils/ticker_test.go @@ -111,7 +111,7 @@ func TestTicker(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - MinimumInterval = time.Duration(1) + MinimumInterval = 100000 fmt.Println("Test", tt.name) var ticker Ticker if tt.args.start { From b0f25d14d7bd05b23e4f68246c94752bab28e959 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Sun, 25 Sep 2022 14:00:49 +0300 Subject: [PATCH 02/30] set ns guard-service as default --- pkg/guard-gate/gate.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/guard-gate/gate.go b/pkg/guard-gate/gate.go index 23dcc07a..88b6e8eb 100644 --- a/pkg/guard-gate/gate.go +++ b/pkg/guard-gate/gate.go @@ -19,6 +19,7 @@ package guardgate import ( "context" "errors" + "fmt" "net/http" "strings" "time" @@ -155,8 +156,8 @@ func (p *plug) preInit(ctx context.Context, c map[string]string, sid string, ns ctx, cancelFunction := context.WithCancel(ctx) - // Defaults used without config when used as a sidecar - guardServiceUrl := "http://guard-service.knative-guard" + // Defaults used without config when used as a qpoption + guardServiceUrl := fmt.Sprintf("http://%s.knative-guard", ns) useCm := false monitorPod := true From b80c3f16b6c3bd13ef37709fb8362122cb708638 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Sun, 25 Sep 2022 14:29:58 +0300 Subject: [PATCH 03/30] test correction --- pkg/guard-gate/gate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/guard-gate/gate_test.go b/pkg/guard-gate/gate_test.go index 3ed918ef..ac9e85cf 100644 --- a/pkg/guard-gate/gate_test.go +++ b/pkg/guard-gate/gate_test.go @@ -251,7 +251,7 @@ func Test_plug_Initialize(t *testing.T) { name: "no c", c: nil, monitorPod: true, - guardServiceUrl: "http://guard-service.knative-guard", + guardServiceUrl: "http://myns.knative-guard", useCm: false, }, } From ceb4fa1f0022c023c2eb5ddca22f0fc1e8cc6212 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Sun, 25 Sep 2022 14:59:38 +0300 Subject: [PATCH 04/30] test correction --- pkg/guard-gate/gate_test.go | 157 ++++++++---------------------------- 1 file changed, 35 insertions(+), 122 deletions(-) diff --git a/pkg/guard-gate/gate_test.go b/pkg/guard-gate/gate_test.go index ac9e85cf..05ad10d3 100644 --- a/pkg/guard-gate/gate_test.go +++ b/pkg/guard-gate/gate_test.go @@ -56,107 +56,7 @@ func testInit(c map[string]string) *plug { return p } -//func TestMain(m *testing.M) { -// code := m.Run() -// os.Exit(code) -//} - -func Test_plug_guardMainEventLoop(t *testing.T) { - t.Run("guardianLoadTicker", func(t *testing.T) { - p := new(plug) - p.version = plugVersion - p.name = plugName - c := make(map[string]string) - c["guard-url"] = "url" - c["use-cm"] = "true" - c["monitor-pod"] = "x" - - pi.RegisterPlug(p) - ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) - p.gateState = fakeGateState() - p.gateState.loadConfig() - p.gateState.srv.pile.Count = 7 - utils.MinimumInterval = 100000 - td, _ := time.ParseDuration("10ms") - - p.gateState.stat.Init() - p.guardianLoadTicker.Parse("", 300000) - // lets rely on timeout - go p.guardMainEventLoop(ctx) - time.Sleep(td) - cancelFunction() - if ret := p.gateState.stat.Log(); ret != "map[]" { - t.Errorf("expected stat %s received %s", "map[]", ret) - } - if p.gateState.srv.pile.Count != 7 { - t.Errorf("expected no pile flush") - } - }) - t.Run("podMonitorTicker", func(t *testing.T) { - p := new(plug) - p.version = plugVersion - p.name = plugName - - c := make(map[string]string) - c["guard-url"] = "url" - c["use-cm"] = "true" - c["monitor-pod"] = "x" - - pi.RegisterPlug(p) - ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) - p.gateState = fakeGateState() - p.gateState.loadConfig() - p.gateState.srv.pile.Count = 7 - utils.MinimumInterval = 100000 - td, _ := time.ParseDuration("10ms") - - p.gateState.stat.Init() - p.podMonitorTicker.Parse("", 300000) - // lets rely on timeout - go p.guardMainEventLoop(ctx) - time.Sleep(td) - cancelFunction() - if ret := p.gateState.stat.Log(); ret != "map[]" { - t.Errorf("expected stat %s received %s", "map[]", ret) - } - if p.gateState.srv.pile.Count != 7 { - t.Errorf("expected no pile flush") - } - }) - t.Run("reportPileTicker", func(t *testing.T) { - p := new(plug) - p.version = plugVersion - p.name = plugName - - c := make(map[string]string) - c["guard-url"] = "url" - c["use-cm"] = "true" - c["monitor-pod"] = "x" - - pi.RegisterPlug(p) - ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) - p.gateState = fakeGateState() - p.gateState.loadConfig() - - utils.MinimumInterval = 100000 - td, _ := time.ParseDuration("10ms") - - p.gateState.stat.Init() - p.reportPileTicker.Parse("", 300000) - // lets rely on timeout - go p.guardMainEventLoop(ctx) - time.Sleep(td) - cancelFunction() - if ret := p.gateState.stat.Log(); ret != "map[]" { - t.Errorf("expected stat %s received %s", "map[]", ret) - } - if p.gateState.srv.pile.Count != 0 { - t.Errorf("expected pile flush") - } - }) -} - -func Test_plug_guardMainEventLoop_1(t *testing.T) { +func initTickerTest() (context.Context, context.CancelFunc, *plug) { p := new(plug) p.version = plugVersion p.name = plugName @@ -167,46 +67,60 @@ func Test_plug_guardMainEventLoop_1(t *testing.T) { c["monitor-pod"] = "x" pi.RegisterPlug(p) + utils.MinimumInterval = 100000 + ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) p.gateState = fakeGateState() p.gateState.loadConfig() + p.gateState.stat.Init() + return ctx, cancelFunction, p +} - t.Run("simple", func(t *testing.T) { - utils.MinimumInterval = 100000 - td, _ := time.ParseDuration("10ms") - - p.gateState.stat.Init() +func Test_plug_guardMainEventLoop_1(t *testing.T) { + td, _ := time.ParseDuration("10ms") + t.Run("guardianLoadTicker", func(t *testing.T) { + ctx, cancelFunction, p := initTickerTest() p.guardianLoadTicker.Parse("", 300000) // lets rely on timeout go p.guardMainEventLoop(ctx) - time.Sleep(td) - cancelFunction() + <-time.After(td) if ret := p.gateState.stat.Log(); ret != "map[]" { t.Errorf("expected stat %s received %s", "map[]", ret) } - - p.gateState.stat.Init() - p.podMonitorTicker.Parse("", 200000) + cancelFunction() + }) +} +func Test_plug_guardMainEventLoop_2(t *testing.T) { + td, _ := time.ParseDuration("10ms") + t.Run("podMonitorTicker", func(t *testing.T) { + ctx, cancelFunction, p := initTickerTest() + p.podMonitorTicker.Parse("", 300000) // lets rely on timeout go p.guardMainEventLoop(ctx) - td, _ = time.ParseDuration("10ms") - time.Sleep(td) - cancelFunction() + <-time.After(td) if ret := p.gateState.stat.Log(); ret != "map[]" { t.Errorf("expected stat %s received %s", "map[]", ret) } - - p.gateState.stat.Init() - p.reportPileTicker.Parse("", 100000) + cancelFunction() + }) +} +func Test_plug_guardMainEventLoop_3(t *testing.T) { + td, _ := time.ParseDuration("10ms") + t.Run("reportPileTicker", func(t *testing.T) { + ctx, cancelFunction, p := initTickerTest() + p.reportPileTicker.Parse("", 300000) // lets rely on timeout go p.guardMainEventLoop(ctx) - td, _ = time.ParseDuration("10ms") - time.Sleep(td) - cancelFunction() + <-time.After(td) if ret := p.gateState.stat.Log(); ret != "map[]" { t.Errorf("expected stat %s received %s", "map[]", ret) } - + cancelFunction() + }) +} +func Test_plug_guardMainEventLoop_4(t *testing.T) { + t.Run("reportPileTicker", func(t *testing.T) { + ctx, cancelFunction, p := initTickerTest() cancelFunction() p.guardMainEventLoop(ctx) if ret := p.gateState.stat.Log(); ret != "map[]" { @@ -214,7 +128,6 @@ func Test_plug_guardMainEventLoop_1(t *testing.T) { } }) - } func Test_plug_Initialize(t *testing.T) { From 47634cc8f96c4aaa2a8782c0dcd070cb1cb67d11 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 09:58:14 +0300 Subject: [PATCH 05/30] fix time based tests --- pkg/guard-gate/session_test.go | 2 +- pkg/guard-utils/ticker_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/guard-gate/session_test.go b/pkg/guard-gate/session_test.go index 8ddd7ebe..a93f2306 100644 --- a/pkg/guard-gate/session_test.go +++ b/pkg/guard-gate/session_test.go @@ -182,12 +182,12 @@ func Test_session_sessionEventLoopTicker(t *testing.T) { gs.stat.Init() gs.ctrl.Block = true go s.sessionEventLoop(ctx) - <-ctx.Done() td, _ := time.ParseDuration("1ms") <-time.After(td) if ret := gs.stat.Log(); ret != "map[SessionLevelAlert:1]" { t.Errorf("expected stat %s received %s", "map[SessionLevelAlert:1]", ret) } + cancelFunction() }) } diff --git a/pkg/guard-utils/ticker_test.go b/pkg/guard-utils/ticker_test.go index e9407631..0d2a93cb 100644 --- a/pkg/guard-utils/ticker_test.go +++ b/pkg/guard-utils/ticker_test.go @@ -129,7 +129,7 @@ func TestTicker(t *testing.T) { if tt.args.stop { ticker.Stop() } - testticker := time.NewTicker(1000) + testticker := time.NewTicker(10000000) select { case <-ticker.Ch(): if !tt.args.ok { From a9e412fad650120a396a159b0e8ab9b7662e35bd Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 10:23:27 +0300 Subject: [PATCH 06/30] fix time based tests --- pkg/guard-gate/session_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/guard-gate/session_test.go b/pkg/guard-gate/session_test.go index a93f2306..96a58459 100644 --- a/pkg/guard-gate/session_test.go +++ b/pkg/guard-gate/session_test.go @@ -182,7 +182,7 @@ func Test_session_sessionEventLoopTicker(t *testing.T) { gs.stat.Init() gs.ctrl.Block = true go s.sessionEventLoop(ctx) - td, _ := time.ParseDuration("1ms") + td, _ := time.ParseDuration("10ms") <-time.After(td) if ret := gs.stat.Log(); ret != "map[SessionLevelAlert:1]" { t.Errorf("expected stat %s received %s", "map[SessionLevelAlert:1]", ret) From f77de0862d22a0e4d9d5cc40f7a2d073b76d0dee Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 11:26:19 +0300 Subject: [PATCH 07/30] moved to go cancel() --- pkg/guard-gate/gate_test.go | 25 ++++++++++++------------- pkg/guard-gate/session.go | 4 ++++ pkg/guard-gate/session_test.go | 12 ++++-------- pkg/guard-utils/log.go | 2 +- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/pkg/guard-gate/gate_test.go b/pkg/guard-gate/gate_test.go index 05ad10d3..57210aff 100644 --- a/pkg/guard-gate/gate_test.go +++ b/pkg/guard-gate/gate_test.go @@ -76,46 +76,46 @@ func initTickerTest() (context.Context, context.CancelFunc, *plug) { return ctx, cancelFunction, p } -func Test_plug_guardMainEventLoop_1(t *testing.T) { +func cancelLater(cancel context.CancelFunc) { td, _ := time.ParseDuration("10ms") + <-time.After(td) + cancel() +} + +func Test_plug_guardMainEventLoop_1(t *testing.T) { t.Run("guardianLoadTicker", func(t *testing.T) { ctx, cancelFunction, p := initTickerTest() p.guardianLoadTicker.Parse("", 300000) // lets rely on timeout - go p.guardMainEventLoop(ctx) - <-time.After(td) + go cancelLater(cancelFunction) + p.guardMainEventLoop(ctx) if ret := p.gateState.stat.Log(); ret != "map[]" { t.Errorf("expected stat %s received %s", "map[]", ret) } - cancelFunction() }) } func Test_plug_guardMainEventLoop_2(t *testing.T) { - td, _ := time.ParseDuration("10ms") t.Run("podMonitorTicker", func(t *testing.T) { ctx, cancelFunction, p := initTickerTest() p.podMonitorTicker.Parse("", 300000) // lets rely on timeout - go p.guardMainEventLoop(ctx) - <-time.After(td) + go cancelLater(cancelFunction) + p.guardMainEventLoop(ctx) if ret := p.gateState.stat.Log(); ret != "map[]" { t.Errorf("expected stat %s received %s", "map[]", ret) } - cancelFunction() }) } func Test_plug_guardMainEventLoop_3(t *testing.T) { - td, _ := time.ParseDuration("10ms") t.Run("reportPileTicker", func(t *testing.T) { ctx, cancelFunction, p := initTickerTest() p.reportPileTicker.Parse("", 300000) // lets rely on timeout - go p.guardMainEventLoop(ctx) - <-time.After(td) + go cancelLater(cancelFunction) + p.guardMainEventLoop(ctx) if ret := p.gateState.stat.Log(); ret != "map[]" { t.Errorf("expected stat %s received %s", "map[]", ret) } - cancelFunction() }) } func Test_plug_guardMainEventLoop_4(t *testing.T) { @@ -126,7 +126,6 @@ func Test_plug_guardMainEventLoop_4(t *testing.T) { if ret := p.gateState.stat.Log(); ret != "map[]" { t.Errorf("expected stat %s received %s", "map[]", ret) } - }) } diff --git a/pkg/guard-gate/session.go b/pkg/guard-gate/session.go index dafea45e..a1952719 100644 --- a/pkg/guard-gate/session.go +++ b/pkg/guard-gate/session.go @@ -87,6 +87,8 @@ func (s *session) sessionEventLoop(ctx context.Context) { s.sessionTicker.Start() defer func() { + logAlert("Session defer() ") + s.sessionTicker.Stop() // Should we learn? @@ -123,8 +125,10 @@ func (s *session) sessionEventLoop(ctx context.Context) { s.screenPod() if s.gateState.shouldBlock() && (s.hasAlert() || s.gateState.hasAlert()) { s.cancel() + pi.Log.Debugf("Session Canceled") return } + pi.Log.Debugf("Session Tick") } } } diff --git a/pkg/guard-gate/session_test.go b/pkg/guard-gate/session_test.go index 96a58459..eef9d313 100644 --- a/pkg/guard-gate/session_test.go +++ b/pkg/guard-gate/session_test.go @@ -165,14 +165,13 @@ func Test_session_sessionEventLoop(t *testing.T) { } func Test_session_sessionEventLoopTicker(t *testing.T) { - gs := fakeGateState() - gs.loadConfig() - gs.stat.Init() t.Run("simple", func(t *testing.T) { ctx, cancelFunction := context.WithCancel(context.Background()) + gs := fakeGateState() + gs.loadConfig() + gs.stat.Init() s := newSession(gs, nil) s.cancelFunc = cancelFunction - gs.stat.Init() s.alert = "x" @@ -181,13 +180,10 @@ func Test_session_sessionEventLoopTicker(t *testing.T) { s.sessionTicker.Parse("", 100000) gs.stat.Init() gs.ctrl.Block = true - go s.sessionEventLoop(ctx) - td, _ := time.ParseDuration("10ms") - <-time.After(td) + s.sessionEventLoop(ctx) if ret := gs.stat.Log(); ret != "map[SessionLevelAlert:1]" { t.Errorf("expected stat %s received %s", "map[SessionLevelAlert:1]", ret) } - cancelFunction() }) } diff --git a/pkg/guard-utils/log.go b/pkg/guard-utils/log.go index 9b0e60c1..921db2d3 100644 --- a/pkg/guard-utils/log.go +++ b/pkg/guard-utils/log.go @@ -42,7 +42,7 @@ func CreateLogger(logLevel string) *zap.SugaredLogger { "level": "info", "encoding": "console", "outputPaths": ["stdout"], - "development": true, + "development": false, "errorOutputPaths": ["stderr"], "encoderConfig": { "messageKey": "message", From ebe577e3cfd13aef3565218f59d9832180cf97e3 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 12:34:03 +0300 Subject: [PATCH 08/30] moved MinimalInterval to const to avoid race during tests --- pkg/guard-gate/gate.go | 11 +++++---- pkg/guard-gate/gate_test.go | 7 +++++- pkg/guard-gate/session.go | 4 ++-- pkg/guard-gate/session_test.go | 2 +- pkg/guard-utils/log_test.go | 43 ++++++++++++++++++++++++++++++++++ pkg/guard-utils/ticker.go | 22 +++++++++++------ pkg/guard-utils/ticker_test.go | 5 +--- 7 files changed, 75 insertions(+), 19 deletions(-) create mode 100644 pkg/guard-utils/log_test.go diff --git a/pkg/guard-gate/gate.go b/pkg/guard-gate/gate.go index 88b6e8eb..af342b68 100644 --- a/pkg/guard-gate/gate.go +++ b/pkg/guard-gate/gate.go @@ -47,10 +47,10 @@ type plug struct { version string // guard gate plug specifics - gateState *gateState // maintainer of the criteria and ctrl, include pod profile, gate stats and gate level alert - guardianLoadTicker utils.Ticker // tick to gateState.loadConfig() gateState - reportPileTicker utils.Ticker // tick to gateState.flushPile() - podMonitorTicker utils.Ticker // tick to gateState.profileAndDecidePod() + gateState *gateState // maintainer of the criteria and ctrl, include pod profile, gate stats and gate level alert + guardianLoadTicker *utils.Ticker // tick to gateState.loadConfig() gateState + reportPileTicker *utils.Ticker // tick to gateState.flushPile() + podMonitorTicker *utils.Ticker // tick to gateState.profileAndDecidePod() } func (p *plug) Shutdown() { @@ -174,6 +174,9 @@ func (p *plug) preInit(ctx context.Context, c map[string]string, sid string, ns if v, ok = c["monitor-pod"]; ok && !strings.EqualFold(v, "true") { monitorPod = false } + p.guardianLoadTicker = utils.NewTicker(utils.MinimumInterval) + p.reportPileTicker = utils.NewTicker(utils.MinimumInterval) + p.podMonitorTicker = utils.NewTicker(utils.MinimumInterval) p.guardianLoadTicker.Parse(c["guardian-load-interval"], guardianLoadIntervalDefault) p.reportPileTicker.Parse(c["report-pile-interval"], reportPileIntervalDefault) p.podMonitorTicker.Parse(c["pod-monitor-interval"], podMonitorIntervalDefault) diff --git a/pkg/guard-gate/gate_test.go b/pkg/guard-gate/gate_test.go index 57210aff..e95cd0e8 100644 --- a/pkg/guard-gate/gate_test.go +++ b/pkg/guard-gate/gate_test.go @@ -67,7 +67,6 @@ func initTickerTest() (context.Context, context.CancelFunc, *plug) { c["monitor-pod"] = "x" pi.RegisterPlug(p) - utils.MinimumInterval = 100000 ctx, cancelFunction := p.preInit(context.Background(), c, "svcName", "myns", defaultLog) p.gateState = fakeGateState() @@ -85,6 +84,7 @@ func cancelLater(cancel context.CancelFunc) { func Test_plug_guardMainEventLoop_1(t *testing.T) { t.Run("guardianLoadTicker", func(t *testing.T) { ctx, cancelFunction, p := initTickerTest() + p.guardianLoadTicker = utils.NewTicker(100000) p.guardianLoadTicker.Parse("", 300000) // lets rely on timeout go cancelLater(cancelFunction) @@ -97,6 +97,7 @@ func Test_plug_guardMainEventLoop_1(t *testing.T) { func Test_plug_guardMainEventLoop_2(t *testing.T) { t.Run("podMonitorTicker", func(t *testing.T) { ctx, cancelFunction, p := initTickerTest() + p.podMonitorTicker = utils.NewTicker(100000) p.podMonitorTicker.Parse("", 300000) // lets rely on timeout go cancelLater(cancelFunction) @@ -109,6 +110,7 @@ func Test_plug_guardMainEventLoop_2(t *testing.T) { func Test_plug_guardMainEventLoop_3(t *testing.T) { t.Run("reportPileTicker", func(t *testing.T) { ctx, cancelFunction, p := initTickerTest() + p.reportPileTicker = utils.NewTicker(100000) p.reportPileTicker.Parse("", 300000) // lets rely on timeout go cancelLater(cancelFunction) @@ -172,6 +174,9 @@ func Test_plug_Initialize(t *testing.T) { p := new(plug) p.version = plugVersion p.name = plugName + p.podMonitorTicker = utils.NewTicker(utils.MinimumInterval) + p.guardianLoadTicker = utils.NewTicker(utils.MinimumInterval) + p.reportPileTicker = utils.NewTicker(utils.MinimumInterval) pi.RegisterPlug(p) ctx, cancelFunction := p.preInit(context.Background(), tt.c, "svcName", "myns", defaultLog) diff --git a/pkg/guard-gate/session.go b/pkg/guard-gate/session.go index a1952719..96e03f95 100644 --- a/pkg/guard-gate/session.go +++ b/pkg/guard-gate/session.go @@ -31,7 +31,7 @@ import ( const sessionKey = "GuardSession" type session struct { - sessionTicker utils.Ticker + sessionTicker *utils.Ticker gotResponse bool alert string // session alert reqTime time.Time // time when session was started @@ -47,7 +47,7 @@ func newSession(state *gateState, cancel context.CancelFunc) *session { s.respTime = s.reqTime // indicates that we do not know the response time s.gateState = state s.cancelFunc = cancel - + s.sessionTicker = utils.NewTicker(utils.MinimumInterval) if err := s.sessionTicker.Parse("", podMonitorIntervalDefault); err != nil { pi.Log.Debugf("Error on Ticker Parse: %v", err) } diff --git a/pkg/guard-gate/session_test.go b/pkg/guard-gate/session_test.go index eef9d313..9633c869 100644 --- a/pkg/guard-gate/session_test.go +++ b/pkg/guard-gate/session_test.go @@ -176,7 +176,7 @@ func Test_session_sessionEventLoopTicker(t *testing.T) { s.alert = "x" // lets rely on timeout - utils.MinimumInterval = 100000 + s.sessionTicker = utils.NewTicker(100000) s.sessionTicker.Parse("", 100000) gs.stat.Init() gs.ctrl.Block = true diff --git a/pkg/guard-utils/log_test.go b/pkg/guard-utils/log_test.go new file mode 100644 index 00000000..4de8795d --- /dev/null +++ b/pkg/guard-utils/log_test.go @@ -0,0 +1,43 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package guardutils + +import ( + "testing" +) + +func TestCreateLogger(t *testing.T) { + tests := []struct { + logLevel string + }{ + { + logLevel: "debug", + }, { + logLevel: "info", + }, { + logLevel: "warn", + }, { + logLevel: "x", + }, + } + for _, tt := range tests { + t.Run(tt.logLevel, func(t *testing.T) { + l := CreateLogger(tt.logLevel) + l.Debug("test") + }) + } +} diff --git a/pkg/guard-utils/ticker.go b/pkg/guard-utils/ticker.go index bec02bea..40452ec2 100644 --- a/pkg/guard-utils/ticker.go +++ b/pkg/guard-utils/ticker.go @@ -21,12 +21,20 @@ import ( "time" ) -// default values -var MinimumInterval = 5 * time.Second +const ( + MinimumInterval = 5 * time.Second +) type Ticker struct { - interval time.Duration - ticker *time.Ticker + minimumInterval time.Duration + interval time.Duration + ticker *time.Ticker +} + +func NewTicker(minimumInterval time.Duration) *Ticker { + t := new(Ticker) + t.minimumInterval = minimumInterval + return t } func (t *Ticker) Parse(intervalStr string, defaultInterval time.Duration) error { @@ -47,8 +55,8 @@ func (t *Ticker) Parse(intervalStr string, defaultInterval time.Duration) error } func (t *Ticker) Start() { - if t.interval < MinimumInterval { - t.interval = MinimumInterval + if t.interval < t.minimumInterval { + t.interval = t.minimumInterval } t.ticker = time.NewTicker(t.interval) } @@ -61,7 +69,7 @@ func (t *Ticker) Stop() { func (t *Ticker) Ch() <-chan time.Time { if t.ticker == nil { - t.ticker = time.NewTicker(MinimumInterval) + t.ticker = time.NewTicker(t.minimumInterval) t.ticker.Stop() } return t.ticker.C diff --git a/pkg/guard-utils/ticker_test.go b/pkg/guard-utils/ticker_test.go index 0d2a93cb..4c9cdc77 100644 --- a/pkg/guard-utils/ticker_test.go +++ b/pkg/guard-utils/ticker_test.go @@ -17,7 +17,6 @@ limitations under the License. package guardutils import ( - "fmt" "testing" "time" ) @@ -111,9 +110,7 @@ func TestTicker(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - MinimumInterval = 100000 - fmt.Println("Test", tt.name) - var ticker Ticker + ticker := NewTicker(100000) if tt.args.start { ticker.Start() } From 909b5d972bf891b86811985b820f95bbb1af8547 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 12:40:43 +0300 Subject: [PATCH 09/30] moved MinimalInterval to const to avoid race during tests --- cmd/guard-service/main.go | 2 +- cmd/guard-service/main_test.go | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/guard-service/main.go b/cmd/guard-service/main.go index 6b02f504..82a7f47d 100644 --- a/cmd/guard-service/main.go +++ b/cmd/guard-service/main.go @@ -77,7 +77,7 @@ func main() { l := new(learner) l.services = newServices() - l.pileLearnTicker = new(utils.Ticker) + l.pileLearnTicker = utils.NewTicker(utils.MinimumInterval) log = utils.CreateLogger(env.GuardServiceLogLevel) l.pileLearnTicker.Parse(env.GuardServiceInterval, serviceIntervalDefault) l.pileLearnTicker.Start() diff --git a/cmd/guard-service/main_test.go b/cmd/guard-service/main_test.go index ed564c1b..bdf64452 100644 --- a/cmd/guard-service/main_test.go +++ b/cmd/guard-service/main_test.go @@ -46,9 +46,8 @@ func Test_learner_mainEventLoop(t *testing.T) { s.namespaces = make(map[string]bool, 4) s.kmgr = new(fakeKmgr) - utils.MinimumInterval = 1000 - ticker := new(utils.Ticker) - ticker.Parse("", 1000) + ticker := utils.NewTicker(100000) + ticker.Parse("", 100000) ticker.Start() addToPile(s) From 501ed39c925397e796ed803a595c4412fb765913 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 13:31:43 +0300 Subject: [PATCH 10/30] avoiding race in ApproveResponse --- pkg/guard-gate/gate_test.go | 13 +++++++++++-- pkg/guard-kubemgr/watcher.go | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/guard-gate/gate_test.go b/pkg/guard-gate/gate_test.go index e95cd0e8..ea8e6e37 100644 --- a/pkg/guard-gate/gate_test.go +++ b/pkg/guard-gate/gate_test.go @@ -276,7 +276,13 @@ func Test_plug_ApproveResponse(t *testing.T) { t.Errorf("ApproveResponse expected error ! ") } - req1, _ := p.ApproveRequest(req) + ctx, cancelFunction := context.WithCancel(req.Context()) + s := newSession(p.gateState, cancelFunction) // maintainer of the profile + ctx = s.addSessionToContext(ctx) + ctx = context.WithValue(ctx, ctxKey("GuardSession"), s) + + req1 := req.WithContext(ctx) + resp.Request = req1 _, err2 := p.ApproveResponse(req1, resp) @@ -311,6 +317,8 @@ func Test_plug_ApproveRequest(t *testing.T) { if req1 == nil { t.Errorf("ApproveRequest did not return a req ") } + _, cancelFunction1 := context.WithCancel(req1.Context()) + cancelFunction1() p.gateState.alert = "x" p.gateState.ctrl.Block = true @@ -323,7 +331,8 @@ func Test_plug_ApproveRequest(t *testing.T) { if req2 != nil { t.Errorf("ApproveRequest did not return a req ") } - + _, cancelFunction2 := context.WithCancel(req1.Context()) + cancelFunction2() }) } diff --git a/pkg/guard-kubemgr/watcher.go b/pkg/guard-kubemgr/watcher.go index f1dec2df..829bae1e 100644 --- a/pkg/guard-kubemgr/watcher.go +++ b/pkg/guard-kubemgr/watcher.go @@ -125,7 +125,7 @@ func (k *KubeMgr) WatchOnce(ns string, cmFlag bool, set func(ns string, sid stri gdata := []byte(cm.Data["Guardian"]) jsonErr := json.Unmarshal(gdata, g) if jsonErr != nil { - fmt.Printf("wsgate getConfig: unmarshel error %v", jsonErr) + fmt.Printf("wsgate getConfig: unmarshel error %v\n", jsonErr) set(ns, sid, cmFlag, nil) continue } From 503490027cad32bc7a20d7a4e65be1422480e9b3 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 14:55:44 +0300 Subject: [PATCH 11/30] fixed fmt.printf use --- pkg/guard-kubemgr/watcher.go | 9 +++++---- pkg/guard-kubemgr/watcher_test.go | 3 --- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/guard-kubemgr/watcher.go b/pkg/guard-kubemgr/watcher.go index 829bae1e..d35f6279 100644 --- a/pkg/guard-kubemgr/watcher.go +++ b/pkg/guard-kubemgr/watcher.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/watch" spec "knative.dev/security-guard/pkg/apis/guard/v1alpha1" + pi "knative.dev/security-guard/pkg/pluginterfaces" ) // Watch never returns - use with a goroutine @@ -75,7 +76,7 @@ func (k *KubeMgr) WatchOnce(ns string, cmFlag bool, set func(ns string, sid stri case watch.Added: g, ok := event.Object.(*spec.Guardian) if !ok { - fmt.Printf("kubernetes cant convert to type Guardian\n") + pi.Log.Infof("kubernetes cant convert to type Guardian\n") return } ns := g.ObjectMeta.Namespace @@ -88,7 +89,7 @@ func (k *KubeMgr) WatchOnce(ns string, cmFlag bool, set func(ns string, sid stri set(ns, sid, cmFlag, g.Spec) case watch.Error: s := event.Object.(*metav1.Status) - fmt.Printf("Error during watch CRD: \n\tListMeta %v\n\tTypeMeta %v\n", s.ListMeta, s.TypeMeta) + pi.Log.Infof("Error during watch CRD: \n\tListMeta %v\n\tTypeMeta %v\n", s.ListMeta, s.TypeMeta) } case event, ok := <-chCm: if !ok { @@ -125,14 +126,14 @@ func (k *KubeMgr) WatchOnce(ns string, cmFlag bool, set func(ns string, sid stri gdata := []byte(cm.Data["Guardian"]) jsonErr := json.Unmarshal(gdata, g) if jsonErr != nil { - fmt.Printf("wsgate getConfig: unmarshel error %v\n", jsonErr) + pi.Log.Infof("wsgate getConfig: unmarshel error %v\n", jsonErr) set(ns, sid, cmFlag, nil) continue } set(ns, sid, cmFlag, g) case watch.Error: s := event.Object.(*metav1.Status) - fmt.Printf("Error during watch CM: \n\tListMeta %v\n\tTypeMeta %v\n", s.ListMeta, s.TypeMeta) + pi.Log.Infof("Error during watch CM: \n\tListMeta %v\n\tTypeMeta %v\n", s.ListMeta, s.TypeMeta) } case <-time.After(10 * time.Minute): // deal with the issue where we get no events diff --git a/pkg/guard-kubemgr/watcher_test.go b/pkg/guard-kubemgr/watcher_test.go index ceb26bd2..918e6488 100644 --- a/pkg/guard-kubemgr/watcher_test.go +++ b/pkg/guard-kubemgr/watcher_test.go @@ -17,7 +17,6 @@ limitations under the License. package guardkubemgr import ( - "fmt" "testing" "time" @@ -75,7 +74,6 @@ func TestKubeMgr_CM_WatchOnce(t *testing.T) { go func() { defer tt.fields.cmWatcher.Stop() for i := 0; i < 3; i++ { - fmt.Printf("i=%d\n", i) time.Sleep(30 * time.Millisecond) tt.fields.cmWatcher.Delete(&v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -202,7 +200,6 @@ func TestKubeMgr_CRD_WatchOnce(t *testing.T) { go func() { defer tt.fields.crdWatcher.Stop() for i := 0; i < 3; i++ { - fmt.Printf("i=%d\n", i) time.Sleep(30 * time.Millisecond) tt.fields.crdWatcher.Delete(&spec.Guardian{ From 17d57a2ce435c87ddccf652b453d3681c086c79a Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 15:07:43 +0300 Subject: [PATCH 12/30] alignment --- cmd/guard-service/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/guard-service/main.go b/cmd/guard-service/main.go index 82a7f47d..6060a340 100644 --- a/cmd/guard-service/main.go +++ b/cmd/guard-service/main.go @@ -76,12 +76,13 @@ func main() { } l := new(learner) - l.services = newServices() l.pileLearnTicker = utils.NewTicker(utils.MinimumInterval) log = utils.CreateLogger(env.GuardServiceLogLevel) l.pileLearnTicker.Parse(env.GuardServiceInterval, serviceIntervalDefault) l.pileLearnTicker.Start() + l.services = newServices() + http.HandleFunc("/config", l.fetchConfig) http.HandleFunc("/pile", l.processPile) From b815b087e67e9ee6db254a04ece4eb48b08f1c42 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 26 Sep 2022 15:27:24 +0300 Subject: [PATCH 13/30] updates --- cmd/guard-service/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/guard-service/main.go b/cmd/guard-service/main.go index 6060a340..12671069 100644 --- a/cmd/guard-service/main.go +++ b/cmd/guard-service/main.go @@ -74,10 +74,10 @@ func main() { fmt.Fprintf(os.Stderr, "Failed to process environment: %s\n", err.Error()) os.Exit(1) } + log = utils.CreateLogger(env.GuardServiceLogLevel) l := new(learner) l.pileLearnTicker = utils.NewTicker(utils.MinimumInterval) - log = utils.CreateLogger(env.GuardServiceLogLevel) l.pileLearnTicker.Parse(env.GuardServiceInterval, serviceIntervalDefault) l.pileLearnTicker.Start() From f13a27a93a133122658f8c37e98fbd8bbf7a88ce Mon Sep 17 00:00:00 2001 From: David Hadas Date: Thu, 29 Sep 2022 11:26:20 +0300 Subject: [PATCH 14/30] bug fix http.client --- pkg/guard-gate/client.go | 2 +- pkg/guard-gate/session.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/guard-gate/client.go b/pkg/guard-gate/client.go index 4f25d33d..ef25561d 100644 --- a/pkg/guard-gate/client.go +++ b/pkg/guard-gate/client.go @@ -32,7 +32,7 @@ type httpClientInterface interface { } type httpClient struct { - client *http.Client + client http.Client } func (hc *httpClient) Do(req *http.Request) (*http.Response, error) { diff --git a/pkg/guard-gate/session.go b/pkg/guard-gate/session.go index 96e03f95..f741ba59 100644 --- a/pkg/guard-gate/session.go +++ b/pkg/guard-gate/session.go @@ -87,8 +87,6 @@ func (s *session) sessionEventLoop(ctx context.Context) { s.sessionTicker.Start() defer func() { - logAlert("Session defer() ") - s.sessionTicker.Stop() // Should we learn? From ad6ed73d6a1d8b8ffbea7e9688663a5451c241ec Mon Sep 17 00:00:00 2001 From: David Hadas Date: Thu, 29 Sep 2022 18:05:49 +0300 Subject: [PATCH 15/30] nits --- cmd/guard-service/auth.go | 3 ++- cmd/guard-service/main.go | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/guard-service/auth.go b/cmd/guard-service/auth.go index ba9b1c08..a56d4a45 100644 --- a/cmd/guard-service/auth.go +++ b/cmd/guard-service/auth.go @@ -17,4 +17,5 @@ limitations under the License. package main // Uncomment when running in a development environment out side of the cluster -// import _ "k8s.io/client-go/plugin/pkg/client/auth" +//import _ "k8s.io/client-go/plugin/pkg/client/auth" +import _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" diff --git a/cmd/guard-service/main.go b/cmd/guard-service/main.go index 92169909..b80e90df 100644 --- a/cmd/guard-service/main.go +++ b/cmd/guard-service/main.go @@ -132,8 +132,6 @@ func (l *learner) processPile(w http.ResponseWriter, req *http.Request) { } func (l *learner) mainEventLoop(quit chan string) { - log.Infof("l.pileLearnTicker %v", l.pileLearnTicker) - for { select { case <-l.pileLearnTicker.Ch(): @@ -172,7 +170,7 @@ func preMain(minimumInterval time.Duration) (*learner, *http.ServeMux, string, c quit := make(chan string) - log.Infof("Starting guard-learner on %s", target) + log.Infof("Starting guard-service on %s", target) return l, mux, target, quit } From bdfbf7967c7eb17a2cb9a0bd689c5040aa465cc7 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Thu, 29 Sep 2022 18:15:21 +0300 Subject: [PATCH 16/30] nits --- cmd/guard-service/auth.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/guard-service/auth.go b/cmd/guard-service/auth.go index a56d4a45..4657fb50 100644 --- a/cmd/guard-service/auth.go +++ b/cmd/guard-service/auth.go @@ -17,5 +17,5 @@ limitations under the License. package main // Uncomment when running in a development environment out side of the cluster -//import _ "k8s.io/client-go/plugin/pkg/client/auth" -import _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" +// import _ "k8s.io/client-go/plugin/pkg/client/auth" +// import _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" From 699af57d6aaf31376188babecc314a8c6071c2d4 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Thu, 29 Sep 2022 20:29:08 +0300 Subject: [PATCH 17/30] deployment of services --- deploy/deploy-guard-service.yaml | 37 ++++++++++++++++++++++++++++++++ deploy/serviceAccount.yaml | 10 ++++----- hack/setup-guard-service.sh | 6 ++++++ hack/update-guard-service.sh | 2 ++ pkg/guard-gate/gate.go | 2 +- pkg/guard-gate/gate_test.go | 2 +- 6 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 deploy/deploy-guard-service.yaml create mode 100755 hack/setup-guard-service.sh create mode 100755 hack/update-guard-service.sh diff --git a/deploy/deploy-guard-service.yaml b/deploy/deploy-guard-service.yaml new file mode 100644 index 00000000..6c2c7e6a --- /dev/null +++ b/deploy/deploy-guard-service.yaml @@ -0,0 +1,37 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: guard-service + labels: + app: guard-service +spec: + replicas: 1 + selector: + matchLabels: + app: guard-service + template: + metadata: + labels: + app: guard-service + spec: + serviceAccountName: guard-service-account + imagePullSecrets: + - name: all-icr-io + containers: + - name: guard-service + image: ko://knative.dev/security-guard/cmd/guard-service + imagePullPolicy: Always + ports: + - containerPort: 8888 +--- +apiVersion: v1 +kind: Service +metadata: + name: guard-service +spec: + selector: + app: guard-service + ports: + - protocol: TCP + port: 80 + targetPort: 8888 diff --git a/deploy/serviceAccount.yaml b/deploy/serviceAccount.yaml index 777c8498..ba4aa574 100644 --- a/deploy/serviceAccount.yaml +++ b/deploy/serviceAccount.yaml @@ -1,7 +1,7 @@ -kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole metadata: - name: guardian-cluster-role + name: guardian-service labels: rbac.authorization.k8s.io/guardian: 'true' rules: @@ -22,17 +22,15 @@ apiVersion: v1 kind: ServiceAccount metadata: name: guard-service-account - namespace: knative-guard --- apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding +kind: RoleBinding metadata: name: guardian-admin subjects: - kind: ServiceAccount name: guard-service-account - namespace: knative-guard roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: guardian-cluster-role + name: guardian-service \ No newline at end of file diff --git a/hack/setup-guard-service.sh b/hack/setup-guard-service.sh new file mode 100755 index 00000000..d8b3146d --- /dev/null +++ b/hack/setup-guard-service.sh @@ -0,0 +1,6 @@ +kubectl apply -f deploy/gateAccount.yaml +kubectl apply -f deploy/serviceAccount.yaml +kubectl apply -f deploy/guardiansCrd.yaml + +./hack/update-guard-service.sh + diff --git a/hack/update-guard-service.sh b/hack/update-guard-service.sh new file mode 100755 index 00000000..6ec07d08 --- /dev/null +++ b/hack/update-guard-service.sh @@ -0,0 +1,2 @@ +export KO_DOCKER_REPO=us.icr.io/knat +ko apply -Rf ./deploy/deploy-guard-service.yaml \ No newline at end of file diff --git a/pkg/guard-gate/gate.go b/pkg/guard-gate/gate.go index af342b68..e118517a 100644 --- a/pkg/guard-gate/gate.go +++ b/pkg/guard-gate/gate.go @@ -157,7 +157,7 @@ func (p *plug) preInit(ctx context.Context, c map[string]string, sid string, ns ctx, cancelFunction := context.WithCancel(ctx) // Defaults used without config when used as a qpoption - guardServiceUrl := fmt.Sprintf("http://%s.knative-guard", ns) + guardServiceUrl := fmt.Sprintf("http://guard-service.%s", ns) useCm := false monitorPod := true diff --git a/pkg/guard-gate/gate_test.go b/pkg/guard-gate/gate_test.go index ea8e6e37..574677c1 100644 --- a/pkg/guard-gate/gate_test.go +++ b/pkg/guard-gate/gate_test.go @@ -165,7 +165,7 @@ func Test_plug_Initialize(t *testing.T) { name: "no c", c: nil, monitorPod: true, - guardServiceUrl: "http://myns.knative-guard", + guardServiceUrl: "http://guard-service.myns", useCm: false, }, } From 74f74ae559100707ba8956fd54d801fceda1a0ff Mon Sep 17 00:00:00 2001 From: David Hadas Date: Thu, 29 Sep 2022 21:04:03 +0300 Subject: [PATCH 18/30] mv deploy config + add cmd/queue/main.go --- cmd/queue/main.go | 35 ++++++++++++++++++++++ config/apiservice.yaml | 17 +++++++++++ config/deploy-guard-service.yaml | 37 +++++++++++++++++++++++ config/gateAccount.yaml | 32 ++++++++++++++++++++ config/guardiansCrd.yaml | 50 ++++++++++++++++++++++++++++++++ config/serviceAccount.yaml | 37 +++++++++++++++++++++++ 6 files changed, 208 insertions(+) create mode 100644 cmd/queue/main.go create mode 100644 config/apiservice.yaml create mode 100644 config/deploy-guard-service.yaml create mode 100644 config/gateAccount.yaml create mode 100644 config/guardiansCrd.yaml create mode 100644 config/serviceAccount.yaml diff --git a/cmd/queue/main.go b/cmd/queue/main.go new file mode 100644 index 00000000..faf35833 --- /dev/null +++ b/cmd/queue/main.go @@ -0,0 +1,35 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "os" + + _ "knative.dev/security-guard/pkg/guard-gate" + "knative.dev/security-guard/pkg/qpoption" + "knative.dev/serving/pkg/queue/sharedmain" +) + +func main() { + qOpt := qpoption.NewGateQPOption() + defer qOpt.Shutdown() + + if sharedmain.Main(qOpt.Setup) != nil { + qOpt.Shutdown() + os.Exit(1) + } +} diff --git a/config/apiservice.yaml b/config/apiservice.yaml new file mode 100644 index 00000000..4ca2808d --- /dev/null +++ b/config/apiservice.yaml @@ -0,0 +1,17 @@ +apiVersion: apiregistration.k8s.io/v1 +kind: APIService +metadata: + name: v1alpha1.guard.security.knative.dev +spec: + group: guard.security.knative.dev + groupPriorityMinimum: 1000 + versionPriority: 15 + service: + name: guard-service + namespace: knative-guard + port: 8888 + version: v1alpha1 + # development + insecureSkipTLSVerify: true + #caBundle + \ No newline at end of file diff --git a/config/deploy-guard-service.yaml b/config/deploy-guard-service.yaml new file mode 100644 index 00000000..6c2c7e6a --- /dev/null +++ b/config/deploy-guard-service.yaml @@ -0,0 +1,37 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: guard-service + labels: + app: guard-service +spec: + replicas: 1 + selector: + matchLabels: + app: guard-service + template: + metadata: + labels: + app: guard-service + spec: + serviceAccountName: guard-service-account + imagePullSecrets: + - name: all-icr-io + containers: + - name: guard-service + image: ko://knative.dev/security-guard/cmd/guard-service + imagePullPolicy: Always + ports: + - containerPort: 8888 +--- +apiVersion: v1 +kind: Service +metadata: + name: guard-service +spec: + selector: + app: guard-service + ports: + - protocol: TCP + port: 80 + targetPort: 8888 diff --git a/config/gateAccount.yaml b/config/gateAccount.yaml new file mode 100644 index 00000000..8c4f255f --- /dev/null +++ b/config/gateAccount.yaml @@ -0,0 +1,32 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: guardian-reader + labels: + rbac.authorization.k8s.io/guardian: 'true' +rules: + - verbs: + - get + - list + - watch + apiGroups: + - guard.security.knative.dev + resources: + - guardians +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: guardian-reader +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: read-guardians +subjects: + - kind: ServiceAccount + name: default +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: guardian-reader diff --git a/config/guardiansCrd.yaml b/config/guardiansCrd.yaml new file mode 100644 index 00000000..ae1de77b --- /dev/null +++ b/config/guardiansCrd.yaml @@ -0,0 +1,50 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: guardians.guard.security.knative.dev + # for more information on the below annotation, please see + # https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/2337-k8s.io-group-protection/README.md +spec: + group: guard.security.knative.dev + versions: + - name: v1alpha1 + served: true + storage: true + schema: + # schema used for validation + openAPIV3Schema: + type: object + properties: + spec: + type: object + x-kubernetes-preserve-unknown-fields: true + properties: + configured: + type: object + x-kubernetes-preserve-unknown-fields: true + learned: + type: object + x-kubernetes-preserve-unknown-fields: true + control: + type: object + x-kubernetes-preserve-unknown-fields: true + properties: + alert: + type: boolean + default: false + auto: + type: boolean + default: false + block: + type: boolean + default: false + force: + type: boolean + default: false + learn: + type: boolean + default: false + names: + kind: Guardian + plural: guardians + scope: Namespaced diff --git a/config/serviceAccount.yaml b/config/serviceAccount.yaml new file mode 100644 index 00000000..f3e43036 --- /dev/null +++ b/config/serviceAccount.yaml @@ -0,0 +1,37 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: guardian-service + labels: + rbac.authorization.k8s.io/guardian: 'true' +rules: + - verbs: + - get + - list + - watch + - create + - update + - patch + - delete + apiGroups: + - guard.security.knative.dev + resources: + - guardians +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: guard-service-account +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: guardian-admin +subjects: + - kind: ServiceAccount + name: guard-service-account +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: guardian-service + \ No newline at end of file From 5240423ba909aa09c8ea21c3f89d536f59d985b4 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Thu, 29 Sep 2022 22:39:03 +0300 Subject: [PATCH 19/30] mv deploy config + add cmd/queue/main.go --- config/apiservice.yaml | 1 - config/serviceAccount.yaml | 1 - deploy/deploy-guard-service.yaml | 37 ----------------------- deploy/gateAccount.yaml | 32 -------------------- deploy/guardiansCrd.yaml | 50 -------------------------------- deploy/serviceAccount.yaml | 36 ----------------------- hack/update-guard-service.sh | 2 +- 7 files changed, 1 insertion(+), 158 deletions(-) delete mode 100644 deploy/deploy-guard-service.yaml delete mode 100644 deploy/gateAccount.yaml delete mode 100644 deploy/guardiansCrd.yaml delete mode 100644 deploy/serviceAccount.yaml diff --git a/config/apiservice.yaml b/config/apiservice.yaml index 4ca2808d..2e6fe50d 100644 --- a/config/apiservice.yaml +++ b/config/apiservice.yaml @@ -14,4 +14,3 @@ spec: # development insecureSkipTLSVerify: true #caBundle - \ No newline at end of file diff --git a/config/serviceAccount.yaml b/config/serviceAccount.yaml index f3e43036..67fe8437 100644 --- a/config/serviceAccount.yaml +++ b/config/serviceAccount.yaml @@ -34,4 +34,3 @@ roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole name: guardian-service - \ No newline at end of file diff --git a/deploy/deploy-guard-service.yaml b/deploy/deploy-guard-service.yaml deleted file mode 100644 index 6c2c7e6a..00000000 --- a/deploy/deploy-guard-service.yaml +++ /dev/null @@ -1,37 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: guard-service - labels: - app: guard-service -spec: - replicas: 1 - selector: - matchLabels: - app: guard-service - template: - metadata: - labels: - app: guard-service - spec: - serviceAccountName: guard-service-account - imagePullSecrets: - - name: all-icr-io - containers: - - name: guard-service - image: ko://knative.dev/security-guard/cmd/guard-service - imagePullPolicy: Always - ports: - - containerPort: 8888 ---- -apiVersion: v1 -kind: Service -metadata: - name: guard-service -spec: - selector: - app: guard-service - ports: - - protocol: TCP - port: 80 - targetPort: 8888 diff --git a/deploy/gateAccount.yaml b/deploy/gateAccount.yaml deleted file mode 100644 index 8c4f255f..00000000 --- a/deploy/gateAccount.yaml +++ /dev/null @@ -1,32 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: guardian-reader - labels: - rbac.authorization.k8s.io/guardian: 'true' -rules: - - verbs: - - get - - list - - watch - apiGroups: - - guard.security.knative.dev - resources: - - guardians ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: guardian-reader ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: read-guardians -subjects: - - kind: ServiceAccount - name: default -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: guardian-reader diff --git a/deploy/guardiansCrd.yaml b/deploy/guardiansCrd.yaml deleted file mode 100644 index ae1de77b..00000000 --- a/deploy/guardiansCrd.yaml +++ /dev/null @@ -1,50 +0,0 @@ -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - name: guardians.guard.security.knative.dev - # for more information on the below annotation, please see - # https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/2337-k8s.io-group-protection/README.md -spec: - group: guard.security.knative.dev - versions: - - name: v1alpha1 - served: true - storage: true - schema: - # schema used for validation - openAPIV3Schema: - type: object - properties: - spec: - type: object - x-kubernetes-preserve-unknown-fields: true - properties: - configured: - type: object - x-kubernetes-preserve-unknown-fields: true - learned: - type: object - x-kubernetes-preserve-unknown-fields: true - control: - type: object - x-kubernetes-preserve-unknown-fields: true - properties: - alert: - type: boolean - default: false - auto: - type: boolean - default: false - block: - type: boolean - default: false - force: - type: boolean - default: false - learn: - type: boolean - default: false - names: - kind: Guardian - plural: guardians - scope: Namespaced diff --git a/deploy/serviceAccount.yaml b/deploy/serviceAccount.yaml deleted file mode 100644 index ba4aa574..00000000 --- a/deploy/serviceAccount.yaml +++ /dev/null @@ -1,36 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: guardian-service - labels: - rbac.authorization.k8s.io/guardian: 'true' -rules: - - verbs: - - get - - list - - watch - - create - - update - - patch - - delete - apiGroups: - - guard.security.knative.dev - resources: - - guardians ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: guard-service-account ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: guardian-admin -subjects: - - kind: ServiceAccount - name: guard-service-account -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: guardian-service \ No newline at end of file diff --git a/hack/update-guard-service.sh b/hack/update-guard-service.sh index 6ec07d08..d44b9deb 100755 --- a/hack/update-guard-service.sh +++ b/hack/update-guard-service.sh @@ -1,2 +1,2 @@ export KO_DOCKER_REPO=us.icr.io/knat -ko apply -Rf ./deploy/deploy-guard-service.yaml \ No newline at end of file +ko apply -Rf ./deploy/deploy-guard-service.yaml From aa6f2f116e07258e36358433c8e9abdbb0c4c19c Mon Sep 17 00:00:00 2001 From: David Hadas Date: Thu, 29 Sep 2022 22:49:34 +0300 Subject: [PATCH 20/30] mv deploy config + add cmd/queue/main.go --- config/apiservice.yaml | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 config/apiservice.yaml diff --git a/config/apiservice.yaml b/config/apiservice.yaml deleted file mode 100644 index 2e6fe50d..00000000 --- a/config/apiservice.yaml +++ /dev/null @@ -1,16 +0,0 @@ -apiVersion: apiregistration.k8s.io/v1 -kind: APIService -metadata: - name: v1alpha1.guard.security.knative.dev -spec: - group: guard.security.knative.dev - groupPriorityMinimum: 1000 - versionPriority: 15 - service: - name: guard-service - namespace: knative-guard - port: 8888 - version: v1alpha1 - # development - insecureSkipTLSVerify: true - #caBundle From 6abdac68b65d9266a8a215f7fae9c1b80abd927f Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 30 Sep 2022 16:36:46 +0300 Subject: [PATCH 21/30] fix bugs --- cmd/guard-service/main.go | 18 ++++++++++++------ config/deploy-queue-proxy.yaml | 28 ++++++++++++++++++++++++++++ hack/setup-guard-service.sh | 6 +++--- hack/update-guard-service.sh | 2 +- pkg/guard-gate/client.go | 13 ++++++------- pkg/guard-gate/gate.go | 3 ++- pkg/guard-gate/state.go | 2 +- pkg/guard-utils/santize.go | 14 ++++++++------ 8 files changed, 61 insertions(+), 25 deletions(-) create mode 100644 config/deploy-queue-proxy.yaml diff --git a/cmd/guard-service/main.go b/cmd/guard-service/main.go index b80e90df..7a1195cb 100644 --- a/cmd/guard-service/main.go +++ b/cmd/guard-service/main.go @@ -40,7 +40,6 @@ const ( type config struct { GuardServiceLogLevel string `split_words:"true" required:"false"` - GuardServicePort string `split_words:"true" required:"false"` GuardServiceInterval string `split_words:"true" required:"false"` } @@ -65,12 +64,21 @@ func (l *learner) baseHandler(query url.Values) (record *serviceRecord, err erro ns := utils.Sanitize(nsSlice[0]) if strings.HasPrefix(sid, "ns-") { - log.Infof("baseHandler illegal sid") sid = "" err = fmt.Errorf("illegal sid %s", sid) return } + if len(sid) < 1 { + err = fmt.Errorf("wrong sid %s", sidSlice[0]) + return + } + + if len(ns) < 1 { + err = fmt.Errorf("wrong ns %s", nsSlice[0]) + return + } + // extract and sanitize cmFlag var cmFlag bool if len(cmFlagSlice) > 0 { @@ -78,6 +86,7 @@ func (l *learner) baseHandler(query url.Values) (record *serviceRecord, err erro } // get session record, create one if does not exist + log.Debugf("** baseHandler ** ns %s, sid %s, cmFlag %t", ns, sid, cmFlag) record = l.services.get(ns, sid, cmFlag) if record == nil { // should never happen @@ -164,9 +173,6 @@ func preMain(minimumInterval time.Duration) (*learner, *http.ServeMux, string, c mux.HandleFunc("/pile", l.processPile) target := ":8888" - if env.GuardServicePort != "" { - target = fmt.Sprintf(":%s", env.GuardServicePort) - } quit := make(chan string) @@ -183,6 +189,6 @@ func main() { go l.mainEventLoop(quit) err := http.ListenAndServe(target, mux) - log.Infof("Failed to start %v", err) + log.Infof("Using target: %s - Failed to start %v", target, err) quit <- "ListenAndServe failed" } diff --git a/config/deploy-queue-proxy.yaml b/config/deploy-queue-proxy.yaml new file mode 100644 index 00000000..ea5196fe --- /dev/null +++ b/config/deploy-queue-proxy.yaml @@ -0,0 +1,28 @@ +# Copyright 2019 The Knative Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-deployment + namespace: knative-serving + labels: + app.kubernetes.io/name: knative-serving + app.kubernetes.io/component: controller + app.kubernetes.io/version: devel +data: + # This overrides the configmap produced by knative serving + # It is useful for demonstration purposes or when using an unchanged configmap + queue-sidecar-image: ko://knative.dev/security-guard/cmd/queue + \ No newline at end of file diff --git a/hack/setup-guard-service.sh b/hack/setup-guard-service.sh index d8b3146d..a38b45a5 100755 --- a/hack/setup-guard-service.sh +++ b/hack/setup-guard-service.sh @@ -1,6 +1,6 @@ -kubectl apply -f deploy/gateAccount.yaml -kubectl apply -f deploy/serviceAccount.yaml -kubectl apply -f deploy/guardiansCrd.yaml +kubectl apply -f config/gateAccount.yaml +kubectl apply -f config/serviceAccount.yaml +kubectl apply -f config/guardiansCrd.yaml ./hack/update-guard-service.sh diff --git a/hack/update-guard-service.sh b/hack/update-guard-service.sh index d44b9deb..1b8458e5 100755 --- a/hack/update-guard-service.sh +++ b/hack/update-guard-service.sh @@ -1,2 +1,2 @@ export KO_DOCKER_REPO=us.icr.io/knat -ko apply -Rf ./deploy/deploy-guard-service.yaml +ko apply -Rf ./config/deploy-guard-service.yaml diff --git a/pkg/guard-gate/client.go b/pkg/guard-gate/client.go index ef25561d..1c1f7ebe 100644 --- a/pkg/guard-gate/client.go +++ b/pkg/guard-gate/client.go @@ -73,19 +73,17 @@ func (srv *gateClient) reportPile() { } defer srv.clearPile() - pi.Log.Infof("Reporting a pile with pileCount %d records to guard-service", srv.pile.Count) - postBody, marshalErr := json.Marshal(srv.pile) if marshalErr != nil { // should never happen - pi.Log.Warnf("Error during marshal: %v", marshalErr) + pi.Log.Infof("Error during marshal: %v", marshalErr) return } reqBody := bytes.NewBuffer(postBody) req, err := http.NewRequest(http.MethodPost, srv.guardServiceUrl+"/pile", reqBody) if err != nil { - pi.Log.Warnf("Http.NewRequest error %v", err) + pi.Log.Infof("Http.NewRequest error %v", err) return } query := req.URL.Query() @@ -95,10 +93,11 @@ func (srv *gateClient) reportPile() { query.Add("cm", "true") } req.URL.RawQuery = query.Encode() + pi.Log.Infof("Reporting a pile with pileCount %d records to guard-service - Url %v", srv.pile.Count, req.URL) res, postErr := srv.httpClient.Do(req) if postErr != nil { - pi.Log.Warnf("httpClient.Do error %v", postErr) + pi.Log.Infof("httpClient.Do error %v", postErr) return } if res.Body != nil { @@ -135,7 +134,7 @@ func (srv *gateClient) loadGuardian() *spec.GuardianSpec { func (srv *gateClient) loadGuardianFromService() *spec.GuardianSpec { req, err := http.NewRequest(http.MethodGet, srv.guardServiceUrl+"/config", nil) if err != nil { - pi.Log.Warnf("loadGuardianFromService Http.NewRequest error %v", err) + pi.Log.Infof("loadGuardianFromService Http.NewRequest error %v", err) return nil } query := req.URL.Query() @@ -148,7 +147,7 @@ func (srv *gateClient) loadGuardianFromService() *spec.GuardianSpec { res, err := srv.httpClient.Do(req) if err != nil { - pi.Log.Warnf("loadGuardianFromService httpClient.Do error %v", err) + pi.Log.Infof("loadGuardianFromService httpClient.Do error %v", err) return nil } diff --git a/pkg/guard-gate/gate.go b/pkg/guard-gate/gate.go index e118517a..373d3e66 100644 --- a/pkg/guard-gate/gate.go +++ b/pkg/guard-gate/gate.go @@ -97,7 +97,7 @@ func (p *plug) ApproveRequest(req *http.Request) (*http.Request, error) { func (p *plug) ApproveResponse(req *http.Request, resp *http.Response) (*http.Response, error) { s := getSessionFromContext(req.Context()) if s == nil { // This should never happen! - pi.Log.Warnf("%s ........... Blocked During Response! Missing context!", p.name) + pi.Log.Infof("%s ........... Blocked During Response! Missing context!", p.name) return nil, errors.New("missing context") } @@ -197,6 +197,7 @@ func (p *plug) preInit(ctx context.Context, c map[string]string, sid string, ns p.gateState = new(gateState) p.gateState.init(cancelFunction, monitorPod, guardServiceUrl, sid, ns, useCm) + pi.Log.Infof("guardServiceUrl %s", guardServiceUrl) pi.Log.Infof("Loading Guardian") return ctx, cancelFunction } diff --git a/pkg/guard-gate/state.go b/pkg/guard-gate/state.go index ca1cff4a..b66df7c9 100644 --- a/pkg/guard-gate/state.go +++ b/pkg/guard-gate/state.go @@ -26,7 +26,7 @@ import ( ) func logAlert(alert string) { - pi.Log.Warnf("SECURITY ALERT! %s", alert) + pi.Log.Infof("SECURITY ALERT! %s", alert) } type gateState struct { diff --git a/pkg/guard-utils/santize.go b/pkg/guard-utils/santize.go index 2d13aa75..1caf1c6c 100644 --- a/pkg/guard-utils/santize.go +++ b/pkg/guard-utils/santize.go @@ -24,14 +24,15 @@ func Sanitize(in string) string { if len(in) > 60 { return "" } - var letter bool + var alphanumeric bool for i, r := range in { - if 97 <= r && r <= 122 { - letter = true + if (97 <= r && r <= 122) || (48 <= r && r <= 57) { + alphanumeric = true continue } - if (48 <= r && r <= 57) || r == 45 { - letter = false + if r == 45 { + alphanumeric = false + // first letter if i == 0 { return "" } @@ -39,7 +40,8 @@ func Sanitize(in string) string { } return "" } - if !letter { + // last letter + if !alphanumeric { return "" } return in From 78e78b733e0b45be2079d76ca664604be7ad193f Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 30 Sep 2022 16:38:33 +0300 Subject: [PATCH 22/30] fix bugs --- config/deploy-queue-proxy.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/deploy-queue-proxy.yaml b/config/deploy-queue-proxy.yaml index ea5196fe..52eb7225 100644 --- a/config/deploy-queue-proxy.yaml +++ b/config/deploy-queue-proxy.yaml @@ -25,4 +25,3 @@ data: # This overrides the configmap produced by knative serving # It is useful for demonstration purposes or when using an unchanged configmap queue-sidecar-image: ko://knative.dev/security-guard/cmd/queue - \ No newline at end of file From 2f299420c99d48313c183c2161dca7b7d9c25dfe Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 30 Sep 2022 16:44:51 +0300 Subject: [PATCH 23/30] fix bugs --- pkg/guard-utils/santize_test.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/guard-utils/santize_test.go b/pkg/guard-utils/santize_test.go index 16591c89..b060b710 100644 --- a/pkg/guard-utils/santize_test.go +++ b/pkg/guard-utils/santize_test.go @@ -42,17 +42,17 @@ func Test_sanitize(t *testing.T) { { name: "1a", in: "1a", - want: "", + want: "1a", }, { name: "a1", in: "a1", - want: "", + want: "a1", }, { name: "1", in: "1", - want: "", + want: "1", }, { name: "-a", @@ -64,6 +64,16 @@ func Test_sanitize(t *testing.T) { in: "a-", want: "", }, + { + name: "1-", + in: "1-", + want: "", + }, + { + name: "-1", + in: "-1", + want: "", + }, { name: "a-", in: "abc.d", From ca2ea32d0e661b329f90c56dc651c1f75c552361 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 30 Sep 2022 16:50:37 +0300 Subject: [PATCH 24/30] fix bugs --- cmd/guard-service/main_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/cmd/guard-service/main_test.go b/cmd/guard-service/main_test.go index f4997c2a..7117409a 100644 --- a/cmd/guard-service/main_test.go +++ b/cmd/guard-service/main_test.go @@ -233,15 +233,6 @@ func TestFetchConfigHandler_main(t *testing.T) { if target != ":8888" { t.Errorf("handler returned wrong default target code: got %s want %s", target, ":8888") } - - os.Setenv("GUARD_SERVICE_PORT", "9999") - _, _, target, _ = preMain(utils.MinimumInterval) - - if target != ":9999" { - t.Errorf("handler returned wrong default target code: got %s want %s", target, ":9999") - } - - os.Unsetenv("GUARD_SERVICE_PORT") } func TestFetchConfigHandler_POST(t *testing.T) { From 722b4247f31fb772b8d127a116cee5d44c213809 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 30 Sep 2022 17:01:16 +0300 Subject: [PATCH 25/30] fix bugs --- cmd/guard-service/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/guard-service/main.go b/cmd/guard-service/main.go index 7a1195cb..40e992fd 100644 --- a/cmd/guard-service/main.go +++ b/cmd/guard-service/main.go @@ -86,7 +86,6 @@ func (l *learner) baseHandler(query url.Values) (record *serviceRecord, err erro } // get session record, create one if does not exist - log.Debugf("** baseHandler ** ns %s, sid %s, cmFlag %t", ns, sid, cmFlag) record = l.services.get(ns, sid, cmFlag) if record == nil { // should never happen From 21a280365ebfd14644534292015a168e28c41958 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 30 Sep 2022 17:04:18 +0300 Subject: [PATCH 26/30] fix bugs --- pkg/guard-gate/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/guard-gate/client.go b/pkg/guard-gate/client.go index 1c1f7ebe..4e79d1e2 100644 --- a/pkg/guard-gate/client.go +++ b/pkg/guard-gate/client.go @@ -93,7 +93,7 @@ func (srv *gateClient) reportPile() { query.Add("cm", "true") } req.URL.RawQuery = query.Encode() - pi.Log.Infof("Reporting a pile with pileCount %d records to guard-service - Url %v", srv.pile.Count, req.URL) + pi.Log.Infof("Reporting a pile with pileCount %d records to guard-service", srv.pile.Count) res, postErr := srv.httpClient.Do(req) if postErr != nil { From 4e7abe9578c0e1b87bd89f279f7d3328c5d8cc92 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 30 Sep 2022 17:15:40 +0300 Subject: [PATCH 27/30] fix bugs --- cmd/guard-service/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/guard-service/main.go b/cmd/guard-service/main.go index 40e992fd..7a1195cb 100644 --- a/cmd/guard-service/main.go +++ b/cmd/guard-service/main.go @@ -86,6 +86,7 @@ func (l *learner) baseHandler(query url.Values) (record *serviceRecord, err erro } // get session record, create one if does not exist + log.Debugf("** baseHandler ** ns %s, sid %s, cmFlag %t", ns, sid, cmFlag) record = l.services.get(ns, sid, cmFlag) if record == nil { // should never happen From bf92ec5d597fe06a0ef094f6d6024f6c28a7031e Mon Sep 17 00:00:00 2001 From: David Hadas Date: Sat, 1 Oct 2022 13:55:53 +0300 Subject: [PATCH 28/30] review comments --- cmd/queue/main.go | 1 + .../guard-service.yaml} | 3 +++ .../queue-proxy.yaml} | 0 config/{ => resources}/gateAccount.yaml | 0 config/{ => resources}/guardiansCrd.yaml | 0 config/{ => resources}/serviceAccount.yaml | 0 hack/setup-guard-service.sh | 20 ++++++++++++++--- hack/update-guard-service.sh | 22 +++++++++++++++++-- pkg/guard-gate/gate.go | 16 ++++++++++++-- pkg/guard-gate/session.go | 4 ++-- 10 files changed, 57 insertions(+), 9 deletions(-) rename config/{deploy-guard-service.yaml => deploy/guard-service.yaml} (90%) rename config/{deploy-queue-proxy.yaml => deploy/queue-proxy.yaml} (100%) rename config/{ => resources}/gateAccount.yaml (100%) rename config/{ => resources}/guardiansCrd.yaml (100%) rename config/{ => resources}/serviceAccount.yaml (100%) diff --git a/cmd/queue/main.go b/cmd/queue/main.go index faf35833..0b96d5e6 100644 --- a/cmd/queue/main.go +++ b/cmd/queue/main.go @@ -24,6 +24,7 @@ import ( "knative.dev/serving/pkg/queue/sharedmain" ) +// Knative Serving Queue Proxy with support for a guard-gate QPOption func main() { qOpt := qpoption.NewGateQPOption() defer qOpt.Shutdown() diff --git a/config/deploy-guard-service.yaml b/config/deploy/guard-service.yaml similarity index 90% rename from config/deploy-guard-service.yaml rename to config/deploy/guard-service.yaml index 6c2c7e6a..f89f1463 100644 --- a/config/deploy-guard-service.yaml +++ b/config/deploy/guard-service.yaml @@ -23,6 +23,9 @@ spec: imagePullPolicy: Always ports: - containerPort: 8888 + env: + - name: GUARD_SERVICE_LOG_LEVEL + value: "debug" --- apiVersion: v1 kind: Service diff --git a/config/deploy-queue-proxy.yaml b/config/deploy/queue-proxy.yaml similarity index 100% rename from config/deploy-queue-proxy.yaml rename to config/deploy/queue-proxy.yaml diff --git a/config/gateAccount.yaml b/config/resources/gateAccount.yaml similarity index 100% rename from config/gateAccount.yaml rename to config/resources/gateAccount.yaml diff --git a/config/guardiansCrd.yaml b/config/resources/guardiansCrd.yaml similarity index 100% rename from config/guardiansCrd.yaml rename to config/resources/guardiansCrd.yaml diff --git a/config/serviceAccount.yaml b/config/resources/serviceAccount.yaml similarity index 100% rename from config/serviceAccount.yaml rename to config/resources/serviceAccount.yaml diff --git a/hack/setup-guard-service.sh b/hack/setup-guard-service.sh index a38b45a5..015855b5 100755 --- a/hack/setup-guard-service.sh +++ b/hack/setup-guard-service.sh @@ -1,6 +1,20 @@ -kubectl apply -f config/gateAccount.yaml -kubectl apply -f config/serviceAccount.yaml -kubectl apply -f config/guardiansCrd.yaml +#!/usr/bin/env bash + +# Copyright 2022 The Knative Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +kubectl apply -f config/resources ./hack/update-guard-service.sh diff --git a/hack/update-guard-service.sh b/hack/update-guard-service.sh index 1b8458e5..a1090217 100755 --- a/hack/update-guard-service.sh +++ b/hack/update-guard-service.sh @@ -1,2 +1,20 @@ -export KO_DOCKER_REPO=us.icr.io/knat -ko apply -Rf ./config/deploy-guard-service.yaml +#!/usr/bin/env bash + +# Copyright 2022 The Knative Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Add export KO_DOCKER_REPO to your image repository +# E.g. export KO_DOCKER_REPO=us.icr.io/knat + +ko apply -Rf ./config/deploy diff --git a/pkg/guard-gate/gate.go b/pkg/guard-gate/gate.go index 373d3e66..ec034209 100644 --- a/pkg/guard-gate/gate.go +++ b/pkg/guard-gate/gate.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "net/http" + "path" "strings" "time" @@ -156,15 +157,26 @@ func (p *plug) preInit(ctx context.Context, c map[string]string, sid string, ns ctx, cancelFunction := context.WithCancel(ctx) + sid = utils.Sanitize(sid) + ns = utils.Sanitize(ns) + if sid == "" { + // mandatory + panic("Ileal sid") + } + if ns == "" { + // mandatory + panic("Ileal ns") + } + // Defaults used without config when used as a qpoption - guardServiceUrl := fmt.Sprintf("http://guard-service.%s", ns) + guardServiceUrl := path.Clean(fmt.Sprintf("http://guard-service.%s", ns)) useCm := false monitorPod := true if c != nil { if v, ok = c["guard-url"]; ok && v != "" { // use default - guardServiceUrl = v + guardServiceUrl = path.Clean(v) } if v, ok = c["use-cm"]; ok && strings.EqualFold(v, "true") { diff --git a/pkg/guard-gate/session.go b/pkg/guard-gate/session.go index f741ba59..9eef16fd 100644 --- a/pkg/guard-gate/session.go +++ b/pkg/guard-gate/session.go @@ -132,12 +132,12 @@ func (s *session) sessionEventLoop(ctx context.Context) { } func (s *session) screenResponseBody(req *http.Response) { - // TBD add screenResponseBody in a future PR + // TODO profile screenResponseBody in a future PR s.alert += s.gateState.decideRespBody(&s.profile.RespBody) } func (s *session) screenRequestBody(req *http.Request) { - // TBD add screenRequestBody in a future PR + // TODO profile screenRequestBody in a future PR s.alert += s.gateState.decideReqBody(&s.profile.ReqBody) } From 68080b9a35522bace90230e027911370f09ae0b1 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Sat, 1 Oct 2022 14:11:52 +0300 Subject: [PATCH 29/30] review comments --- hack/update-guard-service.sh | 2 +- pkg/guard-gate/gate.go | 16 ++-------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/hack/update-guard-service.sh b/hack/update-guard-service.sh index a1090217..66d4623b 100755 --- a/hack/update-guard-service.sh +++ b/hack/update-guard-service.sh @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Add export KO_DOCKER_REPO to your image repository +# Add export KO_DOCKER_REPO to your image repository # E.g. export KO_DOCKER_REPO=us.icr.io/knat ko apply -Rf ./config/deploy diff --git a/pkg/guard-gate/gate.go b/pkg/guard-gate/gate.go index ec034209..373d3e66 100644 --- a/pkg/guard-gate/gate.go +++ b/pkg/guard-gate/gate.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "net/http" - "path" "strings" "time" @@ -157,26 +156,15 @@ func (p *plug) preInit(ctx context.Context, c map[string]string, sid string, ns ctx, cancelFunction := context.WithCancel(ctx) - sid = utils.Sanitize(sid) - ns = utils.Sanitize(ns) - if sid == "" { - // mandatory - panic("Ileal sid") - } - if ns == "" { - // mandatory - panic("Ileal ns") - } - // Defaults used without config when used as a qpoption - guardServiceUrl := path.Clean(fmt.Sprintf("http://guard-service.%s", ns)) + guardServiceUrl := fmt.Sprintf("http://guard-service.%s", ns) useCm := false monitorPod := true if c != nil { if v, ok = c["guard-url"]; ok && v != "" { // use default - guardServiceUrl = path.Clean(v) + guardServiceUrl = v } if v, ok = c["use-cm"]; ok && strings.EqualFold(v, "true") { From 098db6810bd5f8f5747ee581a981c62d58273eb7 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Mon, 3 Oct 2022 16:06:32 +0300 Subject: [PATCH 30/30] Update config/deploy/guard-service.yaml Co-authored-by: Paul Schweigert --- config/deploy/guard-service.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/deploy/guard-service.yaml b/config/deploy/guard-service.yaml index f89f1463..6857df4b 100644 --- a/config/deploy/guard-service.yaml +++ b/config/deploy/guard-service.yaml @@ -15,8 +15,6 @@ spec: app: guard-service spec: serviceAccountName: guard-service-account - imagePullSecrets: - - name: all-icr-io containers: - name: guard-service image: ko://knative.dev/security-guard/cmd/guard-service