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

KMS Key not found error during dataset import #712

Closed
manjulaK opened this issue Aug 28, 2023 · 4 comments · Fixed by #748
Closed

KMS Key not found error during dataset import #712

manjulaK opened this issue Aug 28, 2023 · 4 comments · Fixed by #748
Assignees
Labels
effort: low priority: medium status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up type: enhancement Feature enhacement
Milestone

Comments

@manjulaK
Copy link
Contributor

Describe the bug

On dataset import If a KMS key cannot be read due to permissions error there should be a very clear error saying that datalll does not have permissions to access the KEY and must provide clear instructions how the key policy must be altered. Currently it just says “key not found” which is confusing and misleading. We would like to fix this as soon as possible because this will be a source of problems for us and everyone will keep running into this issue because their KMS key will not be open to the pivotRole.

How to Reproduce

*P.S. Please do not attach files as it's considered a security risk. Add code snippets directly in the message body as much as possible.*

Expected behavior

Provide options such as : 1) Check if the KMS key for the bucket has a key policy defined and if pivot role has permissions to all kms resources in the key policy,

Your project

No response

Screenshots

No response

OS

All

Python version

3.1

AWS data.all version

all

Additional context

No response

@noah-paige noah-paige self-assigned this Aug 28, 2023
@marora90
Copy link
Contributor

Probably ties into #614

@dlpzx
Copy link
Contributor

dlpzx commented Sep 5, 2023

Yes, Agree with @blitzmohit. We can think of how the keys are imported.

@dlpzx dlpzx added type: enhancement Feature enhacement status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up labels Sep 5, 2023
@anmolsgandhi anmolsgandhi added this to the v2.1.0 milestone Sep 8, 2023
noah-paige added a commit that referenced this issue Sep 15, 2023
…748)

### Feature or Bugfix
<!-- please choose -->
- Feature Enhancement

### Detail
- Adding additional error messages for KMS Key lookup when importing a
new dataset
  - 1 Error message to determine if the KMS Key Alias Exists
- 1 Error message to determine if the PivotRole has permissions to
describe the KMS Key

### Relates
- #712 

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/).

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)?
  - Is the input sanitized?
- What precautions are you taking before deserializing the data you
consume?
  - Is injection prevented by parametrizing queries?
  - Have you ensured no `eval` or similar functions are used?
- Does this PR introduce any functionality or component that requires
authorization?
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
  - Are you logging failed auth attempts?
- Are you using or adding any cryptographic features?
  - Do you use a standard proven implementations?
  - Are the used keys controlled by the customer? Where are they stored?
- Are you introducing any new policies/roles/users?
  - Have you used the least-privilege principle? How?


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
@fourtyplustwo
Copy link
Contributor

@dlpzx would be good to have some kind of brainstorm/review session how we would like to implement this

@dlpzx
Copy link
Contributor

dlpzx commented Nov 8, 2023

Merged and released with v2.1.0 🚀 Although more enhancements are expected for the updates on KMS policies in sharing requests

@dlpzx dlpzx closed this as completed Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: low priority: medium status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up type: enhancement Feature enhacement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants