Uploaded image for project: 'Apache NiFi'
  1. Apache NiFi
  2. NIFI-6734

S3EncryptionService fixes and improvements

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.2
    • Fix Version/s: 1.10.0
    • Component/s: Extensions
    • Labels:
      None

      Description

      I found some issues while I was setting up S3 encryption controller service.
      I think these should be addressed before the initial release of the CS.

      Bugs:

      • multipart upload not works in case of SSE S3 encryption
      • multipart upload not works in case of CSE* encryptions
      • SSE S3 and SSE KMS strategies don't do anything in case of FetchS3Object (it is not needed to configure them, the decryption handled implicitly). On the other hand, if SSE S3 is set for an SSE KMS (or a CSE*) encrypted object, it won't cause any error (CSE encrypted object won't be decrypted though) and SSE S3 will be set on the outgoing FlowFile (s3.encryptionStrategy attribute) which is false info => SSE S3 and SSE KMS should be disabled for FetchS3Object
      • StandardS3EncryptionService.customValidate() runs on wrong encryptionStrategy instance (it must be retrieved from ValidationContext)
      • StandardS3EncryptionService 'Key ID or Key Material' property does not evaluate EL despite of its documentation (supporting variable registry)

      Code cleanup:

      • CSE CMK encryption strategy sets the KMS region, but it will not be used (as the key does not come from KMS, but will be specified by the client) => setting the KMS region is not necessary / misleading in the code
      • CSE* encryption strategies set the KMS region on the client, but the client needs the bucket region (which can be different than the KMS region) and it will be set later in the code flow => setting the KMS region on the client is not necessary / misleading in the code

      Documentation enhancements:

      • 'Key ID or Key Material' property: document in the property description that it is not used (should be empty) in case of SSE S3, for other encryption types use the same names as in the Encryption Strategy combo (eg. 'Server-side Customer Key' instead of 'Server-side CEK')
      • 'region' property: add display name + description, document in the property description that it is the KMS region and is only used in case of Client-side KMS
      • documentation of PutS3Object and FetchS3Object should be separated: eg. FetchS3Object does not have 'Server Side Encryption' property referred in the docs and the controller service is not needed for fetching SSE S3 and SSE KMS encrypted objects
      • add 'aws' and 's3' tags to the CS
      • additionalDetails not linked properly (not accessible)
      • key alias does not work for KMS keys, only key id => remove alias from docs
      • add validator with informative error messages to help configuration

      Renaming:

      • 'Client-side Customer Master Key' property value: CMK (Customer Master Key) is generally used for the client side encryption keys in the AWS docs, regardless that the key provided by the client or stored in KMS. For this reason, 'Client-side KMS' vs 'Client-side Customer Master Key' is a bit confusing for me, I would use 'Client-side Customer Key' for the latter (similar to 'Server-side KMS' and 'Server-side Customer Key')
      • 'region' property: should be renamed to kms-region (to avoid confusion with the bucket region in the code)

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                turcsanyip Peter Turcsanyi
                Reporter:
                turcsanyip Peter Turcsanyi
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 1h 40m
                  1h 40m