pimd: MSDP peer MD5 authentication #8980
Conversation
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/50a46aa04393d826585e3fab539b1284/raw/b2ee620b5fbf24e39189f4afb38867d1fb6972f5/cr_8980_1625409243.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index e1b940111..a317f9e0e 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -6580,12 +6580,9 @@ DEFUN (clear_ip_mroute_count,
DEFPY(clear_ip_msdp_peer, clear_ip_msdp_peer_cmd,
"clear ip msdp peer A.B.C.D$peer [vrf WORD$vrfname]",
- CLEAR_STR
- IP_STR
- MSDP_STR
+ CLEAR_STR IP_STR MSDP_STR
"Restart MSDP peer\n"
- "MSDP peer address\n"
- VRF_CMD_HELP_STR)
+ "MSDP peer address\n" VRF_CMD_HELP_STR)
{
struct vrf *vrf;
struct pim_instance *pim;
@@ -9798,8 +9795,7 @@ DEFUN (no_ip_msdp_peer,
DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
"ip msdp peer A.B.C.D$peer password WORD$psk",
- IP_STR
- CFG_MSDP_STR
+ IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
@@ -9822,19 +9818,16 @@ DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
return CMD_SUCCESS;
}
- nb_cli_enqueue_change(vty, "./authentication-type",
- NB_OP_MODIFY, "MD5");
- nb_cli_enqueue_change(vty, "./authentication-key",
- NB_OP_MODIFY, psk);
+ nb_cli_enqueue_change(vty, "./authentication-type", NB_OP_MODIFY,
+ "MD5");
+ nb_cli_enqueue_change(vty, "./authentication-key", NB_OP_MODIFY, psk);
return nb_cli_apply_changes(vty, xpath);
}
DEFPY(no_ip_msdp_peer_md5, no_ip_msdp_peer_md5_cmd,
"no ip msdp peer A.B.C.D$peer password [WORD]",
- NO_STR
- IP_STR
- CFG_MSDP_STR
+ NO_STR IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
@@ -9857,8 +9850,8 @@ DEFPY(no_ip_msdp_peer_md5, no_ip_msdp_peer_md5_cmd,
return CMD_SUCCESS;
}
- nb_cli_enqueue_change(vty, "./authentication-type",
- NB_OP_MODIFY, "None");
+ nb_cli_enqueue_change(vty, "./authentication-type", NB_OP_MODIFY,
+ "None");
return nb_cli_apply_changes(vty, xpath);
}
diff --git a/pimd/pim_msdp.c b/pimd/pim_msdp.c
index ce3785c97..43416d695 100644
--- a/pimd/pim_msdp.c
+++ b/pimd/pim_msdp.c
@@ -1371,8 +1371,8 @@ bool pim_msdp_peer_config_write(struct vty *vty, struct pim_instance *pim,
&mp->peer, &mp->local);
if (mp->auth_type == MSDP_AUTH_MD5)
- vty_out(vty, "%sip msdp peer %pI4 password %s\n", spaces,
- &mp->peer, mp->auth_key);
+ vty_out(vty, "%sip msdp peer %pI4 password %s\n",
+ spaces, &mp->peer, mp->auth_key);
written = true;
}
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/f0ead3390b99922182c6d74e976914f1/raw/04294bcb2f9e1acd547633a8c3c3b5fe93ac41f1/cr_8980_1625409477.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index bb945df8b..a317f9e0e 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -6580,12 +6580,9 @@ DEFUN (clear_ip_mroute_count,
DEFPY(clear_ip_msdp_peer, clear_ip_msdp_peer_cmd,
"clear ip msdp peer A.B.C.D$peer [vrf WORD$vrfname]",
- CLEAR_STR
- IP_STR
- MSDP_STR
+ CLEAR_STR IP_STR MSDP_STR
"Restart MSDP peer\n"
- "MSDP peer address\n"
- VRF_CMD_HELP_STR)
+ "MSDP peer address\n" VRF_CMD_HELP_STR)
{
struct vrf *vrf;
struct pim_instance *pim;
@@ -9798,8 +9795,7 @@ DEFUN (no_ip_msdp_peer,
DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
"ip msdp peer A.B.C.D$peer password WORD$psk",
- IP_STR
- CFG_MSDP_STR
+ IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
@@ -9831,9 +9827,7 @@ DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
DEFPY(no_ip_msdp_peer_md5, no_ip_msdp_peer_md5_cmd,
"no ip msdp peer A.B.C.D$peer password [WORD]",
- NO_STR
- IP_STR
- CFG_MSDP_STR
+ NO_STR IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedNetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build: NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/CI012BUILD/config.status/config.status Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/DEB11AMD64/config.status/config.statusMake failed for Debian 11 amd64 build: Debian 11 amd64 build: Unknown Log <config.log.gz> FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/CI009BUILD/config.status/config.status FreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build: FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/FBSD12AMD64/config.status/config.status Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsNetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build: NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/CI012BUILD/config.status/config.status Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/DEB11AMD64/config.status/config.statusMake failed for Debian 11 amd64 build: Debian 11 amd64 build: Unknown Log <config.log.gz> FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/CI009BUILD/config.status/config.status FreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build: FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20045/artifact/FBSD12AMD64/config.status/config.status |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedNetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build: NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/CI012BUILD/config.status/config.status Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/DEB11AMD64/config.status/config.statusMake failed for Debian 11 amd64 build: Debian 11 amd64 build: Unknown Log <config.log.gz> FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/CI009BUILD/config.status/config.status FreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build: FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/FBSD12AMD64/config.status/config.status Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsNetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build: NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/CI012BUILD/config.status/config.status Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/DEB11AMD64/config.status/config.statusMake failed for Debian 11 amd64 build: Debian 11 amd64 build: Unknown Log <config.log.gz> FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/CI009BUILD/config.status/config.status FreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build: FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20046/artifact/FBSD12AMD64/config.status/config.status |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/04ed4a213322acd28c896600c5ef7ca4/raw/04294bcb2f9e1acd547633a8c3c3b5fe93ac41f1/cr_8980_1625413990.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index bb945df8b..a317f9e0e 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -6580,12 +6580,9 @@ DEFUN (clear_ip_mroute_count,
DEFPY(clear_ip_msdp_peer, clear_ip_msdp_peer_cmd,
"clear ip msdp peer A.B.C.D$peer [vrf WORD$vrfname]",
- CLEAR_STR
- IP_STR
- MSDP_STR
+ CLEAR_STR IP_STR MSDP_STR
"Restart MSDP peer\n"
- "MSDP peer address\n"
- VRF_CMD_HELP_STR)
+ "MSDP peer address\n" VRF_CMD_HELP_STR)
{
struct vrf *vrf;
struct pim_instance *pim;
@@ -9798,8 +9795,7 @@ DEFUN (no_ip_msdp_peer,
DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
"ip msdp peer A.B.C.D$peer password WORD$psk",
- IP_STR
- CFG_MSDP_STR
+ IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
@@ -9831,9 +9827,7 @@ DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
DEFPY(no_ip_msdp_peer_md5, no_ip_msdp_peer_md5_cmd,
"no ip msdp peer A.B.C.D$peer password [WORD]",
- NO_STR
- IP_STR
- CFG_MSDP_STR
+ NO_STR IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedFreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build: FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/FBSD12AMD64/config.status/config.status NetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build: NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/CI012BUILD/config.status/config.status FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/CI009BUILD/config.status/config.status Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsFreeBSD 12 amd64 build: Failed (click for details)Make failed for FreeBSD 12 amd64 build: FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/FBSD12AMD64/config.status/config.status NetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/CI012BUILD/config.log/config.log.gzMake failed for NetBSD 8 amd64 build: NetBSD 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/CI012BUILD/config.status/config.status FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20047/artifact/CI009BUILD/config.status/config.status |
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/befaaec12014fc8df4e33a1591b0440d/raw/04294bcb2f9e1acd547633a8c3c3b5fe93ac41f1/cr_8980_1625422884.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index bb945df8b..a317f9e0e 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -6580,12 +6580,9 @@ DEFUN (clear_ip_mroute_count,
DEFPY(clear_ip_msdp_peer, clear_ip_msdp_peer_cmd,
"clear ip msdp peer A.B.C.D$peer [vrf WORD$vrfname]",
- CLEAR_STR
- IP_STR
- MSDP_STR
+ CLEAR_STR IP_STR MSDP_STR
"Restart MSDP peer\n"
- "MSDP peer address\n"
- VRF_CMD_HELP_STR)
+ "MSDP peer address\n" VRF_CMD_HELP_STR)
{
struct vrf *vrf;
struct pim_instance *pim;
@@ -9798,8 +9795,7 @@ DEFUN (no_ip_msdp_peer,
DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
"ip msdp peer A.B.C.D$peer password WORD$psk",
- IP_STR
- CFG_MSDP_STR
+ IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
@@ -9831,9 +9827,7 @@ DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
DEFPY(no_ip_msdp_peer_md5, no_ip_msdp_peer_md5_cmd,
"no ip msdp peer A.B.C.D$peer password [WORD]",
- NO_STR
- IP_STR
- CFG_MSDP_STR
+ NO_STR IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 8: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 8: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 arm8 part 8: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 8: No useful log foundCLANG Static Analyzer Summary
2 Static Analyzer issues remaining.See details at |
Implement MSDP peer MD5 authentication. Notes: - Move the MSDP socket creation code to a generic function so it can be parametrized to be used with/without authentication. - The MSDP peer connection will not change when the configuration is set, instead it will only be applied next connection or when `clear ip msdp peer A.B.C.D` is called. Signed-off-by: Rafael Zalamena <[email protected]>
Tell user how to use the new authentication and clear commands. Signed-off-by: Rafael Zalamena <[email protected]>
Modify existing MSDP topology to use authentication. Signed-off-by: Rafael Zalamena <[email protected]>
Add definition of `TCP_MD5SIG_MAXKEYLEN` in the `sockopt.h` header so users of it have the definition of the maximum key length for socket authentication operations. The following OSes reported failure in CI while building: - NetBSD 8 - FreeBSD 11 - FreeBSD 12 Signed-off-by: Rafael Zalamena <[email protected]>
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/37f2d4fdfaa6463dfe176769f0c7e47e/raw/04294bcb2f9e1acd547633a8c3c3b5fe93ac41f1/cr_8980_1625446266.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index bb945df8b..a317f9e0e 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -6580,12 +6580,9 @@ DEFUN (clear_ip_mroute_count,
DEFPY(clear_ip_msdp_peer, clear_ip_msdp_peer_cmd,
"clear ip msdp peer A.B.C.D$peer [vrf WORD$vrfname]",
- CLEAR_STR
- IP_STR
- MSDP_STR
+ CLEAR_STR IP_STR MSDP_STR
"Restart MSDP peer\n"
- "MSDP peer address\n"
- VRF_CMD_HELP_STR)
+ "MSDP peer address\n" VRF_CMD_HELP_STR)
{
struct vrf *vrf;
struct pim_instance *pim;
@@ -9798,8 +9795,7 @@ DEFUN (no_ip_msdp_peer,
DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
"ip msdp peer A.B.C.D$peer password WORD$psk",
- IP_STR
- CFG_MSDP_STR
+ IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
@@ -9831,9 +9827,7 @@ DEFPY(ip_msdp_peer_md5, ip_msdp_peer_md5_cmd,
DEFPY(no_ip_msdp_peer_md5, no_ip_msdp_peer_md5_cmd,
"no ip msdp peer A.B.C.D$peer password [WORD]",
- NO_STR
- IP_STR
- CFG_MSDP_STR
+ NO_STR IP_STR CFG_MSDP_STR
"Configure MSDP peer\n"
"MSDP Peer address\n"
"Use MD5 authentication\n"
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20050/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
PR is very outdated and the code changed a lot, I'll close this until I get some spare time to get it back on shape. |
Summary
Implement MD5 authentication for MSDP peers.
Technical Detail
Move MSDP socket creation code to a generic helper function and parameterize it to toggle between authenticated and non authenticated.
MSDP peers that are already connected will not be disconnected when authentication is configured or removed instead you must either:
clear ip msdp peer A.B.C.Dto apply it immediately