Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CAMARA OIDC Profile #113

Closed

Conversation

garciasolero
Copy link
Contributor

@garciasolero garciasolero commented Jan 25, 2024

What type of PR is this?

  • documentation

What this PR does / why we need it:

Add first draft of CAMARA OIDC Profile:

  • Authorization flows:
    • Authorization Code Flow.
    • Client Initiated Back Channel Authentication (CIBA).
    • Client Credentials.
  • Current definition of scope parameter format applicable to CAMARA APIs that could be evolve in the future.
  • ID Token content proposal based on OIDC standard.
  • Client Authentication.

Which issue(s) this PR fixes:

Special notes for reviewers:

Pending topics to be discussed for inclusion in future versions of the profile:

  • Purpose/Scope redefinition
  • PKCE
  • request parameter in CIBA flow
  • DPoP

Changelog input

N/A

Additional documentation

N/A

@mhfoo
Copy link
Collaborator

mhfoo commented Jan 26, 2024

Copy link
Collaborator

@AxelNennker AxelNennker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First a general remark: I think this document would be easier to read, if it only specifies what decisions Camara made to improve interoperability and security. E.g. repeating all the request parameters that are already according to the standard makes it hard to see what Camara requires.

Detailed reviews on specific points follow.

@bigludo7
Copy link
Collaborator

Thanks @garciasolero for this contribution. May I ask you to add my colleague @sebdewet in the reviewer list (I cannot do it). Thanks

@jpengar jpengar requested a review from sebdewet January 26, 2024 12:30
@jpengar
Copy link
Collaborator

jpengar commented Jan 26, 2024

Thanks @garciasolero for this contribution. May I ask you to add my colleague @sebdewet in the reviewer list (I cannot do it). Thanks

done.


* **scope**

OPTIONAL. A space delimited and a case-sensitive list of ASCII strings.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scope must be REQUIRED for CAMARA, because all endpoints will have a scope

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to REQUIRED.

@nickvenezia
Copy link

Is this a Developer Profile being created?
PII should be addressed if this is a profile for consumer.


* **redirect_uri**

REQUIRED. Redirection URI to which the response will be sent. This URI MUST exactly match one of the Redirection URI values for the Client pre-registered at the OpenID Provider, with the matching performed as described in [RFC 3986](https://www.rfc-editor.org/rfc/rfc3986.html#section-6.2.1). When using this flow, the Redirection URI SHOULD use the https scheme; however, it MAY use the http scheme. The Redirection URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using this flow, the Redirection URI SHOULD use the https scheme; however, it MAY use the http scheme.
When using this flow, the Redirection URI MUST use the https scheme; Insecure schemes like http MUST not be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inclusion of the http scheme was only intended for testing scenarios in development environments. If you agree, we could make that restriction explicit in the profile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added restriction to support http schema in non-production environments only.

documentation/CAMARA-OIDC-profile.md Outdated Show resolved Hide resolved

For OpenID requests, the scope values MUST include `openid`, to indicate that the request is an OpenID Connect request, followed by other values depending on the specific CAMARA services being requested.

A purpose must be declared within the requested scope for the following grant types:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A scope is an ascii string which is compared character by character by the AZ and other entities.
If an API working group defines their scopes then these scopes are fixed for all parties involved.
There could be a convention how scopes are build.
If Camara requires that an AZ adds semantic to the scope which requires parsing the scope, then it gets harder to exchange one AZ vendor by another. Do we want this vendor lock-in?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, but a purpose is not the same as a scope. The purpose is the reason why you are accessing the data (e.g. "I want a token to access the 'sim-swap' scope for the purpose 'dpv:FraudManagement'").

As I say in https://github.com/camaraproject/IdentityAndConsentManagement/pull/113/files#r1470035920 the current approach to convey the purpose and its format was a short-term agreement. There were other options on the table (e.g. sending the purpose in a separate "purpose" parameter). We need to work on a better solution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fully agree, the purpose is not the same as a scope. It is the reason for calling an API.
I concur palmerabollo's analysis: let's go for a separate purpose parameter. If then an authorization server does not understand it, it can ignore it.

## Authorization using the Authorization Code Flow

The [OAuth 2.0 Authorization Code](https://datatracker.ietf.org/doc/html/rfc6749#section-4.1) flow involves exchanging an authorization code for a token. Instead of requesting authorization directly from the resource owner, the client directs the resource owner to an authorization server (via its user-agent), which in turn directs the resource owner back to the client with the authorization code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-Site Request Forgery (CSRF)

Camara choses PKCE for CSRF protection.

Authorization Servers shall require PKCE [RFC7636] with S256 as the code challenge method.

For the Authorization Code flow, Clients shall use PKCE [RFC7636] with S256 as the code challenge method.

Protection provided by state against CSRF is provided by PKCE.
Protection provided by nonce against CSRF is provided by PKCE.


The keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119.


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Camara security and interoperability best practises

Network Layer Protections

Regarding network layer protections Camara follows FAPI 2.0 Security


Requirements for all endpoints

All TLS connections between web browsers, clients, authorization
servers, and resource servers shall be protected against network
attackers. To this end, clients, authorization servers, and resource
servers

  1. shall only offer TLS protected endpoints and shall establish connections
    to other servers using TLS;
  2. shall set up TLS connections using TLS version 1.2 or later;
  3. when using TLS 1.2, shall follow the recommendations for Secure Use of Transport Layer Security in [@!BCP195];
  4. should use DNSSEC to protect against DNS spoofing attacks that can lead to
    the issuance of rogue domain-validated TLS certificates; and
  5. shall perform a TLS server certificate check, as per [@!RFC6125].

NOTE: Even if an endpoint uses only organization validated (OV) or extended
validation (EV) TLS certificates, rogue domain-validated certificates can be used
to impersonate the endpoint and conduct man-in-the-middle attacks. CAA records
[@!RFC8659] can help to mitigate this risk.

Requirements for endpoints not used by web browsers

For server-to-server communication endpoints that are not used by web
browsers, the following requirements apply:

  1. When using TLS 1.2, servers shall only permit the cipher suites listed in (#tls-12-ciphers).
  2. When using TLS 1.2, clients should only permit the cipher suites listed in (#tls-12-ciphers).
  3. When using the TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 cipher suites,
    key lengths of at least 2048 bits are required.

TLS 1.2 permitted cipher suites {#tls-12-ciphers}

For TLS 1.2, only the following cipher suites shall be used:

  • TLS_DHE_RSA_WITH_AES_128_GCM_SHA256
  • TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
  • TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
  • TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384

Requirements for endpoints used by web browsers

For endpoints that are used by web browsers, the following additional
requirements apply:

  1. Servers shall use methods to ensure that connections cannot be
    downgraded using TLS Stripping attacks. A preloaded [@preload] HTTP
    Strict Transport Security policy [@!RFC6797] can be used for this
    purpose.
  2. When using TLS 1.2, servers shall only use cipher suites allowed in
    [@!BCP195].

Copy link
Collaborator

@AxelNennker AxelNennker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Camara should avoid shared secrets



The client MUST authenticate with the authorization server as described in the [Client Authentication Section](#client-authentication).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authentication between Aggregator and Operator API server MUST use private_key_jwt because private_key_jwt avoids shared secrets.
Authentication between Client and Aggregator or between Client and Operator API server SHOULD be private_key_jwt, but MAY be if the Client is not capable of using private_key_jwt.

Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @AxelNennker. If there is no one explicitly requesting for client authentication method using client_secret_basic to be part of this profile, we can get rid of the option within this profile.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with supporting private_key_jwt, and only private_key_jwt ("SHOULD ... BUT ..." leaves it open to interpretations and interoperability issues).

I think it is important to omit references to aggregators in CAMARA; this profile should be about API Clients and API Servers.

Copy link
Contributor

@sfnuser sfnuser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @garciasolero for the proposal and effort.


## Introduction

Telco network capabilities exposed through APIs provide great value to customers. By simplifying the complexity of telco networks with APIs and making the APIs available across telco networks and countries, CAMARA enables easy and seamless access. To ensure seamless interoperability and increased security, this technical document introduces the OpenID Connect (OIDC) profile, which is specifically tailored for CAMARA APIs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please clarify the scope of this document? Is it meant to be a specification or a reference document containing recommendations?
How do APIs refer to this profile in their respective documentation (i.e.) will it be a standalone version controlled document or will be addressed collectively as part of this project's release?

It would be great to include this as part of introduction.

It would also be nice to mention how future enhancements/backwards compatibility will be addressed.

"access_token": "SlAV32hkKG",
"token_type": "Bearer",
"refresh_token": "8xLOxBtZp8",
"expires_in": 3600,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to reflect the 10min token expiry recommendation in the example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 10-minute recommendation only applies to the authorization code. If you want to raise the debate of token lifetime recommendations, it may be best to open a new issue to discuss it so that it can be included in future versions of the profile.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

documentation/CAMARA-OIDC-profile.md Outdated Show resolved Hide resolved

* `<technicalParameter>` must be either:

* one technical scope to limit the request to one API endpoint (this technical scope is described in the API spec YAML file)
Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The restriction to allow just one technical scope should be removed from this profile.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was the short-term agreement reached at #32. It served to move things forward, but nobody really likes it, so we need to work in a solution for the mid-term.

In my view, this first profile should reflect the current status of the agreements, to release a 1.0 version as soon as possible and add further improvements from there, such as a better way to send the "purpose", support for PKCE (also mentioned in this issue), etc. in a 2.0 version.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @palmerabollo for your feedback. The current short term agreements are already documented in https://github.com/camaraproject/IdentityAndConsentManagement/blob/release-0.1.0/documentation/CAMARA-API-access-and-user-consent.md and are part of release 0.1 of ICM. Release 0.2 is now focussed on urgent problems/open issues that arise due to open interpretations of release 0.1. This profile can now focus on improvements. If we are able to get this review completed early with active feedbacks & participation, we could add it as a part of release 0.2. If not, we can bring it in 0.3 release.
I am not sure if we get any advantage by documenting this profile to reflect again the short term agreements. But we can also get more feedback from others on this. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shilpa-padgaonkar: fully agree, some text phrases above already allow for multiple scopes, so we must not limit here,

Copy link

@palmerabollo palmerabollo Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @shilpa-padgaonkar. If I get you right, what you suggest is:

  • Release 0.2 - Include a first version of this profile focused on urgent problems/open issues that aries due to open interpretations of release 0.1. (e.g. clearly state possible error codes, required fields that weren't clear, clarify whether private_key_jwt or client_secret_basic should be used since current docs are ambiguous, etc)
  • Release 0.3 - Include further improvements in topics not yet covered (e.g. PKCE is mentioned in some comments, adherence to FAPI 2.0, etc).

That sounds good to me. However, this particular point (purposes & user consents) is not a minor detail nor something open to interpretations. It is a significant topic that requires some thinking to devise a better solution this time —more than just a patch on top of what we have. That is why I'd like start the discussion as soon as possible but in a separate issue, with the release 0.3 in mind.

Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@palmerabollo @jpengar @diegogonmar : I went through the last TSC meeting minutes (audio recording) dated 18th Jan and @diegogonmar as a part of ICM updates has correctly mentioned that 0.2 will focus on solving open urgent issues and 0.3 will include work on profile which will include enhancements. Are we now changing this agreement by saying profile will include contents of 0.1 and possibly 0.2?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @shilpa-padgaonkar. Activity in TSC was reporting on Identity & Consent group relevant progress, not escalation to taking any decision there. Decission in Identity&Consent was to use v0.2 for clarifications and urgent issues, I think that is clear. In parallel we decided to write OIDC profile in CAMARA. Then, once first version was proposed, as part of this github thread discussion and as per yesterday call, it was proposed to use a first version of OIDC profile as an appropriate way to cover current picture and such urgent issues targeted for v0.2. This is indeed a proposal as it could be the most effective way to solve points that need to be clarified and the urgent issues, in an organized manner. We had some interesting discussions yesterday and you weren't opposing to have profile already in v0.2, not sure if you are objecting to that now, as you sent this comment after yesterday call

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diegogonmar I don't see that @shilpa-padgaonkar referred to an escalation and/or decision in TSC, just to the report you gave within the TSC on Jan 18th, that the profile is planned for 0.3.0 (I remember that as well). That WG decision is - according to your statement above - now revised. We are questioning that such a new decision has happened, at least we would have objected it, if that means that the short-comings and deviations from OIDC standard within the short-term solution (e.g. single scope only) will get codified into the profile.

The profile is not needed to clarify the urgent topics left open in v0.1. See as an indication that none of the issues labeled as v0.2 are referred in the PR description. It might also that the agreement on the profile will block solving the urgent issues.

As a comprise I would propose that the profile can be already part of v0.2, if in time agreed, and if it does not codify the short-comings of the short-term solution. Which includes that it also not refer to (https://github.com/camaraproject/IdentityAndConsentManagement/blob/release-0.2.0/documentation/CAMARA-API-access-and-user-consent.md#applying-purpose-concept-in-the-authorization-request) but only the other way round (the API-access-and-user-consent.md refers for additional clarifications to the profile).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @hdamker , thanks for your view. I didn't say anything about Shilpa referring to an escalation, I just added that as an information.

I think you say something correct: the profile was planned for 0.3.0. Then please read again my comment above: as part of this github thread discussion and as per yesterday call, it was proposed to use a first version of OIDC profile as an appropriate way to cover current picture and such urgent issues targeted for v0.2. Thus, I think its clear and I hope its fine, that this group opened a discussion on whether to include a first version of the profile for v0.2 was the best way to reflect urgent issues.
It's not in conflict with existing agreement of having an OIDC profile for v0.3, which of course as agreed will include enhancements. So after last wednesday call you challenge if having a profile already in v0.2 it's appropriate, or at least you consider it adds no benefit.

I'd close this part of the discussion and focus on way forward. Regarding your last paragraph, it's a valid suggestion (not saying I agree with it), can you put it in separate thread comment so we collect group opinion on that suggestion?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diegogonmar I agree to close this discussion and look forward. Have followed your proposal and created #113 (comment).


An authentication request is made using the HTTP POST method with the aforementioned parameters in the `application/x-www-form-urlencoded` format and a character encoding of UTF-8 in the HTTP request entity-body. When applicable, additional parameters required by the given client authentication method are also included (e.g. JWT assertion-based client authentication uses `client_assertion` and `client_assertion_type`).

The client MUST authenticate with the authorization server as described in [Client Authentication Section](#client-authentication).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the client authentication method is private_key_jwt, then it should be mandated that the authorisation request be signed (as defined here). An example of a signed request should be provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, the use of the request parameter is not approved by all of us. This profile only includes what is already agreed, but we could add it in the future if there is consensus. Anyway, the standard does not say that it is mandatory to pass the request parameter if the authentication method is private_key_jwt. In fact a few lines above the link you sent there is an example of private_key_jwt authentication without request object parameter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but if we were only following what the standard says, then we would not need this profile.

The purpose of the profile is to reduce the number of options available. But here we still have two - signed or unsigned requests, applicable to all allowed client authorisation schemes. But if the API provider already knows the client public key for private_key_jwt, then verifying a signed request is trivial.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. But the goal is to establish a set of minimum requirements that must be met. From our point of view, the only authentication mechanism should be private_key_jwt. We added client_secret_basic because some operators did not support it yet. If we remove this authentication mechanism from the profile and use only private_key_jwt, the request object parameter will be redundant for security purposes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the request object parameter will be redundant for security purposes

No, not redundant. Signed authorisation requests ensure the integrity of the request, and prevent it being modified by a MITM (or, more accurately, allow such modifications to be detected).

Now it could be argued that TLS will prevent such attacks and thus render them purely theoretical. But nonetheless signed requests are defined as an option by CIBA and, given that there is no good reason not to support signed requests if the client public key is anyway known, will likely be mandated by at least some API providers.

So I think signed authorisation requests should be mandatory (given that client_secret_basic is no longer being proposed as a valid client authentication scheme). That would remove another CIBA option, thus increasing the likelihood of the API provider and consumer supporting a compatible authorisation flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to client_secret_basic as a supported authentication method is removed, as agreed at the last meeting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but signed or unsigned authorisation requests both remain as options. One of those options should be removed. I propose to mandate signed requests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eric-murray , you are using the MITM argument to refute my claim that it could be used in the same way to not trust the public keys exposed in the jwks_uri.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if a MITM is intercepting all traffic between the API consumer and provider at HTTP level, then indeed that client is hopelessly compromised. But that is an even more theoretical attack. I think we have to make the assumption that the API provider knows the correct client public key.

But the key point remains that this profile allows both signed and unsigned authorisation requests. Only one of these options should be allowed, and my preference is signed authorisation requests. As long as signed authorisation requests remain an option, some API providers will mandate it because their own internal security rules will not allow accepting unsigned requests when they could be signed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not particularly keen on rejecting the inclusion of the request parameter in the CIBA flow. We can include this requirement in future versions of the profile if there is consensus.

Nice theoretical discussion. 😃


* **sub**

REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client. It MUST NOT exceed 255 ASCII characters in length. The sub value is a case-sensitive string.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Globally unique" end user identifiers, such as MSISDN, IMSI or public IP:port, should never be allowed. It should not be possible to identify the end user from the "sub" field alone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is why it is explained that the sub would be a local unique identifier for authorization server and not reassignable, and the MSISDN or IP could be reassigned.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That explanation is not nearly sufficient. A "globally unique" identifier is also "locally unique", and many MSISDNs have never been re-assigned. What must be avoided is that the API provider just echoes the login_hint in the sub field. The identifier must be pseudonymous, and this needs to be clearly explained.

But your comment raises another issue. If the identifier identifies an end user (i.e. a person) and always identifies that same user, then the API consumer may learn that a given end user is using a new MSISDN if the sub field matches a previous sub field obtained for a different MSISDN. This is information that they will not necessarily already know, and would count as a privacy breach. So the identifier should not be constant for a given end user for this reason. An end user who changes their MSISDN should get a new sub.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have modified the text to indicate that the sub claim should be a pseudonymous customer reference.

The value of the [OAuth.Assertions](https://www.rfc-editor.org/info/rfc7521) `client_assertion_type` parameter MUST be `urn:ietf:params:oauth:client-assertion-type:jwt-bearer`, per [OAuth.JWT](https://www.rfc-editor.org/info/rfc7523).


* **_client_secret_basic_**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client_secret_basic should be removed as an option. private_key_jwt should be mandatory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to client_secret_basic as a supported authentication method is removed, as agreed at the last meeting.


* **_private_key_jwt_**

RECOMMENDED. Clients that have registered a public key sign a JWT using that key. The Client authenticates in accordance with JSON Web Token (JWT) Profile for OAuth 2.0 Client Authentication and Authorization Grants [OAuth.JWT] and Assertion Framework for OAuth 2.0 Client Authentication and Authorization Grants [OAuth.Assertions]. At least the Authorization Server MUST support JWS-signed Object with the RS256 algorithm. The JWT MUST contain the following REQUIRED Claim Values and MAY contain the following OPTIONAL Claim Values:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RFC 7521 should not be referenced here because the relevant sections of that document for client authentication are already referenced by RFC 7523. So RFC 7523 is the only inline reference required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The links have been changed to point to the new RFC 7523 reference.


* **scope**

REQUIRED. A space delimited and a case-sensitive list of ASCII strings for OAuth 2.0 scope values. OpenID Connect requests MUST contain the `openid` scope value. If the `openid` scope value is not present, the ID Token will not be returned. Other scope values MAY be present. Scope values used that are not understood by an implementation SHOULD be ignored. More information about scope values as described in [The Scope Parameter Section](#the-scope-parameter).
Copy link
Collaborator

@eric-murray eric-murray Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either the openid scope value is mandatory, or it is not.

If you are defining different behaviour when the openid scope value is not present, then that is an additional authorisation scheme whose baseline is the OAuth2 authorisation code flow, and not the OpenId Connect Core authorisation code flow.

So the authorisation code flow section should be separated into two parts:

  • Authorisation code flow with openid scope value, with OpenId Connect Core as the baseline
  • Authorisation code flow without openid scope value, with RFC 6749 as the baseline

@AxelNennker
Copy link
Collaborator

During the meeting there was the argument that a repeat-everything profile would be easier to read for developers.
I do not think that argument is relevant because nobody sane is going to start developing e.g. an AZ from scratch.
OIDF recommends to start from scratch but to use existing libraries.
https://openid.net/developers/certified-openid-connect-implementations/

Most of the telcos either already have an OIDC implementation and if not we pick a vendor who has some proven track record implementing OIDC and OAuth2 SW.

So, I think that this policy should list security requirements that improve the security of OIDC.
And this policy should reduce the number of features an implementer has to implement fostering interoperability.
Example: if we require PKCE to be used in authorization code flow as an Anti-CSRF then developers know that other measures to counter CSRF are not needed.
Example: if we require sender-constraint access tokens with DPoP then developers know what to do.

If this profiles repeats all the standards it profiles than this document gets quite long, which is then unreadable for developers.
So, I think that "readability" is not an argument that holds water.

@Elisabeth-Ericsson
Copy link
Contributor

@ALL: First of all I want to give a big appreciation to the colleagues from Telefonica for putting this great document together. In my view we should find a way to properly review it and get it into the best possible form. This will be the central document used by all developers and thus requires detailed review from all participants. I think that we need time for that.

Given all the discussions in the team, I read the agreement documented in the technical steering group about the release content. (2024-01-18 TSC Minutes - CAMARA Project - Confluence)
extracted from the meeting minutes is the following:
1. V0.1 to be generated. All issues marked as required for the release are solved (Intended date for release will be next Jan 22 - Jan 26.)
2. V0.2 to be progressed as fast as possible. Scope: clarifications on existing scope requested by existing issues
3. V0.3 will include enhancements

OIDC profile:
To be created in CAMARA. Not to rely on starting it in OIDF working group to avoid risk of delay.

The agreement does not state the release, the OIDC profile document should be put in.
In order to buy us some discussion time, would it be possible to include the most needed clarifications on the short term agreement in the already existing document (https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation/CAMARA-API-access-and-user-consent.md) as 0,2 - and only concentrate on the most urgent problems only ? In parallel we would safe time and get some room for discussions on the new document for the OIDC profile?
Then we would use the new document on OIDC profile (content of this pull request) to be the embryo of 0,3. I acknowledge that the ODIC profile document is very much needed. But is it reasonable to spend so much effort in this (very good) document and have it focused on the short term agreement (which is not really future proof) ?

I'm a bit afraid that this new document will become the bible. As soon as the implementations will take off now, gain speed and then - with this short term agreement - we cement it and build a burden into our solution we cannot get rid of any more.

@jpengar
Copy link
Collaborator

jpengar commented Feb 1, 2024

During the meeting there was the argument that a repeat-everything profile would be easier to read for developers. I do not think that argument is relevant because nobody sane is going to start developing e.g. an AZ from scratch. OIDF recommends to start from scratch but to use existing libraries. https://openid.net/developers/certified-openid-connect-implementations/

Most of the telcos either already have an OIDC implementation and if not we pick a vendor who has some proven track record implementing OIDC and OAuth2 SW.

So, I think that this policy should list security requirements that improve the security of OIDC. And this policy should reduce the number of features an implementer has to implement fostering interoperability. Example: if we require PKCE to be used in authorization code flow as an Anti-CSRF then developers know that other measures to counter CSRF are not needed. Example: if we require sender-constraint access tokens with DPoP then developers know what to do.

If this profiles repeats all the standards it profiles than this document gets quite long, which is then unreadable for developers. So, I think that "readability" is not an argument that holds water.

I understand your point, and you presented it correctly in yesterday's call, but after asking the WG participants and considering the different opinions presented during the meeting, the way forward was agreed to go with the self-contained format + compromise to properly note any changes related to the reused standard.

@@ -100,7 +100,7 @@ The CAMARA profile uses the following OpenID Connect request parameters with the

* **redirect_uri**

REQUIRED. Redirection URI to which the response will be sent. This URI MUST exactly match one of the Redirection URI values for the Client pre-registered at the OpenID Provider, with the matching performed as described in [RFC 3986](https://www.rfc-editor.org/rfc/rfc3986.html#section-6.2.1). When using this flow, the Redirection URI SHOULD use the https scheme; however, it MAY use the http scheme. The Redirection URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application.
REQUIRED. Redirection URI to which the response will be sent. This URI MUST exactly match one of the Redirection URI values for the Client pre-registered at the OpenID Provider, with the matching performed as described in [RFC 3986](https://www.rfc-editor.org/rfc/rfc3986.html#section-6.2.1). When using this flow, the Redirection URI MUST use the https scheme; however, it MAY use the http scheme only for non-production environments. The Redirection URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the text of the original standard, or delete this definition because it has not changed.
I think, this profile should not care what developers do in non-production. Also there are ways to have certificates on internal servers and developer machines e.g. self-signed certs and /etc/host entries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are precisely saying that it should be referenced because there are no changes when in fact there are, or we want there to be. In another thread in this PR you said that the use of the https schema should be mandatory.

This is exactly what the OIDC standard says:

redirect_uri
REQUIRED. Redirection URI to which the response will be sent. This URI MUST exactly match one of the Redirection URI values for the Client pre-registered at the OpenID Provider, with the matching performed as described in Section 6.2.1 of [RFC3986] (Simple String Comparison). When using this flow, the Redirection URI SHOULD use the https scheme; however, it MAY use the http scheme, provided that the Client Type is confidential, as defined in Section 2.1 of OAuth 2.0, and provided the OP allows the use of http Redirection URIs in this case. Also, if the Client is a native application, it MAY use the http scheme with localhost or the IP loopback literals 127.0.0.1 or [::1] as the hostname. The Redirection URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application.

And what exactly the OAuth2.0 standard says:

This specification does not mandate the use of TLS because at the time of this writing, requiring clients to deploy TLS is a significant hurdle for many client developers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant with "don't change the standard" is that the text of a parameter definition should be verbatim the same as in the standard.
We agreed in Camara that we use OIDC authorization code flow, so the OIDC standard is the basis of this PR.

Regarding "at this time":

This specification does not mandate the use of TLS because at the time of this writing, requiring clients to deploy TLS is a significant hurdle for many client developers.

OpenId Connect is now ten years old and OAuth2 even older. A lot has changed and improved.

The document is intended to be a profile of OIDC not a redefinition of OIDC based on OAuth2.
Please keep the OIDC definitions of parameters, if possible. If you change a definition, then please clearly mark the change.

I still think that just stating the differences to the profiled standards leads to a document that is easier to read and better for implementors. Please see https://github.com/AxelNennker/IdentityAndConsentManagement/blob/camara_oidc_profile/documentation/CAMARA-Security-Interoperability.md

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AxelNennker brings up a lot of great points. I see a major issue with this and the demand for Data Minimization. @AxelNennker did you come across issues with the OIDC Profile and Data Minimization Regulation in your research?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garciasolero the definition of redirect_uri is different to the standard e.g. the part about confidential clients is not here instead there is the mention of non-production environments.

I read this definition as being more restrictive than the OIDC standard and I support that https is used for all client types and url-schemes for native apps. But maybe this is not the only interpretation.
I think, changes to the standard should be clearly marked.

@jpengar
Copy link
Collaborator

jpengar commented Feb 1, 2024

@Elisabeth-Ericsson Regarding your comment #113 (comment), please see Diego's comment here -> #113 (comment)

And regarding your concerns, the existing purpose agreement is what's going to be implemented in the short term anyway. It is not more or less valid/official because we documented it in CAMARA-API-access-and-user-consent.md and/or the OIDC profile. And as I said yesterday, the OIDC profile is not only about the purpose solution, but also about including other implementation details that have already been requested as needed by other participants in their current implementations (client authentication, CIBA login_hint, error scenarios, etc.). That's why the profile is proposed for the v0.2.0 release. And in fact, in the latest updates of the profile in this PR, the purpose solution is just a reference to API-access-and-user-consent.md.

@AxelNennker
Copy link
Collaborator

AxelNennker commented Feb 1, 2024

During the meeting there was the argument that a repeat-everything profile would be easier to read for developers. I do not think that argument is relevant because nobody sane is going to start developing e.g. an AZ from scratch. OIDF recommends NOT (corrected) to start from scratch but to use existing libraries. https://openid.net/developers/certified-openid-connect-implementations/
Most of the telcos either already have an OIDC implementation and if not we pick a vendor who has some proven track record implementing OIDC and OAuth2 SW.
So, I think that this policy should list security requirements that improve the security of OIDC. And this policy should reduce the number of features an implementer has to implement fostering interoperability. Example: if we require PKCE to be used in authorization code flow as an Anti-CSRF then developers know that other measures to counter CSRF are not needed. Example: if we require sender-constraint access tokens with DPoP then developers know what to do.
If this profiles repeats all the standards it profiles than this document gets quite long, which is then unreadable for developers. So, I think that "readability" is not an argument that holds water.

I understand your point, and you presented it correctly in yesterday's call, but after asking the WG participants and considering the different opinions presented during the meeting, the way forward was agreed to go with the self-contained format + compromise to properly note any changes related to the reused standard.

Yesterday I presented a different point. This point is about the "readablity"-argument, which yesterday I had no counter argument to, but today I have the above and I think that the readability argument is wrong.

I think the changes-only policy document is much easier to read.

Usually, in this working group we discuss arguments for a few days and give participants time to review and comment.

See the redirect_uri example: Today a change was merged, that I think is wrong and it also does not mark the definition as changed from the standard.

@jpengar
Copy link
Collaborator

jpengar commented Feb 1, 2024

Yesterday I presented a different point. This point is about the "readablity"-argument, which yesterday I had no counter argument to, but today I have the above and I think that the readability argument is wrong.

I think the changes-only policy document is much easier to read.

Usually, in this working group we discuss arguments for a few days and give participants time to review and comment.

See the redirect_uri example: Today a change was merged, that I think is wrong and it also does not mark the definition as changed from the standard.

There have been days of feedback on the profile format since the PR was created, there have been different opinions, and yesterday's meeting was precisely to agree on a way forward and unblock this hot topic.

If you've thought about it a second time and now you've come up with some more analysis, that's fine. But yesterday we did agree on a way forward. So I understand that you are now proposing to reopen the discussion??

If the rest of the participants feel it's necessary, I would disagree, but it would be the WG decision. And in that case we can give some time for feedback as we always do. But this has a big impact on the PR, and we were already working and updating the PR according to yesterday's agreement. It is an inconvenience.

@AxelNennker
Copy link
Collaborator

I created https://github.com/AxelNennker/IdentityAndConsentManagement/blob/camara_oidc_profile/documentation/CAMARA-Security-Interoperability.md

Much shorter much easier to read, I think.
Hopefully I captured everything that CAMARA-OIDC-Profile wants to say about usage of OIDC, CIBA, etc

@hdamker
Copy link
Collaborator

hdamker commented Feb 5, 2024

@garciasolero @jpengar @Elisabeth-Ericsson @shilpa-padgaonkar @eric-murray

First of all thanks to @garciasolero for the initial draft of this document. I understand that it is written with two different audiences in mind

The target audience for this document is the service/technical departments of operators exposing network functions via standard CAMARA APIs. And applications or client systems that consume CAMARA standard APIs to make use of the operator's network capabilities.

But despite the great effort I think that it is unfortunately missing both audiences with the attempt to be "self contained".

Let me start with "service/technical departments of operators exposing network functions via standard CAMARA APIs". They would rely on the referred standards and need to know (only) the points which CAMARA is defining in addition to ensure interoperability and security. For them the document has too much duplicated information which is already defined with in the referenced standards. They would need exactly the information which is provided within the short profile document proposed by @AxelNennker. For the exact definition of the expected behaviour the document in this PR has a high risk to get an "alternative standards" to OIDC, but without being ever complete (e.g. the complete definition of terminology is missing).

On the other side are the "applications or client systems that consume CAMARA standard APIs". Actually the audience are here the developers of the these applications or client systems. For them indeed a self-contained document which explains what they need to initiate the flows, authenticate, what the expect in the tokens, explanations of potential errors, etc would be helpful. We should not require them to read multiple documents (also not within CAMARA) and to dig into standards which are written for implementators. But that should be a documentation from a developer perspective, not mixed up with design decision how to implement the OIDC standards. Reading over the current document I don't have the impression that the current PR would fulfil the need of this audience.

Said this my recommendation would be to reduce the profile document in this PR to just the differences to the referred standards and consider to write an additional document which describes with examples, flow diagrams etc the use of CAMARA APIs from developers perspective, referring to the standards for further details, including mentioning the differences / limitations imposed by the CAMARA profile to ensure interoperability. This approach would ensure that we can fix in short time the necessary agreements for implementations and then in parallel to implementations work on proper documentation for developers (the main target audience of CAMARA APIs).

@hdamker
Copy link
Collaborator

hdamker commented Feb 13, 2024

@diegogonmar asked that I put my following suggestion from a now closed discussion into a separate thread comment so we collect group opinion on it (origin: #113 (comment)):

As a comprise I would propose that the profile can be already part of v0.2, if in time agreed, and if it does not codify the short-comings of the short-term solution. Which includes that it also not refer to (https://github.com/camaraproject/IdentityAndConsentManagement/blob/release-0.2.0/documentation/CAMARA-API-access-and-user-consent.md#applying-purpose-concept-in-the-authorization-request) but only the other way round (the API-access-and-user-consent.md refers for additional clarifications to the profile).

I would like to see it together with my proposal within the previous comment, to focus first on a short profile document to get the necessary clarifications agreed, and doing a comprehensive developer documentation afterwards based on the profile.

@davidgtonge
Copy link

Just adding to the discussion, there is quite a lot of repetition here from existing specs. Is it worth considering referencing https://openid.bitbucket.io/fapi/fapi-2_0-security-profile.html

This is a secure profile of OAuth2 that has the following advantages:

  1. extensive conformance tests
  2. extensive vendor support
  3. formal security analysis

Axel's PR looks like it could build on this existing work.

@AxelNennker
Copy link
Collaborator

Regarding offline_access, please mark this as a change to the OIDC standard on offline_access.
The standard defines that the client gets an access token for the user-info endpoint.
While this profile defines that the clients gets a refresh token and and access token for a RS endpoint

Copy link
Collaborator

@AxelNennker AxelNennker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mark changes to the standards

@@ -100,7 +100,7 @@ The CAMARA profile uses the following OpenID Connect request parameters with the

* **redirect_uri**

REQUIRED. Redirection URI to which the response will be sent. This URI MUST exactly match one of the Redirection URI values for the Client pre-registered at the OpenID Provider, with the matching performed as described in [RFC 3986](https://www.rfc-editor.org/rfc/rfc3986.html#section-6.2.1). When using this flow, the Redirection URI SHOULD use the https scheme; however, it MAY use the http scheme. The Redirection URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application.
REQUIRED. Redirection URI to which the response will be sent. This URI MUST exactly match one of the Redirection URI values for the Client pre-registered at the OpenID Provider, with the matching performed as described in [RFC 3986](https://www.rfc-editor.org/rfc/rfc3986.html#section-6.2.1). When using this flow, the Redirection URI MUST use the https scheme; however, it MAY use the http scheme only for non-production environments. The Redirection URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garciasolero the definition of redirect_uri is different to the standard e.g. the part about confidential clients is not here instead there is the mention of non-production environments.

I read this definition as being more restrictive than the OIDC standard and I support that https is used for all client types and url-schemes for native apps. But maybe this is not the only interpretation.
I think, changes to the standard should be clearly marked.

@garciasolero
Copy link
Contributor Author

This self-contained profile pull request is being closed because it has been decided to use another profile format that has been merged into main branch in pull request #121.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet