New issue
Advanced search Search tips

Issue 601903 link

Starred by 3 users
Status: Fixed
Owner: ----
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 601900



Sign in to add a comment

ParseCertificateDate() may succeed with an invalid date

Project Member Reported by eroman@chromium.org, Apr 8 2016 
https://code.google.com/p/chromium/codesearch#chromium/src/net/cert/x509_cert_types.cc&rcl=1460128611&l=74

------------------
  valid &= exploded.HasValidValues();

  if (!valid)
    return false;

  *time = base::Time::FromUTCExploded(exploded);
  return true;
------------------

The problem in the above is that base::Time::FromUTCExploded() can fail even though |exploded.HasValidValues()| returned true.

The Windows implementation for instance will return Time(0) in this case.

This would be bad in the context of say parsing a notBefore constraint, since caller might imagine it is valid before it really is.

Comment 1 by rsleevi@chromium.org, Apr 8 2016

Labels: -Pri-2 OS-All Pri-3
Status: Available (was: Untriaged)
In practice, this should have limited security impact, because non-representable dates will fail to parse into OS cert handles (OS X, Windows, NSS). It's only a concern for BoringSSL, and only if the notBefore check was based on this, which it isn't.

Other forms of security policy code (such as enforcement of date-based constraints) handle cases of Time(0) just fine.

Marking Available / P-3 - this is a good cleanup/consistency, but not a necessarily urgent or high-priority one.

Comment 2 by eroman@chromium.org, Jun 21 2016

Components: -Internals>Network>SSL Internals>Network>Certificate
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 20 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8c6cd130d570c7b269e6c10e6716631afd3a648

commit c8c6cd130d570c7b269e6c10e6716631afd3a648
Author: maksim.sisov <maksim.sisov@intel.com>
Date: Tue Dec 20 13:36:00 2016

Make callers of FromUTC(Local)Exploded in net/ use new time API.

Use new time conversion API in accordance with
https://codereview.chromium.org/1988663002/

BUG= 601905 , 601903 , 601900 

Review-Url: https://codereview.chromium.org/2090713003
Cr-Commit-Position: refs/heads/master@{#439789}

[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/cert/ct_policy_enforcer_unittest.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/cert/x509_cert_types.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/cert/x509_cert_types_unittest.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/disk_cache/blockfile/eviction.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/extras/sqlite/sqlite_channel_id_store_unittest.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/ftp/ftp_directory_listing_parser_ls.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/ftp/ftp_directory_listing_parser_unittest.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/ftp/ftp_directory_listing_parser_unittest.h
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/ftp/ftp_directory_listing_parser_vms.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/ftp/ftp_util.cc
[modify] https://crrev.com/c8c6cd130d570c7b269e6c10e6716631afd3a648/net/http/http_util_unittest.cc

Comment 4 by maksim.s...@intel.com, Dec 21 2016 

can we close it now?

Comment 5 by maksim.s...@intel.com, Dec 21 2016

Status: Fixed (was: Available)
Feel free to reopen if anything.
►

Sign in to add a comment