Bug 986664 - Make Android FxAccountClient* HAWK requests always include request payload hash. r=rnewman
Relanding with correct bug number.
--- a/mobile/android/base/background/fxa/FxAccountClient10.java
+++ b/mobile/android/base/background/fxa/FxAccountClient10.java
@@ -171,42 +171,45 @@ public class FxAccountClient10 {
*/
protected abstract class ResourceDelegate<T> extends BaseResourceDelegate {
protected abstract void handleSuccess(final int status, HttpResponse response, final ExtendedJSONObject body);
protected final RequestDelegate<T> delegate;
protected final byte[] tokenId;
protected final byte[] reqHMACKey;
- protected final boolean payload;
protected final SkewHandler skewHandler;
/**
* Create a delegate for an un-authenticated resource.
*/
public ResourceDelegate(final Resource resource, final RequestDelegate<T> delegate) {
- this(resource, delegate, null, null, false);
+ this(resource, delegate, null, null);
}
/**
* Create a delegate for a Hawk-authenticated resource.
+ * <p>
+ * Every Hawk request that encloses an entity (PATCH, POST, and PUT) will
+ * include the payload verification hash.
*/
- public ResourceDelegate(final Resource resource, final RequestDelegate<T> delegate, final byte[] tokenId, final byte[] reqHMACKey, final boolean authenticatePayload) {
+ public ResourceDelegate(final Resource resource, final RequestDelegate<T> delegate, final byte[] tokenId, final byte[] reqHMACKey) {
super(resource);
this.delegate = delegate;
this.reqHMACKey = reqHMACKey;
this.tokenId = tokenId;
- this.payload = authenticatePayload;
this.skewHandler = SkewHandler.getSkewHandlerForResource(resource);
}
@Override
public AuthHeaderProvider getAuthHeaderProvider() {
if (tokenId != null && reqHMACKey != null) {
- return new HawkAuthHeaderProvider(Utils.byte2Hex(tokenId), reqHMACKey, payload, skewHandler.getSkewInSeconds());
+ // We always include the payload verification hash for FxA Hawk-authenticated requests.
+ final boolean includePayloadVerificationHash = true;
+ return new HawkAuthHeaderProvider(Utils.byte2Hex(tokenId), reqHMACKey, includePayloadVerificationHash, skewHandler.getSkewInSeconds());
}
return super.getAuthHeaderProvider();
}
@Override
public String getUserAgent() {
return FxAccountConstants.USER_AGENT;
}
@@ -478,17 +481,17 @@ public class FxAccountClient10 {
BaseResource resource;
try {
resource = new BaseResource(new URI(serverURI + "session/create"));
} catch (URISyntaxException e) {
invokeHandleError(delegate, e);
return;
}
- resource.delegate = new ResourceDelegate<TwoTokens>(resource, delegate, tokenId, reqHMACKey, false) {
+ resource.delegate = new ResourceDelegate<TwoTokens>(resource, delegate, tokenId, reqHMACKey) {
@Override
public void handleSuccess(int status, HttpResponse response, ExtendedJSONObject body) {
try {
byte[] keyFetchToken = new byte[32];
byte[] sessionToken = new byte[32];
unbundleBody(body, requestKey, FxAccountUtils.KW("session/create"), keyFetchToken, sessionToken);
delegate.handleSuccess(new TwoTokens(keyFetchToken, sessionToken));
return;
@@ -514,17 +517,17 @@ public class FxAccountClient10 {
BaseResource resource;
try {
resource = new BaseResource(new URI(serverURI + "session/destroy"));
} catch (URISyntaxException e) {
invokeHandleError(delegate, e);
return;
}
- resource.delegate = new ResourceDelegate<Void>(resource, delegate, tokenId, reqHMACKey, false) {
+ resource.delegate = new ResourceDelegate<Void>(resource, delegate, tokenId, reqHMACKey) {
@Override
public void handleSuccess(int status, HttpResponse response, ExtendedJSONObject body) {
delegate.handleSuccess(null);
}
};
post(resource, null, delegate);
}
@@ -603,17 +606,17 @@ public class FxAccountClient10 {
BaseResource resource;
try {
resource = new BaseResource(new URI(serverURI + "account/keys"));
} catch (URISyntaxException e) {
invokeHandleError(delegate, e);
return;
}
- resource.delegate = new ResourceDelegate<TwoKeys>(resource, delegate, tokenId, reqHMACKey, false) {
+ resource.delegate = new ResourceDelegate<TwoKeys>(resource, delegate, tokenId, reqHMACKey) {
@Override
public void handleSuccess(int status, HttpResponse response, ExtendedJSONObject body) {
try {
byte[] kA = new byte[FxAccountUtils.CRYPTO_KEY_LENGTH_BYTES];
byte[] wrapkB = new byte[FxAccountUtils.CRYPTO_KEY_LENGTH_BYTES];
unbundleBody(body, requestKey, FxAccountUtils.KW("account/keys"), kA, wrapkB);
delegate.handleSuccess(new TwoKeys(kA, wrapkB));
return;
@@ -665,17 +668,17 @@ public class FxAccountClient10 {
BaseResource resource;
try {
resource = new BaseResource(new URI(serverURI + "recovery_email/status"));
} catch (URISyntaxException e) {
invokeHandleError(delegate, e);
return;
}
- resource.delegate = new ResourceDelegate<StatusResponse>(resource, delegate, tokenId, reqHMACKey, false) {
+ resource.delegate = new ResourceDelegate<StatusResponse>(resource, delegate, tokenId, reqHMACKey) {
@Override
public void handleSuccess(int status, HttpResponse response, ExtendedJSONObject body) {
try {
String[] requiredStringFields = new String[] { JSON_KEY_EMAIL };
body.throwIfFieldsMissingOrMisTyped(requiredStringFields, String.class);
String email = body.getString(JSON_KEY_EMAIL);
Boolean verified = body.getBoolean(JSON_KEY_VERIFIED);
delegate.handleSuccess(new StatusResponse(email, verified));
@@ -707,17 +710,17 @@ public class FxAccountClient10 {
BaseResource resource;
try {
resource = new BaseResource(new URI(serverURI + "certificate/sign"));
} catch (URISyntaxException e) {
invokeHandleError(delegate, e);
return;
}
- resource.delegate = new ResourceDelegate<String>(resource, delegate, tokenId, reqHMACKey, true) {
+ resource.delegate = new ResourceDelegate<String>(resource, delegate, tokenId, reqHMACKey) {
@Override
public void handleSuccess(int status, HttpResponse response, ExtendedJSONObject body) {
String cert = body.getString("cert");
if (cert == null) {
delegate.handleError(new FxAccountClientException("cert must be a non-null string"));
return;
}
delegate.handleSuccess(cert);
@@ -748,17 +751,17 @@ public class FxAccountClient10 {
BaseResource resource;
try {
resource = new BaseResource(new URI(serverURI + "recovery_email/resend_code"));
} catch (URISyntaxException e) {
invokeHandleError(delegate, e);
return;
}
- resource.delegate = new ResourceDelegate<Void>(resource, delegate, tokenId, reqHMACKey, false) {
+ resource.delegate = new ResourceDelegate<Void>(resource, delegate, tokenId, reqHMACKey) {
@Override
public void handleSuccess(int status, HttpResponse response, ExtendedJSONObject body) {
try {
delegate.handleSuccess(null);
return;
} catch (Exception e) {
delegate.handleError(e);
return;
deleted file mode 100644
--- a/mobile/android/base/fxa/activities/FxAccountCreateAccountFragment.java
+++ /dev/null
@@ -1,191 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.gecko.fxa.activities;
-
-import android.support.v4.app.Fragment;
-
-public class FxAccountCreateAccountFragment extends Fragment { // implements OnClickListener {
- protected static final String LOG_TAG = FxAccountCreateAccountFragment.class.getSimpleName();
-//
-// protected FxAccountSetupActivity activity;
-//
-// protected EditText emailEdit;
-// protected EditText passwordEdit;
-// protected EditText password2Edit;
-// protected Button button;
-//
-// protected TextView emailError;
-// protected TextView passwordError;
-//
-// protected TextChangedListener textChangedListener;
-// protected EditorActionListener editorActionListener;
-// protected OnFocusChangeListener focusChangeListener;
-// @Override
-// public void onCreate(Bundle savedInstanceState) {
-// super.onCreate(savedInstanceState);
-// // Retain this fragment across configuration changes. See, for example,
-// // http://www.androiddesignpatterns.com/2013/04/retaining-objects-across-config-changes.html
-// // This fragment will own AsyncTask instances which should not be
-// // interrupted by configuration changes (and activity changes).
-// setRetainInstance(true);
-// }
-
-// @Override
-// public View onCreateView(LayoutInflater inflater, ViewGroup container,
-// Bundle savedInstanceState) {
-// View v = inflater.inflate(R.layout.fxaccount_create_account_fragment, container, false);
-//
-// FxAccountSetupActivity.linkifyTextViews(v, new int[] { R.id.description, R.id.policy });
-//
-// emailEdit = (EditText) ensureFindViewById(v, R.id.email, "email");
-// passwordEdit = (EditText) ensureFindViewById(v, R.id.password, "password");
-// // Second password can be null.
-// password2Edit = (EditText) v.findViewById(R.id.password2);
-//
-// emailError = (TextView) ensureFindViewById(v, R.id.email_error, "email error");
-// passwordError = (TextView) ensureFindViewById(v, R.id.password_error, "password error");
-//
-// textChangedListener = new TextChangedListener();
-// editorActionListener = new EditorActionListener();
-// focusChangeListener = new FocusChangeListener();
-//
-// addListeners(emailEdit);
-// addListeners(passwordEdit);
-// if (password2Edit != null) {
-// addListeners(password2Edit);
-// }
-//
-// button = (Button) ensureFindViewById(v, R.id.create_account_button, "button");
-// button.setOnClickListener(this);
-// return v;
-// }
-
-// protected void onCreateAccount(View button) {
-// Logger.debug(LOG_TAG, "onCreateAccount: Asking for username/password for new account.");
-// String email = emailEdit.getText().toString();
-// String password = passwordEdit.getText().toString();
-// activity.signUp(email, password);
-// }
-//
-// @Override
-// public void onClick(View v) {
-// switch (v.getId()) {
-// case R.id.create_account_button:
-// if (!validate(false)) {
-// return;
-// }
-// onCreateAccount(v);
-// break;
-// }
-// }
-//
-// protected void addListeners(EditText editText) {
-// editText.addTextChangedListener(textChangedListener);
-// editText.setOnEditorActionListener(editorActionListener);
-// editText.setOnFocusChangeListener(focusChangeListener);
-// }
-//
-// protected class FocusChangeListener implements OnFocusChangeListener {
-// @Override
-// public void onFocusChange(View v, boolean hasFocus) {
-// if (hasFocus) {
-// return;
-// }
-// validate(false);
-// }
-// }
-//
-// protected class EditorActionListener implements OnEditorActionListener {
-// @Override
-// public boolean onEditorAction(TextView v, int actionId, KeyEvent event) {
-// validate(false);
-// return false;
-// }
-// }
-//
-// protected class TextChangedListener implements TextWatcher {
-// @Override
-// public void afterTextChanged(Editable s) {
-// validate(true);
-// }
-//
-// @Override
-// public void beforeTextChanged(CharSequence s, int start, int count, int after) {
-// // Do nothing.
-// }
-//
-// @Override
-// public void onTextChanged(CharSequence s, int start, int before, int count) {
-// // Do nothing.
-// }
-// }
-//
-// /**
-// * Show or hide error messaging.
-// *
-// * @param removeOnly
-// * if true, possibly remove existing error messages but do not set an
-// * error message if one was not present.
-// * @param errorResourceId
-// * of error string, or -1 to hide.
-// * @param errorView
-// * <code>TextView</code> instance to display error message in.
-// * @param edits
-// * <code>EditText</code> instances to style.
-// */
-// protected void setError(boolean removeOnly, int errorResourceId, TextView errorView, EditText... edits) {
-// if (removeOnly && errorResourceId != -1) {
-// return;
-// }
-//
-// int res = errorResourceId == -1 ? R.drawable.fxaccount_textfield_background : R.drawable.fxaccount_textfield_error_background;
-// for (EditText edit : edits) {
-// if (edit == null) {
-// continue;
-// }
-// edit.setBackgroundResource(res);
-// }
-// if (errorResourceId == -1) {
-// errorView.setVisibility(View.GONE);
-// errorView.setText(null);
-// } else {
-// errorView.setText(errorResourceId);
-// errorView.setVisibility(View.VISIBLE);
-// }
-// }
-//
-// protected boolean validate(boolean removeOnly) {
-// boolean enabled = true;
-// final String email = emailEdit.getText().toString();
-// final String password = passwordEdit.getText().toString();
-//
-// if (email.length() == 0 || Patterns.EMAIL_ADDRESS.matcher(email).matches()) {
-// setError(removeOnly, -1, emailError, emailEdit);
-// } else {
-// enabled = false;
-// setError(removeOnly, R.string.fxaccount_bad_email, emailError, emailEdit);
-// }
-//
-// if (password2Edit != null) {
-// final String password2 = password2Edit.getText().toString();
-// enabled = enabled && password2.length() > 0;
-//
-// boolean passwordsMatch = password.equals(password2);
-// if (passwordsMatch) {
-// setError(removeOnly, -1, passwordError, passwordEdit, password2Edit);
-// } else {
-// enabled = false;
-// setError(removeOnly, R.string.fxaccount_bad_passwords, passwordError, passwordEdit, password2Edit);
-// }
-// }
-//
-// if (enabled != button.isEnabled()) {
-// Logger.debug(LOG_TAG, (enabled ? "En" : "Dis") + "abling button.");
-// button.setEnabled(enabled);
-// }
-//
-// return enabled;
-// }
-}
--- a/mobile/android/base/fxa/sync/FxAccountSyncAdapter.java
+++ b/mobile/android/base/fxa/sync/FxAccountSyncAdapter.java
@@ -360,17 +360,22 @@ public class FxAccountSyncAdapter extend
ClientsDataDelegate clientsDataDelegate = new SharedPreferencesClientsDataDelegate(sharedPrefs);
// We compute skew over time using SkewHandler. This yields an unchanging
// skew adjustment that the HawkAuthHeaderProvider uses to adjust its
// timestamps. Eventually we might want this to adapt within the scope of a
// global session.
final SkewHandler storageServerSkewHandler = SkewHandler.getSkewHandlerForHostname(storageHostname);
final long storageServerSkew = storageServerSkewHandler.getSkewInSeconds();
- final AuthHeaderProvider authHeaderProvider = new HawkAuthHeaderProvider(token.id, token.key.getBytes("UTF-8"), false, storageServerSkew);
+ // We expect Sync to upload large sets of records. Calculating the
+ // payload verification hash for these record sets could be expensive,
+ // so we explicitly do not send payload verification hashes to the
+ // Sync storage endpoint.
+ final boolean includePayloadVerificationHash = false;
+ final AuthHeaderProvider authHeaderProvider = new HawkAuthHeaderProvider(token.id, token.key.getBytes("UTF-8"), includePayloadVerificationHash, storageServerSkew);
final Context context = getContext();
final SyncConfiguration syncConfig = new SyncConfiguration(token.uid, authHeaderProvider, sharedPrefs, syncKeyBundle);
Collection<String> knownStageNames = SyncConfiguration.validEngineNames();
syncConfig.stagesToSync = Utils.getStagesToSyncFromBundle(knownStageNames, extras);
syncConfig.setClusterURL(storageServerURI);
--- a/mobile/android/base/sync/net/HawkAuthHeaderProvider.java
+++ b/mobile/android/base/sync/net/HawkAuthHeaderProvider.java
@@ -13,16 +13,17 @@ import java.security.InvalidKeyException
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Locale;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import org.mozilla.apache.commons.codec.binary.Base64;
+import org.mozilla.gecko.background.common.log.Logger;
import org.mozilla.gecko.sync.Utils;
import ch.boye.httpclientandroidlib.Header;
import ch.boye.httpclientandroidlib.HttpEntity;
import ch.boye.httpclientandroidlib.HttpEntityEnclosingRequest;
import ch.boye.httpclientandroidlib.client.methods.HttpRequestBase;
import ch.boye.httpclientandroidlib.client.methods.HttpUriRequest;
import ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
@@ -50,24 +51,30 @@ public class HawkAuthHeaderProvider impl
protected final boolean includePayloadHash;
protected final long skewSeconds;
/**
* Create a Hawk Authorization header provider.
* <p>
* Hawk specifies no mechanism by which a client receives an
* identifier-and-key pair from the server.
+ * <p>
+ * Hawk requests can include a payload verification hash with requests that
+ * enclose an entity (PATCH, POST, and PUT requests). <b>You should default
+ * to including the payload verification hash<b> unless you have a good reason
+ * not to -- the server can always ignore payload verification hashes provided
+ * by the client.
*
* @param id
* to name requests with.
* @param key
* to sign request with.
*
* @param includePayloadHash
- * true if message integrity hash should be included in signed
+ * true if payload verification hash should be included in signed
* request header. See <a href="https://github.com/hueniverse/hawk#payload-validation">https://github.com/hueniverse/hawk#payload-validation</a>.
*
* @param skewSeconds
* a number of seconds by which to skew the current time when
* computing a header.
*/
public HawkAuthHeaderProvider(String id, byte[] key, boolean includePayloadHash, long skewSeconds) {
if (id == null) {
@@ -77,21 +84,16 @@ public class HawkAuthHeaderProvider impl
throw new IllegalArgumentException("key must not be null");
}
this.id = id;
this.key = key;
this.includePayloadHash = includePayloadHash;
this.skewSeconds = skewSeconds;
}
- public HawkAuthHeaderProvider(String id, byte[] key, boolean includePayloadHash) {
- this(id, key, includePayloadHash, 0L);
- }
-
-
/**
* @return the current time in milliseconds.
*/
@SuppressWarnings("static-method")
protected long now() {
return System.currentTimeMillis();
}
@@ -136,24 +138,19 @@ public class HawkAuthHeaderProvider impl
throw new IllegalArgumentException("nonce must not be null.");
}
if (nonce.length() == 0) {
throw new IllegalArgumentException("nonce must not be empty.");
}
String payloadHash = null;
if (includePayloadHash) {
- if (!(request instanceof HttpEntityEnclosingRequest)) {
- throw new IllegalArgumentException("cannot specify payload for request without an entity");
- }
- HttpEntity entity = ((HttpEntityEnclosingRequest) request).getEntity();
- if (entity == null) {
- throw new IllegalArgumentException("cannot specify payload for request with a null entity");
- }
- payloadHash = Base64.encodeBase64String(getPayloadHash(entity));
+ payloadHash = getPayloadHashString(request);
+ } else {
+ Logger.debug(LOG_TAG, "Configured to not include payload hash for this request.");
}
String app = null;
String dlg = null;
String requestString = getRequestString(request, "header", timestamp, nonce, payloadHash, extra, app, dlg);
String macString = getSignature(requestString.getBytes("UTF-8"), this.key);
StringBuilder sb = new StringBuilder();
@@ -179,16 +176,48 @@ public class HawkAuthHeaderProvider impl
sb.append("mac=\"");
sb.append(macString);
sb.append("\"");
return new BasicHeader("Authorization", sb.toString());
}
/**
+ * Get the payload verification hash for the given request, if possible.
+ * <p>
+ * Returns null if the request does not enclose an entity (is not an HTTP
+ * PATCH, POST, or PUT). Throws if the payload verification hash cannot be
+ * computed.
+ *
+ * @param request
+ * to compute hash for.
+ * @return verification hash, or null if the request does not enclose an entity.
+ * @throws IllegalArgumentException if the request does not enclose a valid non-null entity.
+ * @throws UnsupportedEncodingException
+ * @throws NoSuchAlgorithmException
+ * @throws IOException
+ */
+ protected static String getPayloadHashString(HttpRequestBase request)
+ throws UnsupportedEncodingException, NoSuchAlgorithmException, IOException, IllegalArgumentException {
+ final boolean shouldComputePayloadHash = request instanceof HttpEntityEnclosingRequest;
+ if (!shouldComputePayloadHash) {
+ Logger.debug(LOG_TAG, "Not computing payload verification hash for non-enclosing request.");
+ return null;
+ }
+ if (!(request instanceof HttpEntityEnclosingRequest)) {
+ throw new IllegalArgumentException("Cannot compute payload verification hash for enclosing request without an entity");
+ }
+ final HttpEntity entity = ((HttpEntityEnclosingRequest) request).getEntity();
+ if (entity == null) {
+ throw new IllegalArgumentException("Cannot compute payload verification hash for enclosing request with a null entity");
+ }
+ return Base64.encodeBase64String(getPayloadHash(entity));
+ }
+
+ /**
* Escape the user-provided extra string for the ext="" header attribute.
* <p>
* Hawk escapes the header ext="" attribute differently than it does the extra
* line in the normalized request string.
* <p>
* See <a href="https://github.com/hueniverse/hawk/blob/871cc597973110900467bd3dfb84a3c892f678fb/lib/browser.js#L385">https://github.com/hueniverse/hawk/blob/871cc597973110900467bd3dfb84a3c892f678fb/lib/browser.js#L385</a>.
*
* @param extra to escape.