Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import io.privkey.keep.uniffi.Nip55RequestType
import io.privkey.keep.uniffi.SignPolicySelection
import io.privkey.keep.uniffi.SignPolicySelectionStorage
import io.privkey.keep.uniffi.SignPolicyStore
import io.privkey.keep.uniffi.StorageRead
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
Expand Down Expand Up @@ -814,12 +815,11 @@ class AppSignPolicyOverridesInstrumentedTest {
}

/**
* Why the marker rather than the absence check carries this. A core that reads as
* empty is indistinguishable from one whose value would not decrypt, and by the time
* a second run happens the core may durably hold the stricter Manual an unconfirmed
* write pinned while the row still holds the looser tier the user asked for. Copying
* the row again would then widen the app, so an attempted package is never copied
* twice whatever the core answers.
* Why the marker rather than the absence check carries this. By the time a second run
* happens the core may durably hold the stricter Manual an unconfirmed write pinned
* while the row still holds the looser tier the user asked for. Copying the row again
* would then widen the app, so an attempted package is never copied twice, even when
* the core answers that nothing is stored.
*/
@Test
fun anAttemptedPackageIsNotCopiedWhenTheCoreReadsAsEmpty() = runBlocking {
Expand Down Expand Up @@ -952,7 +952,8 @@ private class UncommittableStorage : SignPolicySelectionStorage {

private val values = HashMap<String, String>()

override fun load(key: String): String? = values[key]
override fun load(key: String): StorageRead =
values[key]?.let { StorageRead.Found(it) } ?: StorageRead.Absent

override fun save(key: String, value: String): Boolean {
values[key] = value
Expand All @@ -976,7 +977,8 @@ private class UnremovableStorage(private val unremovablePackage: String) : SignP

private val values = HashMap<String, String>()

override fun load(key: String): String? = values[key]
override fun load(key: String): StorageRead =
values[key]?.let { StorageRead.Found(it) } ?: StorageRead.Absent

override fun save(key: String, value: String): Boolean {
values[key] = value
Expand All @@ -991,16 +993,16 @@ private class UnremovableStorage(private val unremovablePackage: String) : SignP
}

/**
* A backend whose reads all come back empty while its writes persist: the decrypt fault
* the encrypted-prefs layer reports as an absent value, since it returns the default
* when a value cannot be decrypted. Records what it was asked to store so a test can
* tell a copy that was skipped from one that landed.
* A backend whose reads all come back absent while its writes persist, so a test can
* see what the migration does when the core answers that nothing is stored whatever it
* holds. Records what it was asked to store so a test can tell a copy that was skipped
* from one that landed.
*/
private class UnreadableStorage : SignPolicySelectionStorage {

val saved = mutableListOf<Pair<String, String>>()

override fun load(key: String): String? = null
override fun load(key: String): StorageRead = StorageRead.Absent

override fun save(key: String, value: String): Boolean {
saved += key to value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,9 +629,9 @@ class PermissionStoreIntegrationTest {
@Test
fun flippingAGrantToAskDropsTheLifetimeItWouldHaveExpiredWith() = runBlocking {
// Same rule from the other side, and the lifetime here is the grant's own
// rather than the clamp's. An ask that expires stops blocking both the policy
// auto-approve and the whitelisted-relay auto-accept, which is the setting the
// user just switched on, and the flip that set it offered no duration.
// rather than the clamp's. An ask that expires stops blocking the policy
// auto-approve, which is the setting the user just switched on, and the flip
// that set it offered no duration.
store.grantPermission("com.test.app", Nip55RequestType.SIGN_EVENT, SENSITIVE_KIND, PermissionDuration.ONE_HOUR)
val row = store.getPermissionsForCaller("com.test.app").single()
assertEquals("precondition: the grant is bounded", HOUR_MS, row.durationMs)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package io.privkey.keep.nip55

import android.content.Context
import android.os.SystemClock
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.privkey.keep.storage.RelayAuthWhitelistStore
import io.privkey.keep.uniffi.Nip55DecisionInputs
import io.privkey.keep.uniffi.Nip55Outcome
import io.privkey.keep.uniffi.Nip55RequestType
import io.privkey.keep.uniffi.Nip55StoredPermission
import io.privkey.keep.uniffi.Nip55VelocityCheck
import io.privkey.keep.uniffi.SignPolicySelection
import io.privkey.keep.uniffi.evaluateNip55Request
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith

/**
* What the background path decides for a relay auth, through the pinned core and the
* same whitelist fold the ContentProvider uses.
*
* The gate tests stop at the gate value. Whether a listed relay is then signed is
* decided below it, in the core, and that is where membership used to authorize an
* app that had never been granted anything. These pin the core this app ships with,
* so a pin that moved back to a core without the fix fails here.
*/
@RunWith(AndroidJUnit4::class)
class RelayAuthDecisionTest {

private val context: Context = ApplicationProvider.getApplicationContext()
private lateinit var store: RelayAuthWhitelistStore

@Before
fun setup() {
context.deleteSharedPreferences(RelayAuthWhitelistStore.PREFS_NAME)
store = RelayAuthWhitelistStore(context)
}

@After
fun teardown() {
context.deleteSharedPreferences(RelayAuthWhitelistStore.PREFS_NAME)
}

@Test
fun aListedRelayWithNoStandingPermissionPromptsRatherThanSigning() {
store.add("relay.example.com")
val outcome = decide(exact = null)
assertTrue(
"membership must not authorize signing on its own, got $outcome",
outcome is Nip55Outcome.RequireUi
)
}

// The control: the same listed relay is signed once the app holds a grant for it,
// so the case above cannot pass on inputs that would prompt for anything.
@Test
fun aListedRelayWithAStandingPermissionIsSigned() {
store.add("relay.example.com")
assertEquals(Nip55Outcome.AutoApprove, decide(exact = allow()))
}

@Test
fun anUnlistedRelayIsRejectedEvenWithAStandingPermission() {
store.add("other.example.com")
assertEquals(Nip55Outcome.Reject("deny_relay_whitelist"), decide(exact = allow()))
}

private fun allow() = Nip55StoredPermission(
decision = "allow",
expiresAt = null,
createdAt = System.currentTimeMillis(),
createdAtElapsed = SystemClock.elapsedRealtime(),
durationMs = null
)

private fun decide(exact: Nip55StoredPermission?): Nip55Outcome {
val (hosts, readFailed) = relayWhitelistInputs(store)
return evaluateNip55Request(
Nip55DecisionInputs(
packageName = "com.test.app",
callerVerified = true,
signingKilled = false,
isLocked = false,
requestType = Nip55RequestType.SIGN_EVENT,
eventJson = EVENT_22242,
frontDoorWithinLimit = true,
velocityCheck = Nip55VelocityCheck.ALLOWED,
relayWhitelist = hosts,
relayWhitelistReadFailed = readFailed,
policySelection = SignPolicySelection.MANUAL,
isOptedIn = false,
optInRateCheck = null,
hasSignedKindBefore = true,
appAgeMs = null,
currentHour = 12u,
appExpired = false,
permissionLookupOk = true,
storedExactPermission = exact,
storedGenericPermission = null,
nowElapsedMs = SystemClock.elapsedRealtime(),
nowWallMs = System.currentTimeMillis()
)
)
}

private companion object {
const val EVENT_22242 =
"""{"kind":22242,"content":"","tags":[["relay","wss://relay.example.com/"],["challenge","abc"]]}"""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ class RelayAuthGateTest {
assertEquals(emptyList<String>(), hosts)
}

// The same relay, listed and readable, is accepted. Without this the test
// The same relay, listed and readable, is not rejected. Without this the test
// above could pass on a gate that rejected everything.
@Test
fun whitelistedRelayAutoAccepts() {
fun whitelistedRelayResolvesToWhitelisted() {
store.add("relay.example.com")
val (gate, host) = evaluateRelayAuthGate(store, event22242)
assertEquals(Nip55RelayAuthGate.AUTO_ACCEPT, gate)
assertEquals(Nip55RelayAuthGate.WHITELISTED, gate)
assertEquals("relay.example.com", host)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.privkey.keep.uniffi.SignPolicySelection
import io.privkey.keep.uniffi.SignPolicyStore
import io.privkey.keep.uniffi.StorageRead
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
Expand Down Expand Up @@ -69,9 +69,9 @@ class SignPolicySelectionPrefsTest {

@Test
fun migrationNeverResurrectsTheLegacySelectionAfterTheStoreIsLost() {
// The legacy value is kept on disk, and the encrypted-prefs layer reports an
// undecryptable value as absent, so gating the migration on "new value is
// absent" would re-run it and restore the old, possibly looser, selection.
// The legacy value is kept on disk, and a lost store reads as absent, so gating
// the migration on "new value is absent" alone would re-run it and restore the
// old, possibly looser, selection.
// The one-shot marker must prevent that: losing the store falls back to
// MANUAL (strictest), never back to the legacy AUTO.
writeLegacyOrdinal(SignPolicy.AUTO.ordinal)
Expand All @@ -86,6 +86,26 @@ class SignPolicySelectionPrefsTest {
assertEquals(SignPolicySelection.MANUAL, newStore().globalPolicy())
}

/**
* The migration copies only over a value that is not there. One that is stored but
* will not decrypt resolves to MANUAL, and reading it as absent would put the legacy
* selection, here a looser one, in its place.
*/
@Test
fun migrationNeverCopiesOverAValueThatCannotBeRead() {
val storedName = writeAndFindStoredName(context, SELECTION_PREFS) {
SignPolicySelectionPrefs(context).save(GLOBAL_KEY, SignPolicySelection.BASIC.ordinal.toString())
}
corruptStoredValue(context, SELECTION_PREFS, storedName)
// As on an install that has not run the migration yet.
context.getSharedPreferences(SignPolicySelectionPrefs.MARKER_PREFS_NAME, Context.MODE_PRIVATE)
.edit().remove(SignPolicySelectionPrefs.MIGRATION_MARKER).commit()
writeLegacyOrdinal(SignPolicy.AUTO.ordinal)

assertEquals(SignPolicySelection.MANUAL, newStore().globalPolicy())
forgetWriteProbe(context, SELECTION_PREFS)
}

@Test
fun migrationLeavesTheLegacyValueInPlace() {
writeLegacyOrdinal(SignPolicy.AUTO.ordinal)
Expand All @@ -102,7 +122,7 @@ class SignPolicySelectionPrefsTest {
fun noLegacyValueLeavesTheStoreUnsetAndDefaultsToManual() {
val prefs = SignPolicySelectionPrefs(context)

assertNull(prefs.load(GLOBAL_KEY))
assertEquals(StorageRead.Absent, prefs.load(GLOBAL_KEY))
assertEquals(SignPolicySelection.MANUAL, SignPolicyStore(prefs).globalPolicy())
}

Expand All @@ -116,15 +136,15 @@ class SignPolicySelectionPrefsTest {
@Test
fun unreadableStoredValueFallsBackToManual() {
// A value of the wrong type is what the pre-migration prefs file held; reading
// it must yield "unset" rather than throwing into the signing path.
// it must report a fault rather than throwing into the signing path.
KeystoreEncryptedPrefs.create(context, SELECTION_PREFS)
.edit()
.putInt(GLOBAL_KEY, SignPolicy.AUTO.ordinal)
.commit()

val prefs = SignPolicySelectionPrefs(context)

assertNull(prefs.load(GLOBAL_KEY))
assertEquals(StorageRead.Unavailable, prefs.load(GLOBAL_KEY))
assertEquals(SignPolicySelection.MANUAL, SignPolicyStore(prefs).globalPolicy())
}

Expand Down Expand Up @@ -152,11 +172,53 @@ class SignPolicySelectionPrefsTest {

prefs.remove(GLOBAL_KEY)

assertNull(prefs.load(GLOBAL_KEY))
assertEquals(StorageRead.Absent, prefs.load(GLOBAL_KEY))
assertEquals(SignPolicySelection.MANUAL, SignPolicyStore(prefs).globalPolicy())
}

/**
* An override that is stored but will not decrypt must not fall back to the global
* selection, which can be looser than the override the owner set. Reported as
* absent, it would: the core reads absent as "no override". Reported as
* unavailable, the core resolves it to MANUAL.
*
* Staged on disk, so this is the production backend's answer for a real
* undecryptable entry rather than a fake's.
*/
@Test
fun anUndecryptableOverrideResolvesToManualRatherThanTheGlobal() {
newStore().setGlobalPolicy(SignPolicySelection.AUTO)
val storedName = writeAndFindStoredName(context, SELECTION_PREFS) {
newStore().setAppOverride(PKG, SignPolicySelection.BASIC)
}
corruptStoredValue(context, SELECTION_PREFS, storedName)

val store = newStore()
assertEquals(SignPolicySelection.MANUAL, store.appOverride(PKG))
assertEquals(SignPolicySelection.MANUAL, store.effectivePolicy(PKG))
forgetWriteProbe(context, SELECTION_PREFS)
}

// The controls: a readable override wins over the global, and a package with no
// override gets the global, so the case above cannot pass on a store that answers
// MANUAL for everything.
@Test
fun aReadableOverrideWinsOverTheGlobal() {
newStore().setGlobalPolicy(SignPolicySelection.AUTO)
newStore().setAppOverride(PKG, SignPolicySelection.BASIC)

assertEquals(SignPolicySelection.BASIC, newStore().effectivePolicy(PKG))
}

@Test
fun aPackageWithNoOverrideGetsTheGlobal() {
newStore().setGlobalPolicy(SignPolicySelection.AUTO)

assertEquals(SignPolicySelection.AUTO, newStore().effectivePolicy(PKG))
}

private companion object {
const val PKG = "com.test.app"
const val SELECTION_PREFS = "keep_sign_policy_selection"
const val LEGACY_PREFS = "keep_sign_policy"
const val GLOBAL_KEY = SignPolicySelectionPrefs.GLOBAL_POLICY_KEY
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class UnreadableReadGuardTest {
assertEquals("relay.example.com", repaired.add("wss://relay.example.com"))
assertEquals(
"and the gate works again afterwards",
Nip55RelayAuthGate.AUTO_ACCEPT,
Nip55RelayAuthGate.WHITELISTED,
evaluateRelayAuthGate(repaired, EVENT_22242).first
)
forgetWriteProbe(context, WHITELIST_PREFS)
Expand Down
Loading