Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CNAME not displayed with -ports flag #604

Closed
JoshuaMart opened this issue May 4, 2022 · 3 comments · Fixed by #625
Closed

CNAME not displayed with -ports flag #604

JoshuaMart opened this issue May 4, 2022 · 3 comments · Fixed by #625
Assignees
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Milestone

Comments

@JoshuaMart
Copy link
Contributor

JoshuaMart commented May 4, 2022

HTTPX Version : v1.2.1

Hi,
When the -ports option is used with -cname it is not displayed anymore

Current Behavior:

root@5131dca14ebb: echo "www.dnsimple.com" | httpx -silent -cname
https://www.dnsimple.com [dnsimple.com]

root@5131dca14ebb: echo "www.dnsimple.com" | httpx -silent -cname -ports 80,443
http://www.dnsimple.com
https://www.dnsimple.com

Expected Behavior:

That the cname is also displayed with the -ports option

Steps To Reproduce:

echo "www.dnsimple.com" | httpx -silent -cname -ports 80,443

Regards

@JoshuaMart JoshuaMart added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label May 4, 2022
@jimen0
Copy link
Contributor

jimen0 commented May 5, 2022

The difference comes from the following snippet:

+	gologger.Info().Msgf("[-] About to get DNS data of domain=%s", domain)
	ips, cnames, err := getDNSData(hp, domain)
	if err != nil {
		ips = append(ips, ip)
	}
+	gologger.Info().Msgf("[-] IPs=%#+v, cnames=%#+v, err=%v", ips, cnames, err)
$ echo "www.dnsimple.com" | httpx -cname -ports 80,443
[INF] [-] About to get DNS data of domain=https://www.dnsimple.com:443
[INF] [-] About to get DNS data of domain=https://www.dnsimple.com:80
[INF] [-] IPs=[]string{}, cnames=[]string(nil), err=<nil>
http://www.dnsimple.com
[INF] [-] IPs=[]string{}, cnames=[]string(nil), err=<nil>
https://www.dnsimple.com

...

$ echo "www.dnsimple.com" | httpx -cname              

[INF] [-] About to get DNS data of domain=www.dnsimple.com
[INF] [-] IPs=[]string{"104.245.210.170"}, cnames=[]string{"dnsimple.com"}, err=<nil>
https://www.dnsimple.com [dnsimple.com]

The difference between valid and empty results comes from the argument with which fastdialer's GetDNSData method is being called. The hostnames I used below are the ones httpx passes to it as you can see in the above debug logs I added to httpx. Here is a minimal reproducer:

$ go run ./cmd/fastdialer/main.go
[] []
[104.245.210.170] []
package main

import (
	"fmt"

	"github.com/projectdiscovery/fastdialer/fastdialer"
)

func main() {
	fd, err := fastdialer.NewDialer(fastdialer.DefaultOptions)
	if err != nil {
		panic(err)
	}

	data, err := fd.GetDNSData("https://www.dnsimple.com:443")
	if err != nil {
		panic(err)
	}

	fmt.Println(data.A, data.AAAA)

	data, err = fd.GetDNSData("www.dnsimple.com")
	if err != nil {
		panic(err)
	}

	fmt.Println(data.A, data.AAAA)
}

@jimen0
Copy link
Contributor

jimen0 commented May 5, 2022

Hi @Mzack9999 @ehsandeep - based on the above, is fastdialer's GetDNSData(hostname string) expected to be able to work with strings such as https://www.dnsimple.com:443 or should it always receive just DNS names such as www.dnsimple.com? Once you answer that, I can work on a patch and submit it.

If we go with the last one, which is the one that sounds right to me, the fix goes in httpx by cleaning up the hostname before passing it to fastdialer.

Thanks!

@ehsandeep ehsandeep added the Priority: Medium This issue may be useful, and needs some attention. label May 6, 2022
@LuitelSamikshya LuitelSamikshya self-assigned this May 13, 2022
@LuitelSamikshya LuitelSamikshya added the Status: In Progress This issue is being worked on, and has someone assigned. label May 13, 2022
@LuitelSamikshya LuitelSamikshya linked a pull request May 16, 2022 that will close this issue
@LuitelSamikshya LuitelSamikshya added Status: Review Needed The issue has a PR attached to it which needs to be reviewed and removed Status: In Progress This issue is being worked on, and has someone assigned. labels May 16, 2022
@Mzack9999
Copy link
Member

@jimen0 Unfortunately, I've seen your comment only now, and meanwhile, a solution was already worked at #625. Your suggestion is 100% correct and should be implemented shortly. Thanks for your help!

@Mzack9999 Mzack9999 added Status: In Progress This issue is being worked on, and has someone assigned. Status: Review Needed The issue has a PR attached to it which needs to be reviewed and removed Status: Review Needed The issue has a PR attached to it which needs to be reviewed Status: In Progress This issue is being worked on, and has someone assigned. labels May 17, 2022
@ehsandeep ehsandeep added Status: Completed Nothing further to be done with this issue. Awaiting to be closed. and removed Status: Review Needed The issue has a PR attached to it which needs to be reviewed labels May 17, 2022
@ehsandeep ehsandeep added this to the v1.2.2 milestone Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants