GitLab Secure Stage, 26 Jul 2019

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Secure Brown Bag Sessions: Security Approvals Backend Implementation

Description

In this video we discuss the backend implementation around the Security Approvals in Merge Requests MVC.

https://gitlab.com/gitlab-org/gitlab-ee/issues/9928

A

Okay, hi everyone, so the feature we're talking about today is security approvals and merge requests. This has been also called security dates: I, don't love the term security gates because it's kind of um point isn't to make security a blocker, it's providing a place for other routines to collaborate. So um the basic idea with what we were working on before so first I'll start with a agenda. Doc should be on the invite.

A

What's the goal, a quick demo which I was having some GDK issues so we're gonna, do it live and see how that goes um a little bit on background jobs, because we need to discuss how background jobs work in the rails, app to understand where to plug things in we're going to talk about some of the key data models and some of the syncing workers that make the magic happen.

A

And hopefully we never have to look at these two files, but we may have to to make sense of the gaps cool so before we jump into that. Does anyone have any questions.

B

Okay, where are you I'll, reduce our staff, our plan? You should come for you too, okay, so.

A

My name is Lucas Charles I'm, a software engineer on the secure stage on the static and static and dynamic analysis team, and this is security approvals coming outs to a gitlab instance near you, starting today.

A

Okay, so let's go back here.

A

The basic idea with this is that we were looking at for a way to hand leverage a new approval system where a security team, that's defined by a user on a project, can add their approvals to before a merge request, gets merged and cheating. So we wanted to leverage our existing security reports and pipelines.

A

So with the proposal, II idea is that requiring an ultimate license? You go into your product settings. You create a special approval group with a in this case of predefined name. Ideally, this would be more of a state toggle on the rule type any 7a approval count of greater than 1 and right now it needs to be named exactly this and we'll get into how that works in a second.

A

The existing approval UI has a bit of code, yet so to clarify terminology, this is adding approval rule. These are users and groups, and you have the you add the approver from the users and groups to the approval rule, and so each row here is an approval rule.

A

So there's also a lot of legacy. Support around the idea of them approvals before approval rules were a thing. So you'll see this row right here, all members with developer role or higher. If I set this, then that adds a require approval to the project, but there is no real approval rule. This is kind of a like a shim that is um it's a project setting. So it's not an actual database instance or database record for an approval- maybe that's too granular, but that kind of is important later.

A

Okay, so we add the approver it's out here with this handy tool tip and then once you actually add it here, MRR shows up right here and I am the approver in this approval rule, and there is one required approval now. That would just be how approval rules work, but in this case the important thing is that this number right here changes according to the results of a security report.

A

So, in our case here, if we change this from a low to a critical, it is now required. But if the severity reported back is medium or lower, then it will become optional.

A

A

Here is a overly complicated flowchart for all the conditions. The reason I want to touch on this is because life cycle is, we need to. We looked at different ways. We could set up this gate and the basic idea is that we need to require by default. So if your pipeline never runs the default stage should be an approval is required, so we need to toggle the approval state off at the end of the pipeline execution rather than on at the end of a pipeline execution.

A

So in the case of a high severity vulnerability, there are two directions: one is opening an M R before the pipeline finishes and the other one is opening an M are after a pipeline finishes, and those have slightly different life cycles, but high severity vulnerabilities should always require approval, missing vulnerability report. So there's no report at all that should also require approval, low severity or medium severity should remove the approval or what we call is make it optional. So you lower the approval, is required count and an empty vulnerability report also makes approvals optional.

A

Now this chart is in the parent issue, but I'll link here.

A

So this is important because we wanted to understand that at the beginning, when you start a pipeline, if an approval was added and then someone changes code, we need to reset to the original count. So there's essentially two life cycle stages, there's sinking to the original state and then there's decrementing the approvals required if necessary. And if you look at I'll zoom in here to okay, maybe I can't zoom in.

A

Assuming people can read this, there's two slightly different paths here pipeline finishes: we open an mr when we're opening an mr. We copy project rules to the mr and we run sync report: approver approval rules, which is essentially the the job that Cotton's the rules and then sync security reports to report approval rules which syncs the approval rules to the contents in the security report.

A

Now these two steps are slightly broken out, because the second one depends on the result of the pipeline. So we have to check if the pipeline is finished and in this case they run separately, but the result is still supposed to be the same.

A

Does that make sense so far.

C

Okay, a quick question, so basically the same code actually drives both of the life cycles. Yes, yes, yes,.

D

C

The trigger that is executed when one is finished.

A

That's right, there's one other part of the life cycle that we need to pay attention to as well, which is when you delete a project approval rule because we're again we're talking about the two data models. Here we have project rules and then we have merge, request rules and this first worker copies the project rules to the M R.

A

So if you delete a project rule should that task agent emerge across, we said yes, although that's a question that we we had to go back with product on and if you add a rule, does that cascade to merge across as well, but a so that's what these two extra boxes are delete, a product rule. It does nothing, so it doesn't revoke approvals required on all outstanding, merge across and then over.

D

A

Are their outstanding, merge requests when you're, adding a project approval rule update all non merged? Mrs wood approvals.

A

Cool, so this look like in the code well.

A

Here's our approval project rule model which belongs to projects and it has a rule type so there's a regular approval rules, code, owner approval rules and report approval rules, which is our new rule, type that we added with this work code owner isn't used because it's dynamically generated according to the code owner file, but it's here to reduce confusion because we are trying to we have a model or we have a concern right here.

A

The approval rule like that creates parity between the approval project rule in the approval, merge request rule model as such the approval rules require a source rule. So if you're a merge request rule here, your source rule will be where, where this was generated from it will be the project rule. Currently, project rules have no source rule, but it could make sense to add them at the group level later on.

A

And so we have the approval project rules with a regular code owner report, approver types we have the same. We will type here on the merger quest rules, but there's actually a subtype, which is our report type in this case. There's only one security, the idea being.

A

Perhaps we want a code quality report approver at some point and then there's also the approval rule like which is this concern shared by both, and this is where a lot of the same logic lives like belongs to users and has many groups by the name and has approvals required.

A

Ok, so looking back at these lifecycle, when you open NMR and have no pipeline, we call this new merge across worker.

A

This executes it starts it, fires off and it basically just sinks the merge across to the pipeline. Then we update the head pipeline for merge request and this basically says what shot is this merge across point and it links it to the pipeline when pushing changes to a branch with an open? Mr then, a number of I believe 18 workers fire, but the only two we really care about is this update, merging press worker and update head pipeline, and this only occurs for openmrs. So this is why this part is important.

A

If there is no open, mr then we have to plug into a different point to actually sync I say: I've merged I have the reefer or service listed here, because the Refresh service is really what handles the bulk of that interaction and that's the same thing that gets fired from the numerous requests. So, let's take a look at this refresh service.

A

Now this is overriding a for the non è é refresh service. We don't really care about here, but we'll just look at it quickly. To give you an idea of what this is doing, refresh merge requests, it realized my do cross it out gates, suggestions, refreshes pipelines marks pending to use done a lot of just standard behavior. For what happens every time you need to refresh a merge request from a code change.

A

The EE version is the one that's on top of that and it has the East Pacific behavior. So we override the Refresh merge, request service that main method, and we add on one additional behavior. We call super, we update the approvers and then we reset approvals for merging quest the resetting happens. If it's a new merging quest or if it's existing one, if it's new, then we don't care, it doesn't really do anything, and if it's the existing one, this would um increase the approvers required back to what whatever the approval project number is.

A

So if you say your approval project rule with approval is required of 2. Then from this you create an approval merger quest rule.

A

With approval is required of 2 from their pipeline runs and it decreases. The approval is required to zero because there's no security vulnerabilities and then you push code and on pushing code. It caused this refresh service, which resyncs rules and resets approval is required to so that's the flow. What we're talking about here.

A

So what we do is we grab the branch name. We find all the merger quests for this branch and these would be siblings from our current merge request. If there's multiple, then we need to sync all of them. Freeze merge requests. We find the target project because this merger press could go across projects and then we reset the approvals and we delete all existing approvals on the merger quest update, Cooper's.

A

Murder quest for source branch, so fine I'll, merge requests for the source branch. We sync her down rolls and we sync what we're calling report approver rules. So a reporter approvers currently is all security report. A perverse, but it's kind of a generic model for, like I, said a code quality or another report. Approval toad.

A

So this sync report approvals.

A

Is pretty straightforward? If merge request is merged, then we don't care it's been merged, so we don't want to affect what is considered like a locked state and we sink project approval rules to merge across rules, which means we take all approval rules on the trip on the target project.

A

In this case, all we care has a report, approver scope, and then we find the report approval rule. So we first are initialized on that, and this would vary if we have more than one report approver. Currently we don't and we update it with the standard attributes which basically says reset it to the project rules, settings, and this would also sink it. If you change the user group, the users or groups on the project rule during the sync, it would read, it would update users, the groups and the approval is required, counts.

A

Cool any questions at this point.

A

I'm not checking chat. If this is for me, oh.

A

D

Okay, I've got a question so right now we just have one type of report: approver law, so this is where we would plug nice and slash compliance approvers. If you were to create such a feature. Similar features at all licenses: yeah, okay, yeah.

A

So so we currently, we call the security um I think it could very well be the same. It could be a security type, the so what we would really need to change if we we can call compliance if we wanted to change that, but um this worker it's should sync all report approver rules, regardless of what they are. um The only thing is this part is currently hard-coded, so we should change that, but um I think that this should be fairly minimal in terms of the change, because they're just they're just approval straight.

E

Yes, what was the point then, to distinguish these types from.

A

Just like a regular approval rule, you.

E

Know when whether the goal to introduce this rebook type here, yeah.

A

So so there's a couple things in in one case: we need we need extra behavior to occur, and so we haven't. We looked at a rule that copyist project rules to merge request, but we haven't yet looked at the service that lowers the approvals required count. So we need to distinguish the rule right there, um but the other one is this: you like it's pretty complicated here, because let me use a different one. So here here is a plain project and.

A

Let this take its time loading, but the current UI remember I mentioned that fallback rule, so I'll just use the screenshot. Instead, this here is I. They call it a fallback rule, it's not an actual rule. So if you change this number right here, this changes a thing called like projects, settings approvals required count and it's a project setting and if you change this number you're actually creating a new database record.

A

So we need a way to render this. We only render this row if there are no existing regular approval rules, and this, like Sam, was getting into this, because this is kind of complicated. What you need to do is you say: are there any regular rules, if not render this row if there are render those? Are there?

A

Is there a report? Approver rule, if so render this one otherwise render none, and so it creates like a fairly complex chain to understand how to do how to fill out the default state of this as well. So those are the two major reasons.

E

He ever even that already achieved with the servi port type. If you can go back to the current self, please yeah.

A

So the world cyberspace.

E

A

Yeah the this was not necessary for us right now. This was anticipating that we will need more report types. Okay,.

E

Now, because we are seeing that if we are introducing licence compliant, it could be the same. So I was curious. What kind of different behavior would imply having a different report idea: yeah.

A

So, let's, let's look at their next service, so I can show you sync security reports to report from rules this service.

A

Lives currently within the security namespace, but it's sinking reports to approvals, so what it does is it finds the reports it considers a it concerned, it's safe if there's any reports and none have an unsafe severity, and this basically is.

A

Medium or I guess I'd be high, critical and unknown.

A

So if we wanted to make this service more generic, this right here would have to depend on some condition and we could probably do something like if reported security like contain security and then have a different condition for a different report type, but it seemed like a better idea to keep them distinct and introduced that state.

A

Originally, there wasn't really any state on the report approval rules, so we have a lot of like kind of scary things and they're like looking at the name, vulnerability check and determining if it's a security rule according to the name. That is just like a lot of code dead, so introducing a single state state machine seemed like the the most explicit way of showing that does. That answer. Your question yeah.

E

A

Yeah, so this service here sync reports to approval rules.

A

This is called during.

A

The sync security report- super rules is called during the pipeline life cycle, so that's occurs right here, so in our CI pipeline, when the status transitions to a completed state. This is the same place that we sync our security reports. So in the same place that we process our security reports, we now sync our reports to the approval rules as well, and since this occurs on all states, it doesn't need the default branch condition that we currently use.

C

Is there any potential for race for long default branches when security reports are not being processed by the moment when the security report approval rules are being synchronized, so the N.

A

There there is a possibility of that part. Part of the part of the design, for this is that it's always secure by default. So any condition that fails to sync. The reports ensures that there is an approval by default, so that should keep things safe um if this is a pipeline completed, but the security report here was not done processing that won't really affect this one. Since these sync security reports worker a.

A

Couple of levels deep here since it uses the reports object here. This is a poor. Oh, this is not looking at database records, so it doesn't really matter if the other worker is processed or not. This will just check the JSON it's okay, so.

C

It's just purely parses, the artifacts each every time, yeah.

A

Ii I think it's here.

A

Yeah, so the security reports object right here gets initialized and it parses out the the artifact directly. You have to go a couple like layers into here to see where that's happening, but it shouldn't it shouldn't depend on the other background, processing yeah thanks.

A

What did I out? What else did I have on this agenda? Yeah, so key models of a product, rule, rule, emerging quest, rule and I would also add here. The approval rule like, which is that concern.

A

Sinking, workers sink reporter pool rules initially, when you open an NMR or update one and then sinks. The carrier reports always fires after a pipeline high squeaky good.

A

There is one case where it could occur before a pipeline has completed, and we have that in our refresh service as well.

A

There we go in merge, requests, create service.

A

When you're creating at em are assuming the pipeline has finished before the mr has been created, we attempt to sync reports immediately after syncing the rules.

A

A

This is a generic state wrapper around all rule like objects. That should give them a common behavior, and it has a very large, complex spec, but I would avoid looking inside.

A

A

So any other questions.

A

Okay, just slightly over time, then thanks for attending everybody- and let me know if you have any other questions. Thank.

D

A

For demoing yeah.

D

This was big. Thank you. Thanks.