Skip to content

Commit

Permalink
cros_healthd: Add HttpsFirewall routine
Browse files Browse the repository at this point in the history
Add the HttpsFirewall routine to the ServiceConnection.

BUG=chromium:956783
TEST=1) chromeos_unittests --gtest_filter=CrosHealthdServiceConnectionTest.*
2) unit_tests --gtest_filter=DeviceCommandRunRoutineJobTest*
3) Applied HttpFirewall changes and successfully ran the
HttpFirewall routine on a DUT (verified using cros-health-tool diag
--action=run_routine --routine=https_firewall).

Change-Id: I55dafa03d296c3451b40dd9720a3447259ebb5fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2482091
Commit-Queue: Kartik Hegde <khegde@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Paul Moy <pmoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828392}
  • Loading branch information
Kartik Hegde authored and Commit Bot committed Nov 17, 2020
1 parent f80f895 commit a65d6bf
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,14 @@ void DeviceCommandRunRoutineJob::RunImpl(CallbackWithResult succeeded_callback,
std::move(failed_callback)));
break;
}
case chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kHttpsFirewall: {
chromeos::cros_healthd::ServiceConnection::GetInstance()
->RunHttpsFirewallRoutine(base::BindOnce(
&DeviceCommandRunRoutineJob::OnCrosHealthdResponseReceived,
weak_ptr_factory_.GetWeakPtr(), std::move(succeeded_callback),
std::move(failed_callback)));
break;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1297,4 +1297,23 @@ TEST_F(DeviceCommandRunRoutineJobTest, RunHttpFirewallRoutineSuccess) {
})));
}

// Note that the HTTPS firewall routine has no parameters, so we only need to
// test that it can be run successfully.
TEST_F(DeviceCommandRunRoutineJobTest, RunHttpsFirewallRoutineSuccess) {
auto run_routine_response =
chromeos::cros_healthd::mojom::RunRoutineResponse::New(kId, kStatus);
chromeos::cros_healthd::FakeCrosHealthdClient::Get()
->SetRunRoutineResponseForTesting(run_routine_response);
base::Value params_dict(base::Value::Type::DICTIONARY);
EXPECT_TRUE(RunJob(
chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kHttpsFirewall,
std::move(params_dict),
base::BindLambdaForTesting([](RemoteCommandJob* job) {
EXPECT_EQ(job->status(), RemoteCommandJob::SUCCEEDED);
std::unique_ptr<std::string> payload = job->GetResultPayload();
EXPECT_TRUE(payload);
EXPECT_EQ(CreateSuccessPayload(kId, kStatus), *payload);
})));
}

} // namespace policy
5 changes: 5 additions & 0 deletions chromeos/dbus/cros_healthd/fake_cros_healthd_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ void FakeCrosHealthdService::RunHttpFirewallRoutine(
std::move(callback).Run(run_routine_response_.Clone());
}

void FakeCrosHealthdService::RunHttpsFirewallRoutine(
RunHttpsFirewallRoutineCallback callback) {
std::move(callback).Run(run_routine_response_.Clone());
}

void FakeCrosHealthdService::AddBluetoothObserver(
mojom::CrosHealthdBluetoothObserverPtr observer) {
bluetooth_observers_.Add(observer.PassInterface());
Expand Down
2 changes: 2 additions & 0 deletions chromeos/dbus/cros_healthd/fake_cros_healthd_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ class FakeCrosHealthdService final
void RunCaptivePortalRoutine(
RunCaptivePortalRoutineCallback callback) override;
void RunHttpFirewallRoutine(RunHttpFirewallRoutineCallback callback) override;
void RunHttpsFirewallRoutine(
RunHttpsFirewallRoutineCallback callback) override;

// CrosHealthdEventService overrides:
void AddBluetoothObserver(
Expand Down
12 changes: 12 additions & 0 deletions chromeos/services/cros_healthd/public/cpp/service_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ class ServiceConnectionImpl : public ServiceConnection {
void RunHttpFirewallRoutine(
mojom::CrosHealthdDiagnosticsService::RunHttpFirewallRoutineCallback
callback) override;
void RunHttpsFirewallRoutine(
mojom::CrosHealthdDiagnosticsService::RunHttpsFirewallRoutineCallback
callback) override;
void AddBluetoothObserver(
mojo::PendingRemote<mojom::CrosHealthdBluetoothObserver> pending_observer)
override;
Expand Down Expand Up @@ -475,6 +478,15 @@ void ServiceConnectionImpl::RunHttpFirewallRoutine(
std::move(callback));
}

void ServiceConnectionImpl::RunHttpsFirewallRoutine(
mojom::CrosHealthdDiagnosticsService::RunHttpsFirewallRoutineCallback
callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
BindCrosHealthdDiagnosticsServiceIfNeeded();
cros_healthd_diagnostics_service_->RunHttpsFirewallRoutine(
std::move(callback));
}

void ServiceConnectionImpl::AddBluetoothObserver(
mojo::PendingRemote<mojom::CrosHealthdBluetoothObserver> pending_observer) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,13 @@ class ServiceConnection {
mojom::CrosHealthdDiagnosticsService::RunHttpFirewallRoutineCallback
callback) = 0;

// Requests that cros_healthd runs the HTTPS firewall routine. See
// src/chromeos/service/cros_healthd/public/mojom/cros_healthd.mojom for
// details.
virtual void RunHttpsFirewallRoutine(
mojom::CrosHealthdDiagnosticsService::RunHttpsFirewallRoutineCallback
callback) = 0;

// Subscribes to cros_healthd's Bluetooth-related events. See
// src/chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom for
// details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,19 @@ TEST_F(CrosHealthdServiceConnectionTest, RunHttpFirewallRoutine) {
run_loop.Run();
}

// Test that we can run the HTTPS firewall routine.
TEST_F(CrosHealthdServiceConnectionTest, RunHttpsFirewallRoutine) {
auto response = MakeRunRoutineResponse();
FakeCrosHealthdClient::Get()->SetRunRoutineResponseForTesting(response);
base::RunLoop run_loop;
ServiceConnection::GetInstance()->RunHttpsFirewallRoutine(
base::BindLambdaForTesting([&](mojom::RunRoutineResponsePtr response) {
EXPECT_EQ(response, MakeRunRoutineResponse());
run_loop.Quit();
}));
run_loop.Run();
}

// Test that we can add a Bluetooth observer.
TEST_F(CrosHealthdServiceConnectionTest, AddBluetoothObserver) {
MockCrosHealthdBluetoothObserver observer;
Expand Down
10 changes: 10 additions & 0 deletions chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,16 @@ interface CrosHealthdDiagnosticsService {
// * |response| - contains a unique identifier and status for the created
// routine.
RunHttpFirewallRoutine() => (RunRoutineResponse response);

// Requests that the HttpsFirewall routine is created and started on the
// platform. This routine checks whether a firewall is blocking HTTPS port
// 443. This routine is only available if GetAvailableRoutines returned
// kHttpsFirewall.
//
// The response:
// * |response| - contains a unique identifier and status for the created
// routine.
RunHttpsFirewallRoutine() => (RunRoutineResponse response);
};

// Event interface exposed by the cros_healthd daemon.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ enum DiagnosticRoutineEnum {
kDnsResolution = 21,
kCaptivePortal = 22,
kHttpFirewall = 23,
kHttpsFirewall = 24,
};

// Enumeration of the possible DiskRead routine's command type
Expand Down

0 comments on commit a65d6bf

Please sign in to comment.