Clarify that a null value is distinct from being omitted

[Michael Jones]

We should use language parallel to this from Core 5.3.2:

"If a Claim is not returned, that Claim Name SHOULD be omitted from the JSON object representing the Claims; it SHOULD NOT be present with a null or empty string value."

‌

[Michael Jones]

We discussed this on the 21-Sep-23 working group call. People agreed that null is a potential value that is not the same as omitting a value.

[Michael Jones]

We talked about this on the 22-Sep-23 Federation Editors' call. It seems that the impetus behind this discussion was a statement by @Vladimir Dzhuvinov that

The objective is to define a policy that removes a metadata parameter (by setting it to null).

We agreed that if we need this capability, we should add an explicit operator that does this - not to do it by assigning a special meaning to the value null. It's a separate question whether we need a delete_value operator or something along those lines. Is there a concrete use case for it?

[Vladimir Dzhuvinov]

This is incorrect. The impetus was my discovery how trivial it is to bypass default and essential policies when null is treated as a value and the underlying metadata parser treats null values as no parameter.

How to create an attack that bypasses these operators:

1. The Trust Chain includes policy operators default and / or essential for the metadata parameter.
2. The Entity Configuration sets a metadata parameter with a null value.
3. Assuming the null is treated as a distinct value, the Trust Chain validation will leave the null parameter as it is.
4. The Trust Chain thus validates successfully.
5. The metadata JSON object is taken and passed to say an RP metadata parser that treats no metadata parameter and parameter a with null identically, i.e. it doesn’t produce an invalid_metadata error.
6. The RP gets registered with metadata that has violated the policy.

The JSON RFC 8259 defines null as a value. Quite a number of JSON parser implementations however don’t make this distinction.

https://www.rfc-editor.org/rfc/rfc8259#section-3

For instance, in Java JSON objects can be represented as a java.util.Map object, and developers would then often use a simple Map.get call to check if the JSON member is present and has a value. The get method returns null in both cases - parameter omitted and parameter with null value. The proper way to do this is to use the containsKey method of Map, however this doesn’t seem to be used much in practise.

My proposal is to specify that default and essential treat a null in the same way as a missing metadata parameter.

Unless we have a good use case for null valued metadata parameters? Do we have such a use case?

To clarify, I don’t have a use case for metadata parameter deletion and this proposal is not to facilitate deletion policies (even though it will have this effect when null is treated the same as no parameter).

[Michael Jones]

By describing the attack when

the underlying metadata parser treats null values as no parameter

I believe you just strengthened the case to not treat null values as no parameter.

‌

[Michael Jones]

Oops, I just saw that https://www.rfc-editor.org/rfc/rfc7592#section-2.2 says this:

Valid values of client metadata fields in this request MUST replace, not augment, the values previously associated with this client. Omitted fields MUST be treated as null or empty values by the server, indicating the client's request to delete them from the client's registration. The authorization server MAY ignore any null or empty value in the request just as any other value.

Filip’s response got me to look there. (So it wasn’t DCR itself that assigned a value to null - I looked there for it. It was DCR Management.)

I’ve come to the conclusion, after all the discussion and evidence of likely implementation problems in Java and Python, that we need to follow suit. I’ll create a PR.

[Vladimir Dzhuvinov]

Thank you for discovering this. I never thought about looking there

[Michael Jones]

Fixed by https://bitbucket.org/openid/connect/pull-requests/630