Skip to content

Commit

Permalink
[flutter_tools] verify checksum of downloaded artifacts (#67839)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jonahwilliams
jonahwilliams committed Oct 13, 2020
1 parent eb24393 commit ac751cc
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 3 deletions.
64 changes: 61 additions & 3 deletions packages/flutter_tools/lib/src/cache.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
107 changes: 107 additions & 0 deletions packages/flutter_tools/test/general.shard/artifact_updater_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -353,6 +444,7 @@ class MockHttpClient extends Mock implements HttpClient {
return testRequest;
}
}

class MockHttpClientRequest extends Mock implements HttpClientRequest {
final MockHttpClientResponse testResponse = MockHttpClientResponse();

Expand All @@ -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];
}
}

0 comments on commit ac751cc

Please sign in to comment.