From 87d092c404f124cabfe919d73864932c9cf33a85 Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Tue, 8 Feb 2022 19:00:20 -0300 Subject: [PATCH 1/8] ENVVars not working for Storage Services because ENVVARS where expected to be encrypted --- .../db/driver/ExternalProviderBase.java | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java index c24ec35f9..0fd976bf3 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java @@ -51,13 +51,18 @@ public String getEncryptedPropertyValue(String propertyName, String alternativeP } public String getEncryptedPropertyValue(String propertyName, String alternativePropertyName, String defaultValue) { - String value = getPropertyValue(propertyName, alternativePropertyName, defaultValue); + String value = readFromEnvVars(propertyName, alternativePropertyName); + if (value != null) { + return value; + } + + value = getServicePropertyValue(propertyName, alternativePropertyName, defaultValue); if (value != null && value.length() > 0) { try { value = Encryption.decrypt64(value); } catch (Exception e) { - logger.warn("Could not decrypt property name: " + resolvePropertyName(propertyName)); + logger.error("Could not decrypt property name: " + resolvePropertyName(propertyName)); } } return value; @@ -74,22 +79,35 @@ public String getPropertyValue(String propertyName, String alternativePropertyNa } public String getPropertyValue(String propertyName, String alternativePropertyName, String defaultValue) { - propertyName = resolvePropertyName(propertyName); - String value = System.getenv(propertyName); - if (value == null || value.length() == 0){ - value = System.getenv(alternativePropertyName); + String value = readFromEnvVars(propertyName, alternativePropertyName); + if (value != null) { + return value; } + + return getServicePropertyValue(propertyName, alternativePropertyName, defaultValue); + } + + private String getServicePropertyValue(String propertyName, String alternativePropertyName, String defaultValue) { + String value = null; + String resolvedPtyName = resolvePropertyName(propertyName); if (this.service != null) { - value = this.service.getProperties().get(propertyName); + value = this.service.getProperties().get(resolvedPtyName); if (value == null || value.length() == 0) { value = this.service.getProperties().get(alternativePropertyName); } } - return value != null? value: defaultValue; + return value != null ? value: defaultValue; + } + + private String readFromEnvVars(String propertyName, String alternativePropertyName) { + String value = System.getenv(resolvePropertyName(propertyName)); + if (value == null || value.length() == 0){ + value = System.getenv(alternativePropertyName); + } + return value; } private String resolvePropertyName(String propertyName) { return String.format("STORAGE_%s_%s", getName(), propertyName); } - } From 573e1c35c687bb395b45045e92d4209024a0a362 Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Tue, 8 Feb 2022 19:09:43 -0300 Subject: [PATCH 2/8] Check for unecntrpyed value --- .../db/driver/ExternalProviderBase.java | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java index 0fd976bf3..86ce44080 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java @@ -27,7 +27,6 @@ public ExternalProviderBase() { init(); } - public ExternalProviderBase(GXService s) { this.service = s; init(); @@ -51,21 +50,18 @@ public String getEncryptedPropertyValue(String propertyName, String alternativeP } public String getEncryptedPropertyValue(String propertyName, String alternativePropertyName, String defaultValue) { - String value = readFromEnvVars(propertyName, alternativePropertyName); - if (value != null) { - return value; - } - - value = getServicePropertyValue(propertyName, alternativePropertyName, defaultValue); - if (value != null && value.length() > 0) { + String decryptedValue = null; + String encryptedOrClearTextValue = getPropertyValue(propertyName, alternativePropertyName, defaultValue); + if (encryptedOrClearTextValue != null && encryptedOrClearTextValue.length() > 0) { try { - value = Encryption.decrypt64(value); + String decryptedTemp = Encryption.decrypt64(encryptedOrClearTextValue); + decryptedValue = (decryptedTemp != null) ? decryptedTemp: encryptedOrClearTextValue; } catch (Exception e) { - logger.error("Could not decrypt property name: " + resolvePropertyName(propertyName)); + logger.warn("Could not decrypt property name: " + resolvePropertyName(propertyName)); } } - return value; + return decryptedValue; } public String getPropertyValue(String propertyName, String alternativePropertyName) throws Exception{ @@ -83,20 +79,18 @@ public String getPropertyValue(String propertyName, String alternativePropertyNa if (value != null) { return value; } - - return getServicePropertyValue(propertyName, alternativePropertyName, defaultValue); - } - - private String getServicePropertyValue(String propertyName, String alternativePropertyName, String defaultValue) { - String value = null; String resolvedPtyName = resolvePropertyName(propertyName); - if (this.service != null) { + + if (value == null && this.service != null) { value = this.service.getProperties().get(resolvedPtyName); if (value == null || value.length() == 0) { value = this.service.getProperties().get(alternativePropertyName); } } - return value != null ? value: defaultValue; + if (value != null) { + logger.warn(String.format("Service getPropertyValue - (%s : %s)", resolvedPtyName, value)); + } + return value != null? value: defaultValue; } private String readFromEnvVars(String propertyName, String alternativePropertyName) { @@ -110,4 +104,5 @@ private String readFromEnvVars(String propertyName, String alternativePropertyNa private String resolvePropertyName(String propertyName) { return String.format("STORAGE_%s_%s", getName(), propertyName); } + } From c6ab6ab78b22ebcd846deb6005b4a0ccfcb77069 Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Tue, 8 Feb 2022 19:11:55 -0300 Subject: [PATCH 3/8] Remove redundant logging --- .../java/com/genexus/db/driver/ExternalProviderBase.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java index 86ce44080..3ede04b49 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java @@ -80,22 +80,18 @@ public String getPropertyValue(String propertyName, String alternativePropertyNa return value; } String resolvedPtyName = resolvePropertyName(propertyName); - - if (value == null && this.service != null) { + if (this.service != null) { value = this.service.getProperties().get(resolvedPtyName); if (value == null || value.length() == 0) { value = this.service.getProperties().get(alternativePropertyName); } } - if (value != null) { - logger.warn(String.format("Service getPropertyValue - (%s : %s)", resolvedPtyName, value)); - } return value != null? value: defaultValue; } private String readFromEnvVars(String propertyName, String alternativePropertyName) { String value = System.getenv(resolvePropertyName(propertyName)); - if (value == null || value.length() == 0){ + if (value == null){ value = System.getenv(alternativePropertyName); } return value; From b13b482077852434c01c1b4cf5d00dd036f26ffc Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Wed, 9 Feb 2022 09:16:18 -0300 Subject: [PATCH 4/8] Fix UnitTests --- .../com/genexus/db/driver/ExternalProviderBase.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java index 3ede04b49..f53fd00f9 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java @@ -50,12 +50,12 @@ public String getEncryptedPropertyValue(String propertyName, String alternativeP } public String getEncryptedPropertyValue(String propertyName, String alternativePropertyName, String defaultValue) { - String decryptedValue = null; - String encryptedOrClearTextValue = getPropertyValue(propertyName, alternativePropertyName, defaultValue); - if (encryptedOrClearTextValue != null && encryptedOrClearTextValue.length() > 0) { + String encryptedOrUnEncryptedValue = getPropertyValue(propertyName, alternativePropertyName, defaultValue); + String decryptedValue = encryptedOrUnEncryptedValue; + if (encryptedOrUnEncryptedValue != null && encryptedOrUnEncryptedValue.length() > 0) { try { - String decryptedTemp = Encryption.decrypt64(encryptedOrClearTextValue); - decryptedValue = (decryptedTemp != null) ? decryptedTemp: encryptedOrClearTextValue; + String decryptedTemp = Encryption.decrypt64(encryptedOrUnEncryptedValue); + decryptedValue = (decryptedTemp != null) ? decryptedTemp: encryptedOrUnEncryptedValue; } catch (Exception e) { logger.warn("Could not decrypt property name: " + resolvePropertyName(propertyName)); From 125a725b4fafd5631ebe004a347a017348d6e56b Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Wed, 9 Feb 2022 12:19:01 -0300 Subject: [PATCH 5/8] Improved Logging for External Services. Checksum for Decrpt Method --- .../java/com/genexus/util/Encryption.java | 42 ++++++++++++++++--- .../db/driver/ExternalProviderBase.java | 2 +- .../main/java/com/genexus/Application.java | 14 +++++-- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/common/src/main/java/com/genexus/util/Encryption.java b/common/src/main/java/com/genexus/util/Encryption.java index 378130108..5ac6b8472 100644 --- a/common/src/main/java/com/genexus/util/Encryption.java +++ b/common/src/main/java/com/genexus/util/Encryption.java @@ -148,10 +148,40 @@ public static String decrypt16(String value, String key) return ""; } - public static String decrypt64(String value){ - value= decrypt64(value, SpecificImplementation.Application.getModelContext().getServerKey()); - return value.substring(0, value.length()-CHECKSUM_LENGTH); - } + public static String decrypt64(String value){ + value = decrypt64(value, SpecificImplementation.Application.getModelContext().getServerKey()); + return value.substring(0, value.length()-CHECKSUM_LENGTH); + } + + /** + * Returns decrpyted value if the checksum verification succedes. Otherwise, original value is returned + * @param encryptedOrDecryptedValue + * @return Decrypted Value + */ + public static String tryDecrypt64(String encryptedOrDecryptedValue) { + return tryDecrypt64(encryptedOrDecryptedValue, SpecificImplementation.Application.getModelContext().getServerKey()); + } + + public static String tryDecrypt64(String encryptedOrDecryptedValue, String key) { + if (encryptedOrDecryptedValue == null) { + return null; + } + + int checkSumLength = Encryption.getCheckSumLength(); + if (encryptedOrDecryptedValue.length() > checkSumLength) { + String dec = Encryption.decrypt64(encryptedOrDecryptedValue, key); + // Ojo, el = de aca es porque sino no me deja tener passwords vacias, dado que el length queda igual al length del checksum + if (dec.length() >= checkSumLength) { + String checksum = CommonUtil.right(dec, checkSumLength); + String decryptedValue = CommonUtil.left(dec, dec.length() - checkSumLength); + if (checksum.equals(Encryption.checksum(decryptedValue, Encryption.getCheckSumLength()))) { + return decryptedValue; + } + } + } + return encryptedOrDecryptedValue; + } + public static String decrypt64(String value, String key) { @@ -193,7 +223,9 @@ public static String decrypt64(String value, String key, boolean safeEncoding) return ""; } } - + + + private static final int CHECKSUM_LENGTH = 6; public static int getCheckSumLength() diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java index f53fd00f9..507b29145 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java @@ -54,7 +54,7 @@ public String getEncryptedPropertyValue(String propertyName, String alternativeP String decryptedValue = encryptedOrUnEncryptedValue; if (encryptedOrUnEncryptedValue != null && encryptedOrUnEncryptedValue.length() > 0) { try { - String decryptedTemp = Encryption.decrypt64(encryptedOrUnEncryptedValue); + String decryptedTemp = Encryption.tryDecrypt64(encryptedOrUnEncryptedValue); decryptedValue = (decryptedTemp != null) ? decryptedTemp: encryptedOrUnEncryptedValue; } catch (Exception e) { diff --git a/java/src/main/java/com/genexus/Application.java b/java/src/main/java/com/genexus/Application.java index 0fd045916..d982b4376 100644 --- a/java/src/main/java/com/genexus/Application.java +++ b/java/src/main/java/com/genexus/Application.java @@ -177,15 +177,23 @@ private static ExternalProvider getExternalProviderImpl(String service) GXService providerService = getGXServices().get(service); if (providerService != null) { + Class providerClass; try { - Class providerClass = Class.forName(providerService.getClassName()); + providerClass = Class.forName(providerService.getClassName()); + } + catch (ClassNotFoundException e) + { + logger.fatal("Unrecognized External Provider class (ClassNotFound) : " + providerService.getName() + " / " + providerService.getClassName(), e); + throw new InternalError("Unrecognized External Provider class (ClassNotFound) : " + providerService.getName() + " / " + providerService.getClassName()); + } + try { externalProviderImpl = (ExternalProvider) providerClass.getConstructor(String.class).newInstance(service); } catch (Exception e) { - logger.error("Unrecognized External Provider class : " + providerService.getName() + " / " + providerService.getClassName(), e); - throw new InternalError("Unrecognized External Provider class : " + providerService.getName() + " / " + providerService.getClassName()); + logger.fatal("Unable to Initialize External Provider Class: " + providerService.getClassName(), e); + throw new InternalError("Unable to Initialize External Provider Class: " + providerService.getClassName(), e); } } return externalProviderImpl; From 01ba6d3acf471495a8d7b7703dbe3cc1ae2437ce Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Wed, 9 Feb 2022 16:13:50 -0300 Subject: [PATCH 6/8] AWS S3 Use IAM Cretendials when possible. --- .../genexus/db/driver/ExternalProviderS3.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderS3.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderS3.java index 865d0bb69..4e4d70a92 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderS3.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderS3.java @@ -1,5 +1,6 @@ package com.genexus.db.driver; +import com.amazonaws.auth.*; import com.amazonaws.client.builder.AwsClientBuilder; import com.amazonaws.services.s3.model.*; import com.amazonaws.services.s3.AmazonS3ClientBuilder; @@ -14,9 +15,7 @@ import java.io.File; import java.io.InputStream; import java.io.ByteArrayInputStream; -import com.amazonaws.auth.AWSCredentials; -import com.amazonaws.auth.BasicAWSCredentials; -import com.amazonaws.auth.AWSStaticCredentialsProvider; + import com.amazonaws.services.s3.AmazonS3; import com.amazonaws.services.s3.AmazonS3Client; import com.amazonaws.util.IOUtils; @@ -41,6 +40,7 @@ public class ExternalProviderS3 extends ExternalProviderBase implements External static final String STORAGE_ENDPOINT = "ENDPOINT"; static final String BUCKET = "BUCKET_NAME"; static final String REGION = "REGION"; + static final String USE_IAM = "USE_IAM"; //Keep it for compatibility reasons @Deprecated @@ -90,8 +90,8 @@ public ExternalProviderS3(GXService providerService) throws Exception{ } private void initialize() throws Exception{ - String accessKey = getEncryptedPropertyValue(ACCESS_KEY, ACCESS_KEY_ID_DEPRECATED); - String secretKey = getEncryptedPropertyValue(SECRET_ACCESS_KEY, SECRET_ACCESS_KEY_DEPRECATED); + String accessKey = getEncryptedPropertyValue(ACCESS_KEY, ACCESS_KEY_ID_DEPRECATED, ""); + String secretKey = getEncryptedPropertyValue(SECRET_ACCESS_KEY, SECRET_ACCESS_KEY_DEPRECATED, ""); String bucket = getEncryptedPropertyValue(BUCKET, BUCKET_DEPRECATED); String folder = getPropertyValue(FOLDER, FOLDER_DEPRECATED, ""); String region = getPropertyValue(REGION, REGION_DEPRECATED, DEFAULT_REGION); @@ -109,10 +109,11 @@ private void initialize() throws Exception{ if (region.length() == 0) { region = DEFAULT_REGION; } + this.bucket = bucket; this.folder = folder; - this.client = buildS3Client(accessKey, secretKey, endpointValue, region); + this.client = buildS3Client(accessKey, secretKey, endpointValue, region); bucketExists(); ensureFolder(folder); } @@ -120,8 +121,16 @@ private void initialize() throws Exception{ private AmazonS3 buildS3Client(String accessKey, String secretKey, String endpoint, String region) { AmazonS3 s3Client; - AWSCredentials credentials = new BasicAWSCredentials(accessKey, secretKey); - AmazonS3ClientBuilder builder = AmazonS3ClientBuilder.standard().withCredentials(new AWSStaticCredentialsProvider(credentials)); + + boolean bUseIAM = !getPropertyValue(USE_IAM, "", "").isEmpty() || (accessKey.equals("") && secretKey.equals("")); + + AmazonS3ClientBuilder builder = bUseIAM ? + AmazonS3ClientBuilder.standard(): + AmazonS3ClientBuilder.standard().withCredentials(new AWSStaticCredentialsProvider(new BasicAWSCredentials(accessKey, secretKey))); + + if (bUseIAM) { + logger.debug("Using IAM Credentials"); + } if (endpoint.length() > 0 && !endpoint.contains(".amazonaws.com")) { pathStyleUrls = true; From e4cb38fa50bcf9f8aac4f20c63c6bbbbca7803c4 Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Mon, 14 Feb 2022 15:29:04 -0300 Subject: [PATCH 7/8] Prevent Override with EnvVarSettings when using .Create() External Storage programatically --- .../main/java/com/genexus/util/GXService.java | 21 ++++++++++++++++--- .../db/driver/ExternalProviderBase.java | 4 ++++ .../configuration/ExternalStorage.java | 3 ++- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/common/src/main/java/com/genexus/util/GXService.java b/common/src/main/java/com/genexus/util/GXService.java index ac9217b81..21e015ce6 100644 --- a/common/src/main/java/com/genexus/util/GXService.java +++ b/common/src/main/java/com/genexus/util/GXService.java @@ -6,8 +6,13 @@ public class GXService private String type; private String className; private boolean allowMultiple; + private boolean allowOverrideWithEnvVarSettings; private GXProperties properties; - + + public GXService() { + this.allowOverrideWithEnvVarSettings = true; + } + public String getName() { return name; @@ -42,12 +47,22 @@ public boolean getAllowMultiple() { return allowMultiple; } - + public void setAllowMultiple(boolean allowMultiple) { this.allowMultiple = allowMultiple; } - + + public void setAllowOverrideWithEnvVarSettings(boolean allowOverride) + { + this.allowOverrideWithEnvVarSettings = allowOverride; + } + + public boolean getAllowOverrideWithEnvVarSettings() + { + return allowOverrideWithEnvVarSettings; + } + public GXProperties getProperties() { return properties; diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java index 507b29145..dbd89e5b9 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java @@ -90,6 +90,10 @@ public String getPropertyValue(String propertyName, String alternativePropertyNa } private String readFromEnvVars(String propertyName, String alternativePropertyName) { + if (!service.getAllowOverrideWithEnvVarSettings()){ + return null; + } + String value = System.getenv(resolvePropertyName(propertyName)); if (value == null){ value = System.getenv(alternativePropertyName); diff --git a/java/src/main/java/com/genexus/configuration/ExternalStorage.java b/java/src/main/java/com/genexus/configuration/ExternalStorage.java index 32fa85db4..ce6cd51ae 100644 --- a/java/src/main/java/com/genexus/configuration/ExternalStorage.java +++ b/java/src/main/java/com/genexus/configuration/ExternalStorage.java @@ -37,7 +37,7 @@ public boolean create(String name, GXProperties properties, GXStorageProvider[] if (isNullOrEmpty(name)) { - GXutil.ErrorToMessages("Unsopported", "Provider cannot be empty", messages[0]); + GXutil.ErrorToMessages("Unsupported", "Provider name cannot be empty", messages[0]); return false; } @@ -49,6 +49,7 @@ public boolean create(String name, GXProperties properties, GXStorageProvider[] providerService.setType(GXServices.STORAGE_SERVICE); providerService.setName(name); providerService.setAllowMultiple(false); + providerService.setAllowOverrideWithEnvVarSettings(false); providerService.setProperties(new GXProperties()); } From 11e00fd7a8b6d858a3b39e1810d4a204bcace3eb Mon Sep 17 00:00:00 2001 From: Gonzalo Gallotti Date: Mon, 14 Feb 2022 19:03:51 -0300 Subject: [PATCH 8/8] In UnitTest service may be null --- .../main/java/com/genexus/db/driver/ExternalProviderBase.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java index dbd89e5b9..775877765 100644 --- a/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java +++ b/gxexternalproviders/src/main/java/com/genexus/db/driver/ExternalProviderBase.java @@ -80,7 +80,7 @@ public String getPropertyValue(String propertyName, String alternativePropertyNa return value; } String resolvedPtyName = resolvePropertyName(propertyName); - if (this.service != null) { + if (service != null) { value = this.service.getProperties().get(resolvedPtyName); if (value == null || value.length() == 0) { value = this.service.getProperties().get(alternativePropertyName); @@ -90,7 +90,7 @@ public String getPropertyValue(String propertyName, String alternativePropertyNa } private String readFromEnvVars(String propertyName, String alternativePropertyName) { - if (!service.getAllowOverrideWithEnvVarSettings()){ + if (service != null && !service.getAllowOverrideWithEnvVarSettings()){ return null; }