ConsumerDataStandardsAustralia / standards-maintenance

This repository houses the interactions, consultations and work management to support the maintenance of baselined components of the Consumer Data Right API Standards and Information Security profile.
41 stars 9 forks source link

ADR should not initiate Authorisation Code Flow with PKCE if the Data Holder does not support this flow #500

Closed WestpacOpenBanking closed 2 months ago

WestpacOpenBanking commented 2 years ago

Description:

As per Data Standards v1.16, Data Recipient should use the OIDC Authorization Code Flow with PKCE from Sept 2022 and Data Holders must support the OIDC Authorization Code Flow with PKCE from April 2023.

During the transition period, Data Recipients can be on OIDC Authorization Code flow with PKCE and Data Holders still being on Hybrid Flow (until April 2023). The OpenID-configuration metadata is available for Data Recipients to discover whether the Authorisation Code Flow is supported by a Data Holder yet. The Data Recipient must first check this metadata, and if the Data Holder does not support the Authorization Code Flow, the Data Recipient should not use this flow.

Otherwise a scenario could arise where the Data Holder is not performing Auth Code validation (being on Hybrid Flow) and the Data Recipient is also not performing the Auth Code validation (being on the Authorization Code Flow, and expecting the Data Holder to be validating whether the Auth Code is valid). This is a security risk.

Area Affected:

Authentication Flows - Baseline Security Provisions

Change Proposed:

The Data Recipient should first check the OpenID-configuration metadata to see if the Authorisation Code Flow is supported by the Data Holder. Data Recipients should not use Authorisation Code Flow / Auth Code with PKCE if the Data Holder does not support it.

The requirement under Data Holders “where Data Holders that do not support [PKCE] MUST ignore PKCE claims and MUST NOT reject clients sending PKCE claims” should be removed (or amended to avoid the scenario described above).

perlboy commented 2 years ago

This proposal is in contravention of FAPI Conformance Suite which explicitly tests that a PKCE request is accepted (and ignored if unsupported). This is clarified in the description of fapi-rw-id2-ensure-valid-pkce-succeeds conformance test:

This test makes a FAPI authorization request using valid PKCE (RFC7636), which must succeed. FAPI-RW-ID2 does not require servers to support PKCE, but as per https://tools.ietf.org/html/rfc6749#section-3.1 'The authorization server MUST ignore unrecognized request parameters' - i.e. whether the server supports PKCE or not, a valid PKCE request must succeed. The reason for this test is that many OpenID Connect clients speculatively use PKCE, and the OAuth2 standard requires that requests from such clients must not fail.

code id_token does not preclude the use of PKCE (although it would be odd) so using this discovery metadata would be invalid.

Conversely if an ADR attempts code only flow when it is not advertised it should be rejected by the Holder anyway. PKCE itself does not define code vs. code id_token, rather code is most common because PKCE solves for the integrity protections hybrid flow previously solved with code id_token, notably c_hash. That is to say you could do code id_token with PKCE if desired...

Proceeding with this proposal would be a contravention of RFC6749.

WestpacOpenBanking commented 2 years ago

Thank you Perlboy for bringing our attention to the relevant sections of the upstream standards. Our purpose is not to override these standards, but to make the regime secure.

As Hybrid with PKCE is not a standard pattern, we continue to believe that there exists a risk in the Hybrid Flow whereby the Data Recipient Software Product is expecting the Data Holder to validate the code challenge and the Data Holder is expecting the Data Recipient Software Product to validate the c_hash.

Updated Change Proposed: From July 4th 2022, Data Recipient Software Products: • MUST continue to perform all necessary validations, including validation of c_hash and s_hash values, when using the OIDC Hybrid Flow, even when sending PKCE claims (unless the Data Holder has advertised PKCE support through its Discovery endpoint).

perlboy commented 2 years ago

I don't understand why it needs to be explicitly stated, what is wrong with the upstream standards on this that wouldn't requite that validation for hybrid flow anyway?

CDR-API-Stream commented 2 years ago

This issue was discussed in the Maintenance Iteration 11 call. It was agreed to incorporate this change request into this maintenance iteration.

nils-work commented 7 months ago

Hi @WestpacOpenBanking

Considering the obligations for Authorization Code Flow with PKCE have passed, it appears there may no longer be a requirement for this change. Can we consider closing this issue?

nils-work commented 2 months ago

If there are no further comments, this issue will be closed on 26 July 2024.