Skip to content

Commit ad00939

Browse files
authored
Hotfix/audit fixes (#1193)
* Fix - Application Allows Cleartext Traffic * Fix: Insecure Keychain Protection Class * fix: Local Authentication Bypass * remove console.logs * feat: Add migrateToSecureKeychain function placeholder for web * Improve clearText traffic fix * update review comments * update review comments
1 parent c68ef2b commit ad00939

File tree

13 files changed

+522
-76
lines changed

13 files changed

+522
-76
lines changed

app/android/app/src/debug/AndroidManifest.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@
1212
android:usesCleartextTraffic="true"
1313
tools:targetApi="28"
1414
tools:ignore="GoogleAppIndexingWarning"
15+
tools:replace="android:usesCleartextTraffic"
1516
/>
1617
</manifest>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<network-security-config>
3+
<!-- Debug configuration: Allow cleartext traffic for local development -->
4+
<base-config cleartextTrafficPermitted="true">
5+
<trust-anchors>
6+
<!-- Trust system certificates -->
7+
<certificates src="system" />
8+
9+
<!-- Trust user-added certificates in debug build -->
10+
<certificates src="user" />
11+
</trust-anchors>
12+
</base-config>
13+
14+
<!-- Explicitly allow cleartext for common development endpoints -->
15+
<domain-config cleartextTrafficPermitted="true">
16+
<domain includeSubdomains="true">localhost</domain>
17+
<domain includeSubdomains="false">10.0.2.2</domain>
18+
<domain includeSubdomains="false">127.0.0.1</domain>
19+
</domain-config>
20+
</network-security-config>
21+

app/android/app/src/main/AndroidManifest.xml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818
android:icon="@mipmap/ic_launcher"
1919
android:roundIcon="@mipmap/ic_launcher"
2020
android:extractNativeLibs="false"
21-
tools:replace="android:icon, android:roundIcon, android:name, android:extractNativeLibs"
21+
tools:replace="android:icon, android:roundIcon, android:name, android:extractNativeLibs"
2222
android:theme="@style/AppTheme"
2323
android:supportsRtl="true"
24-
android:usesCleartextTraffic="true"
24+
android:usesCleartextTraffic="false"
25+
android:networkSecurityConfig="@xml/network_security_config"
2526
>
2627
<activity
2728
android:name=".MainActivity"
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<network-security-config>
3+
<!-- Base configuration: Disallow cleartext traffic by default (production security) -->
4+
<base-config cleartextTrafficPermitted="false">
5+
<trust-anchors>
6+
<!-- Trust only system certificates -->
7+
<certificates src="system" />
8+
</trust-anchors>
9+
</base-config>
10+
</network-security-config>

app/src/navigation/system.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import React from 'react';
66
import { SystemBars } from 'react-native-edge-to-edge';
77
import type { NativeStackNavigationOptions } from '@react-navigation/native-stack';
88

9+
import type { DocumentCategory } from '@selfxyz/common/utils/types';
10+
911
import DeferredLinkingInfoScreen from '@/screens/system/DeferredLinkingInfoScreen';
1012
import LaunchScreen from '@/screens/system/LaunchScreen';
1113
import LoadingScreen from '@/screens/system/Loading';
@@ -24,6 +26,11 @@ const systemScreens = {
2426
options: {
2527
headerShown: false,
2628
} as NativeStackNavigationOptions,
29+
params: {} as {
30+
documentCategory?: DocumentCategory;
31+
signatureAlgorithm?: string;
32+
curveOrExponent?: string;
33+
},
2734
},
2835
Modal: {
2936
screen: ModalScreen,

app/src/providers/authProvider.tsx

Lines changed: 131 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,64 @@ import React, {
1212
useState,
1313
} from 'react';
1414
import ReactNativeBiometrics from 'react-native-biometrics';
15-
import Keychain from 'react-native-keychain';
15+
import Keychain, { GetOptions, SetOptions } from 'react-native-keychain';
1616

1717
import { AuthEvents } from '@selfxyz/mobile-sdk-alpha/constants/analytics';
1818

1919
import { useSettingStore } from '@/stores/settingStore';
2020
import type { Mnemonic } from '@/types/mnemonic';
2121
import analytics from '@/utils/analytics';
22+
import {
23+
createKeychainOptions,
24+
detectSecurityCapabilities,
25+
GetSecureOptions,
26+
} from '@/utils/keychainSecurity';
2227

2328
const { trackEvent } = analytics();
2429

2530
const SERVICE_NAME = 'secret';
2631

2732
type SignedPayload<T> = { signature: string; data: T };
33+
type KeychainOptions = {
34+
getOptions: GetOptions;
35+
setOptions: SetOptions;
36+
};
2837
const _getSecurely = async function <T>(
29-
fn: () => Promise<string | false>,
38+
fn: (keychainOptions: KeychainOptions) => Promise<string | false>,
3039
formatter: (dataString: string) => T,
40+
options: GetSecureOptions,
3141
): Promise<SignedPayload<T> | null> {
32-
const dataString = await fn();
33-
if (dataString === false) {
34-
return null;
42+
try {
43+
const capabilities = await detectSecurityCapabilities();
44+
const { getOptions, setOptions } = await createKeychainOptions(
45+
options,
46+
capabilities,
47+
);
48+
const dataString = await fn({ getOptions, setOptions });
49+
if (dataString === false) {
50+
return null;
51+
}
52+
53+
trackEvent(AuthEvents.BIOMETRIC_AUTH_SUCCESS);
54+
return {
55+
signature: 'authenticated',
56+
data: formatter(dataString),
57+
};
58+
} catch (error: unknown) {
59+
const message = error instanceof Error ? error.message : String(error);
60+
trackEvent(AuthEvents.BIOMETRIC_AUTH_FAILED, {
61+
reason: 'unknown_error',
62+
error: message,
63+
});
64+
throw error;
3565
}
66+
};
3667

68+
const _getWithBiometrics = async function <T>(
69+
fn: () => Promise<string | false>,
70+
formatter: (dataString: string) => T,
71+
options: GetSecureOptions,
72+
): Promise<SignedPayload<T> | null> {
3773
try {
3874
const simpleCheck = await biometrics.simplePrompt({
3975
promptMessage: 'Allow access to identity',
@@ -47,13 +83,16 @@ const _getSecurely = async function <T>(
4783
throw new Error('Authentication failed');
4884
}
4985

50-
trackEvent(AuthEvents.BIOMETRIC_AUTH_SUCCESS);
86+
const dataString = await fn();
87+
if (dataString === false) {
88+
return null;
89+
}
90+
5191
return {
5292
signature: 'authenticated',
5393
data: formatter(dataString),
5494
};
5595
} catch (error: unknown) {
56-
console.error('Error in _getSecurely:', error);
5796
const message = error instanceof Error ? error.message : String(error);
5897
trackEvent(AuthEvents.BIOMETRIC_AUTH_FAILED, {
5998
reason: 'unknown_error',
@@ -79,7 +118,10 @@ async function checkBiometricsAvailable(): Promise<boolean> {
79118
}
80119
}
81120

82-
async function restoreFromMnemonic(mnemonic: string): Promise<string | false> {
121+
async function restoreFromMnemonic(
122+
mnemonic: string,
123+
options: KeychainOptions,
124+
): Promise<string | false> {
83125
if (!mnemonic || !ethers.Mnemonic.isValidMnemonic(mnemonic)) {
84126
trackEvent(AuthEvents.MNEMONIC_RESTORE_FAILED, {
85127
reason: 'invalid_mnemonic',
@@ -91,6 +133,7 @@ async function restoreFromMnemonic(mnemonic: string): Promise<string | false> {
91133
const restoredWallet = ethers.Wallet.fromPhrase(mnemonic);
92134
const data = JSON.stringify(restoredWallet.mnemonic);
93135
await Keychain.setGenericPassword('secret', data, {
136+
...options.setOptions,
94137
service: SERVICE_NAME,
95138
});
96139
trackEvent(AuthEvents.MNEMONIC_RESTORE_SUCCESS);
@@ -104,8 +147,14 @@ async function restoreFromMnemonic(mnemonic: string): Promise<string | false> {
104147
}
105148
}
106149

107-
async function loadOrCreateMnemonic(): Promise<string | false> {
150+
async function loadOrCreateMnemonic(
151+
keychainOptions: KeychainOptions,
152+
): Promise<string | false> {
153+
// Get adaptive security configuration
154+
const { setOptions, getOptions } = keychainOptions;
155+
108156
const storedMnemonic = await Keychain.getGenericPassword({
157+
...getOptions,
109158
service: SERVICE_NAME,
110159
});
111160
if (storedMnemonic) {
@@ -129,7 +178,9 @@ async function loadOrCreateMnemonic(): Promise<string | false> {
129178
ethers.Mnemonic.fromEntropy(ethers.randomBytes(32)),
130179
);
131180
const data = JSON.stringify(mnemonic);
181+
132182
await Keychain.setGenericPassword('secret', data, {
183+
...setOptions,
133184
service: SERVICE_NAME,
134185
});
135186
trackEvent(AuthEvents.MNEMONIC_CREATED);
@@ -154,6 +205,7 @@ interface IAuthContext {
154205
isAuthenticating: boolean;
155206
loginWithBiometrics: () => Promise<void>;
156207
_getSecurely: typeof _getSecurely;
208+
_getWithBiometrics: typeof _getWithBiometrics;
157209
getOrCreateMnemonic: () => Promise<SignedPayload<Mnemonic> | null>;
158210
restoreAccountFromMnemonic: (
159211
mnemonic: string,
@@ -165,6 +217,7 @@ export const AuthContext = createContext<IAuthContext>({
165217
isAuthenticating: false,
166218
loginWithBiometrics: () => Promise.resolve(),
167219
_getSecurely,
220+
_getWithBiometrics,
168221
getOrCreateMnemonic: () => Promise.resolve(null),
169222
restoreAccountFromMnemonic: () => Promise.resolve(null),
170223
checkBiometricsAvailable: () => Promise.resolve(false),
@@ -219,15 +272,25 @@ export const AuthProvider = ({
219272
}, [authenticationTimeoutinMs, isAuthenticatingPromise]);
220273

221274
const getOrCreateMnemonic = useCallback(
222-
() => _getSecurely<Mnemonic>(loadOrCreateMnemonic, str => JSON.parse(str)),
275+
() =>
276+
_getSecurely<Mnemonic>(
277+
keychainOptions => loadOrCreateMnemonic(keychainOptions),
278+
str => JSON.parse(str),
279+
{
280+
requireAuth: false,
281+
},
282+
),
223283
[],
224284
);
225285

226286
const restoreAccountFromMnemonic = useCallback(
227287
(mnemonic: string) =>
228288
_getSecurely<boolean>(
229-
() => restoreFromMnemonic(mnemonic),
289+
keychainOptions => restoreFromMnemonic(mnemonic, keychainOptions),
230290
str => !!str,
291+
{
292+
requireAuth: true,
293+
},
231294
),
232295
[],
233296
);
@@ -241,6 +304,7 @@ export const AuthProvider = ({
241304
restoreAccountFromMnemonic,
242305
checkBiometricsAvailable,
243306
_getSecurely,
307+
_getWithBiometrics,
244308
}),
245309
[
246310
getOrCreateMnemonic,
@@ -259,6 +323,54 @@ export async function hasSecretStored() {
259323
return !!seed;
260324
}
261325

326+
// Migrates existing mnemonic to use new security settings with accessControl.
327+
export async function migrateToSecureKeychain(): Promise<boolean> {
328+
try {
329+
const { hasCompletedKeychainMigration, setKeychainMigrationCompleted } =
330+
useSettingStore.getState();
331+
332+
if (hasCompletedKeychainMigration) {
333+
return false;
334+
}
335+
336+
// we try to get with old settings (no accessControl)
337+
const existingMnemonic = await Keychain.getGenericPassword({
338+
service: SERVICE_NAME,
339+
});
340+
341+
if (!existingMnemonic) {
342+
setKeychainMigrationCompleted();
343+
return false;
344+
}
345+
346+
const capabilities = await detectSecurityCapabilities();
347+
const { setOptions } = await createKeychainOptions(
348+
{ requireAuth: true },
349+
capabilities,
350+
);
351+
352+
await Keychain.setGenericPassword(SERVICE_NAME, existingMnemonic.password, {
353+
...setOptions,
354+
service: SERVICE_NAME,
355+
});
356+
357+
trackEvent(AuthEvents.MNEMONIC_CREATED, { migrated: true });
358+
359+
setKeychainMigrationCompleted();
360+
361+
return true;
362+
} catch (error: unknown) {
363+
console.error('Error during keychain migration:', error);
364+
const message = error instanceof Error ? error.message : String(error);
365+
trackEvent(AuthEvents.MNEMONIC_RESTORE_FAILED, {
366+
reason: 'migration_failed',
367+
error: message,
368+
});
369+
370+
return false;
371+
}
372+
}
373+
262374
export async function unsafe_clearSecrets() {
263375
if (__DEV__) {
264376
await Keychain.resetGenericPassword({ service: SERVICE_NAME });
@@ -269,8 +381,14 @@ export async function unsafe_clearSecrets() {
269381
* The only reason this is exported without being locked behind user biometrics is to allow `loadPassportDataAndSecret`
270382
* to access both the privatekey and the passport data with the user only authenticating once
271383
*/
272-
export async function unsafe_getPrivateKey() {
273-
const foundMnemonic = await loadOrCreateMnemonic();
384+
export async function unsafe_getPrivateKey(keychainOptions?: KeychainOptions) {
385+
const options =
386+
keychainOptions ||
387+
(await createKeychainOptions({
388+
requireAuth: true,
389+
}));
390+
391+
const foundMnemonic = await loadOrCreateMnemonic(options);
274392
if (!foundMnemonic) {
275393
return null;
276394
}

app/src/providers/authProvider.web.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ const _getSecurely = async function <T>(
109109
data: formatter(dataString),
110110
};
111111
} catch (error: unknown) {
112-
console.error('Error in _getSecurely:', error);
113112
const message = error instanceof Error ? error.message : String(error);
114113
trackEvent(AuthEvents.BIOMETRIC_AUTH_FAILED, {
115114
reason: 'unknown_error',
@@ -271,6 +270,11 @@ export async function hasSecretStored() {
271270
return true;
272271
}
273272

273+
export async function migrateToSecureKeychain(): Promise<boolean> {
274+
console.warn('migrateToSecureKeychain is not implemented for web');
275+
return false;
276+
}
277+
274278
export async function unsafe_clearSecrets() {
275279
if (__DEV__) {
276280
console.warn('unsafe_clearSecrets is not implemented for web');

0 commit comments

Comments
 (0)