Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4658b253 authored Nov 14, 2019 by Lorenzo Colitti

Don't allow a non-empty CA certificate in production code.

The caCertificate field in ResolverParamsParcel is documented as
being for testing only. Because setting it could have security
implications, don't allow the system server to pass it in.

Bug: 144475682
Test: existing tests pass
Change-Id: Ie0b826127cc48f18945a9de5f3579e47706462ee

parent d77eef74

DnsResolverService.cpp

+7 −0

Original line number	Diff line number	Diff line
		@@ -166,6 +166,13 @@ binder_status_t DnsResolverService::dump(int fd, const char**, uint32_t) {
		const ResolverParamsParcel& resolverParams) {
		// Locking happens in PrivateDnsConfiguration and res_* functions.
		ENFORCE_INTERNAL_PERMISSIONS();

		uid_t uid = AIBinder_getCallingUid();
		if (resolverParams.caCertificate.size() != 0 && uid == AID_SYSTEM) {
		auto err = StringPrintf("UID %d is not authorized to set a non-empty CA certificate", uid);
		return ::ndk::ScopedAStatus(AStatus_fromExceptionCodeWithMessage(EX_SECURITY, err.c_str()));
		}

		// TODO: Remove this log after AIDL gen_log supporting more types, b/129732660
		auto entry =
		gDnsResolverLog.newEntry()

DnsTlsSocket.cpp

+3 −1

Original line number	Diff line number	Diff line
		@@ -160,7 +160,9 @@ bool DnsTlsSocket::initialize() {
		// For discussion of alternative, sustainable approaches see b/71909242.
		if (RESOLV_INJECT_CA_CERTIFICATE && !mServer.certificate.empty()) {
		// Inject test CA certs from ResolverParamsParcel.caCertificate for internal testing.
		LOG(WARNING) << "test CA certificate is valid";
		// This is only allowed by DnsResolverService if the caller is not AID_SYSTEM, and on
		// debug builds.
		LOG(WARNING) << "Setting test CA certificate. This should never happen in production code.";
		if (!setTestCaCertificate()) {
		LOG(ERROR) << "Failed to set test CA certificate";
		return false;