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

Support TCP-MD5 authentication for BGP #215

Closed
ghorofamike opened this Issue Apr 4, 2018 · 3 comments

Comments

Projects
2 participants
@ghorofamike
Copy link
Contributor

ghorofamike commented Apr 4, 2018

Is this a bug report or a feature request?:
Feature Request

What happened:
We would like to be able to set a password field in the BGP configuration, needed when working with some peers: Like Vultr's BGP, which accept a password field to authenticate a user, see the protocol bgp vultr section in documentation link above, there is a password line. Looking through the code, seems like it would go to some place like this though of course im not a go programmer yet.

What you expected to happen:
We cant set the password, so we will obviously not be able to use their BGP

How to reproduce it (as minimally and precisely as possible):
Always

Anything else we need to know?:
Not sure if there is a downside, but we only see the upsides.

Environment:

  • MetalLB version: 0.5.0
  • Kubernetes version: 1.9X
  • BGP router type/version: OSS Bird
  • OS (e.g. from /etc/os-release): Fedora
  • Kernel (e.g. uname -a): 4.15.12

@danderson danderson added this to To Do in BGP mode via automation Apr 7, 2018

@danderson danderson added this to the v0.6.0 milestone Apr 7, 2018

@danderson

This comment has been minimized.

Copy link
Member

danderson commented Apr 7, 2018

This looks like a great feature to have.

General overview of implementation:

  • Add the field to the Peer struct in internal/config, and add some config parsing tests (are there any constraints on the format of the BGP password? I don't know)
  • Add the password to the Session struct in internal/bgp, and set the TCP_MD5 socket option in Session.connect() if the password is non-empty. For this, you probably need to be familiar with low-level networking and syscall interfaces in Go, because Go doesn't have an easy API to set the TCP-MD5 key.
  • Connect things up in speaker/bgp_controller.go: take the password out of the config objects and pass them into the BGP session creations.
  • Add documentation: update manifests/example-config.yaml, check if the website needs an update (probably minor edits, but no need for a full doc section about BGP passwords).
  • Add to release notes for 0.6.

Are you interested in contributing a PR for this? I'm happy to implement it myself, but if you want to do it, I would be happy to do code reviews and give you advice!

@danderson danderson changed the title Custom field in BGP protocol config Support TCP-MD5 authentication for BGP Apr 8, 2018

@ghorofamike

This comment has been minimized.

Copy link
Contributor

ghorofamike commented Apr 10, 2018

So i have made some attempt, not yet ready for a PR since its not functional, but see here
code
i need some pointers for solving this error:

E0410 12:19:51.481824       1 bgp.go:52] Connecting to "169.254.169.254:179": setting TCP_MD5 sockopt to "169.254.169.254:179": invalid argument

this line is clearly the issue: Line
its clearly not doing what i intended, but otherwise i think i got the bulk of the checklist done

  • Add the field to the Peer struct in internal/config ...
  • Add the password to the Session struct in internal/bgp ...
  • Connect things up in speaker/bgp_controller.go ...
  • Add documentation: update (Partial, edited manifests/example-config.yaml)
  • Add to release notes :( not done.

I have had a look at gobgp, which seems to do this by not using net.Conn, but using some fancy legwork with os file descriptors link, which i wish we could avoid if the net.Conn api's allow.

@ghorofamike

This comment has been minimized.

Copy link
Contributor

ghorofamike commented Apr 18, 2018

closing, PR #236 merged.

BGP mode automation moved this from To Do to Done Apr 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment