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

VerifyIP blocked DNS style #973

Open
aireet opened this issue Dec 14, 2022 · 3 comments · May be fixed by #1010
Open

VerifyIP blocked DNS style #973

aireet opened this issue Dec 14, 2022 · 3 comments · May be fixed by #1010
Labels
bug Something isn't working

Comments

@aireet
Copy link
Contributor

aireet commented Dec 14, 2022

BUG REPORT

  1. Please describe the issue you observed:

    • What did you do (The steps to reproduce)?
      I deploy rocketmq on k8s, and use coreDNS addr to connect it. then i was blocked by VerifyIP function

    • What did you expect to see?
      do not intercept connect when the addr not like ipv4 and ipv6
      if the addr actually wrong, the tcp connection will failed and it will sign definite error

@francisoliverlee francisoliverlee added the bug Something isn't working label Feb 22, 2023
@francisoliverlee
Copy link
Member

francisoliverlee commented Feb 22, 2023

从现在的逻辑看,是需要检查namesrv地址的有效性,目前支持以下几种:

  1. ipv4
  2. ipv6
  3. http://或者https://开头。

这个检查没有覆盖namesrv1.com, namesrv1.com;namesrv2.com这种域名场景, 非常欢迎你提交PR修复它。

VerifyIP()代码:

func NewNamesrvAddr(s ...string) (NamesrvAddr, error) {
if len(s) == 0 {
return nil, errors.ErrNoNameserver
}
ss := s
if len(ss) == 1 {
// compatible with multi server env string: "a;b;c"
ss = strings.Split(s[0], ";")
}
for _, srv := range ss {
if err := verifyIP(srv); err != nil {
return nil, err
}
}
addrs := make(NamesrvAddr, 0)
addrs = append(addrs, ss...)
return addrs, nil
}
func (addr NamesrvAddr) Check() error {
for _, srv := range addr {
if err := verifyIP(srv); err != nil {
return err
}
}
return nil
}
var (
httpPrefixRegex, _ = regexp.Compile("^(http|https)://")
)
func verifyIP(ip string) error {
if httpPrefixRegex.MatchString(ip) {
return nil
}
if strings.Contains(ip, ";") {
return errors.ErrMultiIP
}
ipV4s := ipv4Regex.FindAllString(ip, -1)
ipV6s := ipv6Regex.FindAllString(ip, -1)
if len(ipV4s) == 0 && len(ipV6s) == 0 {
return errors.ErrIllegalIP
}
if len(ipV4s) > 1 || len(ipV6s) > 1 {
return errors.ErrMultiIP
}
return nil
}

@aireet aireet linked a pull request Feb 23, 2023 that will close this issue
@aireet
Copy link
Contributor Author

aireet commented Feb 26, 2023

#1010 PR 在这里, 顺便改了使用net包校验ipv4,6 url host:port的方式

@francisoliverlee francisoliverlee linked a pull request Feb 27, 2023 that will close this issue
@aireet
Copy link
Contributor Author

aireet commented Jun 12, 2023

@francisoliverlee 这个pr有计划merge么,我们确实需要通过coreDNS来访问Namesrv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants