[flutter_tools] verify checksum of downloaded artifacts (flutter#67839)

Jonah Williams · web-flow · commit ac751cc50cc2 · 2020-10-12T17:44:23.000-07:00
All of the network requests from google cloud storage include an x-goog-hash header which contains an MD5 checksum. If present, use to validate that the downloaded binary is valid. This will rule out corrupt files as the cause of getting started crashers in the flutter_tool. flutter#38980 This does not fully resolve the above issue, because while we can check if the checksum matches what was expected from cloud storage, this A) may not necessarily be present and B) may not match up to what should be uploaded as part of the engine build process. But when life gives you lemons you hash those lemons using an outdated hashing algorithm.
diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart
@@ -2,7 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:async';
+
 import 'package:archive/archive.dart';
+import 'package:crypto/crypto.dart';
 import 'package:file/memory.dart';
 import 'package:meta/meta.dart';
 import 'package:package_config/package_config.dart';
@@ -12,12 +15,13 @@ import 'android/gradle_utils.dart';
 import 'base/common.dart';
 import 'base/error_handling_io.dart';
 import 'base/file_system.dart';
-import 'base/io.dart' show HttpClient, HttpClientRequest, HttpClientResponse, HttpStatus, ProcessException, SocketException;
+import 'base/io.dart' show HttpClient, HttpClientRequest, HttpClientResponse, HttpHeaders, HttpStatus, ProcessException, SocketException;
 import 'base/logger.dart';
 import 'base/net.dart';
 import 'base/os.dart' show OperatingSystemUtils;
 import 'base/platform.dart';
 import 'base/process.dart';
+import 'convert.dart';
 import 'dart/package_map.dart';
 import 'dart/pub.dart';
 import 'features.dart';
@@ -1610,7 +1614,7 @@ class ArtifactUpdater {
         retries -= 1;
         if (retries == 0) {
           throwToolExit(
-            'Failed to download $url. Ensure you have network connectivity and then try again.',
+            'Failed to download $url. Ensure you have network connectivity and then try again.\n$err',
           );
         }
         continue;
@@ -1656,15 +1660,69 @@ class ArtifactUpdater {
   }
 
   /// Download bytes from [url], throwing non-200 responses as an exception.
+  ///
+  /// Validates that the md5 of the content bytes matches the provided
+  /// `x-goog-hash` header, if present. This header should contain an md5 hash
+  /// if the download source is Google cloud storage.
+  ///
+  /// See also:
+  ///   * https://cloud.google.com/storage/docs/xml-api/reference-headers#xgooghash
   Future<void> _download(Uri url, File file) async {
     final HttpClientRequest request = await _httpClient.getUrl(url);
     final HttpClientResponse response = await request.close();
     if (response.statusCode != HttpStatus.ok) {
       throw Exception(response.statusCode);
     }
+
+    final String md5Hash = _expectedMd5(response.headers);
+    ByteConversionSink inputSink;
+    StreamController<Digest> digests;
+    if (md5Hash != null) {
+      _logger.printTrace('Content $url md5 hash: $md5Hash');
+      digests = StreamController<Digest>();
+      inputSink = md5.startChunkedConversion(digests);
+    }
+    final RandomAccessFile randomAccessFile = file.openSync(mode: FileMode.writeOnly);
     await response.forEach((List<int> chunk) {
-      file.writeAsBytesSync(chunk, mode: FileMode.append);
+      inputSink?.add(chunk);
+      randomAccessFile.writeFromSync(chunk);
     });
+    randomAccessFile.closeSync();
+    if (inputSink != null) {
+      inputSink.close();
+      final Digest digest = await digests.stream.last;
+      final String rawDigest = base64.encode(digest.bytes);
+      if (rawDigest != md5Hash) {
+        throw Exception(''
+          'Expected $url to have md5 checksum $md5Hash, but was $rawDigest. This '
+          'may indicate a problem with your connection to the Flutter backend servers. '
+          'Please re-try the download after confirming that your network connection is '
+          'stable.'
+        );
+      }
+    }
+  }
+
+  String _expectedMd5(HttpHeaders httpHeaders) {
+    final List<String> values = httpHeaders['x-goog-hash'];
+    if (values == null) {
+      return null;
+    }
+    final String rawMd5Hash = values.firstWhere((String value) {
+      return value.startsWith('md5=');
+    }, orElse: () => null);
+    if (rawMd5Hash == null) {
+      return null;
+    }
+    final List<String> segments = rawMd5Hash.split('md5=');
+    if (segments.length < 2) {
+      return null;
+    }
+    final String md5Hash = segments[1];
+    if (md5Hash.isEmpty) {
+      return null;
+    }
+    return md5Hash;
   }
 
   /// Create a temporary file and invoke [onTemporaryFile] with the file as
diff --git a/packages/flutter_tools/test/general.shard/artifact_updater_test.dart b/packages/flutter_tools/test/general.shard/artifact_updater_test.dart
@@ -44,6 +44,97 @@ void main() {
     expect(fileSystem.file('out/test'), exists);
   });
 
+  testWithoutContext('ArtifactUpdater will not validate the md5 hash if the '
+    'x-goog-hash header is present but missing an md5 entry', () async {
+    final MockOperatingSystemUtils operatingSystemUtils = MockOperatingSystemUtils();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final BufferLogger logger = BufferLogger.test();
+    final MockHttpClient client = MockHttpClient();
+    client.testRequest.testResponse.headers = FakeHttpHeaders(<String, List<String>>{
+      'x-goog-hash': <String>[],
+    });
+
+    final ArtifactUpdater artifactUpdater = ArtifactUpdater(
+      fileSystem: fileSystem,
+      logger: logger,
+      operatingSystemUtils: operatingSystemUtils,
+      platform: testPlatform,
+      httpClient: client,
+      tempStorage: fileSystem.currentDirectory.childDirectory('temp')
+        ..createSync(),
+    );
+
+    await artifactUpdater.downloadZipArchive(
+      'test message',
+      Uri.parse('http:///test.zip'),
+      fileSystem.currentDirectory.childDirectory('out'),
+    );
+    expect(logger.statusText, contains('test message'));
+    expect(fileSystem.file('out/test'), exists);
+  });
+
+  testWithoutContext('ArtifactUpdater will validate the md5 hash if the '
+    'x-goog-hash header is present', () async {
+    final MockOperatingSystemUtils operatingSystemUtils = MockOperatingSystemUtils();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final BufferLogger logger = BufferLogger.test();
+    final MockHttpClient client = MockHttpClient();
+    client.testRequest.testResponse.headers = FakeHttpHeaders(<String, List<String>>{
+      'x-goog-hash': <String>[
+        'foo-bar-baz',
+        'md5=k7iFrf4NoInN9jSQT9WfcQ=='
+      ],
+    });
+
+    final ArtifactUpdater artifactUpdater = ArtifactUpdater(
+      fileSystem: fileSystem,
+      logger: logger,
+      operatingSystemUtils: operatingSystemUtils,
+      platform: testPlatform,
+      httpClient: client,
+      tempStorage: fileSystem.currentDirectory.childDirectory('temp')
+        ..createSync(),
+    );
+
+    await artifactUpdater.downloadZipArchive(
+      'test message',
+      Uri.parse('http:///test.zip'),
+      fileSystem.currentDirectory.childDirectory('out'),
+    );
+    expect(logger.statusText, contains('test message'));
+    expect(fileSystem.file('out/test'), exists);
+  });
+
+  testWithoutContext('ArtifactUpdater will validate the md5 hash if the '
+    'x-goog-hash header is present and throw if it does not match', () async {
+    final MockOperatingSystemUtils operatingSystemUtils = MockOperatingSystemUtils();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final BufferLogger logger = BufferLogger.test();
+    final MockHttpClient client = MockHttpClient();
+    client.testRequest.testResponse.headers = FakeHttpHeaders(<String, List<String>>{
+      'x-goog-hash': <String>[
+        'foo-bar-baz',
+        'md5=k7iFrf4SQT9WfcQ=='
+      ],
+    });
+
+    final ArtifactUpdater artifactUpdater = ArtifactUpdater(
+      fileSystem: fileSystem,
+      logger: logger,
+      operatingSystemUtils: operatingSystemUtils,
+      platform: testPlatform,
+      httpClient: client,
+      tempStorage: fileSystem.currentDirectory.childDirectory('temp')
+        ..createSync(),
+    );
+
+    await expectLater(() async => await artifactUpdater.downloadZipArchive(
+      'test message',
+      Uri.parse('http:///test.zip'),
+      fileSystem.currentDirectory.childDirectory('out'),
+    ), throwsToolExit(message: 'k7iFrf4SQT9WfcQ==')); // validate that the hash mismatch message is included.
+  });
+
   testWithoutContext('ArtifactUpdater will restart the status ticker if it needs to retry the download', () async {
     final MockOperatingSystemUtils operatingSystemUtils = MockOperatingSystemUtils();
     final MemoryFileSystem fileSystem = MemoryFileSystem.test();
@@ -353,6 +444,7 @@ class MockHttpClient extends Mock implements HttpClient {
     return testRequest;
   }
 }
+
 class MockHttpClientRequest extends Mock implements HttpClientRequest {
   final MockHttpClientResponse testResponse = MockHttpClientResponse();
 
@@ -361,13 +453,28 @@ class MockHttpClientRequest extends Mock implements HttpClientRequest {
     return testResponse;
   }
 }
+
 class MockHttpClientResponse extends Mock implements HttpClientResponse {
   @override
   int statusCode = HttpStatus.ok;
 
+  @override
+  HttpHeaders headers = FakeHttpHeaders(<String, List<String>>{});
+
   @override
   Future<void> forEach(void Function(List<int> element) action) async {
     action(<int>[0]);
     return;
   }
 }
+
+class FakeHttpHeaders extends Fake implements HttpHeaders {
+  FakeHttpHeaders(this.values);
+
+  final Map<String, List<String>> values;
+
+  @override
+  List<String> operator [](String key) {
+    return values[key];
+  }
+}
