[APT-10393] Support Rotating Content URLs #46
Conversation
Throw an exception that will be caught when a device needs DRM but isn't able to safely store it.
Change the internal download tracking to be based on the ID passed in from the client, not by the URL of the audio content. This gives the client full control over the content downloaded by Armadillo, regardless if the URL changes or not at a later time. Beforehand, a changed URL could fail to be found in Armadillo, even though it has been downloaded. These downloads should no longer be lost. Removed unused storage reference in the WidevineSessionEventListener.
| EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, | ||
| EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM | ||
| ) | ||
| } catch (ex: Exception) { |
There was a problem hiding this comment.
What exception can actually happen here?
There was a problem hiding this comment.
Also I think this one should maybe fall back to unencrypted storage? It seems better than not being able to do anything.
There was a problem hiding this comment.
I dont know exactly, im following second hand reports on problems with EncryptedSharedPrefs.
Failing into an insecure mode would not be appropriate.
There was a problem hiding this comment.
There was a problem hiding this comment.
I'd recommend maybe doing something like this?
val keySpec = KeyGenParameterSpec.Builder("armadilloStandard", PURPOSE_ENCRYPT or PURPOSE_DECRYPT)
.setKeySize(256)
.setBlockModes(BLOCK_MODE_GCM)
.setEncryptionPaddings(ENCRYPTION_PADDING_NONE)
.build()
val keys = try {
MasterKeys.getOrCreate(keyspec)
} catch (ex: Exception){
val keyStore = KeyStore.getInstance("AndroidKeyStore")
keyStore.load(null)
keyStore.deleteEntry("armadilloStandard")
context.getSharedPreferences("armadillo.standard.secure", Context.MODE_PRIVATE).edit().clear().apply()
MasterKeys.getOrCreate(keySpec)
}
return EncryptedSharedPreferences.create(
"armadillo.standard.secure",
keys,
context,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
)
There was a problem hiding this comment.
I think you may be fine to use MasterKeys.AES256_GCM_SPEC for both instead of a custom keyspec for each too, but your call
| @@ -71,48 +72,53 @@ internal class ArmadilloSecureStorage @Inject constructor( | |||
| } | |||
|
|
|||
| override fun saveDrmDownload(context: Context, audioUrl: String, drmDownload: DrmDownload) { | |||
| if(secureDrmStorage == null){ | |||
| throw DrmDownloadException(UnsupportedOperationException("This device cannot encrypt downloads")) | |||
There was a problem hiding this comment.
What should the user experience be if this happens?
There was a problem hiding this comment.
Is there anything they can do to fix it?
There was a problem hiding this comment.
No. If their device is in the failed mode they can't download. They can still stream even this component fails to initialize.
|
|
||
| downloadHelper.prepare(object : DownloadHelper.Callback { | ||
| override fun onPrepared(helper: DownloadHelper) { | ||
| val request = helper.getDownloadRequest(audioPlayable.id.encodeInByteArray()) | ||
| var request = helper.getDownloadRequest(audioPlayable.id.encodeInByteArray()) | ||
| request = request.copyWithId(audioPlayable.id.toString()) |
There was a problem hiding this comment.
Why are we changing from the byte array to the string?
There was a problem hiding this comment.
It's string everywhere else, and the audioplayable.id is not what's being set as the ID there, its being set as the URL, and then i have to override it back to being the audioplayable.id instead.
There was a problem hiding this comment.
Out of scope: I don't think we use the audioPlayable.id.encodeInByteArray() data anywhere, it's a bit of a red herring. I think we can pass null
Also, the var threw me off for a moment thinking there was reason to reassign it.
Any reason not to do:
val request = helper.getDownloadRequest(audioPlayable.id.encodeInByteArray()).copyWithId(audioPlayable.id.toString())?
Clears the keystore if the EncryptedSharedPref key is lost, which can happen in some random OEMs.
The data isn't used as an ID, but its passed into Exoplayer. We still have to set the ID ourselves.
Change the internal download tracking to be based on the ID passed in from the client, not by the URL of the audio content. This gives the client full control over the content downloaded by Armadillo, regardless if the URL changes or not at a later time.
Beforehand, a changed URL could fail to be found in Armadillo, even though it has been downloaded. These downloads should no longer be lost.
Removed unused storage reference in the WidevineSessionEventListener.
Protect EncryptedSharedPref Instability: Throw an exception that will be caught when a device needs DRM but isn't able to safely store it.