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.