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

[Java] fix parameter tampering for immediate requests #167

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 62 additions & 60 deletions sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleFeedback.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,66 +18,6 @@ public class ModuleFeedback extends ModuleBase {
ModuleFeedback() {
}

@Override
public void init(InternalConfig config) {
super.init(config);
L.v("[ModuleFeedback] Initializing");

cachedAppVersion = config.getApplicationVersion();
feedbackInterface = new Feedback();
}

@Override
public Boolean onRequest(Request request) {
return true;
}

@Override
public void stop(InternalConfig config, boolean clear) {
super.stop(config, clear);
feedbackInterface = null;
}

private void getAvailableFeedbackWidgetsInternal(CallbackOnFinish<List<CountlyFeedbackWidget>> callback) {
L.d("[ModuleFeedback] getAvailableFeedbackWidgetsInternal, callback set:[" + (callback != null) + "]");

if (callback == null) {
L.e("[ModuleFeedback] getAvailableFeedbackWidgetsInternal, available feedback widget list can't be retrieved without a callback");
return;
}

// If someday we decide to support temporary device ID mode, this check will be needed
if (internalConfig.isTemporaryIdEnabled()) {
L.e("[ModuleFeedback] available feedback widget list can't be retrieved when in temporary device ID mode");
callback.onFinished(null, "[ModuleFeedback] available feedback widget list can't be retrieved when in temporary device ID mode");
return;
}

Transport transport = SDKCore.instance.networking.getTransport();
final boolean networkingIsEnabled = internalConfig.getNetworkingEnabled();

String params = ModuleRequests.prepareRequiredParamsAsString(internalConfig, "method", "feedback");
ImmediateRequestGenerator iRGenerator = internalConfig.immediateRequestGenerator;

iRGenerator.createImmediateRequestMaker().doWork("?" + params, "/o/sdk", transport, false, networkingIsEnabled, checkResponse -> {
if (checkResponse == null) {
L.d("[ModuleFeedback] getAvailableFeedbackWidgetsInternal, Not possible to retrieve widget list. Probably due to lack of connection to the server");
callback.onFinished(null, "Not possible to retrieve widget list. Probably due to lack of connection to the server");
return;
}

L.d("[ModuleFeedback] Retrieved request: [" + checkResponse + "]");

List<CountlyFeedbackWidget> feedbackEntries = new ArrayList<>();
String error = parseFeedbackList(checkResponse, feedbackEntries);
if (error != null) {
feedbackEntries = null;
}

callback.onFinished(feedbackEntries, error);
}, L);
}

static String parseFeedbackList(JSONObject requestResponse, List<CountlyFeedbackWidget> parsedRes) {
Log L = SDKCore.instance.L;
L.d("[ModuleFeedback] parseFeedbackList, calling");
Expand Down Expand Up @@ -148,6 +88,67 @@ static String parseFeedbackList(JSONObject requestResponse, List<CountlyFeedback
return null;
}

@Override
public void init(InternalConfig config) {
super.init(config);
L.v("[ModuleFeedback] Initializing");

cachedAppVersion = config.getApplicationVersion();
feedbackInterface = new Feedback();
}

@Override
public Boolean onRequest(Request request) {
return true;
}

@Override
public void stop(InternalConfig config, boolean clear) {
super.stop(config, clear);
feedbackInterface = null;
}

private void getAvailableFeedbackWidgetsInternal(CallbackOnFinish<List<CountlyFeedbackWidget>> callback) {
L.d("[ModuleFeedback] getAvailableFeedbackWidgetsInternal, callback set:[" + (callback != null) + "]");

if (callback == null) {
L.e("[ModuleFeedback] getAvailableFeedbackWidgetsInternal, available feedback widget list can't be retrieved without a callback");
return;
}

// If someday we decide to support temporary device ID mode, this check will be needed
if (internalConfig.isTemporaryIdEnabled()) {
L.e("[ModuleFeedback] available feedback widget list can't be retrieved when in temporary device ID mode");
callback.onFinished(null, "[ModuleFeedback] available feedback widget list can't be retrieved when in temporary device ID mode");
return;
}

Transport transport = SDKCore.instance.networking.getTransport();
final boolean networkingIsEnabled = internalConfig.getNetworkingEnabled();

Params params = ModuleRequests.prepareRequiredParams(internalConfig).add("method", "feedback");
ModuleRequests.prepareSaltedParams(internalConfig, params);
ImmediateRequestGenerator iRGenerator = internalConfig.immediateRequestGenerator;

iRGenerator.createImmediateRequestMaker().doWork(params.toString(), "/o/sdk?", transport, false, networkingIsEnabled, checkResponse -> {
if (checkResponse == null) {
L.d("[ModuleFeedback] getAvailableFeedbackWidgetsInternal, Not possible to retrieve widget list. Probably due to lack of connection to the server");
callback.onFinished(null, "Not possible to retrieve widget list. Probably due to lack of connection to the server");
return;
}

L.d("[ModuleFeedback] Retrieved request: [" + checkResponse + "]");

List<CountlyFeedbackWidget> feedbackEntries = new ArrayList<>();
String error = parseFeedbackList(checkResponse, feedbackEntries);
if (error != null) {
feedbackEntries = null;
}

callback.onFinished(feedbackEntries, error);
}, L);
}

private void reportFeedbackWidgetManuallyInternal(CountlyFeedbackWidget widgetInfo, JSONObject widgetData, Map<String, Object> widgetResult) {
if (widgetInfo == null) {
L.e("[ModuleFeedback] Can't report feedback widget data manually with 'null' widget info");
Expand Down Expand Up @@ -287,6 +288,7 @@ private void getFeedbackWidgetDataInternal(CountlyFeedbackWidget widgetInfo, Cal
.add("platform", internalConfig.getSdkPlatform())
.add("app_version", cachedAppVersion)
.add("av", internalConfig.getApplicationVersion());
ModuleRequests.prepareSaltedParams(internalConfig, params);

Transport cp = SDKCore.instance.networking.getTransport();
final boolean networkingIsEnabled = internalConfig.getNetworkingEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ private String prepareRemoteConfigRequest(@Nullable String keysInclude, @Nullabl
params.add("oi", "1");
}

ModuleRequests.prepareSaltedParams(internalConfig, params);
return params.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ public class ModuleRequests extends ModuleBase {

private static Params metrics;

public interface ParamsInjector {
void call(Params params);
}

@Override
public void initFinished(final InternalConfig config) {
ModuleRequests.metrics = Device.dev.buildMetrics();
}

private static Request sessionRequest(InternalConfig config, SessionImpl session, String type, Long value) {
Request request = Request.build();

Expand Down Expand Up @@ -185,6 +176,19 @@ static void addRequiredParametersToParams(InternalConfig config, Params params)
}
}

/**
* Add checksum to the request if needed.
*
* @param config InternalConfig to run in
* @param params Params to add checksum to
*/
static void prepareSaltedParams(InternalConfig config, Params params) {
if (!Utils.isEmptyOrNull(config.getParameterTamperingProtectionSalt())) {
String calculatedChecksum = Utils.digestHex(Transport.PARAMETER_TAMPERING_DIGEST, params + config.getParameterTamperingProtectionSalt(), config.getLogger());
params.add(Transport.CHECKSUM, calculatedChecksum);
}
}

public static Params prepareRequiredParams(InternalConfig config) {
Params params = new Params();

Expand All @@ -194,10 +198,6 @@ public static Params prepareRequiredParams(InternalConfig config) {
return params;
}

public static String prepareRequiredParamsAsString(InternalConfig config, Object... paramsObj) {
return prepareRequiredParams(config).add(paramsObj).toString();
}

/**
* Common store-request logic: store & send a ping to the service.
*
Expand Down Expand Up @@ -241,4 +241,13 @@ public static Future<Boolean> pushAsync(final InternalConfig config, final Reque
}
});
}

@Override
public void initFinished(final InternalConfig config) {
ModuleRequests.metrics = Device.dev.buildMetrics();
}

public interface ParamsInjector {
void call(Params params);
}
}
45 changes: 22 additions & 23 deletions sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,10 @@
//class Network extends ModuleBase { - may be

public class Transport implements X509TrustManager {
static final String PARAMETER_TAMPERING_DIGEST = "SHA-256";
static final String CHECKSUM = "checksum256";
private Log L = null;
private static final String PARAMETER_TAMPERING_DIGEST = "SHA-256";
private static final String CHECKSUM = "checksum256";
private InternalConfig config;

private SSLContext sslContext; // ssl context to use if pinning is enabled
private List<byte[]> keyPins = null; // list of parsed key pins
private List<byte[]> certPins = null; // list of parsed cert pins
Expand All @@ -65,6 +64,26 @@ public class Transport implements X509TrustManager {
public Transport() {
}

private static String trimPem(String pem) {
pem = pem.trim();

final String beginPK = "-----BEGIN PUBLIC KEY-----";
if (pem.startsWith(beginPK)) {
pem = pem.substring(pem.indexOf(beginPK) + beginPK.length());
}

final String beginCert = "-----BEGIN CERTIFICATE-----";
if (pem.startsWith(beginCert)) {
pem = pem.substring(pem.indexOf(beginCert) + beginCert.length());
}

if (pem.contains("-----END ")) {
pem = pem.substring(0, pem.indexOf("-----END"));
}
String res = pem.replaceAll("\n", "");
return res;
}

/**
* @param config configuration to use
* @throws IllegalArgumentException if certificate exception happens
Expand Down Expand Up @@ -367,26 +386,6 @@ Boolean processResponse(int code, String response, Long requestId) {
}
}

private static String trimPem(String pem) {
pem = pem.trim();

final String beginPK = "-----BEGIN PUBLIC KEY-----";
if (pem.startsWith(beginPK)) {
pem = pem.substring(pem.indexOf(beginPK) + beginPK.length());
}

final String beginCert = "-----BEGIN CERTIFICATE-----";
if (pem.startsWith(beginCert)) {
pem = pem.substring(pem.indexOf(beginCert) + beginCert.length());
}

if (pem.contains("-----END ")) {
pem = pem.substring(0, pem.indexOf("-----END"));
}
String res = pem.replaceAll("\n", "");
return res;
}

private void setPins(Set<String> keys, Set<String> certs) throws CertificateException {
keyPins = new ArrayList<>();
certPins = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public void getAvailableFeedbackWidgets_base(List<CountlyFeedbackWidget> expecte
ImmediateRequestI requestMaker = (requestData, customEndpoint, cp, requestShouldBeDelayed, networkingIsEnabled, callback, log) -> {
Map<String, String> params = TestUtils.parseQueryParams(requestData);
Assert.assertEquals("feedback", params.get("method"));
TestUtils.validateRequestMakerRequiredParams("/o/sdk", customEndpoint, requestShouldBeDelayed, networkingIsEnabled);
TestUtils.validateRequestMakerRequiredParams("/o/sdk?", customEndpoint, requestShouldBeDelayed, networkingIsEnabled);
TestUtils.validateRequiredParams(params);
callback.callback(returnedResponse);
};
Expand Down