Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(176)

Unified Diff: net/cert/internal/verify_name_match.cc

Issue 1125333005: RFC 2459 name comparison. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: . Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..1d80f438fb3f5ca50d47c4f37d57d7f6739b1e10 100644
--- a/net/cert/internal/verify_name_match.cc
+++ b/net/cert/internal/verify_name_match.cc
@@ -2,14 +2,175 @@
// 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 {
+
+class PrintableStringNormalizer {
Ryan Sleevi 2015/05/13 01:27:52 Document :)
mattm 2015/05/13 03:24:05 Acknowledged.
+ public:
+ explicit PrintableStringNormalizer(const der::Input& in)
+ : reader_(in), in_begin_(true) {}
+
+ bool ReadByte(uint8_t* out) {
Ryan Sleevi 2015/05/13 01:27:52 Because these are non-trivial, my suggestion would
mattm 2015/05/13 03:24:05 Acknowledged.
+ if (in_begin_) {
+ // Ignore leading whitespace.
+ SkipWhitespace();
+ in_begin_ = false;
+ }
+
+ uint8_t c;
+ if (!reader_.ReadByte(&c))
+ return false;
+
+ if (c == ' ') {
+ if (SkipWhitespace()) {
+ // If there is non-whitespace characters remaining in input, compress
+ // multiple whitespace chars to a single space.
+ *out = c;
+ return true;
+ } else {
+ // If there is trailing whitespace, ignore it.
+ return false;
+ }
+ }
+
+ *out = base::ToLowerASCII(c);
Ryan Sleevi 2015/05/13 01:27:52 So this doesn't enforce that |c| is validly encode
mattm 2015/05/13 03:24:05 Done.
+ return true;
+ }
+
+ private:
+ // Skip whitespace, if any. Return true if characters remain in input.
+ bool SkipWhitespace() {
+ der::ByteReader peaker(reader_);
+ while (true) {
+ uint8_t c;
+ if (!peaker.ReadByte(&c))
+ return false;
+ if (c == ' ') {
+ if (!reader_.ReadByte(&c))
+ NOTREACHED();
+ } else {
+ return true;
+ }
+ }
+ }
+
+ der::ByteReader reader_;
+ bool in_begin_;
+};
Ryan Sleevi 2015/05/13 01:27:52 So as a design point, this won't scale when it com
mattm 2015/05/13 03:24:05 Done.
+
+// Compare two PrintableString values according to RFC 2459 section 4.1.2.4.
+bool PrintableStringMatch(const der::Input& a, const der::Input& b) {
+ PrintableStringNormalizer a_reader(a);
+ PrintableStringNormalizer b_reader(b);
+
+ while (true) {
+ uint8_t a_byte, b_byte;
+ bool a_done = !a_reader.ReadByte(&a_byte);
+ bool b_done = !b_reader.ReadByte(&b_byte);
+
+ if (a_done && b_done)
+ return true;
+
+ if (a_done || b_done)
+ return false;
+
+ if (a_byte != b_byte)
+ return false;
Ryan Sleevi 2015/05/13 01:27:52 This ends up being pretty inefficient in a tight l
mattm 2015/05/13 03:24:05 Acknowledged.
+ }
+}
+
+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)
+ 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;
+
// 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.
+ return PrintableStringMatch(a_value, b_value);
+ } 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;
Ryan Sleevi 2015/05/13 01:27:52 braces
mattm 2015/05/13 03:24:05 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;
Ryan Sleevi 2015/05/13 01:27:52 braces
mattm 2015/05/13 03:24:06 Done.
+ 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
« no previous file with comments | « no previous file | net/cert/internal/verify_name_match_unittest.cc » ('j') | net/cert/internal/verify_name_match_unittest.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698