appmattus / certificatetransparency

Certificate transparency for Android and JVM
Apache License 2.0
142 stars 29 forks source link

By default all hosts are included if the excluded hosts are not specified #105

Closed hwdavr closed 3 months ago

hwdavr commented 10 months ago

Not sure if this is an intentional design or a bug. From the below code in the library, if the host is not part of the excludeHosts, excludeHosts.any { it.matches(host) } will be false, and the whole condition return true, and the host will be checked for certificate transparency. Why the library have both excludeHosts and includeHosts? To me, we just need one of them.

private fun enabledForCertificateTransparency(host: String) = !excludeHosts.any { it.matches(host) } || includeHosts.any {
        it.matches(
            host
        )
    }
mattmook commented 3 months ago

The idea is you can configure it for just a single sub-domain:

For example:

config {
    excludeHosts = setOf(Host("*.example.com"))
    includeHosts = setOf(Host("included.example.com"))
}

Here, any host that is not example.com will still be fully included, but all sub domains of example.com are excluded except included.example.com.

i.e.:

So basically because all domains are included for CT checks by default (stricter security by default) we check for exclusions first and if there is no exclusion rules for a host then any inclusion rules are redundant. Hope that makes sense.