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
This commit is contained in:
olly 2018-08-03 01:35:17 -07:00 committed by Oliver Woodman
parent 2815ff1b02
commit f549cf5635
2 changed files with 37 additions and 11 deletions

View File

@ -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 {@link DataSource} that decrypts data read from an upstream source, encrypted with AES-128 with
* a 128-bit key and PKCS7 padding. * a 128-bit key and PKCS7 padding.
* <p> *
* Note that this {@link DataSource} does not support being opened from arbitrary offsets. It is * <p>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 * designed specifically for reading whole files as defined in an HLS media playlist. For this
* reason the implementation is private to the HLS package. * 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 DataSource upstream;
private final byte[] encryptionKey; private final byte[] encryptionKey;
@ -65,15 +65,15 @@ import javax.crypto.spec.SecretKeySpec;
} }
@Override @Override
public void addTransferListener(TransferListener transferListener) { public final void addTransferListener(TransferListener transferListener) {
upstream.addTransferListener(transferListener); upstream.addTransferListener(transferListener);
} }
@Override @Override
public long open(DataSpec dataSpec) throws IOException { public final long open(DataSpec dataSpec) throws IOException {
Cipher cipher; Cipher cipher;
try { try {
cipher = Cipher.getInstance("AES/CBC/PKCS7Padding"); cipher = getCipherInstance();
} catch (NoSuchAlgorithmException | NoSuchPaddingException e) { } catch (NoSuchAlgorithmException | NoSuchPaddingException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} }
@ -95,7 +95,7 @@ import javax.crypto.spec.SecretKeySpec;
} }
@Override @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); Assertions.checkNotNull(cipherInputStream);
int bytesRead = cipherInputStream.read(buffer, offset, readLength); int bytesRead = cipherInputStream.read(buffer, offset, readLength);
if (bytesRead < 0) { if (bytesRead < 0) {
@ -105,12 +105,12 @@ import javax.crypto.spec.SecretKeySpec;
} }
@Override @Override
public @Nullable Uri getUri() { public final @Nullable Uri getUri() {
return upstream.getUri(); return upstream.getUri();
} }
@Override @Override
public Map<String, List<String>> getResponseHeaders() { public final Map<String, List<String>> getResponseHeaders() {
return upstream.getResponseHeaders(); return upstream.getResponseHeaders();
} }
@ -121,4 +121,8 @@ import javax.crypto.spec.SecretKeySpec;
upstream.close(); upstream.close();
} }
} }
protected Cipher getCipherInstance() throws NoSuchPaddingException, NoSuchAlgorithmException {
return Cipher.getInstance("AES/CBC/PKCS7Padding");
}
} }

View File

@ -23,6 +23,9 @@ import com.google.android.exoplayer2.upstream.DataSource;
import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.upstream.TransferListener;
import java.io.IOException; import java.io.IOException;
import java.security.NoSuchAlgorithmException;
import javax.crypto.Cipher;
import javax.crypto.NoSuchPaddingException;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner; import org.robolectric.RobolectricTestRunner;
@ -34,7 +37,7 @@ public class Aes128DataSourceTest {
@Test @Test
public void test_OpenCallsUpstreamOpen_CloseCallsUpstreamClose() throws IOException { public void test_OpenCallsUpstreamOpen_CloseCallsUpstreamClose() throws IOException {
UpstreamDataSource upstream = new UpstreamDataSource(); 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(); assertThat(upstream.opened).isFalse();
Uri uri = Uri.parse("http.abc.com/def"); Uri uri = Uri.parse("http.abc.com/def");
@ -54,7 +57,7 @@ public class Aes128DataSourceTest {
throw new IOException(); 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(); assertThat(upstream.opened).isFalse();
Uri uri = Uri.parse("http.abc.com/def"); Uri uri = Uri.parse("http.abc.com/def");
@ -72,6 +75,25 @@ public class Aes128DataSourceTest {
assertThat(upstream.closedCalled).isTrue(); 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 { private static class UpstreamDataSource implements DataSource {
public boolean opened; public boolean opened;