RFC 2459 name comparison.

net/cert/internal/verify_name_match.cc

Index: net/cert/internal/verify_name_match.cc
diff --git a/net/cert/internal/verify_name_match.cc b/net/cert/internal/verify_name_match.cc
index 3f0d7718ed0969444888cc7e9e5d1da16bcd6fee..f162acbbd8e42926f442dd5ff1fdc5fbadee73ff 100644
--- a/net/cert/internal/verify_name_match.cc
+++ b/net/cert/internal/verify_name_match.cc
@@ -2,14 +2,159 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/strings/string_util.h"
 #include "net/cert/internal/verify_name_match.h"
 #include "net/der/input.h"
+#include "net/der/parser.h"
+#include "net/der/tag.h"
 
 namespace net {
 
-bool VerifyNameMatch(const der::Input& a, const der::Input& b) {
+namespace {
+
+// Advance |reader| so that the next read will return a non-space character, or
+// the end of the input. Return true if non-space characters remain in |reader|.
+bool SkipASCIISpace(der::ByteReader* reader) {
+  der::ByteReader peaker(*reader);

[nharper, 2015/05/14 19:21:44]

nit: peeker

[mattm, 2015/05/14 21:38:56]

doh.

+  while (true) {
+    uint8_t c;
+    if (!peaker.ReadByte(&c))
+      return false;
+    if (c == ' ') {
+      if (!reader->ReadByte(&c))
+        NOTREACHED();
+    } else {
+      return true;
+    }
+  }
+}
+
+// Normalize a PrintableString value according to RFC 2459 section 4.1.2.4.
+bool NormalizePrintableString(const der::Input& in, std::string* output) {
+  der::ByteReader reader(in);
+
+  // Normalized version will always be equal or shorter than input, pre-reserve
+  // the space to avoid re-allocations.
+  output->reserve(in.Length());
+
+  // Ignore leading whitespace.
+  SkipASCIISpace(&reader);
+
+  uint8_t c;
+  while (reader.ReadByte(&c)) {
+    if (c == ' ') {
+      // If there is non-whitespace characters remaining in input, compress
+      // multiple whitespace chars to a single space, otherwise ignore trailing
+      // whitespace.
+      if (SkipASCIISpace(&reader))
+        *output += ' ';
+    } else if (c >= 'A' && c <= 'Z') {
+      // Fold case.
+      *output += base::ToLowerASCII(c);
+    } else if ((c >= 'a' && c <= 'z') || (c >= '\'' && c <= ')') ||
+               (c >= '+' && c <= ':') || c == '=' || c == '?') {

[nharper, 2015/05/14 19:21:44]

Add comments indicating what the ranges '\'' thru ')' and '+' thru ':' are?

[mattm, 2015/05/14 21:38:57]

Done.

+      *output += c;
+    } else {
+      // Fail on any characters that are not valid for PrintableString.
+      // TODO(mattm): will we need to include '*'?
+      return false;
+    }
+  }
+
+  return true;
+}
+
+bool VerifyAttributeValueMatch(der::Parser* a, der::Parser* b) {
+  der::Tag a_tag, b_tag;
+  der::Input a_value, b_value;
+
+  // Read the attribute type.
+  if (!a->ReadTagAndValue(&a_tag, &a_value))
+    return false;
+  if (!b->ReadTagAndValue(&b_tag, &b_value))
+    return false;
+  // Type of "Attribute type" must be OBJECT IDENTIFIER.
+  if (a_tag != der::kOid || b_tag != der::kOid)

[nharper, 2015/05/14 19:21:44]

Since you want to match specific tags, why not do this:

if (!a->ReadTag(der::kOid, &a_value))
  return false;
if (!b->ReadTag(der::kOid, &b_value))
  return false;

[mattm, 2015/05/14 21:38:56]

Done.

+    return false;
+  // Attribute types must be equal.
+  if (!a_value.Equals(b_value))
+    return false;
+
+  // Read the attribute value.
+  if (!a->ReadTagAndValue(&a_tag, &a_value))
+    return false;
+  if (!b->ReadTagAndValue(&b_tag, &b_value))
+    return false;

[nharper, 2015/05/14 19:21:44]

After reading the attribute type and value, you should check that there is
nothing left to parse:

if (a->HasMore() || b->HasMore())
  return false;

[mattm, 2015/05/14 21:38:56]

Done.

+
   // TODO(mattm): use normalization as specified in RFC 5280 section 7.
-  return a.Equals(b);
+
+  // RFC 2459 section 4.1.2.4 comparison rules:
+  // Attributes encoded with different types may be assumed to be unequal.
+  if (a_tag != b_tag)
+    return false;
+  if (a_tag == der::kPrintableString) {
+    // PrintableString values should be compared case insenstive and ignoring
+    // extraneous whitespace.
+    std::string a_normalized, b_normalized;
+    if (!NormalizePrintableString(a_value, &a_normalized) ||
+        !NormalizePrintableString(b_value, &b_normalized))
+      return false;
+    return a_normalized == b_normalized;
+  } else {
+    // Types other than PrintableString use binary comparison.
+    return a_value.Equals(b_value);
+  }
+}
+
+bool VerifyRDNMatch(der::Parser* a, der::Parser* b) {
+  while (a->HasMore() && b->HasMore()) {
+    der::Parser a_attr_type_and_value;
+    der::Parser b_attr_type_and_value;
+    if (!a->ReadSequence(&a_attr_type_and_value) ||
+        !b->ReadSequence(&b_attr_type_and_value))
+      return false;
+    if (!VerifyAttributeValueMatch(&a_attr_type_and_value,
+                                   &b_attr_type_and_value))
+      return false;
+  }
+
+  // If one of the RDNs has more elements than the other, not a match.
+  if (a->HasMore() || b->HasMore())
+    return false;
+
+  return true;
+}
+
+}  // namespace
+
+// TODO(mattm): is returning false on parsing errors ok, or should it try to
+// fall back to binary comparison on unexpected input?
+bool VerifyNameMatch(const der::Input& a, const der::Input& b) {
+  der::Parser a_parser(a);
+  der::Parser b_parser(b);
+  der::Parser a_rdn_sequence;
+  der::Parser b_rdn_sequence;
+
+  if (!a_parser.ReadSequence(&a_rdn_sequence) ||
+      !b_parser.ReadSequence(&b_rdn_sequence)) {
+    return false;
+  }

[nharper, 2015/05/14 19:21:44]

Add a check that there's nothing in the inputs after the rdn sequence:

if (a_parser.HasMore() || b_parser.HasMore())
  return false;

[mattm, 2015/05/14 21:38:56]

Done.

+
+  while (a_rdn_sequence.HasMore() && b_rdn_sequence.HasMore()) {
+    der::Parser a_rdn, b_rdn;
+    if (!a_rdn_sequence.ReadConstructed(der::kSet, &a_rdn) ||
+        !b_rdn_sequence.ReadConstructed(der::kSet, &b_rdn)) {
+      return false;
+    }
+    if (!VerifyRDNMatch(&a_rdn, &b_rdn))
+      return false;
+  }
+
+  // If one of the sequences has more elements than the other, not a match.
+  if (a_rdn_sequence.HasMore() || b_rdn_sequence.HasMore())
+    return false;
+
+  return true;
 }
 
 }  // namespace net