-
Notifications
You must be signed in to change notification settings - Fork 26
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
Security Considerations #304
Conversation
✔️ Deploy Preview for gnap-core-protocol-editors-draft ready! 🔨 Explore the source changes: 3eba247 🔍 Inspect the deploy log: https://app.netlify.com/sites/gnap-core-protocol-editors-draft/deploys/6140ea49254cf000082134e4 😎 Browse the preview: https://deploy-preview-304--gnap-core-protocol-editors-draft.netlify.app/draft-ietf-gnap-core-protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the suggested changes, as discussed during the editor's meeting. Maybe we'll find additional topics to cover later on, but that's a very good starting point.
Also includes answers to many open issues, which we'll be able to close
About Section 12. Security Considerations (page 109) Section 12.1 states:
This has nothing to do with the title of this section which is "TLS Protection in Transit". Section 12.1 also states:
This has nothing to do with the title of this section which is "TLS Protection in Transit". The next sentence states:
It should be observed that those keys and signatures associated to a bound access token do not prevent an attacker from using a stolen token, if a legitimate client collaborates with another client. Section 12.2 (Protection of Client Instance Key Material) states:
The rational for authentication client instances using a unique key is not explained. The access tokens contain the rights or privileges of a human user and not those for a client instance. The relationship between client instances and end-users is not explained. Section 12.3 states:
While the text explicitly states that the AS authenticates the client software, the text is mute about the authentication of the end-user by the AS. Section 12.6 is about Bearer Access Tokens states:
In the context of GNAP, bearer tokens are not defined. However, there exists a definition of the bearer flag which is the following:
However, it is possible to obtain a protection against the use of an access token by another client by using bearers tokens, if they include a specific field that does not exist in draft -07. Such a protection is explained in details in draft-pinkas-gnap-core-protocol-00. See the text on page 9 about the "buid" field which is a "binding user identifier" that allows a RS to verify that the access token is associated with the right (short-term or long-term) user account. This field allows to detect the inappropriate use of an access token by a malicious user. Section 12.6 states:
and also:
Both sentences are incorrect, as soon as a "binding user identifier" field is used in the access token. Section 12.8 (Exposure of End-user Credentials to Client Instance) states:
Such an incorrect conclusion is the consequence of a missing trust relationship. A key point is whether the end-user is trusting or not his client instance for not stealing his credentials and for displaying the correct information. The end-user authentication towards the AS is even considered as "dangerous" ! Furthermore, "end-user authentication" is implicitly interpreted as being the use of a password. GNAP should be targeted to be world-wide usable, including within Europe and the EU. As of December 31 st, 2020, the European Union (EU) is requiring that consumer electronic payments over € 50 ($ 60) require MFA (Multi-factor authentication).
In 2016, for example, the Dutch Data Protection Authority (Dutch DPA) specifically indicated that hospitals need to use 2FA in patient portals. It can be concluded that the Dutch DPA believes that medical data, which is sensitive personal data according to the GDPR, needs to be secured with 2FA (Two factors authentication). It is thus fundamental for GNAP to be able to support both 2FA (Two factors authentication) draft-pinkas-gnap-core-protocol-00 supports the end-user authentication as a mandatory feature of the core protocol. Section 12.9 (Mixing Up Authorization Servers) states:
A client instance needs to always be aware of which AS it is talking to throughout a grant process, and ensure that any callback for one AS does not get conflated with the callback to different AS. This threat does not exist as soon as TLS is supported and the end-user verifies that it is connected to the appropriate AS. Section 12.10 (Processing of Client-Presented User Information) states:
This highlights the fact that the information presented by the client about the end-user is only identifying the claimed end-user and by no way authenticating it. This is insecure. Later on the text continues with:
Such a condition would not comply with the EU Payments Services Directive (PSD2) that took effect in January 2018. Section 12.11 (Client Instance Pre-registration) states:
Such a statement would change the model, since access tokens only contain information about end-users and not about client instances. Section 12.12 (Client Instance Impersonation) states:
Since the end-user needs to trust his client, such a threat does not exist. In summary, this Security Considerations section exhibits major security weaknesses present in draft -07. A redesign of the protocol would be necessary, but such a redesign will only be possible once the trust relationships will be reconsidered and rewritten. |
@Denisthemalice since you relate the core of your criticisms to the trust relationships #306, I guess we should resolve this first. But I suspect we just have different world views and objectives, which makes you present GNAP as "insecure" or "not private by design", while those choices are very conscious design decisions (just not yours). And quite importantly, we demonstrated over and over again why your criticisms don't hold true. |
Adds an initial set of security considerations to the GNAP core draft. Additional considerations to be added in future edits as discussion evolves.