length of nonce tested in OP conformance tests

[Joseph Heenan]

The OP tests for FAPI1/FAPI2 currently require OPs to support a 10 character nonce, and to either correctly process or reject with an error a 384 character state.

The certification team identified that more than one certified clients use nonces up to 43 characters in length, hence we would like to require that OPs support 43 character nonces and seek the working groups approval to do so.

By way of background:

We currently warn (but don’t fail) RPs if they use over 43 character nonces, so the suggested change would introduce a consistency between the OP & RP tests.

https://gitlab.com/openid/conformance-suite/-/issues/1217 has the data about lengths we’ve seen in certified clients

https://gitlab.com/openid/conformance-suite/-/issues/1307 tracks adding a check for 43 character nonces

(There’s unfortunately a long history of discussions in this area with no real conclusion being reached, e.g. https://bitbucket.org/openid/connect/issues/1055/limits-on-overall-url-length and the issues linked from it.)

‌

[Filip Skokan]

Make that 64?

> crypto.randomBytes(32).toString('base64').length
44
> crypto.randomBytes(32).toString('base64url').length
43
> crypto.randomBytes(32).toString('hex').length
64

‌

[Joseph Heenan]

Sure - we’re happy to use a larger value than 43 in both RP & OP tests if the working group decides.

[Dima Postnikov]

I support @Filip Skokan 's suggestion re 64.

And my preference is to be explicit in the specification if we issue warnings.

[Dave Tonge]

if possible define minimum for as, for nonce 64 (although issue as its OIDC and not specifically required). State = 512

[Dave Tonge]

https://bitbucket.org/openid/fapi/pull-requests/476

[Dave Tonge]

PR merged

[Dave Tonge]

More feedback from ecosystems

[Dave Tonge]

We received feedback today that 512 is probably too small as some clients use JWTs for state.

We discussed the issue again and there are two positions that we need to reconcile

1. We shouldn’t use an arbitrrary value, and its probably better to not mention state length at all
2. We have a test for state length which is good for interopability, but ideally that test should be based on some normative text. In addition having some sort of guidance about state length is useful for implementers, especially those less familiar with the specs

‌

My suggestion at the moment is that we remove the normative text, but have a note that covers

• state is not used for CSRF, but can be used by the client for “state”
• some clients may use JWTs as state, and therefore state values may be over 1000 characters
• while this specification doesn’t require a minium supported value, the AS should expect to receive and process large state values and should not reject them without good reason

‌

‌

‌

[gffletch]

If lots of RPs are using JWTs to make the state value a parsable entity, 384 chars is most likely not sufficient and I don’t think we should require an OP error if the state is over X bytes. It seems like a “security consideration” would be a better way to address issues (and guidance) around the use of the state parameter. The harm in large state values is memory consumption, breakage in. mobile browsers, etc.

Another option for the certification suite would be to do 4 tests with different sized (2k, 1.5k, 1k, 384bytes) state values and then warn for everything over 384. This helps OPs recognize that they might get large state values but don’t break certification.

[Dave Tonge]

I suggest that we have a note that says:

In this document the state parameter is not used for CSRF protection, but may be used to by the client for application state. In circumstances where clients encode application state in a JWT the length of the state parameter value could be in excess of 1000 characters.

This gives a number, which we can therefore use in a conformance test - but doesn’t prescribe any normative language.

‌

[Dave Tonge]

https://bitbucket.org/openid/fapi/pull-requests/483

[Dave Tonge]

PR merged

[Joseph Heenan]

Issued opened in conformance suite to make this change in the tests: https://gitlab.com/openid/conformance-suite/-/issues/1322