Distinguish between SCT invalidity reasons in UMA

net/cert/sct_status_flags.h

Index: net/cert/sct_status_flags.h
diff --git a/net/cert/sct_status_flags.h b/net/cert/sct_status_flags.h
index 123c25c89052acf0ca3e915d57555b452b04617b..8ee4db1fd280ba3cfdc07bcf34846727a597c290 100644
--- a/net/cert/sct_status_flags.h
+++ b/net/cert/sct_status_flags.h
@@ -21,12 +21,19 @@ enum SCTVerifyStatus {
   // The SCT is from an unknown log, so we cannot verify its signature.
   SCT_STATUS_LOG_UNKNOWN = 1,
 
-  // The SCT is from a known log, but the signature is invalid.
-  SCT_STATUS_INVALID = 2,
+  // SCTVerifyStatus=2 used to represent SCT_STATUS_INVALID, which has now been
+  // split into INVALID_SIGNATURE and INVALID_TIMESTAMP to represent the
+  // different reasons an SCT could be invalid.
 
   // The SCT is from a known log, and the signature is valid.
   SCT_STATUS_OK = 3,
 
+  // The SCT is from a known log, but the signature is invalid.
+  SCT_STATUS_INVALID_SIGNATURE = 4,
+
+  // The SCT is from a known log, but the timestamp is in the future.
+  SCT_STATUS_INVALID_TIMESTAMP = 5,
+
   // Used to bound the enum values.
   SCT_STATUS_MAX,

[Avi (use Gerrit), 2016/08/16 15:25:47]

That's not how MAX works; you assign MAX to the last valid value in the enum:

SCT_STATUS_MAX = SCT_STATUS_INVALID_TIMESTAMP,

(See https://cs.chromium.org/search/?q=FILTER_TYPE_LAST or
https://cs.chromium.org/search/?q=EVENT_MEDIA_TYPE_LAST or any number of other
enums.)

The issue is that SCT_STATUS_MAX is used in content/common/resource_messages.h
with:

IPC_ENUM_TRAITS_MAX_VALUE(net::ct::SCTVerifyStatus, net::ct::SCT_STATUS_MAX)

but if you look at the definition of IPC_ENUM_TRAITS_MAX_VALUE it says:

// Convenience macro for defining enumerated type traits for types which are
// range-checked by the IPC system to be in the range of 0..maxvalue inclusive.
// This macro should not need to be subsequently redefined.

so we're currently allowing the wrong range of enum values to pass over IPC.

We should fix this here in this CL.

[Eran Messeri, 2016/08/16 16:06:09]

Fixed.
The discrepancy comes, I believe, from using the same enum for UMA and IPC. The
convention when used for UMA is to have a _MAX value that's after the last valid
value (since UMA_HISTOGRAM_ENUMERATION  states it expects values that are
strictly less than the boundary value).

 };