From f549cf5635fd921402c2aabe54750660e765fd03 Mon Sep 17 00:00:00 2001 From: olly Date: Fri, 3 Aug 2018 01:35:17 -0700 Subject: [PATCH] Fix Aes128DataSourceTest portability issue For background on why doing this works, see below. I don't want to change how we get our Cipher instance in non-test code, since PKCS7 always works on Android. It's only when the tests are running on a non-Android host machine that they can fail. An alternative would be to make it an androidTest, but androidTests are slow. ------ Background: "While Java considers PKCS5 and PKCS7 padding to be the "same" (and one should always use the string "AES/CBC/PKCS5Padding" because "AES/CBC/PKCS7Padding" will cause NoSuchAlgorithmException to be thrown when initializing an AES block cipher using the Java crypto API), I consider this a gross misnaming in the Java platform because the pure technical definitions of these paddings are not the same." Ref: https://stackoverflow.com/questions/10193567/java-security-nosuchalgorithmexceptioncannot-find-any-provider-supporting-aes-e ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=207234518 --- .../source/hls/Aes128DataSource.java | 22 +++++++++------- .../source/hls/Aes128DataSourceTest.java | 26 +++++++++++++++++-- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/Aes128DataSource.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/Aes128DataSource.java index 2039449e89..55a648a0b8 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/Aes128DataSource.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/Aes128DataSource.java @@ -40,12 +40,12 @@ import javax.crypto.spec.SecretKeySpec; /** * A {@link DataSource} that decrypts data read from an upstream source, encrypted with AES-128 with * a 128-bit key and PKCS7 padding. - *

- * Note that this {@link DataSource} does not support being opened from arbitrary offsets. It is + * + *

Note that this {@link DataSource} does not support being opened from arbitrary offsets. It is * designed specifically for reading whole files as defined in an HLS media playlist. For this * reason the implementation is private to the HLS package. */ -/* package */ final class Aes128DataSource implements DataSource { +/* package */ class Aes128DataSource implements DataSource { private final DataSource upstream; private final byte[] encryptionKey; @@ -65,15 +65,15 @@ import javax.crypto.spec.SecretKeySpec; } @Override - public void addTransferListener(TransferListener transferListener) { + public final void addTransferListener(TransferListener transferListener) { upstream.addTransferListener(transferListener); } @Override - public long open(DataSpec dataSpec) throws IOException { + public final long open(DataSpec dataSpec) throws IOException { Cipher cipher; try { - cipher = Cipher.getInstance("AES/CBC/PKCS7Padding"); + cipher = getCipherInstance(); } catch (NoSuchAlgorithmException | NoSuchPaddingException e) { throw new RuntimeException(e); } @@ -95,7 +95,7 @@ import javax.crypto.spec.SecretKeySpec; } @Override - public int read(byte[] buffer, int offset, int readLength) throws IOException { + public final int read(byte[] buffer, int offset, int readLength) throws IOException { Assertions.checkNotNull(cipherInputStream); int bytesRead = cipherInputStream.read(buffer, offset, readLength); if (bytesRead < 0) { @@ -105,12 +105,12 @@ import javax.crypto.spec.SecretKeySpec; } @Override - public @Nullable Uri getUri() { + public final @Nullable Uri getUri() { return upstream.getUri(); } @Override - public Map> getResponseHeaders() { + public final Map> getResponseHeaders() { return upstream.getResponseHeaders(); } @@ -121,4 +121,8 @@ import javax.crypto.spec.SecretKeySpec; upstream.close(); } } + + protected Cipher getCipherInstance() throws NoSuchPaddingException, NoSuchAlgorithmException { + return Cipher.getInstance("AES/CBC/PKCS7Padding"); + } } diff --git a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/Aes128DataSourceTest.java b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/Aes128DataSourceTest.java index 93f724e630..defc838c6a 100644 --- a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/Aes128DataSourceTest.java +++ b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/Aes128DataSourceTest.java @@ -23,6 +23,9 @@ import com.google.android.exoplayer2.upstream.DataSource; import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.upstream.TransferListener; import java.io.IOException; +import java.security.NoSuchAlgorithmException; +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -34,7 +37,7 @@ public class Aes128DataSourceTest { @Test public void test_OpenCallsUpstreamOpen_CloseCallsUpstreamClose() throws IOException { UpstreamDataSource upstream = new UpstreamDataSource(); - Aes128DataSource testInstance = new Aes128DataSource(upstream, new byte[16], new byte[16]); + Aes128DataSource testInstance = new TestAes123DataSource(upstream, new byte[16], new byte[16]); assertThat(upstream.opened).isFalse(); Uri uri = Uri.parse("http.abc.com/def"); @@ -54,7 +57,7 @@ public class Aes128DataSourceTest { throw new IOException(); } }; - Aes128DataSource testInstance = new Aes128DataSource(upstream, new byte[16], new byte[16]); + Aes128DataSource testInstance = new TestAes123DataSource(upstream, new byte[16], new byte[16]); assertThat(upstream.opened).isFalse(); Uri uri = Uri.parse("http.abc.com/def"); @@ -72,6 +75,25 @@ public class Aes128DataSourceTest { assertThat(upstream.closedCalled).isTrue(); } + private static class TestAes123DataSource extends Aes128DataSource { + + public TestAes123DataSource(DataSource upstream, byte[] encryptionKey, byte[] encryptionIv) { + super(upstream, encryptionKey, encryptionIv); + } + + @Override + protected Cipher getCipherInstance() throws NoSuchPaddingException, NoSuchAlgorithmException { + try { + return super.getCipherInstance(); + } catch (NoSuchAlgorithmException e) { + // Some host machines may not provide an algorithm for "AES/CBC/PKCS7Padding", however on + // such machines it's possible to get a functionally identical algorithm by requesting + // "AES/CBC/PKCS5Padding". + return Cipher.getInstance("AES/CBC/PKCS5Padding"); + } + } + } + private static class UpstreamDataSource implements DataSource { public boolean opened;