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

Fix MM 56723 #7883

Merged
merged 11 commits into from
Apr 24, 2024
Merged
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
6 changes: 6 additions & 0 deletions android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ dependencies {
androidTestImplementation('com.wix:detox:+')
implementation project(':reactnativenotifications')
implementation project(':watermelondb-jsi')

api('io.jsonwebtoken:jjwt-api:0.12.5')
runtimeOnly('io.jsonwebtoken:jjwt-impl:0.12.5')
runtimeOnly('io.jsonwebtoken:jjwt-orgjson:0.12.5') {
exclude(group: 'org.json', module: 'json') //provided by Android natively
}
}

configurations.all {
Expand Down
5 changes: 5 additions & 0 deletions android/app/proguard-rules.pro
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,8 @@
# http://developer.android.com/guide/developing/tools/proguard.html

# Add any project specific keep options here:
-keepattributes InnerClasses

-keep class io.jsonwebtoken.** { *; }
-keepnames class io.jsonwebtoken.* { *; }
-keepnames interface io.jsonwebtoken.* { *; }
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import android.os.Build;
import android.os.Bundle;
import android.text.TextUtils;
import android.util.Base64;
import android.util.Log;

import androidx.annotation.NonNull;
Expand All @@ -32,14 +33,24 @@
import com.nozbe.watermelondb.WMDatabase;

import java.io.IOException;
import java.security.KeyFactory;
import java.security.PublicKey;
import java.security.spec.X509EncodedKeySpec;
import java.util.Date;
import java.util.Objects;

import io.jsonwebtoken.IncorrectClaimException;
import io.jsonwebtoken.JwtException;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.MissingClaimException;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;

import static com.mattermost.helpers.database_extension.GeneralKt.getDatabaseForServer;
import static com.mattermost.helpers.database_extension.GeneralKt.getDeviceToken;
import static com.mattermost.helpers.database_extension.SystemKt.queryConfigServerVersion;
import static com.mattermost.helpers.database_extension.SystemKt.queryConfigSigningKey;
import static com.mattermost.helpers.database_extension.UserKt.getLastPictureUpdate;

public class CustomPushNotificationHelper {
Expand Down Expand Up @@ -227,6 +238,154 @@ public static void createNotificationChannels(Context context) {
}
}

public static boolean verifySignature(final Context context, String signature, String serverUrl, String ackId) {
if (signature == null) {
// Backward compatibility with old push proxies
Log.i("Mattermost Notifications Signature verification", "No signature in the notification");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for backward compatibility with older servers? If so, can we add a comment and ticket to make signature verification mandatory after, say, next two ESRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for compatibility with old push proxies. I can add a comment about that.

return true;
}

if (serverUrl == null) {
Log.i("Mattermost Notifications Signature verification", "No server_url for server_id");
return false;
}

DatabaseHelper dbHelper = DatabaseHelper.Companion.getInstance();
if (dbHelper == null) {
Log.i("Mattermost Notifications Signature verification", "Cannot access the database");
return false;
}

WMDatabase db = getDatabaseForServer(dbHelper, context, serverUrl);
if (db == null) {
Log.i("Mattermost Notifications Signature verification", "Cannot access the server database");
return false;
}

if (signature.equals("NO_SIGNATURE")) {
String version = queryConfigServerVersion(db);
if (version == null) {
Log.i("Mattermost Notifications Signature verification", "No server version");
return false;
}

if (!version.matches("[0-9]+(\\.[0-9]+)*")) {
Log.i("Mattermost Notifications Signature verification", "Invalid server version");
return false;
}

String[] parts = version.split("\\.");
int major = parts.length > 0 ? Integer.parseInt(parts[0]) : 0;
int minor = parts.length > 1 ? Integer.parseInt(parts[1]) : 0;
int patch = parts.length > 2 ? Integer.parseInt(parts[2]) : 0;

int[][] targets = {{9,8,0},{9,7,3},{9,6,3},{9,5,5},{8,1,14}};
boolean rejected = false;
for (int i = 0; i < targets.length; i++) {
boolean first = i == 0;
int[] targetVersion = targets[i];
int majorTarget = targetVersion[0];
int minorTarget = targetVersion[1];
int patchTarget = targetVersion[2];

if (major > majorTarget) {
// Only reject if we are considering the first (highest) version.
// Any version in between should be acceptable.
rejected = first;
break;
}

if (major < majorTarget) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should equal value case also be included here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only check the minor version if the version is exactly the same. So... imagine we have the targets 9.8.0 and 7.5.0.

If we are on version 8.0.0, when comparing with 9.8.0, since the major version is lower, we can continue to the next target. But if we are on version 9.9.0, even if the major version is the same, we want to check the minor version.

I hope it makes sense

// Continue to see if it complies with a smaller target
continue;
}

// Same major
if (minor > minorTarget) {
// Only reject if we are considering the first (highest) version.
// Any version in between should be acceptable.
rejected = first;
break;
}

if (minor < minorTarget) {
// Continue to see if it complies with a smaller target
continue;
}

// Same major and same minor
if (patch >= patchTarget) {
rejected = true;
break;
}

// Patch is lower than target
return true;
}

if (rejected) {
Log.i("Mattermost Notifications Signature verification", "Server version should send signature");
return false;
}

// Version number is below any of the targets, so it should not send the signature
return true;
}

String signingKey = queryConfigSigningKey(db);
if (signingKey == null) {
Log.i("Mattermost Notifications Signature verification", "No signing key");
return false;
}

try {
byte[] encoded = Base64.decode(signingKey, 0);
KeyFactory kf = KeyFactory.getInstance("EC");
PublicKey pubKey = (PublicKey) kf.generatePublic(new X509EncodedKeySpec(encoded));

String storedDeviceToken = getDeviceToken(dbHelper);
if (storedDeviceToken == null) {
Log.i("Mattermost Notifications Signature verification", "No device token stored");
return false;
}
String[] tokenParts = storedDeviceToken.split(":", 2);
if (tokenParts.length != 2) {
Log.i("Mattermost Notifications Signature verification", "Wrong stored device token format");
return false;
}
String deviceToken = tokenParts[1].substring(0, tokenParts[1].length() -1 );
if (deviceToken.isEmpty()) {
Log.i("Mattermost Notifications Signature verification", "Empty stored device token");
return false;
}

Jwts.parser()
.require("ack_id", ackId)
larkox marked this conversation as resolved.
Show resolved Hide resolved
.require("device_id", deviceToken)
.verifyWith((PublicKey) pubKey)
.build()
.parseSignedClaims(signature);
} catch (MissingClaimException e) {
Log.i("Mattermost Notifications Signature verification", String.format("Missing claim: %s", e.getMessage()));
e.printStackTrace();
return false;
} catch (IncorrectClaimException e) {
Log.i("Mattermost Notifications Signature verification", String.format("Incorrect claim: %s", e.getMessage()));
e.printStackTrace();
return false;
} catch (JwtException e) {
Log.i("Mattermost Notifications Signature verification", String.format("Cannot verify JWT: %s", e.getMessage()));
e.printStackTrace();
return false;
} catch (Exception e) {
Log.i("Mattermost Notifications Signature verification", String.format("Exception while parsing JWT: %s", e.getMessage()));
e.printStackTrace();
return false;
}

return true;
}

private static Bitmap getCircleBitmap(Bitmap bitmap) {
final Bitmap output = Bitmap.createBitmap(bitmap.getWidth(),
bitmap.getHeight(), Bitmap.Config.ARGB_8888);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fun DatabaseHelper.getDatabaseForServer(context: Context?, serverUrl: String): W
defaultDatabase!!.rawQuery(query, arrayOf(serverUrl)).use { cursor ->
if (cursor.count == 1) {
cursor.moveToFirst()
val databasePath = cursor.getString(0)
val databasePath = String.format("file://%s", cursor.getString(0))
enahum marked this conversation as resolved.
Show resolved Hide resolved
return WMDatabase.getInstance(databasePath, context!!)
}
}
Expand All @@ -67,6 +67,22 @@ fun DatabaseHelper.getDatabaseForServer(context: Context?, serverUrl: String): W
return null
}

fun DatabaseHelper.getDeviceToken(): String? {
try {
val query = "SELECT value FROM Global WHERE id=?"
defaultDatabase!!.rawQuery(query, arrayOf("deviceToken")).use { cursor ->
if (cursor.count == 1) {
cursor.moveToFirst()
return cursor.getString(0);
}
}
} catch (e: Exception) {
e.printStackTrace()
}

return null
}

fun find(db: WMDatabase, tableName: String, id: String?): ReadableMap? {
try {
db.rawQuery(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,11 @@ fun queryConfigDisplayNameSetting(db: WMDatabase): String? {

return null
}

fun queryConfigSigningKey(db: WMDatabase): String? {
return find(db, "Config", "AsymmetricSigningPublicKey")?.getString("value")
}

fun queryConfigServerVersion(db: WMDatabase): String? {
return find(db, "Config", "Version")?.getString("value")
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public void onReceived() {
final String ackId = initialData.getString("ack_id");
final String postId = initialData.getString("post_id");
final String channelId = initialData.getString("channel_id");
final String signature = initialData.getString("signature");
final boolean isIdLoaded = initialData.getString("id_loaded") != null && initialData.getString("id_loaded").equals("true");
int notificationId = NotificationHelper.getNotificationId(initialData);

Expand All @@ -70,6 +71,11 @@ public void onReceived() {
}
}

if (!CustomPushNotificationHelper.verifySignature(mContext, signature, serverUrl, ackId)) {
Log.i("Mattermost Notifications Signature verification", "Notification skipped because we could not verify it.");
return;
}

finishProcessingNotification(serverUrl, type, channelId, notificationId);
}

Expand Down
8 changes: 8 additions & 0 deletions app/constants/push_notification.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {defineMessage} from 'react-intl';

// Needed for localization on iOS native side
export const notVerifiedErrorMessage = defineMessage({
id: 'native.ios.notifications.not_verified',
defaultMessage: 'We could not verify this notification with the server',
});

export const CATEGORY = 'CAN_REPLY';

export const REPLY_ACTION = 'REPLY_ACTION';
Expand Down
10 changes: 9 additions & 1 deletion app/init/push_notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import EphemeralStore from '@store/ephemeral_store';
import NavigationStore from '@store/navigation_store';
import {isBetaApp} from '@utils/general';
import {isMainActivity, isTablet} from '@utils/helpers';
import {logInfo} from '@utils/log';
import {logDebug, logInfo} from '@utils/log';
import {convertToNotificationData} from '@utils/notification';

class PushNotifications {
Expand Down Expand Up @@ -232,6 +232,10 @@ class PushNotifications {

// This triggers when the app was in the background (iOS)
onNotificationReceivedBackground = async (incoming: Notification, completion: (response: NotificationBackgroundFetchResult) => void) => {
if (incoming.payload.verified === 'false') {
logDebug('not handling background notification because it was not verified, ackId=', incoming.payload.ackId);
return;
}
const notification = convertToNotificationData(incoming, false);
this.processNotification(notification);

Expand All @@ -241,6 +245,10 @@ class PushNotifications {
// This triggers when the app was in the foreground (Android and iOS)
// Also triggers when the app was in the background (Android)
onNotificationReceivedForeground = (incoming: Notification, completion: (response: NotificationCompletion) => void) => {
if (incoming.payload.verified === 'false') {
logDebug('not handling foreground notification because it was not verified, ackId=', incoming.payload.ackId);
return;
}
const notification = convertToNotificationData(incoming, false);
if (AppState.currentState !== 'inactive') {
notification.foreground = AppState.currentState === 'active' && isMainActivity();
Expand Down
1 change: 1 addition & 0 deletions assets/base/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@
"more_messages.text": "{count} new {count, plural, one {message} other {messages}}",
"msg_typing.areTyping": "{users} and {last} are typing...",
"msg_typing.isTyping": "{user} is typing...",
"native.ios.notifications.not_verified": "We could not verify this notification with the server",
"notification_settings.auto_responder": "Automatic Replies",
"notification_settings.auto_responder.default_message": "Hello, I am out of office and unable to respond to messages.",
"notification_settings.auto_responder.footer.message": "Set a custom message that is automatically sent in response to direct messages, such as an out of office or vacation reply. Enabling this setting changes your status to Out of Office and disables notifications.",
Expand Down
6 changes: 4 additions & 2 deletions ios/Gekidou/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@ let package = Package(
],
dependencies: [
// Dependencies declare other packages that this package depends on.
.package(url: "https://github.com/stephencelis/SQLite.swift.git", from: "0.14.1")
.package(url: "https://github.com/stephencelis/SQLite.swift.git", from: "0.14.1"),
.package(url: "https://github.com/Kitura/Swift-JWT.git", from:"3.6.1")
],
targets: [
// Targets are the basic building blocks of a package. A target can define a module or a test suite.
// Targets can depend on other targets in this package, and on products in packages this package depends on.
.target(
name: "Gekidou",
dependencies: [
.product(name: "SQLite", package: "SQLite.swift")
.product(name: "SQLite", package: "SQLite.swift"),
.product(name: "SwiftJWT", package: "Swift-JWT"),
]
),
.testTarget(
Expand Down
Loading
Loading