diff --git a/signature/json.go b/signature/json.go index 1cb3c44727..9e592863da 100644 --- a/signature/json.go +++ b/signature/json.go @@ -14,64 +14,6 @@ func (err jsonFormatError) Error() string { return string(err) } -// validateExactMapKeys returns an error if the keys of m are not exactly expectedKeys, which must be pairwise distinct -func validateExactMapKeys(m map[string]interface{}, expectedKeys ...string) error { - if len(m) != len(expectedKeys) { - return jsonFormatError("Unexpected keys in a JSON object") - } - - for _, k := range expectedKeys { - if _, ok := m[k]; !ok { - return jsonFormatError(fmt.Sprintf("Key %s missing in a JSON object", k)) - } - } - // Assuming expectedKeys are pairwise distinct, we know m contains len(expectedKeys) different values in expectedKeys. - return nil -} - -// int64Field returns a member fieldName of m, if it is an int64, or an error. -func int64Field(m map[string]interface{}, fieldName string) (int64, error) { - untyped, ok := m[fieldName] - if !ok { - return -1, jsonFormatError(fmt.Sprintf("Field %s missing", fieldName)) - } - f, ok := untyped.(float64) - if !ok { - return -1, jsonFormatError(fmt.Sprintf("Field %s is not a number", fieldName)) - } - v := int64(f) - if float64(v) != f { - return -1, jsonFormatError(fmt.Sprintf("Field %s is not an integer", fieldName)) - } - return v, nil -} - -// mapField returns a member fieldName of m, if it is a JSON map, or an error. -func mapField(m map[string]interface{}, fieldName string) (map[string]interface{}, error) { - untyped, ok := m[fieldName] - if !ok { - return nil, jsonFormatError(fmt.Sprintf("Field %s missing", fieldName)) - } - v, ok := untyped.(map[string]interface{}) - if !ok { - return nil, jsonFormatError(fmt.Sprintf("Field %s is not a JSON object", fieldName)) - } - return v, nil -} - -// stringField returns a member fieldName of m, if it is a string, or an error. -func stringField(m map[string]interface{}, fieldName string) (string, error) { - untyped, ok := m[fieldName] - if !ok { - return "", jsonFormatError(fmt.Sprintf("Field %s missing", fieldName)) - } - v, ok := untyped.(string) - if !ok { - return "", jsonFormatError(fmt.Sprintf("Field %s is not a string", fieldName)) - } - return v, nil -} - // paranoidUnmarshalJSONObject unmarshals data as a JSON object, but failing on the slightest unexpected aspect // (including duplicated keys, unrecognized keys, and non-matching types). Uses fieldResolver to // determine the destination for a field value, which should return a pointer to the destination if valid, or nil if the key is rejected. @@ -122,3 +64,25 @@ func paranoidUnmarshalJSONObject(data []byte, fieldResolver func(string) interfa } return nil } + +// paranoidUnmarshalJSONObject unmarshals data as a JSON object, but failing on the slightest unexpected aspect +// (including duplicated keys, unrecognized keys, and non-matching types). Each of the fields in exactFields +// must be present exactly once, and none other fields are accepted. +func paranoidUnmarshalJSONObjectExactFields(data []byte, exactFields map[string]interface{}) error { + seenKeys := map[string]struct{}{} + if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { + if valuePtr, ok := exactFields[key]; ok { + seenKeys[key] = struct{}{} + return valuePtr + } + return nil + }); err != nil { + return err + } + for key := range exactFields { + if _, ok := seenKeys[key]; !ok { + return jsonFormatError(fmt.Sprintf(`Key "%s" missing in a JSON object`, key)) + } + } + return nil +} diff --git a/signature/json_test.go b/signature/json_test.go index fe63a1cd1e..83fe292989 100644 --- a/signature/json_test.go +++ b/signature/json_test.go @@ -2,8 +2,6 @@ package signature import ( "encoding/json" - "fmt" - "math" "testing" "github.com/stretchr/testify/assert" @@ -24,91 +22,6 @@ func x(m mSI, fields ...string) mSI { return m } -func TestValidateExactMapKeys(t *testing.T) { - // Empty map and keys - err := validateExactMapKeys(mSI{}) - assert.NoError(t, err) - - // Success - err = validateExactMapKeys(mSI{"a": nil, "b": 1}, "b", "a") - assert.NoError(t, err) - - // Extra map keys - err = validateExactMapKeys(mSI{"a": nil, "b": 1}, "a") - assert.Error(t, err) - - // Extra expected keys - err = validateExactMapKeys(mSI{"a": 1}, "b", "a") - assert.Error(t, err) - - // Unexpected key values - err = validateExactMapKeys(mSI{"a": 1}, "b") - assert.Error(t, err) -} - -func TestInt64Field(t *testing.T) { - // Field not found - _, err := int64Field(mSI{"a": "x"}, "b") - assert.Error(t, err) - - // Field has a wrong type - _, err = int64Field(mSI{"a": "string"}, "a") - assert.Error(t, err) - - for _, value := range []float64{ - 0.5, // Fractional input - math.Inf(1), // Infinity - math.NaN(), // NaN - } { - _, err = int64Field(mSI{"a": value}, "a") - assert.Error(t, err, fmt.Sprintf("%f", value)) - } - - // Success - // The float64 type has 53 bits of effective precision, so ±1FFFFFFFFFFFFF is the - // range of integer values which can all be represented exactly (beyond that, - // some are representable if they are divisible by a high enough power of 2, - // but most are not). - for _, value := range []int64{0, 1, -1, 0x1FFFFFFFFFFFFF, -0x1FFFFFFFFFFFFF} { - testName := fmt.Sprintf("%d", value) - v, err := int64Field(mSI{"a": float64(value), "b": nil}, "a") - require.NoError(t, err, testName) - assert.Equal(t, value, v, testName) - } -} - -func TestMapField(t *testing.T) { - // Field not found - _, err := mapField(mSI{"a": mSI{}}, "b") - assert.Error(t, err) - - // Field has a wrong type - _, err = mapField(mSI{"a": 1}, "a") - assert.Error(t, err) - - // Success - // FIXME? We can't use mSI as the type of child, that type apparently can't be converted to the raw map type. - child := map[string]interface{}{"b": mSI{}} - m, err := mapField(mSI{"a": child, "b": nil}, "a") - require.NoError(t, err) - assert.Equal(t, child, m) -} - -func TestStringField(t *testing.T) { - // Field not found - _, err := stringField(mSI{"a": "x"}, "b") - assert.Error(t, err) - - // Field has a wrong type - _, err = stringField(mSI{"a": 1}, "a") - assert.Error(t, err) - - // Success - s, err := stringField(mSI{"a": "x", "b": nil}, "a") - require.NoError(t, err) - assert.Equal(t, "x", s) -} - // implementsUnmarshalJSON is a minimalistic type used to detect that // paranoidUnmarshalJSONObject uses the json.Unmarshaler interface of resolved // pointers. @@ -180,3 +93,45 @@ func TestParanoidUnmarshalJSONObject(t *testing.T) { assert.Error(t, err, input) } } + +func TestParanoidUnmarshalJSONObjectExactFields(t *testing.T) { + var stringValue string + var float64Value float64 + var rawValue json.RawMessage + var unmarshallCalled implementsUnmarshalJSON + exactFields := map[string]interface{}{ + "string": &stringValue, + "float64": &float64Value, + "raw": &rawValue, + "unmarshaller": &unmarshallCalled, + } + + // Empty object + err := paranoidUnmarshalJSONObjectExactFields([]byte(`{}`), map[string]interface{}{}) + require.NoError(t, err) + + // Success + err = paranoidUnmarshalJSONObjectExactFields([]byte(`{"string": "a", "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}`), exactFields) + require.NoError(t, err) + assert.Equal(t, "a", stringValue) + assert.Equal(t, 3.5, float64Value) + assert.Equal(t, json.RawMessage(`{"a":"b"}`), rawValue) + assert.Equal(t, implementsUnmarshalJSON(true), unmarshallCalled) + + // Various kinds of invalid input + for _, input := range []string{ + ``, // Empty input + `&`, // Entirely invalid JSON + `1`, // Not an object + `{&}`, // Invalid key JSON + `{1:1}`, // Key not a string + `{"string": "a", "string": "a", "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}`, // Duplicate key + `{"string": "a", "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true, "thisisunknown", 1}`, // Unknown key + `{"string": &, "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}`, // Invalid value JSON + `{"string": 1, "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}`, // Type mismatch + `{"string": "a", "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}{}`, // Extra data after object + } { + err := paranoidUnmarshalJSONObjectExactFields([]byte(input), exactFields) + assert.Error(t, err, input) + } +} diff --git a/signature/policy_config.go b/signature/policy_config.go index e4c93ed528..bc6c5e9a7d 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -255,13 +255,8 @@ var _ json.Unmarshaler = (*prInsecureAcceptAnything)(nil) func (pr *prInsecureAcceptAnything) UnmarshalJSON(data []byte) error { *pr = prInsecureAcceptAnything{} var tmp prInsecureAcceptAnything - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, }); err != nil { return err } @@ -290,13 +285,8 @@ var _ json.Unmarshaler = (*prReject)(nil) func (pr *prReject) UnmarshalJSON(data []byte) error { *pr = prReject{} var tmp prReject - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, }); err != nil { return err } @@ -465,15 +455,9 @@ func (pr *prSignedBaseLayer) UnmarshalJSON(data []byte) error { *pr = prSignedBaseLayer{} var tmp prSignedBaseLayer var baseLayerIdentity json.RawMessage - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - case "baseLayerIdentity": - return &baseLayerIdentity - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, + "baseLayerIdentity": &baseLayerIdentity, }); err != nil { return err } @@ -481,9 +465,6 @@ func (pr *prSignedBaseLayer) UnmarshalJSON(data []byte) error { if tmp.Type != prTypeSignedBaseLayer { return InvalidPolicyFormatError(fmt.Sprintf("Unexpected policy requirement type \"%s\"", tmp.Type)) } - if baseLayerIdentity == nil { - return InvalidPolicyFormatError(fmt.Sprintf("baseLayerIdentity not specified")) - } bli, err := newPolicyReferenceMatchFromJSON(baseLayerIdentity) if err != nil { return err @@ -541,13 +522,8 @@ var _ json.Unmarshaler = (*prmMatchExact)(nil) func (prm *prmMatchExact) UnmarshalJSON(data []byte) error { *prm = prmMatchExact{} var tmp prmMatchExact - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, }); err != nil { return err } @@ -576,13 +552,8 @@ var _ json.Unmarshaler = (*prmMatchRepoDigestOrExact)(nil) func (prm *prmMatchRepoDigestOrExact) UnmarshalJSON(data []byte) error { *prm = prmMatchRepoDigestOrExact{} var tmp prmMatchRepoDigestOrExact - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, }); err != nil { return err } @@ -611,13 +582,8 @@ var _ json.Unmarshaler = (*prmMatchRepository)(nil) func (prm *prmMatchRepository) UnmarshalJSON(data []byte) error { *prm = prmMatchRepository{} var tmp prmMatchRepository - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, }); err != nil { return err } @@ -656,15 +622,9 @@ var _ json.Unmarshaler = (*prmExactReference)(nil) func (prm *prmExactReference) UnmarshalJSON(data []byte) error { *prm = prmExactReference{} var tmp prmExactReference - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - case "dockerReference": - return &tmp.DockerReference - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, + "dockerReference": &tmp.DockerReference, }); err != nil { return err } @@ -704,15 +664,9 @@ var _ json.Unmarshaler = (*prmExactRepository)(nil) func (prm *prmExactRepository) UnmarshalJSON(data []byte) error { *prm = prmExactRepository{} var tmp prmExactRepository - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - switch key { - case "type": - return &tmp.Type - case "dockerRepository": - return &tmp.DockerRepository - default: - return nil - } + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "type": &tmp.Type, + "dockerRepository": &tmp.DockerRepository, }); err != nil { return err } diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 12cb832ab7..c1d930ba22 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -297,8 +297,8 @@ func TestPolicyUnmarshalJSON(t *testing.T) { // Various allowed modifications to the policy allowedModificationFns := []func(mSI){ - // Delete the map of specific policies - func(v mSI) { delete(v, "specific") }, + // Delete the map of transport-specific scopes + func(v mSI) { delete(v, "transports") }, // Use an empty map of transport-specific scopes func(v mSI) { v["transports"] = map[string]PolicyTransportScopes{} }, } diff --git a/signature/signature.go b/signature/signature.go index 30238e6115..79174c845d 100644 --- a/signature/signature.go +++ b/signature/signature.go @@ -120,78 +120,69 @@ func (s *untrustedSignature) UnmarshalJSON(data []byte) error { // strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal jsonFormatError error type. // Splitting it into a separate function allows us to do the jsonFormatError → InvalidSignatureError in a single place, the caller. func (s *untrustedSignature) strictUnmarshalJSON(data []byte) error { - var untyped interface{} - if err := json.Unmarshal(data, &untyped); err != nil { - return err - } - o, ok := untyped.(map[string]interface{}) - if !ok { - return InvalidSignatureError{msg: "Invalid signature format"} - } - if err := validateExactMapKeys(o, "critical", "optional"); err != nil { + var critical, optional json.RawMessage + if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + "critical": &critical, + "optional": &optional, + }); err != nil { return err } - c, err := mapField(o, "critical") - if err != nil { - return err - } - if err := validateExactMapKeys(c, "type", "image", "identity"); err != nil { - return err - } - - optional, err := mapField(o, "optional") - if err != nil { + var creatorID string + var timestamp float64 + var gotCreatorID, gotTimestamp = false, false + if err := paranoidUnmarshalJSONObject(optional, func(key string) interface{} { + switch key { + case "creator": + gotCreatorID = true + return &creatorID + case "timestamp": + gotTimestamp = true + return ×tamp + default: + var ignore interface{} + return &ignore + } + }); err != nil { return err } - if _, ok := optional["creator"]; ok { - creatorID, err := stringField(optional, "creator") - if err != nil { - return err - } + if gotCreatorID { s.UntrustedCreatorID = &creatorID } - if _, ok := optional["timestamp"]; ok { - timestamp, err := int64Field(optional, "timestamp") - if err != nil { - return err + if gotTimestamp { + intTimestamp := int64(timestamp) + if float64(intTimestamp) != timestamp { + return InvalidSignatureError{msg: "Field optional.timestamp is not is not an integer"} } - s.UntrustedTimestamp = ×tamp + s.UntrustedTimestamp = &intTimestamp } - t, err := stringField(c, "type") - if err != nil { + var t string + var image, identity json.RawMessage + if err := paranoidUnmarshalJSONObjectExactFields(critical, map[string]interface{}{ + "type": &t, + "image": &image, + "identity": &identity, + }); err != nil { return err } if t != signatureType { return InvalidSignatureError{msg: fmt.Sprintf("Unrecognized signature type %s", t)} } - image, err := mapField(c, "image") - if err != nil { - return err - } - if err := validateExactMapKeys(image, "docker-manifest-digest"); err != nil { - return err - } - digestString, err := stringField(image, "docker-manifest-digest") - if err != nil { + var digestString string + if err := paranoidUnmarshalJSONObjectExactFields(image, map[string]interface{}{ + "docker-manifest-digest": &digestString, + }); err != nil { return err } s.UntrustedDockerManifestDigest = digest.Digest(digestString) - identity, err := mapField(c, "identity") - if err != nil { - return err - } - if err := validateExactMapKeys(identity, "docker-reference"); err != nil { - return err - } - reference, err := stringField(identity, "docker-reference") - if err != nil { + if err := paranoidUnmarshalJSONObjectExactFields(identity, map[string]interface{}{ + "docker-reference": &s.UntrustedDockerReference, + }); err != nil { return err } - s.UntrustedDockerReference = reference return nil } diff --git a/signature/signature_test.go b/signature/signature_test.go index 3ddcffe574..1afbfd6de5 100644 --- a/signature/signature_test.go +++ b/signature/signature_test.go @@ -153,6 +153,7 @@ func TestUnmarshalJSON(t *testing.T) { func(v mSI) { x(v, "optional")["creator"] = 1 }, // Invalid "timestamp" func(v mSI) { x(v, "optional")["timestamp"] = "unexpected" }, + func(v mSI) { x(v, "optional")["timestamp"] = 0.5 }, // Fractional input } for _, fn := range breakFns { err = tryUnmarshalModifiedSignature(t, &s, validJSON, fn)