The message length should be serialized as a 64-bit value.

base/sha1_portable.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/sha1.h"
 
 #include <string.h>
 
 #include "base/basictypes.h"
 
@@ skipping 41 matching lines @@
 
   uint32 A, B, C, D, E;
 
   uint32 H[5];
 
   union {
     uint32 W[80];
     uint8 M[64];
   };
 
   uint32 cursor;

[wtc, 2014/07/12 01:23:40]

Yukawa also suggested that |cursor| should be defined as size_t. I didn't make
that change because |cursor| is the index for the |M| array (see line 59) and
only varies in the range [0...64].

[yukawa, 2014/07/12 03:40:01]

Makes sense. Thanks.

-  uint32 l;
+  uint64 l;
 };
 
 static inline uint32 f(uint32 t, uint32 B, uint32 C, uint32 D) {
   if (t < 20) {
     return (B & C) | ((~B) & D);
   } else if (t < 40) {
     return B ^ C ^ D;
   } else if (t < 60) {
     return (B & C) | (B & D) | (C & D);
   } else {
@@ skipping 11 matching lines @@
   } else if (t < 40) {
     return 0x6ed9eba1;
   } else if (t < 60) {
     return 0x8f1bbcdc;
   } else {
     return 0xca62c1d6;
   }
 }
 
 static inline void swapends(uint32* t) {
-  *t = ((*t & 0xff000000) >> 24) |
-       ((*t & 0xff0000) >> 8) |
-       ((*t & 0xff00) << 8) |
-       ((*t & 0xff) << 24);
+  *t = (*t >> 24) | ((*t >> 8) & 0xff00) | ((*t & 0xff00) << 8) | (*t << 24);
 }
 
 const int SecureHashAlgorithm::kDigestSizeBytes = 20;
 
 void SecureHashAlgorithm::Init() {
   A = 0;
   B = 0;
   C = 0;
   D = 0;
   E = 0;
@@ skipping 28 matching lines @@
   M[cursor++] = 0x80;
 
   if (cursor > 64-8) {
     // pad out to next block
     while (cursor < 64)
       M[cursor++] = 0;
 
     Process();
   }
 
-  while (cursor < 64-4)
+  while (cursor < 64-8)
     M[cursor++] = 0;
 
-  M[64-4] = (l & 0xff000000) >> 24;
-  M[64-3] = (l & 0xff0000) >> 16;
-  M[64-2] = (l & 0xff00) >> 8;
-  M[64-1] = (l & 0xff);
+  M[cursor++] = (l >> 56) & 0xff;
+  M[cursor++] = (l >> 48) & 0xff;
+  M[cursor++] = (l >> 40) & 0xff;
+  M[cursor++] = (l >> 32) & 0xff;
+  M[cursor++] = (l >> 24) & 0xff;
+  M[cursor++] = (l >> 16) & 0xff;
+  M[cursor++] = (l >> 8) & 0xff;
+  M[cursor++] = l & 0xff;

[wtc, 2014/07/12 01:23:40]

All the & 0xff operations can be omitted.

[Mark Mentovai, 2014/07/12 14:48:47]

wtc wrote:
LGTM with or without this change.

 }
 
 void SecureHashAlgorithm::Process() {
   uint32 t;
 
   // Each a...e corresponds to a section in the FIPS 180-3 algorithm.
 
   // a.
   //
   // W and M are in a union, so no need to memcpy.
@@ skipping 42 matching lines @@
 void SHA1HashBytes(const unsigned char* data, size_t len,
                    unsigned char* hash) {
   SecureHashAlgorithm sha;
   sha.Update(data, len);
   sha.Final();
 
   memcpy(hash, sha.Digest(), SecureHashAlgorithm::kDigestSizeBytes);
 }
 
 }  // namespace base