Skip to content

Commit

Permalink
[PRImpl] Gate-keeping the mojo PaymentRequest untrusted calls.
Browse files Browse the repository at this point in the history
Change logic:
* The untrusted PaymentRequest interface breaks apart from CPRImpl to
become MojoPaymentRequestGateKeeper. The new class would be responsible
for handling the untrusted mojo calls from the renderer. Doing that,
it can ensure that CPRImpl receives no invalid calls from the renderer.
* Establish the requirements for the users of CPRImpl that the CPRImpl
instances should be set null after BPRImpl#close, CPRImpl#teardown and
PRClient#close. For example, CPRImpl#OnClosedListener is provided for
the callers to nullify the instances.

Note that:
* Although MPRGateKeeper and CPRImpl both have plumbings to pass PR
calls, they are necessary, because going forwards, PRImpl will be
diminished, and eventually the plumbings would only exist between
MPRGateKeeper and CPRImpl.

Code change highlights:
* In CPRImpl#initAndValidate(), null-check the request's raw data
before passing them into BPR, so that BPR doesn't need to add @nullable
for them.
* In MPRGateKeeper#init, discards CPRImpl (and its PRClient, BPR) when
the raw data are invalid.
* MPRGateKeeper plumbs the PR calls to CPRImpl, so that the calls are
only passed to the CPRImpl when the PaymentRequest data are valid.

Acronym:
CPRImpl = ComponentPaymentRequestImpl
BPR = BrowserPaymentRequest
PRClient = PaymentRequestClient
MPRGateKeeper = MojoPaymentRequestGateKeeper

Bug: 1102522

Change-Id: Iab160b2c3e74608b9b19ba17e454dedf32411193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346730
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797780}
  • Loading branch information
maxlgu authored and Commit Bot committed Aug 13, 2020
1 parent 3c62dab commit 6731c83
Show file tree
Hide file tree
Showing 6 changed files with 350 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,11 @@ public PaymentRequest createImpl() {
}

WebContents webContents = WebContentsStatics.fromRenderFrameHost(mRenderFrameHost);
return ComponentPaymentRequestImpl.create(mRenderFrameHost,
delegate.isOffTheRecord(webContents), delegate.skipUiForBasicCard(),
(componentPaymentRequest, isOffTheRecord, journeyLogger)
-> new PaymentRequestImpl(mRenderFrameHost, componentPaymentRequest,
return ComponentPaymentRequestImpl.createPaymentRequest(mRenderFrameHost,
/*isOffTheRecord=*/delegate.isOffTheRecord(webContents),
/*skipUiForBasicCard=*/delegate.skipUiForBasicCard(),
(renderFrameHost, componentPaymentRequest, isOffTheRecord, journeyLogger)
-> new PaymentRequestImpl(renderFrameHost, componentPaymentRequest,
isOffTheRecord, journeyLogger, delegate));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -366,23 +366,20 @@ public PaymentRequestImpl(RenderFrameHost renderFrameHost,
mPaymentUIsManager = new PaymentUIsManager(/*delegate=*/this,
/*params=*/this, mWebContents, mIsOffTheRecord, mJourneyLogger);
mComponentPaymentRequestImpl = componentPaymentRequestImpl;
mComponentPaymentRequestImpl.registerPaymentRequestLifecycleObserver(mPaymentUIsManager);
}

// Implement BrowserPaymentRequest:
/**
* Called by the merchant website to initialize the payment request data.
*/
@Override
public void init(PaymentMethodData[] methodData, PaymentDetails details,
public boolean initAndValidate(PaymentMethodData[] methodData, PaymentDetails details,
@Nullable PaymentOptions options, boolean googlePayBridgeEligible) {
assert mComponentPaymentRequestImpl != null;
mMethodData = new HashMap<>();
mComponentPaymentRequestImpl.registerPaymentRequestLifecycleObserver(mPaymentUIsManager);

if (!OriginSecurityChecker.isOriginSecure(mWebContents.getLastCommittedUrl())) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage(ErrorStrings.NOT_IN_A_SECURE_ORIGIN);
return;
return false;
}

mPaymentOptions = options;
Expand All @@ -403,7 +400,7 @@ public void init(PaymentMethodData[] methodData, PaymentDetails details,
// "NotSupportedError".
mQueryForQuota = new HashMap<>();
onDoneCreatingPaymentApps(/*factory=*/null);
return;
return true;
}

mJourneyLogger.setRequestedInformation(
Expand All @@ -419,7 +416,7 @@ public void init(PaymentMethodData[] methodData, PaymentDetails details,
// "NotSupportedError".
mQueryForQuota = new HashMap<>();
onDoneCreatingPaymentApps(/*factory=*/null);
return;
return true;
}

boolean googlePayBridgeActivated = googlePayBridgeEligible
Expand All @@ -430,7 +427,7 @@ public void init(PaymentMethodData[] methodData, PaymentDetails details,
if (mMethodData == null) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage(ErrorStrings.INVALID_PAYMENT_METHODS_OR_DATA);
return;
return false;
}

if (googlePayBridgeActivated) {
Expand All @@ -448,14 +445,14 @@ public void init(PaymentMethodData[] methodData, PaymentDetails details,
mQueryForQuota.put("basic-card-payment-options", paymentMethodData);
}

if (!parseAndValidateDetailsOrDisconnectFromClient(details)) return;
if (!parseAndValidateDetailsOrDisconnectFromClient(details)) return true;
mSpec = new PaymentRequestSpec(mPaymentOptions, details, mMethodData.values(),
LocaleUtils.getDefaultLocaleString());

if (mRawTotal == null) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage(ErrorStrings.TOTAL_REQUIRED);
return;
return false;
}
mId = details.id;

Expand Down Expand Up @@ -499,6 +496,7 @@ public void init(PaymentMethodData[] methodData, PaymentDetails details,
mJourneyLogger.setRequestedPaymentMethodTypes(
/*requestedBasicCard=*/mPaymentUIsManager.merchantSupportsAutofillCards(),
requestedMethodGoogle, requestedMethodOther);
return true;
}

/**
Expand Down Expand Up @@ -821,7 +819,8 @@ private static Map<String, PaymentMethodData> getValidatedMethodData(
PaymentMethodData[] methodData, boolean googlePayBridgeEligible,
CardEditor paymentMethodsCollector) {
// Payment methodData are required.
if (methodData == null || methodData.length == 0) return null;
assert methodData != null;
if (methodData.length == 0) return null;
Map<String, PaymentMethodData> result = new ArrayMap<>();
for (int i = 0; i < methodData.length; i++) {
String method = methodData[i].supportedMethod;
Expand Down
1 change: 1 addition & 0 deletions components/payments/content/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ android_library("java") {
"java/src/org/chromium/components/payments/ErrorMessageUtil.java",
"java/src/org/chromium/components/payments/JniPaymentApp.java",
"java/src/org/chromium/components/payments/JourneyLogger.java",
"java/src/org/chromium/components/payments/MojoPaymentRequestGateKeeper.java",
"java/src/org/chromium/components/payments/MojoStructCollection.java",
"java/src/org/chromium/components/payments/OriginSecurityChecker.java",
"java/src/org/chromium/components/payments/PackageManagerDelegate.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

package org.chromium.components.payments;

import androidx.annotation.Nullable;

import org.chromium.payments.mojom.PaymentDetails;
import org.chromium.payments.mojom.PaymentMethodData;
import org.chromium.payments.mojom.PaymentOptions;
import org.chromium.payments.mojom.PaymentRequest;
import org.chromium.payments.mojom.PaymentValidationErrors;

/**
Expand All @@ -15,16 +18,18 @@
*/
public interface BrowserPaymentRequest {
/**
* The browser part of the {@link PaymentRequest#init} implementation.
* Initialize the browser part of the {@link PaymentRequest} implementation and validate the raw
* payment request data coming from the untrusted mojo.
* @param methodData The supported methods specified by the merchant.
* @param details The payment details specified by the merchant.
* @param options The payment options specified by the merchant.
* @param options The payment options specified by the merchant, can be null.
* @param googlePayBridgeEligible True when the renderer process deems the current request
* eligible for the skip-to-GPay experimental flow. It is ultimately up to the browser
* process to determine whether to trigger it
* @return whether the initialization is successful.
*/
void init(PaymentMethodData[] methodData, PaymentDetails details, PaymentOptions options,
boolean googlePayBridgeEligible);
boolean initAndValidate(PaymentMethodData[] methodData, PaymentDetails details,
@Nullable PaymentOptions options, boolean googlePayBridgeEligible);

/**
* The browser part of the {@link PaymentRequest#show} implementation.
Expand Down
Loading

0 comments on commit 6731c83

Please sign in to comment.