From ac751cc50cc28d329f4b7581bbadd960918a9b64 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 12 Oct 2020 17:44:23 -0700 Subject: [PATCH] [flutter_tools] verify checksum of downloaded artifacts (#67839) 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. #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. --- packages/flutter_tools/lib/src/cache.dart | 64 ++++++++++- .../general.shard/artifact_updater_test.dart | 107 ++++++++++++++++++ 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart index 57c02da99a..126810b29f 100644 --- 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 _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 digests; + if (md5Hash != null) { + _logger.printTrace('Content $url md5 hash: $md5Hash'); + digests = StreamController(); + inputSink = md5.startChunkedConversion(digests); + } + final RandomAccessFile randomAccessFile = file.openSync(mode: FileMode.writeOnly); await response.forEach((List 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 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 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 index f5741fed72..d0c6c81e50 100644 --- 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(>{ + 'x-goog-hash': [], + }); + + 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(>{ + 'x-goog-hash': [ + '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(>{ + 'x-goog-hash': [ + '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(>{}); + @override Future forEach(void Function(List element) action) async { action([0]); return; } } + +class FakeHttpHeaders extends Fake implements HttpHeaders { + FakeHttpHeaders(this.values); + + final Map> values; + + @override + List operator [](String key) { + return values[key]; + } +}