Seeing the Forest AND the Trees

Technology for a better world.

Code Reviews April 12, 2008

Filed under: .NET,Security,WCF — pdxbob @ 3:57 am
Tags: , ,

There was a time when I cringed at the idea of having a formal code review of my code. Well, I’ve been programming for over twenty years and my first five years in the business was for a company that was transitioning from startup to sustained business mode, and there was just no time for good process. Since then I’ve been fortunate to work with people who care about the quality of their code as well as the business objectives. I worked on a team that was broken up last year by shifting corporate priorities due to acquisitions. We practiced scrum, some TDD, some pairing, and most importantly, we developed a code-review practice that was comfortable for everyone. Every other Friday afternoon at 3 we meet in a common area in the company cafe building where one person would walk through code. It was usually something they wrote, but we also wanted to select code others had written because we all picked up whatever task was at the top of the backlog, leading to a more well-rounded team. We reviewed comments, design, coding guidelines, all within the scope of the particular piece of code being reviewed. It worked reasonably well. And the late Friday time had the effect of relaxing everyone.

That team was special because we built a product from scratch and we developed a lot of trust in each other. Today was the last day at work for Philippe, a guy who wasn’t on that team but who worked on the security products development group at my company.  As I had developed an interest in building secure software, I had some interaction with him during the security code review process that he and his team conducted on our product. I took the recommendations that they made and designed and implemented a plan for handling as much of it as was possible in our impossible schedule .  When I said goodbye today, Philippe told me that he really enjoyed working with me back then because I took seriously the engineering of security into the product.

As I was flipping through my aggregator (Google Reader) tonight, I came across Joe Duffy‘s blog post on multi-threaded code review. Joe is one of the key members of the Microsoft team building the Parallel Extensions to the .NET Framework, and is currently writing a book on concurrency development for Addison-Wesley. His blog post is fairly long but then, writing concurrency code is difficult and making sure that it is validated is of critical importance. There is too much to re-state here. Even listing highlights doesn’t give it enough justice. If you’re writing multi-threaded code, go read this blog entry.

Back on security, related to code review I thought I’d point out that the Microsoft Patterns & Practices group has come up with security guidelines for WCF that includes a lot of how-to and application scenario documentation as well as videos. It is crucial to be aware of the common security traps such as buffer overflows and cross-site scripting attacks.  With WCF, however, there is a whole boatload of additional concerns because of the C: Communication.  A WCF service may need to impersonate the caller in order for a component on the receiving end can authorize them for some activity. The P&P team has nicely written docs for explaining how to do this. Or you can watch the video if you’re so inclined. This is great material for identifying questions and concerns for use in a security code review.


3 Responses to “Code Reviews”

  1. Greg Hughes Says:

    God stuff, Bob. I think it’s worth pointing out (so I hope you don’t mind that I do so here) that before you get to a code review, applying many of the best practices that go into these after-the-fact reviews really need to be applied in the design and modeling phases. In fact, without a good solid design process, your code review is bound to be a mess.

    As you know, I (among many others) have been a strong proponent of using a formal design and threat modeling standard at the beginning, middle and end of any lifecycle phase in addition to a solid review process. Well-done design and a formal threat model before coding starts results in a significant benefit when it comes time to ensure quality exists in review. I am glad you mention security, and argue that since security is a key component of quality, especially in this day and age, taking it into account as a first-class citizen starting from the very beginning is critical (and costs less in the long run). The Microsoft Threat Modeling Tool is a terrific, easy to use, and free piece of simple software that really everyone should use from the inception of any software project. It opens eyes and makes practical threat modeling possible and useful.

    Great topic. It’s scary how many organizations don’t review code in any formal manner at all, let alone doing a good job of design and modeling.

    And yes – Philippe is a terrific guy, for sure. Sorry to see him leave there, but glad for him in his new gig. 🙂

  2. Greg Hughes Says:

    Oops – I meant GOOD stuff, heh. Oops!

  3. pdxbob Says:

    I completely agree with everything you said, Greg. I wish that the TMT had taken off at work. In fact, this is a good impetus for me to bring it up again.

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s