Loading Android.bp +0 −1 Original line number Original line Diff line number Diff line Loading @@ -162,7 +162,6 @@ cc_library { debuggable: { debuggable: { cppflags: [ cppflags: [ "-DRESOLV_ALLOW_VERBOSE_LOGGING=1", "-DRESOLV_ALLOW_VERBOSE_LOGGING=1", "-DRESOLV_INJECT_CA_CERTIFICATE=1", ], ], }, }, }, }, Loading DnsResolverService.cpp +4 −1 Original line number Original line Diff line number Diff line Loading @@ -178,8 +178,11 @@ binder_status_t DnsResolverService::dump(int fd, const char** args, uint32_t num // Locking happens in PrivateDnsConfiguration and res_* functions. // Locking happens in PrivateDnsConfiguration and res_* functions. ENFORCE_INTERNAL_PERMISSIONS(); ENFORCE_INTERNAL_PERMISSIONS(); // TODO@: Switch to selinux based permission check if AIBinder_getCallingSid and // AIBinder_setRequestingSid can be supported by libbinder_dnk (b/159135973). uid_t uid = AIBinder_getCallingUid(); uid_t uid = AIBinder_getCallingUid(); if (resolverParams.caCertificate.size() != 0 && uid == AID_SYSTEM) { // CAUTION: caCertificate should NOT be used except for internal testing. if (resolverParams.caCertificate.size() != 0 && uid != AID_ROOT) { auto err = StringPrintf("UID %d is not authorized to set a non-empty CA certificate", uid); 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())); return ::ndk::ScopedAStatus(AStatus_fromExceptionCodeWithMessage(EX_SECURITY, err.c_str())); } } Loading DnsTlsSocket.cpp +3 −9 Original line number Original line Diff line number Diff line Loading @@ -41,11 +41,6 @@ #include "private/android_filesystem_config.h" // AID_DNS #include "private/android_filesystem_config.h" // AID_DNS #include "resolv_private.h" #include "resolv_private.h" // NOTE: Inject CA certificate for internal testing -- do NOT enable in production builds #ifndef RESOLV_INJECT_CA_CERTIFICATE #define RESOLV_INJECT_CA_CERTIFICATE 0 #endif namespace android { namespace android { using base::StringPrintf; using base::StringPrintf; Loading Loading @@ -152,10 +147,9 @@ bool DnsTlsSocket::initialize() { // Load system CA certs from CAPath for hostname verification. // Load system CA certs from CAPath for hostname verification. // // // For discussion of alternative, sustainable approaches see b/71909242. // For discussion of alternative, sustainable approaches see b/71909242. if (RESOLV_INJECT_CA_CERTIFICATE && !mServer.certificate.empty()) { if (!mServer.certificate.empty()) { // Inject test CA certs from ResolverParamsParcel.caCertificate for internal testing. // Inject test CA certs from ResolverParamsParcel.caCertificate for INTERNAL TESTING ONLY. // This is only allowed by DnsResolverService if the caller is not AID_SYSTEM, and on // This is only allowed by DnsResolverService if the caller is AID_ROOT. // debug builds. LOG(WARNING) << "Setting test CA certificate. This should never happen in production code."; LOG(WARNING) << "Setting test CA certificate. This should never happen in production code."; if (!setTestCaCertificate()) { if (!setTestCaCertificate()) { LOG(ERROR) << "Failed to set test CA certificate"; LOG(ERROR) << "Failed to set test CA certificate"; Loading tests/resolv_integration_test.cpp +12 −0 Original line number Original line Diff line number Diff line Loading @@ -4591,6 +4591,18 @@ TEST_F(ResolverTest, RepeatedSetup_KeepChangingPrivateDnsServers) { EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr2)); EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr2)); } } TEST_F(ResolverTest, PermissionCheckOnCertificateInjection) { ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); parcel.caCertificate = kCaCert; ASSERT_TRUE(mDnsClient.resolvService()->setResolverConfiguration(parcel).isOk()); for (const uid_t uid : {AID_SYSTEM, TEST_UID}) { ScopedChangeUID scopedChangeUID(uid); auto status = mDnsClient.resolvService()->setResolverConfiguration(parcel); EXPECT_EQ(status.getExceptionCode(), EX_SECURITY); } } // Parameterized tests. // Parameterized tests. // TODO: Merge the existing tests as parameterized test if possible. // TODO: Merge the existing tests as parameterized test if possible. // TODO: Perhaps move parameterized tests to an independent file. // TODO: Perhaps move parameterized tests to an independent file. Loading tests/resolv_test_utils.h +12 −0 Original line number Original line Diff line number Diff line Loading @@ -62,6 +62,18 @@ class ScopeBlockedUIDRule { const uid_t mSavedUid; const uid_t mSavedUid; }; }; class ScopedChangeUID { public: ScopedChangeUID(uid_t testUid) : mTestUid(testUid), mSavedUid(getuid()) { EXPECT_TRUE(seteuid(mTestUid) == 0); }; ~ScopedChangeUID() { EXPECT_TRUE(seteuid(mSavedUid) == 0); } private: const uid_t mTestUid; const uid_t mSavedUid; }; struct DnsRecord { struct DnsRecord { std::string host_name; // host name std::string host_name; // host name ns_type type; // record type ns_type type; // record type Loading Loading
Android.bp +0 −1 Original line number Original line Diff line number Diff line Loading @@ -162,7 +162,6 @@ cc_library { debuggable: { debuggable: { cppflags: [ cppflags: [ "-DRESOLV_ALLOW_VERBOSE_LOGGING=1", "-DRESOLV_ALLOW_VERBOSE_LOGGING=1", "-DRESOLV_INJECT_CA_CERTIFICATE=1", ], ], }, }, }, }, Loading
DnsResolverService.cpp +4 −1 Original line number Original line Diff line number Diff line Loading @@ -178,8 +178,11 @@ binder_status_t DnsResolverService::dump(int fd, const char** args, uint32_t num // Locking happens in PrivateDnsConfiguration and res_* functions. // Locking happens in PrivateDnsConfiguration and res_* functions. ENFORCE_INTERNAL_PERMISSIONS(); ENFORCE_INTERNAL_PERMISSIONS(); // TODO@: Switch to selinux based permission check if AIBinder_getCallingSid and // AIBinder_setRequestingSid can be supported by libbinder_dnk (b/159135973). uid_t uid = AIBinder_getCallingUid(); uid_t uid = AIBinder_getCallingUid(); if (resolverParams.caCertificate.size() != 0 && uid == AID_SYSTEM) { // CAUTION: caCertificate should NOT be used except for internal testing. if (resolverParams.caCertificate.size() != 0 && uid != AID_ROOT) { auto err = StringPrintf("UID %d is not authorized to set a non-empty CA certificate", uid); 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())); return ::ndk::ScopedAStatus(AStatus_fromExceptionCodeWithMessage(EX_SECURITY, err.c_str())); } } Loading
DnsTlsSocket.cpp +3 −9 Original line number Original line Diff line number Diff line Loading @@ -41,11 +41,6 @@ #include "private/android_filesystem_config.h" // AID_DNS #include "private/android_filesystem_config.h" // AID_DNS #include "resolv_private.h" #include "resolv_private.h" // NOTE: Inject CA certificate for internal testing -- do NOT enable in production builds #ifndef RESOLV_INJECT_CA_CERTIFICATE #define RESOLV_INJECT_CA_CERTIFICATE 0 #endif namespace android { namespace android { using base::StringPrintf; using base::StringPrintf; Loading Loading @@ -152,10 +147,9 @@ bool DnsTlsSocket::initialize() { // Load system CA certs from CAPath for hostname verification. // Load system CA certs from CAPath for hostname verification. // // // For discussion of alternative, sustainable approaches see b/71909242. // For discussion of alternative, sustainable approaches see b/71909242. if (RESOLV_INJECT_CA_CERTIFICATE && !mServer.certificate.empty()) { if (!mServer.certificate.empty()) { // Inject test CA certs from ResolverParamsParcel.caCertificate for internal testing. // Inject test CA certs from ResolverParamsParcel.caCertificate for INTERNAL TESTING ONLY. // This is only allowed by DnsResolverService if the caller is not AID_SYSTEM, and on // This is only allowed by DnsResolverService if the caller is AID_ROOT. // debug builds. LOG(WARNING) << "Setting test CA certificate. This should never happen in production code."; LOG(WARNING) << "Setting test CA certificate. This should never happen in production code."; if (!setTestCaCertificate()) { if (!setTestCaCertificate()) { LOG(ERROR) << "Failed to set test CA certificate"; LOG(ERROR) << "Failed to set test CA certificate"; Loading
tests/resolv_integration_test.cpp +12 −0 Original line number Original line Diff line number Diff line Loading @@ -4591,6 +4591,18 @@ TEST_F(ResolverTest, RepeatedSetup_KeepChangingPrivateDnsServers) { EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr2)); EXPECT_FALSE(hasUncaughtPrivateDnsValidation(addr2)); } } TEST_F(ResolverTest, PermissionCheckOnCertificateInjection) { ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel(); parcel.caCertificate = kCaCert; ASSERT_TRUE(mDnsClient.resolvService()->setResolverConfiguration(parcel).isOk()); for (const uid_t uid : {AID_SYSTEM, TEST_UID}) { ScopedChangeUID scopedChangeUID(uid); auto status = mDnsClient.resolvService()->setResolverConfiguration(parcel); EXPECT_EQ(status.getExceptionCode(), EX_SECURITY); } } // Parameterized tests. // Parameterized tests. // TODO: Merge the existing tests as parameterized test if possible. // TODO: Merge the existing tests as parameterized test if possible. // TODO: Perhaps move parameterized tests to an independent file. // TODO: Perhaps move parameterized tests to an independent file. Loading
tests/resolv_test_utils.h +12 −0 Original line number Original line Diff line number Diff line Loading @@ -62,6 +62,18 @@ class ScopeBlockedUIDRule { const uid_t mSavedUid; const uid_t mSavedUid; }; }; class ScopedChangeUID { public: ScopedChangeUID(uid_t testUid) : mTestUid(testUid), mSavedUid(getuid()) { EXPECT_TRUE(seteuid(mTestUid) == 0); }; ~ScopedChangeUID() { EXPECT_TRUE(seteuid(mSavedUid) == 0); } private: const uid_t mTestUid; const uid_t mSavedUid; }; struct DnsRecord { struct DnsRecord { std::string host_name; // host name std::string host_name; // host name ns_type type; // record type ns_type type; // record type Loading