Amplify-cli: I am subscribing to events and receiving data I am not authorized to see

[rawadrifai]

* Which Category is your question related to? *
Amplify subscriptions

* What AWS Services are you utilizing? *
Amplify, AppSync, DynamoDB

* Provide additional details e.g. code snippets *
I am wondering what's the expected behavior of a subscription generated with codegen.
If I am subscribed to onCreateSomething, and then someone else adds a record that I am not authorized to read (based on owner field). Am I supposed to still receive the event data? I could be doing something wrong, but right now all subscribed clients receive a copy of every event authorized or not. I hope this is not the expected behavior.

[licalac]

Hi there,

Just experiencing this security breach myself with a very simple app.

This security breach is for me as critical as surprising in AWS cloud, because I remember the AWS's CTO was saying that every AWS employee is at first a security engineer ... I thought it was true :(
So disappointing and looking forward for a fast and serious response.

Best regards
Lionel

[kaustavghosh06]

@rawadrifai @licalac What does your annotated schema look like?

[licalac]

Hi,

Many thanks for your reply.

Mine is as follow:

type Deck @model @auth(rules: [{ allow: owner }]) {
id: ID!
name: String!
cards: [Card]! @connection(name: "DeckCards")
}

type Card @model @searchable @auth(rules: [{ allow: owner }]) {
id: ID!
question: String!
answer: String
deck: Deck! @connection(name: "DeckCards")
}

When I make a subscription OnCreateCard and then receive all new card created by whatever user.

Lionel

[mikeparisstuff]

This behavior has been outlined in the #1043. You have the ability to protect subscriptions yourself but it requires custom logic. You have the option to a) turn off subscriptions via the @model(subscriptions: null) argument or b) add custom authorization logic yourself using custom resources. To do so you add a resolver to the subscription field that implements your authorization check. The resolver will be run at connect time (i.e. when you first issue the subscription query) and allows you to implement fine-grained authorization.

[rawadrifai]

@kaustavghosh06 the schema can be anything, I don't think it's specific to me.
Take this for instance:

type Something @model @auth(rules: [
  { allow: owner }
]) {
  id: ID!
  owner: String
}

if I have 2 users subscribed to onCreateSomething. One of them inserts an object that the other is not allowed to see according to the auth rules. The subscribed user will get notified regardless. The event data contain the full record. I think that's a serious security concern if one wants to rely on Amplify and Transform only to do the job.

@mikeparisstuff Can you please point me to an example for locking down the subscriptions I receive to only the ones that fit the auth rules? So for example, for the example above, no one should receive a subscription event except the user that created the record.

[rawadrifai]

Think if you're building a simple messaging app, DM or channels. You want to certain folks to receive a notification through AppSync when a new message is created.. not any new message, only the ones they're authorized to see. I am certain it can be done with AWS, just not certain Amplify provides that yet using Transform alone.

[ajhool]

Can we please add a pre-alpha tag to the Amplify-CLI documentation so that people know that it is not production ready because an attacker can circumvent the @auth directive by simply subscribing to the data model and they will receive all new data objects delivered to them even if they are not the owner?

Also that this is the default implementation and there is no documentation suggesting that you should add this custom logic?

[mikeparisstuff]

@rawadrifai You can read more about how to protect subscription fields here https://docs.aws.amazon.com/appsync/latest/devguide/security-authorization-use-cases.html. You may add custom subscription resolvers via the Amplify CLI using a custom stack within the API category. We will add more explicit documentation around this limitation and how to get around it in the Amplify documentation.

[rawadrifai]

@mikeparisstuff I've read this before. I have not put the effort into learning VTL well. This feels like a lot of friction and I'm not sure it addresses the security concerns behind this. One developer can write bad code and pushes an unfiltered subscription. This will likely drain the battery in a few minutes on all the devices with the app running.

Do you mind if we reach a conclusion whether codegen subscriptions are meant to respect Auth rules? I think they should. It's the right thing to do.

I would like to understand your vision and whether we should accept the conclusion that subscriptions don't respect Auth rules. That's the one variable left for us to make a final determination whether this is a framework we can seriously count on. Security is at the top of our concerns.

[mikeparisstuff]

Hi @rawadrifai we’re not implying that subscriptions should not respect auth rules. Providing simple solutions while also allowing for flexibility and customization is a goal of Amplify, and as outlined in the RFC we’re still trying to get feedback from customers on what their use cases and needs are. We also prioritize based on customer feedback and interest, which is why some of the other auth features were completed first. Your feedback on the RFC with some concrete use cases will help us complete this design and move forward. We always want to provide methods for you still to accomplish tasks even if a feature in the Transformer doesn’t exist yet, which is why custom stack support was added into the API category to allow customers to side-step any situations where the underlying services support features that the CLI/transform have yet to implement. We also provide hooks to turn subscriptions off if you do not need them or do not want to implement your own fine-grained authorization logic.

Auth support for subscriptions is different than that of queries/mutations and will require further
design for us to implement in a simple & flexible manner. It would be helpful to hear more specifics about your use cases to help guide the design of @auth support on subscriptions.

[RossWilliams]

@mikeparisstuff Would you consider having subscriptions disabled by default and having them as opt-in? The current situation defaults users into an insecure configuration. There may also be many tutorials around @auth in the wild that falsely give assurances and neglect to describe subscriptions as bypassing auth.

This would be a breaking change, but thats why we have semver.

[ajhool]

Providing simple solutions while also allowing for flexibility and customization is a goal of Amplify, and as outlined in the RFC we’re still trying to get feedback from customers on what their use cases and needs are. We also prioritize based on customer feedback and interest, which is why some of the other auth features were completed first.

This isn't the absence of a feature, this is an undocumented security vulnerability in the default configuration of the Amplify framework.

If subscriptions were disabled by default and the documentation said:

"Add an @subscriptions directive to enable subscriptions. WARNING: subscriptions do not respect auth rules, so ALL objects will be broadcast to ALL subscribers".

Then, that would be a completely understandable absence of a feature. This current behavior:

1. Developer follows docs and adds "DiaryEntry @auth owner" to protect diary entries (wow Amplify makes auth so easy and explicit!)
2. Amplify framework automatically broadcasts all DiaryEntry objects to any subscriber who knows how to copy and paste code

however, is an undocumented security vulnerability in the default configuration of the framework and the lack of recognition, acknowledgment, or urgency in fixing it is startling. There shouldn't need to be community feedback to realize that this is a terrible default configuration and it shouldn't take half a year (and counting) to fix it. If there are half a year's worth of other auth features that have higher priority than this -- maybe hire more people? I sent an email to aws security in hopes that somebody would have some sense of urgency on this.

Edit: Furthermore, the fix of: @model(subscriptions: null) is also not in documentation -- as in, there is no mention of that syntax anywhere except for in this Issue (as far as I can tell). So, disabling subscription autogeneration is also not currently a real, documented feature, meaning that anybody who uses Amplify is currently exposed to this vulnerability....

[mikeparisstuff]

Hi @ajhool

Amplify being an open source framework is driven by customer requests, and as noted we do want to add this feature but the demand wasn’t as strong as other features. It seems that you and a few others would like us to prioritize this in the coming months which we’re happy to do however getting your concrete use cases listed down would help us do the design. The default setup is fine for many customers we have worked with but we still want to provide more options here. While we do offer the capability to define custom resolvers having the option to add configuration in the @auth directive will be simpler. Could you give us:

• The Use Cases and/or scenarios you’d like to support
• How you might see yourself specifying this in a resolver
• What actions should pass/fail when sending and receiving messages

In the meantime we’ve added a note Authorizing Subscriptions that subscriptions are unprotected by default and given a couple sample templates for you to use.

[rawadrifai]

@ajhool Amplify is work in progress, and open source if we'd like to contribute.
I feel your frustration on this, let's try to give clear use cases so we get this prioritized.

@mikeparisstuff a few months .. I'll take that although I wish it could be quicker. We're planning on going live in a few months, and we're 100% aws / amplify thus far.

[rawadrifai]

@mikeparisstuff here's my feedback copied from the other thread:

@mikeparisstuff here's some feedback on our use case regarding auth and subscriptions. I hope it helps. It's a pretty straight forward scenario.

Consider a slack clone. We have a message table or type. Every message is inserted with the groups and individuals that can see it (according to the auth rules). The identity claims are custom and injected pre token generation, via Cognito triggers.

When a new message is created, we hope that all the users that are authorized to read it, will get notified through an onCreateMessage subscription. The case right now is that every client that is subscribed gets the notification. Clearly that cannot scale and compromises security.

I understand there is custom work that can be done through vtl. Our suggestion is that all the groups and individuals that are authorized to read the record, would receive an event. It is what I expected the case would be before diving in.

[ajhool]

Hi @ajhool

Amplify being an open source framework is driven by customer requests, and as noted we do want to add this feature but the demand wasn’t as strong as other features. It seems that you and a few others would like us to prioritize this in the coming months which we’re happy to do however getting your concrete use cases listed down would help us do the design. The default setup is fine for many customers we have worked with but we still want to provide more options here.

There are two separate concerns in this thread and I think they're being conflated

Concern 1: Providing fine-grained authorization capabilities for Subscription Resolvers. (This is a feature request)

I am not advocating for urgency on Concern 1. I recognize that that is a feature that many people don't need or want (although I do want it!) and that it isn't a priority for the team and I completely respect that the customer demand for that feature isn't there -- I'm not trying to cut in line to get this feature set pushed through. No worries there.

Concern 2: The massive undocumented (now slightly documented) security vulnerability in the default configuration. The vulnerability is that Amplify autogenerates Subscription resolvers by default which allow an attacker to circumvent explicit Auth rules and gain access to a stream of literally all of the data that goes through AppSync. (This is a security vulnerability)

I am only advocating for urgency (and startled by the lack thereof) on Concern 2, which is a totally independent concern from Concern 1. Currently, the default configuration is a vulnerability. Whether Amplify is an open-source framework is irrelevant; it is controlled by AWS, promoted by AWS on the AWS website, the roadmap is privately maintained by AWS, all the repo admins are AWS employees etc., so AWS should be responsible for fixing the massive security vulnerability in the default configuration -- or expect complaints about the massive security vulnerability in the default configuration. Users can't complain about something that they aren't aware of.

The obvious, quick solution would be to make Subscription resolvers not autogenerated -- make them be opt-in in the model schema (as @RossWilliams suggested). Yes, this might cause breaking changes for some people; however, the revelation about this security vulnerability is a breaking change in mine and others' code. Documentation is the bare minimum, but an opt-in approach would fix this security vulnerability for new-comers and existing users.

The default setup is fine for many customers we have worked with but we still want to provide more options here

Concern 2 was not a documented vulnerability until today and the fix was not provided until today so I don't see how they could be okay with it. I also don't see why them being okay with a terrible security vulnerability is relevant to other users' concern about the vulnerability.

Are there other undocumented security vulnerabilities in Amplify that those customers were okay with but that haven't been made generally known?

TLDR; IMO there is a difference between a feature request and a fundamental security vulnerability, and I hope that the Amplify team recognizes this and takes urgent action to notify/resolve future vulnerabilities, while also making design choices that are secure-by-default.

[robert-moore]

Just hit this myself and completely agreed with @ajhool this should be documented as a bug and a massive security vulnerability, not a feature request.

[rawadrifai]

@kaustavghosh06 good sign labeling this as a feature request.
Should we take this as an indication for something?
Sorry happens to be a sensitive time to pick the right tech to use.

[jchris]

Why have auth if by default everything is world-readable? In my world this would be worthy of a CVE.

[ajhool]

@jchris see #1043 for more discussion/vulnerabilities

[rawadrifai]

@mikeparisstuff do you have any update on this?

[RossWilliams]

This is akin to having S3 buckets open by default. If turning off subscriptions by default isn't on your roadmap, consider using a similar approach to S3: have the CLI warn users with a red security message when subscriptions are not turned off on a model and an @auth directive exists. Have a similar warning for @subscriptions with @auth.

A project re-tweeted by dabit3 this week had left subscriptions exposed. AWS owned sample github repos show insecure @searchable usage. Real users are facing security issues. They might not be shouting about it because they don't know.

[undefobj]

We're actively working on a design for this feature request based on some of the input that @rawadrifai has given on his use case. If you have further input on your use case to the design please can you comment on the RFC in https://github.com/aws-amplify/amplify-cli/issues/1043 so that we can incorporate it in the design.

[undefobj]

have the CLI warn users with a red security message when subscriptions are not turned off on a model and an @auth directive exists. Have a similar warning for @subscriptions with @auth.

This is good feedback and something that we were discussing this week, potentially also showing a matrix of permissions. Full evaluation of permissions and displaying the resultant set of policy is not a simple thing to implement, but we're looking into it.

cc @kaustavghosh06

[ajhool]

Just make everything explicit so you don't need to rely on warnings or documentations.

Developers should be forced to set auth to be PUBLIC or AUTHENTICATED or OWNER, etc. There shouldn't be default permissions and there certainly shouldn't be default PUBLIC permissions. If the developer doesn't specify an auth level, they shouldn't be able to push the schema and it should show an error that auth isn't specified.

This should be released in a cardinal semver version (eg. 3.x.x) to specify a breaking change and anybody who is relying on the current public default should need to upgrade their schema. It might seem like a breaking change for them, but IMO this was already a broken system for everybody who thought they were being protected by @auth when they weren't.

Implicit defaults for auth levels provide the illusion of convenience; they save about 2 seconds of time upfront but can lead to massive headaches when data leaks in production

[undefobj]

Hi @ajhool we are actively working on a design to address this request to protect subscriptions. As noted in the RFC, we have been listening to the feedback on the use cases. We do need to provide more explicit authorization controls while being mindful of usability. So, suggestions such as @RossWilliams have made are valuable, and we don’t want to dismiss them. There isn’t a single correct answer, especially when trying to provide authorization controls on request/response patterns (queries/mutations) vs. push (subscriptions). If everything is difficult to use, we have seen customers turn everything off; this is the wrong behavior, and we don’t want to encourage that either. We want to instead setup customers for success. For example, for more explicit settings such as denoting things Public and Private, we’ve been actively working on that and will be exposing this soon in the following PR: https://github.com/aws-amplify/amplify-cli/pull/1916.

These are not easy problems and we are actively spending a lot of time researching, designing, and implementing them. We are also looking at doing a version bump as you suggested.

[ajhool]

I was also making a design suggestion... and I've cited RossWilliams' approach as being a good idea, not dismissed it...

@undefobj or @mikeparisstuff would you please consider responding directly to my post, above, that distinguishes as clearly as possibly the difference between Concern 2, which is the existing terrible security vulnerability that should be fixed with urgency and Concern 1, which is the desire to add features and improve the framework.

IMO there might not be single correct answers for Concern 2, but the current design is clearly the incorrect answer and should be fixed with urgency rather than careful, let's-take-another-year-to-think-it-over debate

[undefobj]

Hello everyone - We have a version of this functionality which has been published to the following tag:

npm install -g @aws-amplify/cli@authSubRelease

In this implementation, when a model is protected using the owner auth strategy, each subscription request will require that the user is passed as an argument to the subscription request. If the user field is not passed, the subscription connection will fail. In the case where it is passed, the user will only get notified of updates to records for which they are the owner.

Alternatively, when the model is protected using the static group auth strategy, the subscription request will only succeed if the user is in an allowed group. Further, the user will only get notifications of updates to records if they are in an allowed group. Note: You don't need to pass the user as an argument in the subscription request, since the resolver will instead check the contents of your JWT token.

For example suppose you have the following schema:

type Post @model 
@auth(rules: [{allow: owner}])
{
  id: ID!
  username: String
  postname: String
  content: String
}

This means that the subscription must look like the following or it will fail:

subscription onCreatePost(username: “Bob”){
  postname
  content
}

In the case of groups if you define the following:

type Post @model 
@model @auth(rules: [{allow: groups, groups: ["Admin"]}]) {
{
  id: ID!
  username: String
  postname: String
  content: String
}

Then you don’t need to pass an argument, as the resolver will check the contents of your JWT token at subscription time and ensure you are in the “Admin” group.

Finally, if you use both owner and group authorization then the username argument becomes optional. This means the following:

• If you don’t pass the user in, but are a member of an allowed group, the subscription will notify you of records added.
• If you don’t pass the user in, but are NOT a member of an allowed group, the subscription will fail to connect.
• If you pass the user in who IS the owner but is NOT a member of a group, the subscription will will notify you of records added of which you are the owner.
• If you pass the user in who is NOT the owner and is NOT a member of a group, the subscription will not notify you of anything as there are no records for which you own

Dynamic groups have no impact to subscriptions, so you will not get notified of any updates to them.

Next week we will add capability that if your type doesn’t already have username the Transformer will automatically add this for you. There are also still a few edge cases with field level auth we need to validate for subscriptions, however the behavior will be that the field will return null if the field auth is not a subset of the object type auth. In the meantime please have a look at the early version published to the tag above. Our aim is to do final validations over the next few days and publish to the latest build next week.

[rawadrifai]

@undefobj @mikeparisstuff This is a great start.
Please keep in mind the use case where Cognito groups do not suffice.

For example, I interject a login request to append the groups a user belongs to. I get those from Dynamo. So to check whether a user belongs in a group or not, please do so with the token not a call to Cognito. I assume this is the case with this release.

[undefobj]

@undefobj @mikeparisstuff This is a great start.
Please keep in mind the use case where Cognito groups do not suffice.

For example, I interject a login request to append the groups a user belongs to. I get those from Dynamo. So to check whether a user belongs in a group or not, please do so with the token not a call to Cognito. I assume this is the case with this release.

Answered in https://github.com/aws-amplify/amplify-cli/issues/1043#issuecomment-522847175

[undefobj]

@rawadrifai cross posting from the other issue:

we have published to the tag again:

sudo npm install -g @aws-amplify/cli@authSubRelease --unsafe-perm=true

I've also just submitted a PR for the new functionality: https://github.com/aws-amplify/docs/pull/868

You should be able to use this for documentation before we merge this to latest build. We're still running final stability tests but are trying to get this out ASAP. Your testing would assist us in expediting this. Note that the custom claims for either the user identity (identityClaim) or the groups (groupClaim) are also part of this PR. While the default behavior is to protect subscriptions, customers have an explicit control now to make them public if they wish to keep previous behavior. Also we've added some automatic inspection (per @RossWilliams suggestion) to flag a warning if you're not using @auth on any @model, listing the ones that are unprotected, with information on how to learn how to add more controls. This happens before an $amplify push sends updates to the cloud allowing you to abort if you made a mistake. Finally there are protections in place if you're using per-field level @auth to protect sensitive data inside a model from being sent over subscriptions.

[undefobj]

@rawadrifai @auth protection on subscriptions have now been released with a major version bump of the CLI to 2.0. The new behavior along with additions of custom claims can be found here: https://aws-amplify.github.io/docs/cli-toolchain/graphql#authorizing-subscriptions

[Flos]

@undefobj
I don't understand how I can use that subscriptions now with the generated JavaScript client. How can I pass the owner to the subscription?

type Todo @model @auth(rules: [{allow: owner}]){
  id: ID!
  title: String!
  owner: String
}

this.apiService.OnCreateTodoListener.subscribe((evt) => {
      console.log("added: ", evt);
});

[undefobj]

@Flos it depends on the platform which you are using. The link gives information on how you could use the owner argument dynamically.

"Passing in the current user can be done dynamically in your code by using Auth.currentAuthenticatedUser() in JavaScript, AWSMobileClient.default().username in iOS, or AWSMobileClient.getInstance().getUsername() in Android."

I'll go ahead and close this issue as the original question was resolved a couple months back. For any other questions on usage please open up a new issue and we can address there.