-
Notifications
You must be signed in to change notification settings - Fork 47
Replace crypto implementation with stronger key generation method #158
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
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
c339e3e
dev: rename StringCipher.java to StringCipherDeprecated.java
YkSix 512d78f
dev: add interface StringCipher
YkSix 9bec8ff
dev: make class StringCipherDeprecated implement StringCipher interface
YkSix f415755
dev: change the type of EncryptorHolder#ENCRYPTOR to StringCipher int…
YkSix 8ca87fb
dev: create empty class: StringAesCipher.kt
YkSix 36bd34e
dev: fix wrong startIndex of keyBytes array although the values are t…
YkSix 8a3c573
dev: implement StringAesCipher; encrypt data using a secret key store…
YkSix 61ec712
dev: move class SecretKeys to a new file and kotlinize it
YkSix 1771435
dev: change property type of CipherData to ByteArray
YkSix 5536deb
dev: calculate HMAC value and store it in CipherData
YkSix d69d5c4
dev: verify HMAC value before proceeding with the decryption
YkSix 6f74696
dev: rename to verifyHmacValue, throw Exception if the HMAC values do…
YkSix 3bfe42a
dev: make StringAesCipher a thread-safe class, wrap code in synchroni…
YkSix ec80a29
dev: add PURPOSE_SIGN in IntegrityKey
YkSix 6a77a22
dev: switch to new AES crypto implementation
YkSix fb4af3b
Revert "dev: move class SecretKeys to a new file and kotlinize it"
YkSix b29e888
dev: modify test
YkSix 3089706
dev: rename functions to encodeToBase64String and decodeFromBase64String
YkSix dcad1d8
dev: use joinToString in encodeToBase64String
YkSix 5701fab
dev: add more comments
YkSix 9240c23
dev: add @deprecated annotation
YkSix de0863f
chore: update version to 5.10.0
YkSix fd7cc70
chore: update versionCode to 5_10_00
YkSix File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
line-sdk/src/main/java/com/linecorp/android/security/encryption/CipherData.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package com.linecorp.android.security.encryption | ||
|
||
import android.util.Base64 | ||
|
||
class CipherData( | ||
val encryptedData: ByteArray, | ||
val initialVector: ByteArray, | ||
val hmacValue: ByteArray, | ||
) { | ||
fun encodeToBase64String(): String = | ||
listOf(encryptedData, initialVector, hmacValue) | ||
.joinToString(SEPARATOR) { it.encodeBase64() } | ||
|
||
companion object { | ||
private const val SEPARATOR = ";" | ||
private const val SIZE_DATA_TYPES = 3 | ||
|
||
fun decodeFromBase64String(cipherDataBase64String: String): CipherData { | ||
val parts = cipherDataBase64String.split(SEPARATOR) | ||
require(parts.size == SIZE_DATA_TYPES) { | ||
"Failed to split encrypted text `$cipherDataBase64String`" | ||
} | ||
|
||
return CipherData( | ||
encryptedData = parts[0].decodeBase64(), | ||
initialVector = parts[1].decodeBase64(), | ||
hmacValue = parts[2].decodeBase64() | ||
) | ||
} | ||
} | ||
} | ||
|
||
private fun ByteArray.encodeBase64(): String = Base64.encodeToString(this, Base64.NO_WRAP) | ||
|
||
private fun String.decodeBase64(): ByteArray = Base64.decode(this, Base64.NO_WRAP) |
187 changes: 187 additions & 0 deletions
187
line-sdk/src/main/java/com/linecorp/android/security/encryption/StringAesCipher.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,187 @@ | ||
package com.linecorp.android.security.encryption | ||
|
||
import android.content.Context | ||
import android.security.keystore.KeyGenParameterSpec | ||
import android.security.keystore.KeyProperties | ||
import android.security.keystore.KeyProperties.PURPOSE_DECRYPT | ||
import android.security.keystore.KeyProperties.PURPOSE_ENCRYPT | ||
import android.security.keystore.KeyProperties.PURPOSE_SIGN | ||
import android.security.keystore.KeyProperties.PURPOSE_VERIFY | ||
import java.security.KeyStore | ||
import java.security.MessageDigest | ||
import javax.crypto.Cipher | ||
import javax.crypto.KeyGenerator | ||
import javax.crypto.Mac | ||
import javax.crypto.SecretKey | ||
import javax.crypto.spec.IvParameterSpec | ||
|
||
/** | ||
* AES cipher by AndroidKeyStore | ||
*/ | ||
class StringAesCipher : StringCipher { | ||
private val keyStore: KeyStore by lazy { | ||
KeyStore.getInstance(ANDROID_KEY_STORE).also { | ||
it.load(null) | ||
} | ||
} | ||
|
||
private lateinit var hmac: Mac | ||
|
||
override fun initialize(context: Context) { | ||
if (::hmac.isInitialized) { | ||
return | ||
} | ||
|
||
synchronized(this) { | ||
getAesSecretKey() | ||
val integrityKey = getIntegrityKey() | ||
|
||
hmac = Mac.getInstance(KeyProperties.KEY_ALGORITHM_HMAC_SHA256).apply { | ||
init(integrityKey) | ||
} | ||
} | ||
} | ||
|
||
override fun encrypt(context: Context, plainText: String): String { | ||
synchronized(this) { | ||
initialize(context) | ||
|
||
try { | ||
val secretKey = getAesSecretKey() | ||
|
||
val cipher = Cipher.getInstance(TRANSFORMATION_FORMAT).apply { | ||
init(Cipher.ENCRYPT_MODE, secretKey) | ||
} | ||
val encryptedData: ByteArray = cipher.doFinal(plainText.toByteArray()) | ||
|
||
return CipherData( | ||
encryptedData = encryptedData, | ||
initialVector = cipher.iv, | ||
hmacValue = hmac.calculateHmacValue(encryptedData, cipher.iv) | ||
).encodeToBase64String() | ||
} catch (e: Exception) { | ||
throw EncryptionException("Failed to encrypt", e) | ||
} | ||
} | ||
} | ||
|
||
override fun decrypt(context: Context, cipherText: String): String { | ||
synchronized(this) { | ||
try { | ||
val secretKey = getAesSecretKey() | ||
|
||
val cipherData = CipherData.decodeFromBase64String(cipherText) | ||
|
||
cipherData.verifyHmacValue(hmac) | ||
|
||
val ivSpec = IvParameterSpec(cipherData.initialVector) | ||
|
||
return Cipher.getInstance(TRANSFORMATION_FORMAT) | ||
.apply { init(Cipher.DECRYPT_MODE, secretKey, ivSpec) } | ||
.run { doFinal(cipherData.encryptedData) } | ||
.let { | ||
String(it) | ||
} | ||
} catch (e: Exception) { | ||
throw EncryptionException("Failed to decrypt", e) | ||
} | ||
} | ||
} | ||
|
||
private fun getAesSecretKey(): SecretKey { | ||
return if (keyStore.containsAlias(AES_KEY_ALIAS)) { | ||
val secretKeyEntry = | ||
keyStore.getEntry(AES_KEY_ALIAS, null) as KeyStore.SecretKeyEntry | ||
|
||
secretKeyEntry.secretKey | ||
} else { | ||
createAesKey() | ||
} | ||
} | ||
|
||
private fun getIntegrityKey(): SecretKey { | ||
return if (keyStore.containsAlias(INTEGRITY_KEY_ALIAS)) { | ||
val secretKeyEntry = | ||
keyStore.getEntry(INTEGRITY_KEY_ALIAS, null) as KeyStore.SecretKeyEntry | ||
|
||
secretKeyEntry.secretKey | ||
} else { | ||
createIntegrityKey() | ||
} | ||
} | ||
|
||
/** | ||
* Create a new AES key in the Android KeyStore. This key will be used for | ||
* encrypting and decrypting data. The key is generated with a size of 256 bits, | ||
* using the CBC block mode and PKCS7 padding. | ||
*/ | ||
private fun createAesKey(): SecretKey { | ||
val keyGenerator = KeyGenerator | ||
.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEY_STORE) | ||
val keyGenParameterSpec = KeyGenParameterSpec.Builder( | ||
AES_KEY_ALIAS, | ||
PURPOSE_ENCRYPT or PURPOSE_DECRYPT | ||
) | ||
.setKeySize(KEY_SIZE_IN_BIT) | ||
.setBlockModes(KeyProperties.BLOCK_MODE_CBC) | ||
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7) | ||
.build() | ||
|
||
keyGenerator.run { | ||
init(keyGenParameterSpec) | ||
return generateKey() | ||
} | ||
} | ||
|
||
private fun createIntegrityKey(): SecretKey { | ||
val keyGenerator = KeyGenerator | ||
.getInstance(KeyProperties.KEY_ALGORITHM_HMAC_SHA256, ANDROID_KEY_STORE) | ||
val keyGenParameterSpec = KeyGenParameterSpec.Builder( | ||
INTEGRITY_KEY_ALIAS, | ||
PURPOSE_SIGN or PURPOSE_VERIFY | ||
) | ||
.build() | ||
|
||
keyGenerator.run { | ||
init(keyGenParameterSpec) | ||
return generateKey() | ||
} | ||
} | ||
|
||
private fun Mac.calculateHmacValue( | ||
encryptedData: ByteArray, | ||
initialVector: ByteArray | ||
): ByteArray = doFinal(encryptedData + initialVector) | ||
|
||
/** | ||
* Validate the HMAC value | ||
* | ||
* @throws SecurityException if the HMAC value doesn't match with [encryptedData] | ||
*/ | ||
private fun CipherData.verifyHmacValue(mac: Mac) { | ||
val expectedHmacValue: ByteArray = mac.calculateHmacValue( | ||
encryptedData = encryptedData, | ||
initialVector = initialVector | ||
) | ||
|
||
if (!MessageDigest.isEqual(expectedHmacValue, hmacValue)) { | ||
throw SecurityException("Cipher text has been tampered with.") | ||
} | ||
} | ||
|
||
companion object { | ||
private const val AES_KEY_ALIAS = | ||
"com.linecorp.android.security.encryption.StringAesCipher" | ||
|
||
private const val INTEGRITY_KEY_ALIAS = | ||
"com.linecorp.android.security.encryption.StringAesCipher.INTEGRITY_KEY" | ||
|
||
private const val ANDROID_KEY_STORE = "AndroidKeyStore" | ||
private const val KEY_SIZE_IN_BIT = 256 | ||
|
||
private const val TRANSFORMATION_FORMAT = | ||
KeyProperties.KEY_ALGORITHM_AES + | ||
"/${KeyProperties.BLOCK_MODE_CBC}" + | ||
"/${KeyProperties.ENCRYPTION_PADDING_PKCS7}" | ||
} | ||
} |
12 changes: 12 additions & 0 deletions
12
line-sdk/src/main/java/com/linecorp/android/security/encryption/StringCipher.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.linecorp.android.security.encryption | ||
|
||
import android.content.Context | ||
|
||
interface StringCipher { | ||
|
||
fun initialize(context: Context) | ||
|
||
fun encrypt(context: Context, plainText: String): String | ||
|
||
fun decrypt(context: Context, cipherText: String): String | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also help to update versionCode to
5_10_00
on line 18.