From 1a515ee74a18d1ec881cc028b6d53bddbfbf2f8b Mon Sep 17 00:00:00 2001 From: Peter Turcsanyi Date: Tue, 1 Jun 2021 11:56:00 +0200 Subject: [PATCH] NIFI-8656 Support EL for SAS Token in the ADLS Gen2 processors This closes #5119 Signed-off-by: Joey Frazee --- .../storage/utils/AzureStorageUtils.java | 55 ++++++++++++------- .../ADLSCredentialsControllerService.java | 3 +- ...reStorageCredentialsControllerService.java | 2 +- .../additionalDetails.html | 44 +++++++++++++++ .../TestADLSCredentialsControllerService.java | 23 ++++++++ 5 files changed, 104 insertions(+), 23 deletions(-) create mode 100644 nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/resources/docs/org.apache.nifi.services.azure.storage.ADLSCredentialsControllerService/additionalDetails.html diff --git a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java index cf3c8a00ae..99ec708cfa 100644 --- a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java +++ b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java @@ -56,37 +56,46 @@ public final class AzureStorageUtils { public static final String STORAGE_SAS_TOKEN_PROPERTY_DESCRIPTOR_NAME = "storage-sas-token"; public static final String STORAGE_ENDPOINT_SUFFIX_PROPERTY_DESCRIPTOR_NAME = "storage-endpoint-suffix"; + public static final String ACCOUNT_KEY_BASE_DESCRIPTION = + "The storage account key. This is an admin-like password providing access to every container in this account. It is recommended " + + "one uses Shared Access Signature (SAS) token instead for fine-grained control with policies."; + + public static final String ACCOUNT_KEY_SECURITY_DESCRIPTION = + " There are certain risks in allowing the account key to be stored as a flowfile " + + "attribute. While it does provide for a more flexible flow by allowing the account key to " + + "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " + + "the event provenance data (e.g., by strictly controlling the policies governing provenance for this processor). " + + "In addition, the provenance repositories may be put on encrypted disk partitions."; + public static final PropertyDescriptor ACCOUNT_KEY = new PropertyDescriptor.Builder() .name(STORAGE_ACCOUNT_KEY_PROPERTY_DESCRIPTOR_NAME) .displayName("Storage Account Key") - .description("The storage account key. This is an admin-like password providing access to every container in this account. It is recommended " + - "one uses Shared Access Signature (SAS) token instead for fine-grained control with policies. " + - "There are certain risks in allowing the account key to be stored as a flowfile " + - "attribute. While it does provide for a more flexible flow by allowing the account key to " + - "be fetched dynamically from a flow file attribute, care must be taken to restrict access to " + - "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " + - "In addition, the provenance repositories may be put on encrypted disk partitions.") + .description(ACCOUNT_KEY_BASE_DESCRIPTION + ACCOUNT_KEY_SECURITY_DESCRIPTION) .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) .required(false) .sensitive(true) .build(); - public static final String ACCOUNT_NAME_BASE_DESCRIPTION = - "The storage account name. There are certain risks in allowing the account name to be stored as a flowfile " + + public static final String ACCOUNT_NAME_BASE_DESCRIPTION = "The storage account name."; + + public static final String ACCOUNT_NAME_SECURITY_DESCRIPTION = + " There are certain risks in allowing the account name to be stored as a flowfile " + "attribute. While it does provide for a more flexible flow by allowing the account name to " + "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " + - "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " + + "the event provenance data (e.g., by strictly controlling the policies governing provenance for this processor). " + "In addition, the provenance repositories may be put on encrypted disk partitions."; + public static final String ACCOUNT_NAME_CREDENTIAL_SERVICE_DESCRIPTION = + " Instead of defining the Storage Account Name, Storage Account Key and SAS Token properties directly on the processor, " + + "the preferred way is to configure them through a controller service specified in the Storage Credentials property. " + + "The controller service can provide a common/shared configuration for multiple/all Azure processors. Furthermore, the credentials " + + "can also be looked up dynamically with the 'Lookup' version of the service."; + public static final PropertyDescriptor ACCOUNT_NAME = new PropertyDescriptor.Builder() .name(STORAGE_ACCOUNT_NAME_PROPERTY_DESCRIPTOR_NAME) .displayName("Storage Account Name") - .description(ACCOUNT_NAME_BASE_DESCRIPTION + - " Instead of defining the Storage Account Name, Storage Account Key and SAS Token properties directly on the processor, " + - "the preferred way is to configure them through a controller service specified in the Storage Credentials property. " + - "The controller service can provide a common/shared configuration for multiple/all Azure processors. Furthermore, the credentials " + - "can also be looked up dynamically with the 'Lookup' version of the service.") + .description(ACCOUNT_NAME_BASE_DESCRIPTION + ACCOUNT_NAME_SECURITY_DESCRIPTION + ACCOUNT_NAME_CREDENTIAL_SERVICE_DESCRIPTION) .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) .required(false) @@ -117,15 +126,19 @@ public final class AzureStorageUtils { .required(true) .build(); + public static final String SAS_TOKEN_BASE_DESCRIPTION = "Shared Access Signature token, including the leading '?'. Specify either SAS token (recommended) or Account Key."; + + public static final String SAS_TOKEN_SECURITY_DESCRIPTION = + " There are certain risks in allowing the SAS token to be stored as a flowfile " + + "attribute. While it does provide for a more flexible flow by allowing the SAS token to " + + "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " + + "the event provenance data (e.g., by strictly controlling the policies governing provenance for this processor). " + + "In addition, the provenance repositories may be put on encrypted disk partitions."; + public static final PropertyDescriptor PROP_SAS_TOKEN = new PropertyDescriptor.Builder() .name(STORAGE_SAS_TOKEN_PROPERTY_DESCRIPTOR_NAME) .displayName("SAS Token") - .description("Shared Access Signature token, including the leading '?'. Specify either SAS Token (recommended) or Account Key. " + - "There are certain risks in allowing the SAS token to be stored as a flowfile " + - "attribute. While it does provide for a more flexible flow by allowing the account name to " + - "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " + - "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " + - "In addition, the provenance repositories may be put on encrypted disk partitions.") + .description(SAS_TOKEN_BASE_DESCRIPTION + SAS_TOKEN_SECURITY_DESCRIPTION) .required(false) .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) .sensitive(true) diff --git a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/ADLSCredentialsControllerService.java b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/ADLSCredentialsControllerService.java index 96851406a8..d275cacc12 100644 --- a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/ADLSCredentialsControllerService.java +++ b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/ADLSCredentialsControllerService.java @@ -67,12 +67,13 @@ public class ADLSCredentialsControllerService extends AbstractControllerService public static final PropertyDescriptor ACCOUNT_KEY = new PropertyDescriptor.Builder() .fromPropertyDescriptor(AzureStorageUtils.ACCOUNT_KEY) + .description(AzureStorageUtils.ACCOUNT_KEY_BASE_DESCRIPTION) .expressionLanguageSupported(ExpressionLanguageScope.NONE) .build(); public static final PropertyDescriptor SAS_TOKEN = new PropertyDescriptor.Builder() .fromPropertyDescriptor(AzureStorageUtils.PROP_SAS_TOKEN) - .expressionLanguageSupported(ExpressionLanguageScope.NONE) + .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) .build(); public static final PropertyDescriptor USE_MANAGED_IDENTITY = new PropertyDescriptor.Builder() diff --git a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/AzureStorageCredentialsControllerService.java b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/AzureStorageCredentialsControllerService.java index f782a9c188..45e995045e 100644 --- a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/AzureStorageCredentialsControllerService.java +++ b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/AzureStorageCredentialsControllerService.java @@ -49,7 +49,7 @@ public class AzureStorageCredentialsControllerService extends AbstractController public static final PropertyDescriptor ACCOUNT_NAME = new PropertyDescriptor.Builder() .name(AzureStorageUtils.ACCOUNT_NAME.getName()) .displayName(AzureStorageUtils.ACCOUNT_NAME.getDisplayName()) - .description(AzureStorageUtils.ACCOUNT_NAME_BASE_DESCRIPTION) + .description(AzureStorageUtils.ACCOUNT_NAME_BASE_DESCRIPTION + AzureStorageUtils.ACCOUNT_NAME_SECURITY_DESCRIPTION) .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) .required(true) diff --git a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/resources/docs/org.apache.nifi.services.azure.storage.ADLSCredentialsControllerService/additionalDetails.html b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/resources/docs/org.apache.nifi.services.azure.storage.ADLSCredentialsControllerService/additionalDetails.html new file mode 100644 index 0000000000..cb5788c2ed --- /dev/null +++ b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/resources/docs/org.apache.nifi.services.azure.storage.ADLSCredentialsControllerService/additionalDetails.html @@ -0,0 +1,44 @@ + + + + + + ADLSCredentialsControllerService + + + + + +

Security considerations of using Expression Language for sensitive properties

+ +

+ Allowing Expression Language for a property has the advantage of configuring the property dynamically via FlowFile attributes + or Variable Registry entries. In case of sensitive properties, it also has a drawback of exposing sensitive information like + passwords, security keys or tokens. When the value of a sensitive property comes from a FlowFile attribute, it travels by the + FlowFile in clear text form and is also saved in the provenance repository. Variable Registry does not support the encryption of + sensitive information either. Due to these, the sensitive credential data can be exposed to unauthorized parties. +

+ Best practices for using Expression Language for sensitive properties: +

+ + + + diff --git a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/services/azure/storage/TestADLSCredentialsControllerService.java b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/services/azure/storage/TestADLSCredentialsControllerService.java index 6837c9ebb5..04a9a18ba7 100644 --- a/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/services/azure/storage/TestADLSCredentialsControllerService.java +++ b/nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/services/azure/storage/TestADLSCredentialsControllerService.java @@ -327,6 +327,24 @@ public class TestADLSCredentialsControllerService { assertNull(actual.getServicePrincipalClientSecret()); } + @Test + public void testGetCredentialsDetailsWithSasTokenUsingEL() throws Exception { + configureAccountName(); + configureSasTokenUsingEL(); + + runner.enableControllerService(credentialsService); + + ADLSCredentialsDetails actual = credentialsService.getCredentialsDetails(new HashMap<>()); + assertEquals(ACCOUNT_NAME_VALUE, actual.getAccountName()); + assertEquals(SAS_TOKEN_VALUE, actual.getSasToken()); + assertNull(actual.getAccountKey()); + assertFalse(actual.getUseManagedIdentity()); + assertNotNull(actual.getEndpointSuffix()); + assertNull(actual.getServicePrincipalTenantId()); + assertNull(actual.getServicePrincipalClientId()); + assertNull(actual.getServicePrincipalClientSecret()); + } + @Test public void testGetCredentialsDetailsWithUseManagedIdentity() throws Exception { // GIVEN @@ -417,6 +435,11 @@ public class TestADLSCredentialsControllerService { runner.setProperty(credentialsService, ADLSCredentialsControllerService.SAS_TOKEN, SAS_TOKEN_VALUE); } + private void configureSasTokenUsingEL() { + String variableName = "sas.token"; + configurePropertyUsingEL(ADLSCredentialsControllerService.SAS_TOKEN, variableName, SAS_TOKEN_VALUE); + } + private void configureUseManagedIdentity() { runner.setProperty(credentialsService, ADLSCredentialsControllerService.USE_MANAGED_IDENTITY, "true"); }