[ 
https://issues.apache.org/jira/browse/NIFI-6734?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Peter Turcsanyi updated NIFI-6734:
----------------------------------
    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|https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingClientSideEncryption.html],
 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)

  was:
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)

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|https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingClientSideEncryption.html],
 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)


> S3EncryptionService fixes and improvements
> ------------------------------------------
>
>                 Key: NIFI-6734
>                 URL: https://issues.apache.org/jira/browse/NIFI-6734
>             Project: Apache NiFi
>          Issue Type: Bug
>          Components: Extensions
>            Reporter: Peter Turcsanyi
>            Assignee: Peter Turcsanyi
>            Priority: Major
>
> 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|https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingClientSideEncryption.html],
>  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)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to