Repository: google/exposure-notifications-server
Pull request #263 (2020-05-14 20:37:08)

align spec and api implementation.
/hold
Please review, but holding merge until people get a chance to look at this.

google-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/263#" title="Author self-approved">mikehelmick</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/master/OWNERS)~~ [mikehelmick]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/README.md b/README.md
index 555377d..a1ac0f8 100644
--- a/README.md
+++ b/README.md
@@ -35,7 +35,7 @@ Server here:
 
 * [Overview](docs/index.md)
 * [Deployment Guide](docs/deploying.md)
-* [Reference Documentation](https://godoc.org/github.com/google/exposure-notifications-server)
+* [Reference Documentation](https://pkg.go.dev/mod/github.com/google/exposure-notifications-server)
 * [Server Functional Requirements](docs/server_functional_requirements.md)
 * [Server Deployment Options](docs/server_deployment_options.md)
 
diff --git a/docs/index.md b/docs/index.md
index 9fcb48e..3326993 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -17,8 +17,8 @@ Server here:
 [Deployment guide](deploying.md)  
 [Contributor guide](/CONTRIBUTING.md)  
 [Server Functional Requirements](server_functional_requirements.md)  
-[Server Deployment Options](server_deployment_options.md)
-[Reference documentation](https://godoc.org/github.com/google/exposure-notifications-server)  
+[Server Deployment Options](server_deployment_options.md)  
+[Reference documentation](https://pkg.go.dev/mod/github.com/google/exposure-notifications-server)  
 
 ## Overview
 
diff --git a/docs/server_functional_requirements.md b/docs/server_functional_requirements.md
index 0251718..4635e78 100644
--- a/docs/server_functional_requirements.md
+++ b/docs/server_functional_requirements.md
@@ -51,34 +51,37 @@ necessary client batches for ingestion into the device for key matching.
 
 Minimum required fields, followed by a JSON example:
 
-`temporaryExposureKeys`  
-**Type**: Array of `TemporaryExposureKey` JSON objects  
-**REQUIRED**: contain at least 1 `TracingKey` object  
-**OPTIONAL**: contain up to 14 (or more if an application is keeping longer history) `TracingKey` objects.  
-**Description**: The verified temporary exposure keys
-
-* `temporaryExposureKeys` object properties
-  * `keyData` (**REQUIRED**)
-    * Type: String
-    * Description: Base64 encoded temporary exposure key from the device
-  * `rollingStartNumber` (**REQUIRED**)
-    * Type: integer (uint32)
-    * Description: Intervals are 10 minute increments since the UTC epoch
-  * `rollingPeriod` (**OPTIONAL** - this may not be present for some keys)
-    * Type: integer (uint32)
-    * Constraints
-      * Valid values are [1..144]
-      * If not present, 144 is the default value (1 day of intervals)
-    * Description: Number of intervals that the key is valid for
-* `regions` (**REQUIRED for federation between regions**)
+* `temporaryExposureKeys`
+  * **Type**: Array of `ExposureKey` JSON objects (below)
+  * **REQUIRED**: contain 1-14 `ExposureKey` object (an individual app/server
+    could keep longer history)
+  * **Description**: The verified temporary exposure keys
+  * `ExposureKey` object properties
+    * `key` (**REQUIRED**)
+      * Type: String
+      * Description: Base64 encoded temporary exposure key from the device
+    * `rollingStartNumber` (**REQUIRED**)
+      * Type: integer (uint32)
+      * Description: Intervals are 10 minute increments since the UTC epoch
+    * `rollingPeriod` (**OPTIONAL** - this may not be present for some keys)
+      * Type: integer (uint32)
+      * Constraints
+        * Valid values are [1..144]
+        * If not present, 144 is the default value (1 day of intervals)
+      * Description: Number of intervals that the key is valid for
+* `regions` (**REQUIRED**)
   [ISO 3166 alpha-2](https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2) format.
   * Type: Array of string
-  * Note: A server operator can use a finer grain definition of region or
-  * Description: 2 letter country code in
-  use different identifiers altogether.
+  * Description: 2 letter country to identify the region(s) a key is valid for.
+* `appPackageName` (**REQUIRED**)
+  * Type: string
+  * Constraints:
+    * For Android device, this must match the device `appPackageName` in the
+      SafetyNet verification payload
+  * Description: name of the application bundle that sent the request.
 * `platform` (**REQUIRED**)
   * Type: string
-  * Description: Mobile device platform, i.e. “ios” or “android” used to indicate which device attestation verification APIs should be used.
+  * Description: Mobile device platform this request originated from.
 * `transmissionRisk` (**REQUIRED**)
   * Type: Integer
   * **The values and meanings of this enum are not finalized at this time.** //TODO(llatif): check status
@@ -91,40 +94,31 @@ Minimum required fields, followed by a JSON example:
     * For Android devices this is a SafetyNet device attestation in JSON Web
     Signature (JWS) format.
     * For iOS devices, this is a DeviceCheck attestation.
-
-Example additional fields that an application might want to collect:
-
-* `appPackageName`
-  * Type: string
-  * Constraints:
-    * For Android device, this must match the device `appPackageName` in the
-    SafetyNet verification payload
-  * Description: name of the application bundle that sent the request.
-* `verificationAuthorityName`
-  * Type: String
-  * Description: opaque string, name of the authority that has verified the diagnosis.
 * `verificationPayload`
   * Type: String
   * Description: some signature / code confirming authorization by the verification authority.
 * `padding`
   * Type: String
-  * Description: Random data to obscure the size of the request network packet sniffers.
+  * Constraints:
+    * Recommend size is random between 1 and 2 kilobytes.
+  * Description: Random data to obscure the size of the request network packet
+    sniffers.
 
 The following snippet is an example POST request payload in JSON format.
 
 ```json
 {
   "temporaryTracingKeys": [
-    {"key": "base64 KEY1", "intervalNumber": 12345, "intervalCount": 144},
-    {"key": "base64 KEY2", "intervalNumber": 12345, "intervalCount": 10},
-    ...
-    {"key": "base64 KEYN", "intervalNumber": 12345, "intervalCount": 100}],
+    {"key": "base64 KEY1", "rollingStartNumber": 12345, "rollingPeriod": 144},
+    {"key": "base64 KEY2", "rollingStartNumber": 12489, "rollingPeriod": 10},
+    {"key": "base64 KEYN", "rollingStartNumber": 12499, "rollingPeriod": 100}],
   "regions": ["US", "CA", "MX"],
   "appPackageName": "com.foo.app",
+  "platform": "android",
   "diagnosisStatus": 2,
-  "deviceVerificationPayload": "base64 encoded payload string",
-  "verificationAuthorityName": "lab name / govt authority",
-  "verificationPayload": "signature /code from  of verifying authority"
+  "deviceVerificationPayload": "base64 encoded attestation payload string",
+  "verificationPayload": "signature /code from  of verifying authority",
+  "padding": "random string data..."
 }
 ```
 
diff --git a/internal/android/nonce.go b/internal/android/nonce.go
index 0cafadb..75a6fdd 100644
--- a/internal/android/nonce.go
+++ b/internal/android/nonce.go
@@ -64,7 +64,7 @@ func NewNonce(publish *model.Publish) Noncer {
 		transmissionRisk: publish.TransmissionRisk,
 		ttKeysBase64:     sortedKeys,
 		regions:          sortedRegions,
-		verification:     publish.VerificationAuthorityName,
+		verification:     publish.VerificationPayload,
 	}
 }
 
diff --git a/internal/model/exposure.go b/internal/model/exposure.go
index 69b623d..d4c2c6a 100644
--- a/internal/model/exposure.go
+++ b/internal/model/exposure.go
@@ -63,12 +63,14 @@ const (
 //  Note: This project doesn't directly include a diagnosis code verification System
 //        but does provide the ability to configure one in `serverevn.ServerEnv`
 type Publish struct {
-	Keys                      []ExposureKey `json:"exposureKeys"`
+	Keys                      []ExposureKey `json:"temporaryExposureKeys"`
 	Regions                   []string      `json:"regions"`
 	AppPackageName            string        `json:"appPackageName"`
+	Platform                  string        `json:"platform"`
 	TransmissionRisk          int           `json:"transmissionRisk"`
-	Verification              string        `json:"verificationPayload"`
-	VerificationAuthorityName string        `json:"verificationAuthorityName"`
+	DeviceVerificationPayload string        `json:"deviceVerificationPayload"`
+	VerificationPayload       string        `json:"verificationPayload"`
+	Padding                   string        `json:"padding"`
 }
 
 // ExposureKey is the 16 byte key, the start time of the key and the
@@ -85,8 +87,8 @@ type Publish struct {
 //   1 - 144 inclusive.
 type ExposureKey struct {
 	Key            string `json:"key"`
-	IntervalNumber int32  `json:"intervalNumber"`
-	IntervalCount  int32  `json:"intervalCount"`
+	IntervalNumber int32  `json:"rollingStartNumber"`
+	IntervalCount  int32  `json:"rollingPeriod"`
 }
 
 // Exposure represents the record as storedin the database
@@ -199,15 +201,14 @@ func (t *Transformer) TransformPublish(inData *Publish, batchTime time.Time) ([]
 		}
 
 		exposure := &Exposure{
-			ExposureKey:               binKey,
-			TransmissionRisk:          inData.TransmissionRisk,
-			AppPackageName:            inData.AppPackageName,
-			Regions:                   upcaseRegions,
-			IntervalNumber:            exposureKey.IntervalNumber,
-			IntervalCount:             exposureKey.IntervalCount,
-			CreatedAt:                 createdAt,
-			LocalProvenance:           true, // This is the origin system for this data.
-			VerificationAuthorityName: strings.ToUpper(strings.TrimSpace(inData.VerificationAuthorityName)),
+			ExposureKey:      binKey,
+			TransmissionRisk: inData.TransmissionRisk,
+			AppPackageName:   inData.AppPackageName,
+			Regions:          upcaseRegions,
+			IntervalNumber:   exposureKey.IntervalNumber,
+			IntervalCount:    exposureKey.IntervalCount,
+			CreatedAt:        createdAt,
+			LocalProvenance:  true,
 		}
 		entities = append(entities, exposure)
 	}
@@ -222,8 +223,8 @@ func (t *Transformer) TransformPublish(inData *Publish, batchTime time.Time) ([]
 	// We know the slice isn't empty, seed w/ the first interval.
 	nextInterval := entities[0].IntervalNumber
 	for _, ex := range entities {
-		if ex.IntervalNumber != nextInterval {
-			return nil, fmt.Errorf("exposure key intervals are not consecutive")
+		if ex.IntervalNumber < nextInterval {
+			return nil, fmt.Errorf("exposure keys have overlapping intervals")
 		}
 		nextInterval = ex.IntervalNumber + ex.IntervalCount
 	}
diff --git a/internal/verification/verify.go b/internal/verification/verify.go
index b4ff48e..dc7fc3d 100644
--- a/internal/verification/verify.go
+++ b/internal/verification/verify.go
@@ -64,7 +64,7 @@ func VerifySafetyNet(ctx context.Context, requestTime time.Time, cfg *apiconfig.
 	}
 
 	opts := cfg.VerifyOpts(requestTime, android.NewNonce(data))
-	if err := androidValidateAttestation(ctx, data.Verification, opts); err != nil {
+	if err := androidValidateAttestation(ctx, data.DeviceVerificationPayload, opts); err != nil {
 		if cfg.BypassSafetyNet {
 			logger.Errorf("bypassing safetynet verification for: '%v'", data.AppPackageName)
 			return nil
@@ -90,7 +90,7 @@ func VerifyDeviceCheck(ctx context.Context, cfg *apiconfig.APIConfig, data *mode
 		PrivateKey: cfg.DeviceCheckPrivateKey,
 	}
 
-	if err := iosValidateDeviceToken(ctx, data.Verification, opts); err != nil {
+	if err := iosValidateDeviceToken(ctx, data.DeviceVerificationPayload, opts); err != nil {
 		if cfg.BypassDeviceCheck {
 			logger.Errorf("devicecheck failed, but bypass enabled for app: '%v', failure: '%v'", data.AppPackageName, err)
 			return nil
diff --git a/tools/exposure-client/main.go b/tools/exposure-client/main.go
index 7cffd90..c0010c9 100644
--- a/tools/exposure-client/main.go
+++ b/tools/exposure-client/main.go
@@ -32,8 +32,8 @@ import (
 
 const (
 	// the length of a diagnosis key, always 16 bytes
-	dkLen                 = 16
-	MAX_TRANSMISSION_RISK = 8
+	dkLen               = 16
+	maxTransmissionRisk = 8
 )
 
 var (
@@ -68,7 +68,7 @@ func main() {
 
 	transmissionRisk := *transmissionRiskFlag
 	if transmissionRisk < 0 {
-		transmissionRisk = randomInt(MAX_TRANSMISSION_RISK)
+		transmissionRisk = randomInt(maxTransmissionRisk) + 1
 	}
 
 	regionIdx := randomInt(len(defaultRegions))
@@ -82,14 +82,21 @@ func main() {
 		verificationAuthorityName = randomArrValue(verificationAuthorityNames)
 	}
 
+	padding := make([]byte, randomInt(1000)+1000)
+	_, err := rand.Read(padding)
+	if err != nil {
+		log.Printf("error generating padding: %v", err)
+	}
+
 	data := model.Publish{
 		Keys:             exposureKeys,
 		Regions:          region,
 		AppPackageName:   *appPackage,
 		TransmissionRisk: transmissionRisk,
 		// This tool cannot generate valid safetynet attestations.
-		Verification:              "some invalid data",
-		VerificationAuthorityName: verificationAuthorityName,
+		DeviceVerificationPayload: "some invalid data",
+		VerificationPayload:       verificationAuthorityName,
+		Padding:                   base64.RawStdEncoding.EncodeToString(padding),
 	}
 
 	jsonData, err := json.Marshal(data)
@@ -99,8 +106,12 @@ func main() {
 
 	sendRequest(jsonData)
 
-	prettyJson, err := json.MarshalIndent(data, "", "  ")
-	log.Printf("payload: \n%v", string(prettyJson))
+	prettyJSON, err := json.MarshalIndent(data, "", "  ")
+	if err != nil {
+		log.Printf("Can't display JSON that was sent, error: %v", err)
+	} else {
+		log.Printf("payload: \n%v", string(prettyJSON))
+	}
 
 	if *twice {
 		time.Sleep(1 * time.Second)


Test patch:
diff --git a/internal/android/nonce_test.go b/internal/android/nonce_test.go
index 878f825..9f952fc 100644
--- a/internal/android/nonce_test.go
+++ b/internal/android/nonce_test.go
@@ -33,10 +33,10 @@ func TestGetNonce(t *testing.T) {
 					{Key: "2mvFSmRsFmJR5r07dxGSjg", IntervalNumber: 263267, IntervalCount: 144},
 					{Key: "6bAd3dv7p+VEuaJVkVItaQ", IntervalNumber: 263411, IntervalCount: 27},
 				},
-				Regions:                   []string{"GB", "US"},
-				AppPackageName:            appPackage,
-				TransmissionRisk:          4,
-				VerificationAuthorityName: "QRTH-ROWO-LOLO-FOOB",
+				Regions:             []string{"GB", "US"},
+				AppPackageName:      appPackage,
+				TransmissionRisk:    4,
+				VerificationPayload: "QRTH-ROWO-LOLO-FOOB",
 			},
 			Expected: "xH8QNR09EKuCCuNitam1RgjPaGHO/9p54VikqFdirVY=",
 		},
@@ -47,10 +47,10 @@ func TestGetNonce(t *testing.T) {
 					{Key: "t+k51ifogJo9jq3GH9LWGQ", IntervalNumber: 263267, IntervalCount: 144},
 					{Key: "3uXRrSlcv1+OMI3oFtdaUw", IntervalNumber: 263411, IntervalCount: 27},
 				},
-				Regions:                   []string{"gB", "us"},
-				AppPackageName:            appPackage,
-				TransmissionRisk:          7,
-				VerificationAuthorityName: "BREA-KMEO-FFAP-IECE",
+				Regions:             []string{"gB", "us"},
+				AppPackageName:      appPackage,
+				TransmissionRisk:    7,
+				VerificationPayload: "BREA-KMEO-FFAP-IECE",
 			},
 			Expected: "LHSwWAjTf3nMVTk7LBwMx9Wg7jEPRjEJf1zRtoxQI64=",
 		},
diff --git a/internal/android/safetynet_test.go b/internal/android/safetynet_test.go
index f6c3c7b..f687518 100644
--- a/internal/android/safetynet_test.go
+++ b/internal/android/safetynet_test.go
@@ -125,10 +125,10 @@ var (
 			{Key: "ko6TsgPP8Wvu+ijpSLbY3A==", IntervalNumber: 2648304, IntervalCount: 144},
 			{Key: "9kMgBy7qdG3o6eh3vAD3mQ==", IntervalNumber: 2648448, IntervalCount: 144},
 		},
-		TransmissionRisk:          1,
-		AppPackageName:            appPackage,
-		VerificationAuthorityName: "PUBLIC_HEALTH_AUTHORITY",
-		Regions:                   []string{"GB"},
+		TransmissionRisk:    1,
+		AppPackageName:      appPackage,
+		VerificationPayload: "PUBLIC_HEALTH_AUTHORITY",
+		Regions:             []string{"GB"},
 	}
 )
 
diff --git a/internal/api/jsonutil/unmarshal_test.go b/internal/api/jsonutil/unmarshal_test.go
index 604dc44..b9da711 100644
--- a/internal/api/jsonutil/unmarshal_test.go
+++ b/internal/api/jsonutil/unmarshal_test.go
@@ -59,20 +59,20 @@ func TestEmptyBody(t *testing.T) {
 }
 func TestMultipleJson(t *testing.T) {
 	invalidJSON := []string{
-		`{"exposureKeys":
+		`{"temporaryExposureKeys":
 			[{"key": "ABC"},
 			 {"key": "DEF"},
 			 {"key": "123"}],
 		"appPackageName": "com.google.android.awesome",
 		"regions": ["us"],
-		"verificationPayload": "foo"}
-		{"exposureKeys":
+		"deviceVerificationPayload": "foo"}
+		{"temporaryExposureKeys":
 			[{"key": "ABC"},
 			 {"key": "DEF"},
 			 {"key": "123"}],
 		"appPackageName": "com.google.android.awesome",
 		"regions": ["us"],
-		"verificationPayload": "foo"}`,
+		"deviceVerificationPayload": "foo"}`,
 	}
 	errors := []string{
 		"body must contain only one JSON object",
@@ -96,15 +96,15 @@ func TestInvalidJSON(t *testing.T) {
 
 func TestInvalidStructure(t *testing.T) {
 	invalidJSON := []string{
-		`{"exposureKeys": 42}`,
-		`{"exposureKeys": ["41", 42]}`,
+		`{"temporaryExposureKeys": 42}`,
+		`{"temporaryExposureKeys": ["41", 42]}`,
 		`{"appPackageName": 4.5}`,
 		`{"regions": "us"}`,
 		`{"badField": "doesn't exist"}`,
 	}
 	errors := []string{
-		`invalid value exposureKeys at position 19`,
-		`invalid value exposureKeys at position 22`,
+		`invalid value temporaryExposureKeys at position 28`,
+		`invalid value temporaryExposureKeys at position 31`,
 		`invalid value appPackageName at position 22`,
 		`invalid value regions at position 16`,
 		`unknown field "badField"`,
@@ -114,14 +114,16 @@ func TestInvalidStructure(t *testing.T) {
 
 func TestValidPublishMessage(t *testing.T) {
 	intervalNumber := int32(time.Date(2020, 04, 17, 20, 04, 01, 1, time.UTC).Unix() / 600)
-	json := `{"exposureKeys": [
-		  {"key": "ABC", "intervalNumber": %v},
-		  {"key": "DEF", "intervalNumber": %v},
-			{"key": "123", "intervalNumber": %v}],
+	json := `{"temporaryExposureKeys": [
+		  {"key": "ABC", "rollingStartNumber": %v, "rollingPeriod": 144},
+		  {"key": "DEF", "rollingStartNumber": %v, "rollingPeriod": 122},
+			{"key": "123", "rollingStartNumber": %v, "rollingPeriod": 1}],
     "appPackageName": "com.google.android.awesome",
+    "platform": "android",
     "regions": ["CA", "US"],
 		"transmissionRisk": 2,
-    "verificationPayload": "foo"}`
+    "DeviceVerificationPayload": "foo",
+    "VerificationPayload": "1234-ABCD-EFGH-5678"}`
 	json = fmt.Sprintf(json, intervalNumber, intervalNumber, intervalNumber)
 
 	body := ioutil.NopCloser(bytes.NewReader([]byte(json)))
@@ -141,14 +143,16 @@ func TestValidPublishMessage(t *testing.T) {
 
 	want := &model.Publish{
 		Keys: []model.ExposureKey{
-			{Key: "ABC", IntervalNumber: intervalNumber},
-			{Key: "DEF", IntervalNumber: intervalNumber},
-			{Key: "123", IntervalNumber: intervalNumber},
+			{Key: "ABC", IntervalNumber: intervalNumber, IntervalCount: 144},
+			{Key: "DEF", IntervalNumber: intervalNumber, IntervalCount: 122},
+			{Key: "123", IntervalNumber: intervalNumber, IntervalCount: 1},
 		},
-		Regions:          []string{"CA", "US"},
-		AppPackageName:   "com.google.android.awesome",
-		TransmissionRisk: 2,
-		Verification:     "foo",
+		Regions:                   []string{"CA", "US"},
+		Platform:                  "android",
+		AppPackageName:            "com.google.android.awesome",
+		TransmissionRisk:          2,
+		DeviceVerificationPayload: "foo",
+		VerificationPayload:       "1234-ABCD-EFGH-5678",
 	}
 	if diff := cmp.Diff(want, got); diff != "" {
 		t.Errorf("unmarshal mismatch (-want +got):\n%v", diff)
diff --git a/internal/model/exposure_test.go b/internal/model/exposure_test.go
index 05a531b..7bade05 100644
--- a/internal/model/exposure_test.go
+++ b/internal/model/exposure_test.go
@@ -339,7 +339,7 @@ func TestTransform(t *testing.T) {
 	}
 }
 
-func TestTransformNonConsecutive(t *testing.T) {
+func TestTransformOverlapping(t *testing.T) {
 	captureStartTime := time.Date(2020, 2, 29, 11, 15, 1, 0, time.UTC)
 	intervalNumber := IntervalNumber(captureStartTime)
 
@@ -368,7 +368,7 @@ func TestTransformNonConsecutive(t *testing.T) {
 			},
 		},
 		{
-			name: "gap",
+			name: "overlap 2",
 			source: Publish{
 				Keys: []ExposureKey{
 					{
@@ -378,7 +378,7 @@ func TestTransformNonConsecutive(t *testing.T) {
 					},
 					{
 						Key:            encodeKey(generateKey(t)),
-						IntervalNumber: intervalNumber + maxIntervalCount + 1,
+						IntervalNumber: intervalNumber - maxIntervalCount + 1,
 						IntervalCount:  maxIntervalCount,
 					},
 				},
@@ -401,7 +401,7 @@ func TestTransformNonConsecutive(t *testing.T) {
 			if err == nil {
 				t.Fatalf("Expected error, got nil")
 			}
-			if err.Error() != "exposure key intervals are not consecutive" {
+			if err.Error() != "exposure keys have overlapping intervals" {
 				t.Errorf("Wrong error, want '%v', got '%v'", "exposure key intervals are not consecutive", err)
 			}
 		})