Skip to content

Commit

Permalink
Fix Android API 23 and below not calling `javax.crypto.MacSpi.engineR…
Browse files Browse the repository at this point in the history
…eset` (#46)
  • Loading branch information
05nelsonm authored Jun 9, 2023
1 parent 9fb6985 commit e3293c5
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 83 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
if: matrix.os == 'ubuntu-latest'
run: >
./gradlew check --stacktrace
-PKMP_TARGETS="JVM,JS,ANDROID,ANDROID_ARM32,ANDROID_ARM64,ANDROID_X64,ANDROID_X86,LINUX_ARM32HFP,LINUX_ARM64,LINUX_MIPS32,LINUX_MIPSEL32,LINUX_X64,WASM,WASM_32"
-PKMP_TARGETS="JVM,JS,ANDROID_ARM32,ANDROID_ARM64,ANDROID_X64,ANDROID_X86,LINUX_ARM32HFP,LINUX_ARM64,LINUX_MIPS32,LINUX_MIPSEL32,LINUX_X64,WASM,WASM_32"
- name: Run Windows Tests
if: matrix.os == 'windows-latest'
run: >
Expand Down
20 changes: 14 additions & 6 deletions library/mac/src/jvmMain/kotlin/org/kotlincrypto/core/Mac.kt
Original file line number Diff line number Diff line change
Expand Up @@ -123,24 +123,32 @@ protected actual constructor(
protected final override fun engineGetMacLength(): Int = macLength()

// Is called immediately from Mac init block with a blanked key (required in order to set
// javax.crypto.Mac.initialized to true.
// javax.crypto.Mac.initialized to true).
protected final override fun engineInit(p0: Key?, p1: AlgorithmParameterSpec?) {
if (isInitialized) {

// Throw an exception b/c if caller is trying to re-init the javax.crypto.Mac with a new key,
// the normal behavior is to blank the Spi state. If they do not know this is an issue,
// any output would not be correct b/c implementations do not re-init. KotlinCrypto users
// the normal behavior is to blank the MacSpi state. If caller does not know this is an issue,
// any further output would not be correct b/c implementations do not re-init. KotlinCrypto users
// already know it's initialized because the API is designed to require the key upon instantiation
// so init is never needed to be called.
// so init is never needed to be called, nor is init function available from commonMain source set.
throw InvalidKeyException(
"org.kotlincrypto.Mac does not support re-initialization " +
"org.kotlincrypto.core.Mac does not support re-initialization " +
"(it's already initialized). A new instance is required to be created."
)
}

isInitialized = true
}
protected final override fun engineDoFinal(): ByteArray = doFinal()
protected final override fun engineDoFinal(): ByteArray {
val b = doFinal()

// Android API 23 and below javax.crypto.Mac does not call engineReset()
@OptIn(InternalKotlinCryptoApi::class)
KC_ANDROID_SDK_INT?.let { sdkInt -> if (sdkInt <= 23) reset() }

return b
}

public final override fun clone(): Any = copy()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,13 @@ internal class AndroidApi21to23MacSpiProvider private constructor(
/* attributes */ null
) {
override fun newInstance(constructorParameter: Any?): Any = synchronized(this@AndroidApi21to23MacSpiProvider) {
// simply return this if spi reference was dropped. b/c this is not
// an instance of MacSpi, android's implementation of javax.crypto.Mac
// will throw an exception which is what we want.
val engine = spi ?: throw NoSuchAlgorithmException("algorithm[$algorithm] not supported")

// javax.crypto.Mac.init was called with a blanked key via org.kotlincrypto.Mac's
// init block in order to set javax.crypto.Mac.initialized to true. return
// the MacSpi (i.e. org.kotlincrypto.Mac.Engine), and null the reference as
// javax.crypto.Mac.init was called with a blanked key via org.kotlincrypto.core.Mac's
// init block in order to set javax.crypto.Mac.initialized to true. Return
// the MacSpi (i.e. org.kotlincrypto.core.Mac.Engine), and null the reference as
// we cannot provide a new instance if called again and do not want to return the
// same, already initialized org.kotlincrypto.Mac.Engine
// same, already initialized org.kotlincrypto.core.Mac.Engine
spi = null

return engine
Expand Down
10 changes: 7 additions & 3 deletions test-android/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ kmpConfiguration {
compileTargetCompatibility = JavaVersion.VERSION_11

android {
namespace = "org.kotlincrypto.test"
namespace = "org.kotlincrypto.core"
compileSdk = 33

defaultConfig {
Expand All @@ -46,11 +46,15 @@ kmpConfiguration {
}

sourceSetTestInstrumented {
kotlin.srcDir("src/androidInstrumentedTest/digest")
kotlin.srcDir("src/androidInstrumentedTest/mac")

dependencies {
// implementation(project(":library:digest"))
implementation(project(":library:mac"))
implementation(libs.androidx.test.runner)
implementation(kotlin("test"))

implementation(project(":library:digest"))
implementation(project(":library:mac"))
}
}
}
Expand Down
1 change: 1 addition & 0 deletions test-android/src/androidInstrumentedTest/digest
Original file line number Diff line number Diff line change
Expand Up @@ -15,83 +15,21 @@
**/
@file:Suppress("UnnecessaryOptInAnnotation")

package org.kotlincrypto.test
package org.kotlincrypto.core

import android.os.Build
import org.kotlincrypto.core.InternalKotlinCryptoApi
import org.kotlincrypto.core.Mac
import java.security.NoSuchAlgorithmException
import java.security.NoSuchProviderException
import java.security.Provider
import javax.crypto.spec.SecretKeySpec
import kotlin.test.*

class AndroidMacTest {

private val key = ByteArray(50) { it.toByte() }

@OptIn(InternalKotlinCryptoApi::class)
class TestMac: Mac {

private val engine: Engine

fun updateCount(): Int = engine.count

constructor(key: ByteArray): this(Engine("Anything????", key))

private constructor(engine: Engine): super(engine.algorithm, engine) {
this.engine = engine
}

private class Engine: Mac.Engine {

var count = 0
val algorithm: String
val delegate: javax.crypto.Mac

constructor(algorithm: String, key: ByteArray): super(key) {
this.algorithm = algorithm

// Use HmacSHA256 for the tests such that we can get a non-static
// result.
this.delegate = getInstance("HmacSHA256")
this.delegate.init(SecretKeySpec(key, "HmacSHA256"))
}

private constructor(state: State, engine: Engine): super(state) {
this.algorithm = engine.algorithm
this.delegate = engine.delegate.clone() as javax.crypto.Mac
}

override fun update(input: Byte) {
delegate.update(input)
}

override fun update(input: ByteArray, offset: Int, len: Int) {
delegate.update(input, offset, len)
count++
}

override fun doFinal(): ByteArray = delegate.doFinal()
override fun macLength(): Int = delegate.macLength
override fun copy(): Mac.Engine = Engine(object : State() {}, this)
override fun reset() { delegate.reset() }
}

override fun copy(engineCopy: Mac.Engine): Mac = TestMac(engineCopy as Engine)
}

@Test
fun givenAndroid_whenMacInstantiated_thenUsesProvidedEngine() {
val testMac = TestMac(key)
testMac.apply { update(key) }.doFinal()

assertEquals(1, testMac.updateCount())
}

@Test
fun givenAndroid_whenApi23OrBelow_thenUsesProvider() {
val provider = TestMac(key).provider
val provider = TestMac(key, TEST_ALGORITHM).provider
if (Build.VERSION.SDK_INT in 21..23) {
assertNotNull(provider)
} else {
Expand Down Expand Up @@ -152,8 +90,12 @@ class AndroidMacTest {
}

private fun testMacAndProviderOrNull(): Pair<Mac, Provider>? {
val mac = TestMac(key)
val mac = TestMac(key, TEST_ALGORITHM)
val provider = mac.provider ?: return null
return Pair(mac, provider)
}

private companion object {
private const val TEST_ALGORITHM = "AndroidMacTest"
}
}
1 change: 1 addition & 0 deletions test-android/src/androidInstrumentedTest/mac
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
**/
package org.kotlincrypto.test
package org.kotlincrypto.core

internal fun stub() { /* no-op */ }

0 comments on commit e3293c5

Please sign in to comment.