BishopFox / sliver

Adversary Emulation Framework
GNU General Public License v3.0
8.36k stars 1.1k forks source link

DNS Issues with resolvers using DNS-0x20 encoding #1587

Open miszr opened 7 months ago

miszr commented 7 months ago

Describe the bug If a host, windows was used during testing, is configured to use a DNS resolver which employs what is known as DNS-0x20 encoding, the DNS C2 communication channel is never established.

The encoding known as DNS-0x20, will basically randomly change the case of each character in a DNS query. This will garble the DNS queries sent to the sliver server which result in error messages in the logs.

Here is an example (cleaned for privacy) of what will be seen in the sliver.log file:

INFO[2024-03-06T09:40:52Z] [sliver/server/c2/dns.go:361] 'bAAKbH898rE8.sUbdoMAIn.eXAmpLe.coM.' is subdomain of 'subdomain.example.com.' 
ERRO[2024-03-06T09:40:52Z] [sliver/server/c2/dns.go:377] [dns] error decoding subdata: invalid dns message

To Reproduce Configure a host to use the Google public DNS resolver (8.8.8.8) and attempt to deploy a DNS beacon.

Expected behavior The beacon is successfully deployed and can be used.

Desktop (please complete the following information):

Additional context Found this article as well as the linked whitepaper

moloch-- commented 7 months ago

I think the real bug is in the detection of case sensitive vs. case insensitive encoding, the implant should* be able to detect this manipulation and fallback to base32 but doesn't.

miszr commented 7 months ago

There are multiple issues here:

First Issue

An example of a message which failes to decode is the initial TOTP message, which is always base32 encoded. https://github.com/BishopFox/sliver/blob/c8a7948671eafba4d6f871c2f2b46b900202699d/implant/sliver/transports/dnsclient/dnsclient.go#L926-L937

The problem is that the DNS-0x20 encoding will alter the case of the subdata, which then fails decoding in the base32 encoder, as there are characters which are uppercase. https://github.com/BishopFox/sliver/blob/c8a7948671eafba4d6f871c2f2b46b900202699d/util/encoders/base32.go#L27-L39

Possible Solution

A solution to this is doing strings.ToLower on the input data.

return sliverBase32.DecodeString(strings.ToLower(string(data)))

This however is partially problematic(programming wise) as, it transfers the responsibility from server/c2/dns.go to the base32 encoder.

Second Issue

Sometimes, a base32 encoded value mangled by DNS-0x20(for example) is interpreted as a base58 encoded value. I have seen you mention this issue here: https://github.com/BishopFox/sliver/issues/1354#issuecomment-1808398666

This can become a problem for example here: https://github.com/BishopFox/sliver/blob/c8a7948671eafba4d6f871c2f2b46b900202699d/server/c2/dns.go#L382-L394

When the data gets interpreted as base58 and produces semi-valid protobuf data, the decodeSubdata functions returns a message and no error. This becomes a problem on line 394 as we use the ID field, which is incorrectly interpreted.

This issue will produce a log warning: https://github.com/BishopFox/sliver/blob/c8a7948671eafba4d6f871c2f2b46b900202699d/server/c2/dns.go#L396

Possible Solution

A possible solution could be that if we attempt to decode the subdata using the current method and we are unable to find a valid session id we could attempt to decode the lowercase value of the subdata.

The solution would look something like this:

    dnsLog.Debugf("[dns] processing req for subdomain = %s", subdomain)
    msg, checksum, err := s.decodeSubdata(subdomain)
    if err != nil {
        dnsLog.Errorf("[dns] error decoding subdata: %v", err)
        return s.nameErrorResp(req)
    }

    // TOTP Handler can be called without dns session ID
    if msg.Type == dnspb.DNSMessageType_TOTP {
        return s.handleHello(domain, msg, req)
    }

    // All other handlers require a valid dns session ID
    _, ok := s.sessions.Load(msg.ID & sessionIDBitMask)
    if !ok {
        subdomain = strings.ToLower(subdomain)

        dnsLog.Debugf("[dns] reprocessing req for subdomain = %s", subdomain)
        msg, checksum, err = s.decodeSubdata(subdomain)
        if err != nil {
            dnsLog.Errorf("[dns] error decoding subdata: %v", err)
            return s.nameErrorResp(req)
        }
        _, ok := s.sessions.Load(msg.ID & sessionIDBitMask)
        if !ok {
            dnsLog.Warnf("[dns] session not found for id %v (%v)", msg.ID, msg.ID&sessionIDBitMask)
            return s.nameErrorResp(req)
        }
    }

This is a bit of a hacky solution, but should work.

Better Solution

Would be nice to be able to determine whether the resolver manipulates the DNS queries using for example DNS-0x20 encoding.

moloch-- commented 7 months ago

We've removed TOTP in v1.6, it would be good to address all these issues in that branch. Perhaps we should just use a single bit to indicate the Base32 vs. Base58 instead of trying to detect it.

miszr commented 7 months ago

Sorry, but I do not understand. The code I referenced in the previous comment is from the master branch and not to the 1.5.x/master branch.

Is there a more updated version of v1.6?

moloch-- commented 7 months ago

Oh yes, you're right. We did remove the TOTP auth, but the message is still called TOTP.

mragusa commented 6 months ago

I am using BIND9 with a custom TLD in a lab environment and encountering the same issue in v1.5.42:

"file":"github.com/bishopfox/sliver/server/c2/dns.go:377","func":"github.com/bishopfox/sliver/server/c2.(*SliverDNSServer).handleC2","level":"eror","msg":"[dns] error decoding subdata: invalid dns message","pkg":"c2","stream":"dns","time":"2024-04-11T13:51:09-04:00"}

{"file":"github.com/bishopfox/sliver/server/c2/dns.go:361","func":"github.com/bishopfox/sliver/server/c2.(*SliverDNSServer).isC2SubDomain","level":"info","msg":"'Y1utwLtf8cjUiQzG79EKeAhKMrKCtAqxd4WPt3nrrEW5DAa4B4hBqGMxXcjHRtR.6EhPZHPu9Mfr3FLgNZTrNm1kvfVGUbJfM9r6y3kTav5TE1kQZDETnZ7MngmqA2m.xskXt92QdYcAwqYEf1SPDr8ncU8nzMKUNUJK8bSkSKqZyqHPkKwKY7ePHJdzuiP.xEgdNGHeWVmhfKB64ro9RZKBK2Ded4Y2iAq8Y.beacon.example.com.' is subdomain of 'beacon.example.com.'","pkg":"c2","stream":"dns","time":"2024-04-11T13:51:09-04:00"}

The client binary is running on debian linux 12.4

derekkddj commented 5 months ago

i am havking the same error

GeneralBison commented 5 months ago

I'm having this issue even with force-base32 enabled. Should this work or is it a problem with my setup?

miszr commented 5 months ago

I'm having this issue even with force-base32 enabled. Should this work or is it a problem with my setup?

Unfortunately no. The base32 encoder does not force lowercase characters, which will mean that decoding will not be possible using base32.

derekkddj commented 5 months ago

for now i can make it work using DNS server 1.1.1.1, but i can not use this is production if the "victim" is using google DNS resolvers.

try-catch-try commented 5 months ago

Our team is also experiencing the same problem when using 8.8.8.8 resolver as well as some others. While using server version 1.5.42 I attempted to start the dns listener with the -D option to disable TOTP auth as suggested but this did not appear to fix the problem. I also tried to compile and run version 1.6 of the server using https://github.com/BishopFox/sliver/commit/9510a5f0c19ce0dca8ad8084abf68741970bb21b and it appears that the DNS command has been removed so I was unable to test.

moloch-- commented 5 months ago

I'm working on a fix for v1.6 and hoping it have it done soon, it's the last major bug before the v1.6 release!

to016 commented 2 months ago

@moloch-- so what is the temporary fix for dns before the release of v1.6 ? i mean are there any pull requests from 1.5.x/master to fully fix the dns's related problem in current sliver version?

to016 commented 2 months ago

@miszr About your proposed solution for second issue, we have to remove the first occurrence of return s.nameErrorResp(req), right ?

miszr commented 2 months ago

@to016 It has been a while since I was looking at this. Based on a cursory look, you are correct. Removing the error, is however not correct.

The first location should be handled in the same way that the retry after the attempted Load function is handled.

Thanks for your feedback.

to016 commented 2 months ago

@miszr I mean we can fix it like this, remove the return so it will keep the subsequent code to run and fix the issue (at least for base32 dns traffix).

    dnsLog.Debugf("[dns] processing req for subdomain = %s", subdomain)
    msg, checksum, err := s.decodeSubdata(subdomain)
    if err != nil {
        dnsLog.Errorf("[dns] error decoding subdata: %v", err)
        //return s.nameErrorResp(req) -> comment it out so the subsequent code can be continutely execute
    }

    // TOTP Handler can be called without dns session ID
    if msg.Type == dnspb.DNSMessageType_TOTP {
        return s.handleHello(domain, msg, req)
    }

    // All other handlers require a valid dns session ID
    _, ok := s.sessions.Load(msg.ID & sessionIDBitMask)
    if !ok {
        subdomain = strings.ToLower(subdomain)

        dnsLog.Debugf("[dns] reprocessing req for subdomain = %s", subdomain)
        msg, checksum, err = s.decodeSubdata(subdomain)
        if err != nil {
            dnsLog.Errorf("[dns] error decoding subdata: %v", err)
            return s.nameErrorResp(req)
        }
        _, ok := s.sessions.Load(msg.ID & sessionIDBitMask)
        if !ok {
            dnsLog.Warnf("[dns] session not found for id %v (%v)", msg.ID, msg.ID&sessionIDBitMask)
            return s.nameErrorResp(req)
        }
    }
miszr commented 2 months ago

@to016 decodeSubdata will return a nil value in the msg variable. Which will fail in subsequent lines. We also don't want to attempt to interpret an invalid message incorrectly.

rustxj commented 1 month ago

How to solve this problem, stuck here.