GitReview: All Git Commits are Reviewed

out I hope everyone's having fun of this conference today um I'm going to go ah and just get started with a quick story about when I was 17 years old I was working for a company out here in the Bay area and we built the servers and installed the routers and built the networks for school districts all over the Bay Area and outside of the Bay Area one of these school districts was in Modesto and if you're not familiar with the Bay Area Modesto is incredibly hot compared to this area here and I was driving this toilet to pickup truck and it was had no air conditioner it it was hot and by the time I get out to the school every

single day I was dripping wet miserable sweaty and I just couldn't cool off so Monday this wonderful week they sent me out to Modesto on Tuesday they sent me to Modesto Wednesday to Modesto so Thursday they sent me to Modesto and I said I'm not going back to Modesto and so I figured out a solution and it turned out that I probably broke the law but what I did is I logged into a network of a school nearby in a cooler climate and I disabled a couple printers in the administration office because I knew that the next day I would get sent out to fix those

printers so that's exactly what happened and ironically now 15 years later or so I am the guy responsible for detecting these types of behavior um so a little bit about me yes I have four kids uh the interesting thing is that they are all under 5 years old uh we have had an onslaught of infants in our house for the last 5 years and I love them to death but we got a lot of them so I've got nearly 20 years overall experience I've worked at a ton of companies um the highlights are Google probably Wells Fargo for the financial stuff and lately I work at startups I love startups um startups have a super

singular Mission it's easy to figure out what's you're doing why you're going to work every day and everyone is pretty smart in that when you say something needs to be more secure they get it they get that the company depends on keeping the infrastructure secure or keeping the software secure and so um I'm going to tell you today about four things I'm going to talk to you about this idea of reflective control I'm going to tell you how github's broken and how we' started fixing the git review and how we think we fix it with this reflective control method the fourth thing is other ways that this reflective control idea can be applied to keeping your information

security secure or your infrastructure your code your development all the above so starting with the reflective control um a reflective control is both a preventative control it's a detective control and it's administrative control so we're going to do a quick review over controls in case anybody here has forgotten a preventive control is kind of like a lock on a door it keeps you in keeps you from or keeps something from going in but you can't really get around it easily detective control is more like the camera against a where you can go and look at the feed of information after the fact and find out what happened and an administrative would be I'm going to to assign somebody

to watch the camera feed all day long um and these are different kinds of controls a reflective control is a composite of all of these so quick story um the other day I was using my credit card I bought something on the Internet I typed in the information and it got declined and I thought that was weird I must have had a typo I did it again I did it a third time because I make mistakes uh so then I checked my email because that's what you do when the internet's not working for you and I saw an email from my credit card company asking me to approve this transaction so I went through I approved the

transaction it worked that's a preventative control it prevented me from spending money but where I got super annoying is that all day long every time I use that credit card when I went to Starbucks when I went to the gas station I'd have to go to Emil click approve and that would allow me to slide my card the second time and complete my transaction and the point of this is that preventative controls which is the standard one of the easiest controls Implement it's also one of the most frustrating for the end users so reflective I think is the best of all um and here's kind of uh what what it boils down to something changes and you've got

software in place to detect that that changed and you're able to identify who the subject matter experts are so maybe this is Cisco firewall and every time there's a set of changes it gets broadcast out to somebody on the firewall team and the second part here is that they they get that change they review it and they acknowledge that yes or no this was a valid change or no it wasn't and then the last part of this is if they don't acknowledge you do something about that um maybe you Pro if it's software development maybe you block the release from going out to production if it's a firewall change maybe you just send off an incident to

your security team or you open it for re-review or you could revert the change um so we'll come back to this again uh we're going to move ahead to this part of the talk how many people in the room use GitHub for development awesome how many of you do code reviews on a daily basis how many of you think the code reviews are important for quality and then for the security of your codebase so quite a few uh so we have this we've we have a control in place all changes need to be peer-reviewed it's super simple and we think it's important we think it's so important we've decided to make it one of our

compliance controls and it's something that we even tell our customers about we say everything's looked at but we also use GitHub so we started to look at GitHub and as we know as anyone who use GitHub knows they've got this fantastic poll request feature where you can do a code review you can see what changed it's colorful it's pretty I like it um so let's figure out how to enforce that every single bit of code gets reviewed so we start looking at the permissions and a GitHub repository has three permissions essentially you have no access you have read access and you have read write access but there's no you can write only through a poll

request which is what we are looking for now we they also have this fork and pole model and I don't mean it fully discount it it works great in some places um the developers that I've talked to including myself I don't like doing Forks it's hard to keep the code in sync it just doesn't solve it it's kind of like the frustrated credit card issue again um so the user experience isn't there with forkin pole but there's another more fundamental issue with it is to do a fork and pole there still has to be somebody with right access to the GitHub repository that's the person who's a maintainer that's a person who can send code into the main branch well

that's either a single point of failure or we still need to monitor that the code he's letting into the branch or the main code base is appropriate so it didn't solve the problem for us even if there wasn't the user experience problem uh GitHub is GitHub Enterprise and I didn't see the features that we needed there as well uh so bottom line GitHub didn't have an answer to this problem there's third Priority Solutions like Garrett um and a bunch of others but I'm not I'm not um I have no problem with those third parties except that my developers no GitHub I've got 15 of them uh we all of our tools are built around GitHub and GitHub is just almost an

industry standard at this point so the bottom line is it's kind of like saying you all have to use open Office nobody can use word anymore it's almost come to the point where GitHub is the deao standard or at least the least uh barrier to

entry so what are we going to do so we look at GitHub and we look at git in general and we see that this is based on a commit every time something's changed a new commit is created and there be a lot of commits uh this repository has got 28,000 commits I don't know about you guys but as a security guy I'm not going to review every single one of those I'm not going to go and track that back to find the poll request and here's a real problem is that there's only 5,000 poll requests so there's 28,000 commits there's 5,000 poll requests um now that number 28,000 is counting raw commits not merges into the main branch so it's

artificially high but there's still there's still a discrepancy

here and so the question is how are we going to find the the commits that didn't go through the proper code review process this is frustrating right so we fixed so we wrote some software uh we wrote some software called git review and git review excuse me so get review analyzes the get repository it also goes out to GitHub and it it pulls down all the poll requests and it pulls out all the comments to those poll requests and inside of those comments there's something pretty familiar to software development it's an lgtm or it looks good to me means if somebody else took a look at that code and decided yeah it looks okay usually is like even know

lgtm but do this and do do that but the bottom line is somebody looked at it said it was good and I call that peer RW at this point so we take all the PRS or the poll requests that have that peer review and we take a list of all the commits and we merge them and what comes out of that are all the commits that didn't get reviewed great we fixed it we fixed GitHub uh we've got a list of violations and that that's fantastic except is it I mean what are we going to do with this list of violations um you know the most obvious answer is we go and we feed people over the head and we

tell them they need to go fix it uh that doesn't sit well with me um I was looking for a better solution to this so we definitely know the software's coming in and it's not getting peer-reviewed and the first question you might be asking is okay well why not just figure out a way to block that change from coming into the main branch and I think that's important to address is that if we we don't want to take away privileges from our development staff because they are the people that know this the best and they know better than me when they need to bypass a peerreview process maybe it's 12:00 at night they need to make they need to add a file

that wasn't included for the de the deployment that's due the next morning and they can't find anyone else to review their code that day thank you so we want to leave it open we want to leave the ability to bypass this process open but we also want to manage the violations that come through and so this is what this reflective control idea is that we were talking about a minute ago how we applied it to source code reviews so what we do is simply take every single violation that comes in and we create a ticket to another developer another engineer to review that code and it's through a ticket process so we can track whether not gets done

they can put their lgtm then into that ticket since there's not a poll request Associated to it the developer reviews the code that was already changed and at that point he can fire off an alert hey this is a huge problem this is a huge security issue or no this looks fine to me it can go out to the release and then the actual like the meat the iron behind this control is that the review all of these open violation reviews have to be closed before we'll actually push go on the release of production and so this way we can generally keep control over the reviews that go through at home without changing developer workflows

without taking the the ability for them to push code in away from them and we can still manage our risk to information security so that's the idea of the control um there's good there's good points to it developers are happy compliance is Happy the customer is happy because they're getting their releases on time I've got 100% code review check we're tracking the time for these out of band changes in our ticketing system so what we didn't have before is any idea of how long it would take to actually do these out of band changes and now we do um probably one of the best things for the developers is adding value to their day-to-day uh because they're

discovering problems that they thought were fine they thought it was a simple change but it turns out that change had impact that they didn't know about that would have normally been caught in a a regular code review but instead it's getting caught a little bit later and repaired before it becomes a bug out in production so now for the bad points because there are definitely bad points to this um I just gave you one of the biggest problems which is I'm deploying this code today and I need to make a quick change and I need to I can't find anyone to do a peer review but this has to get done well this process supports

that and the developer at the end of the day can make that decision whether or not he wants to push an unreviewed change into production or not we'll still review it but it may be a day or two after it's been running in production for a little while so there's some risk there another problem is um this is probably the biggest problem I've seen so far with the system is all of our secure files like our authentications in special directory I get an alert every time there's a change to it um I review those changes and somebody refactored the entire authentication system they changed from one library to another library and number one I didn't know about it

beforehand number two it didn't go through the pr process it went through this process so that is a bad thing because that is definitely something I would have liked to see PR before it got into the main Cod line but at the same time we caught it we not only caught that we had a change of security software but we were able to have a process in place to review that that everybody knew about and it went through the regular review steps and it didn't actually get to production until it did get a full review so to summarize kind of where we're at right now so this idea of reflective control is a composite of

existing controls used in a way to help us um not put a preventive control in the front of the process but put it somewhere back behind the process we showed we decided that I decided I showed that GitHub is broken and how we fixed it with G review and how we fix a peer review entirely with reflective

control woohoo so that's great for code revieww um but I think that this reflective control idea applies more broadly to other areas of our infrastructure and specifically I'm thinking about this concept that infrastructure is code um we we're pretty familiar now with Amazon web services that we can write a script that builds us servers builds us networks builds us firewalls and routers and we can check that script in a source code repository and we can also audit what's actually built against a simware script and we can detect changes so I'm using the same process here to keep TS of our security groups in Amazon web services uh namely that every time a change comes through to those we are

getting alerted and we're verifying that change actually makes sense um another area of course is as we move toward more automation through anible chef and puppet this just grows to I think a better way of auditing everything that changes on our systems and the main difference I see is instead of sending the changes to an audit team to review we're sending it back to the folks who actually work on those systems every single day similar to a developer making source code changes when they see a change to a library they worked on they understand immediately why they made certain decisions and how that change impacts it same thing with the server guys if they see that yeah I set

up DNS or I set up mail for the entire system and somebody just changed that I know exactly what the impact of that change is going to be or I can quickly go find out what it is whereas an audit team is going to have to make a phone call they're going to have to go to somebody's desk they're going to have to go and do investigate of research to understand what the impact of that change might be or they might not even recognize that there is an impact uh and so that's and that's where kind of this operations idea and this whole security idea can combine a little bit I don't believe that your typical

operations guy has the security mindset well I I don't want to say that a lot of Ops guys do but the Ops guys now have an ability to see what's changing um to their systems by other people they're no longer working in a vacuum and so that's that's ultimately what I see with this reflective control summarize one more time um reflective controls a composite of existing controls we show the GitHub is broken we fix GitHub with Git review we applied this reflective control to code reviews and we talked a little bit about other applications of reflective controls but that's not all I've got one more thing um we've also open sourced git review uh so you can go download a

copy of this and you can try it out against your own repository uh and also a link here in the slide uh to a long discussion on GitHub about GitHub and about how development shops use poll requests forks and polls and all that kind of good stuff right so thank you very much and I think we have time for questions we have plenty of time for questions

fantastic yeah so right now it is pulling out two things or three things it's pulling three things into a database it's grabbing the the lgtm and it's looking for a couple other strings like spelling out looks good to me sounds good to me a couple of those other common ones so we use um it's also pulling out the files per change so what files we actually changed and the point of that is that we for us we're managing we know which files have a security impact and that way we can send off an additional alert on security changes or changes to security file files system jir so he asked which uh ticketing systems have we integrated with and

right now it's jir do you have any on that's

changed can you repeat

that yeah absolutely so the question is uh do you differentiate between changes and departments I think l so like let's say you've got one group of people that do frontend development and another group of people that do backend development another group that do the authentication system development or is this is this tool able to differentiate between those different groups and then send that ticket to the correct um to that correct group or that correct list of people uh it's a great question the answer is no uh it's on the Todo and the read me file but being able to pull out like an owner's file for a directory was the initial idea of how to implement

that so all code under a specific directory would go to a group of people and then they would be selected for the peer review

yep yeah so uh the question was how easy is this to deploy to an internal git repo and I think the answer is it won't work out of the box for sure so this definitely depends on the poll request model so the ticketing so you ask it which ticketing systems do they integrate with well there's two pieces right so there's a ticketing system that actually you assign a post review to or an out of- Band review and then there's the PO request which is a change um a change uh review process so that's a diff the online diff tool I guess and so since this is using whole request to understand whether or not there's looks

good to me um yeah it probably won't work easily with an internal G

deployment

well yes and no so if you have a tool for piew uh then you can integrate with that instead so this tool that of Open Source is absolutely GitHub specific but it is modu modulized so that you can replace the poll request portion of it with whatever internal tool you're using and then make the same comparison across the two do you have a question I actually oh great I'm curious actually what other tools for internal peer reviews are you guys looking

at G [Music] GB let git lab

I've heard of

stash any more questions all right guys go get the software let me know what you think and uh thanks for coming

out