Document clarification for ERP records

- Add references between "ERP Payload Overview" and "ERP Encrypted
  Record".
- Add clarifications for each record.

Bug: b:225050507
Change-Id: I3423d196b0d343d03ee4eb7df2acf1716b685ebe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3531410
Auto-Submit: Hong Xu <xuhong@google.com>
Reviewed-by: Leonid Baraz <lbaraz@chromium.org>
Commit-Queue: Hong Xu <xuhong@google.com>
Cr-Commit-Position: refs/heads/main@{#982957}

chrome/browser/policy/messaging_layer/upload/record_upload_request_builder.h

@@ -12,15 +12,17 @@
 
 namespace reporting {
 
+// {{{Note}}} ERP Encrypted Record
+//
 // Builds an upload request payload specific for
-// EncryptedReportingJobConfiguration A JSON version of the payload looks like
+// EncryptedReportingJobConfiguration. A JSON version of the payload looks like
 // this:
 // {
 //   "encryptedRecord": [
 //     {
 //       "encryptedWrappedRecord": "EncryptedMessage",
 //       "encryptionInfo" : {
-//         "encryptionKey": "EncryptedMessage",
+//         "encryptionKey": "LocalPublicValue",
 //         "publicKeyId": 1
 //       },
 //       "sequenceInformation": {
@@ -32,7 +34,7 @@ namespace reporting {
 //     {
 //       "encryptedWrappedRecord": "EncryptedMessage",
 //       "encryptionInfo" : {
-//         "encryptionKey": "EncryptedMessage",
+//         "encryptionKey": "LocalPublicValue",
 //         "publicKeyId": 2
 //       },
 //       "sequenceInformation": {
@@ -47,16 +49,21 @@ namespace reporting {
 // TODO(b/159361496): Periodically add memory and disk space usage.
 //
 // This payload is added to the common payload of all reporting jobs, which
-// includes "device" and "browser" sub-fields:
+// includes other sub-fields such as "device" and "browser" (See note "ERP
+// Payload Overview"):
+//
+//   EncryptedReportingRequestBuilder builder;
+//   builder.AddRecord(record1);
+//   builder.AddRecord(record2);
+//   ...
+//   builder.AddRecord(recordN);
+//   auto payload_result = builder.Build();
+//   DCHECK(payload_result.has_value());
+//   job_payload_.Merge(payload_result.value());
 //
-// EncryptedReportingRequestBuilder builder;
-// builder.AddRecord(record1);
-// builder.AddRecord(record2);
-//  ...
-// builder.AddRecord(recordN);
-// auto payload_result = builder.Build();
-// DCHECK(payload_result.has_value());
-// job_payload_.Merge(payload_result.value());
+// The value of an "encryptedRecord" must be a list, in which each element is a
+// dictionary that represents a record. The details of each record is documented
+// in record.proto.
 
 class UploadEncryptedReportingRequestBuilder {
  public:

components/policy/core/common/cloud/encrypted_reporting_job_configuration.h

@@ -18,14 +18,16 @@
 
 namespace policy {
 
+// {{{Note}}} ERP Payload Overview
+//
 // EncryptedReportingJobConfiguration configures a payload for the Encrypted
 // server endpoint. A JSON version of the payload looks like this:
 // {
 //   "encryptedRecord": [
 //     {
 //       "encryptedWrappedRecord": "EncryptedMessage",
 //       "encryptionInfo" : {
-//         "encryptionKey": "EncryptedMessage",
+//         "encryptionKey": "LocalPublicValue",
 //         "publicKeyId": 1
 //       },
 //       "sequencingInformation": {
@@ -37,7 +39,7 @@ namespace policy {
 //     {
 //       "encryptedWrappedRecord": "EncryptedMessage",
 //       "encryptionInfo" : {
-//         "encryptionKey": "EncryptedMessage",
+//         "encryptionKey": "LocalPublicValue",
 //         "publicKeyId": 2
 //       },
 //       "sequencingInformation": {
@@ -63,6 +65,9 @@ namespace policy {
 // }
 // "device" and "browser" are populated by the base class,
 // the rest needs to be provided as |merging_payload|.
+//
+// Details of "encryptedRecord" are documented at note "ERP Encrypted Record".
+
 class POLICY_EXPORT EncryptedReportingJobConfiguration
     : public ReportingJobConfigurationBase {
  public:

components/reporting/proto/synced/record.proto

@@ -57,31 +57,34 @@ message WrappedRecord {
 message EncryptionInfo {
   // Encryption key (optional).
   // Represents the client's X25519 public key used along with the server's
-  // private key (identified by |public_key_id|) for shared secret recreation
-  // by the server to get the symmetric key used for |encrypted_wrapped_record|
-  // encryption.
+  // private key (identified by |public_key_id|) for shared secret recreation by
+  // the server to compose the symmetric key used for |encrypted_wrapped_record|
+  // encryption. Is transmitted in plaintext.
   optional bytes encryption_key = 1;
 
   // Public key id (optional)
-  // Hash of the public key used to do encryption. Used to identity the
-  // private key for decryption. If no key_id is present, it is assumed that
-  // |key| has been transferred in plaintext.
+  // Hash of the public key used to do encryption. Used to identity the server
+  // private key for decryption. If no key_id is present,
+  // |encrypted_wrapped_record| is transmitted in plaintext (which has become a
+  // test-only feature).
   optional int64 public_key_id = 2;
 }
 
 // Tracking information for what order a record appears in.
 message SequenceInformation {
   // Sequencing ID (monotonic number, required)
   // Tracks records processing progress and is used for confirming that this
-  // and all prior records have been processed. If the same number is
+  // and all prior records have been processed. Always starts from zero, and
+  // increases by one each time in a perfect world. If the same number is
   // encountered more than once, only one instance needs to be processed. If
   // certain number is absent when higher are encountered, it indicates that
-  // some records have been lost and there is a gap in the records stream
-  // (what to do with that is a decision that the caller needs to make).
+  // some records have been lost and there is a gap in the records stream (what
+  // to do with that is a decision that the caller needs to make).
   optional int64 sequencing_id = 1;
 
-  // Generation ID (required). Unique per device and priority. Generated anew
-  // when previous record digest is not found at startup (e.g. after powerwash).
+  // Generation ID (required). Unique per device and priority. Generated
+  // randomly anew when previous record digest is not found at startup (e.g.
+  // after powerwash).
   optional int64 generation_id = 2;
 
   // Priority (required)
@@ -93,10 +96,10 @@ message SequenceInformation {
 // disk and sent to the server.
 message EncryptedRecord {
   // Encrypted Wrapped Record (required)
-  // |WrappedRecord| encrypted with the |encryption_key| in |encryption_info|.
-  // When absent, indicates gap - respective record is irreparably corrupt or
-  // missing from Storage, and server side should log it as such and no longer
-  // expect client to deliver it.
+  // |WrappedRecord| encrypted with the symmetric key. When absent, indicates
+  // gap - respective record is irreparably corrupt or missing from Storage, and
+  // server side should log it as such and no longer expect client to deliver
+  // it. Otherwise, must be present.
   optional bytes encrypted_wrapped_record = 1;
 
   // Must be present to facilitate decryption of encrypted record.