rtrlib: ability to pass configured socket in tcp/ssh attribute by pguibert6WIND · Pull Request #252 · rtrlib/rtrlib

pguibert6WIND · 2019-10-21T16:45:39Z

a new attribute socket is given in tr_ssh_config and tr_tcp_config
structures. as the structures are visible for configuration by the
caller, it becomes possible to create early the socket that will be
used later by rtrlib. some scenarios like vrf usage can be possible.
for instance, the socket is first created in the appropriate vrf, then
submitted to rtrlib that uses it to connect to the target.
Also, tr_ident_fp is modified so as to return the configured socket
ctxt. this is helpful to retrieve the context.

Signed-off-by: Philippe Guibert philippe.guibert@6wind.com

mroethke · 2019-10-22T15:11:09Z

This is a good start, but it won't work.
The socket is closed in tr_tcp_close and I assume ssh_channel_close does the same.
There needs to be a way to request a new one, I suggest a callback with the following signature:

int new_socket(void *data);

where data points to caller provided data, which should be stored in the config struct too.
This assumes the callback does not call connect on it's own, I don't know if that would be useful.

And I don't think it makes sense to put the sockets fd number in the ident string.

pguibert6WIND · 2019-10-25T16:29:09Z

just a quick update to see if it conforms to your counter proposal.

mroethke

Looks good in general, but I have some comments.

Did you check what state libssh expects for the socket? Do we have to connect it ourself?

rtrlib/transport/ssh/ssh_transport.h

rtrlib/transport/ssh/ssh_transport.c

rtrlib/transport/tcp/tcp_transport.c

pguibert6WIND · 2019-10-28T17:23:15Z

Marcel,
do you have an idea on what is wrong with the syntax check:

+++ b/rtrlib/transport/tcp/tcp_transport.c
@@ -65,7 +65,7 @@ int tr_tcp_open(void *tr_socket)

	if (config->new_socket) {
              config->new_socket (...)
       }

actually, I dont know why, test fail because of that piece of code ( call is done despite new_socket is supposed to be null).

mroethke · 2019-10-29T13:57:46Z

You are not initializing the socket config properly in tr_tcp_init, same for tr_ssh_init.
That causes conditional jumps based on uninitialized data.

From valgrind:

==214063== Thread 2:
==214063== Conditional jump or move depends on uninitialised value(s)
==214063==    at 0x11102E: tr_tcp_open (tcp_transport.c:68)
==214063==    by 0x110BD7: tr_open (transport.c:16)
==214063==    by 0x111D87: rtr_fsm_start (rtr.c:130)
==214063==    by 0x48984CE: start_thread (in /usr/lib/libpthread-2.30.so)
==214063==    by 0x4A192D2: clone (in /usr/lib/libc-2.30.so)
==214063==  Uninitialised value was created by a heap allocation
==214063==    at 0x483877F: malloc (vg_replace_malloc.c:309)
==214063==    by 0x10E9CB: lrtr_malloc (alloc_utils.c:35)
==214063==    by 0x11191A: tr_tcp_init (tcp_transport.c:225)
==214063==    by 0x10C5C9: main (test_dynamic_groups.c:37)
==214063==

rtrlib/transport/tcp/tcp_transport.h

mroethke · 2019-10-29T16:40:52Z

Some additional info:

The config struct is copied in tr_{tcp,ssh}_init, which must be extended for the new struct fields.

pguibert6WIND · 2019-10-29T16:51:08Z

Hi @mroethke ,
yes I realized that the crash I had was due to the non initialisation of tr[tcp/ssh]_init

rtrlib/transport/tcp/tcp_transport.h

pguibert6WIND · 2019-10-30T16:19:03Z

When testing, I still encounter an issue with callback update_fp(). in frr code, somehow the calculated tr_socket pointer is wrong ( 0x73) which is invalid pointer. strangely it was working with default rpki. do you know which method I should use to retrieve the right context?

from pfx_table, how could I retrieve the correct tr_socket pointer ?
Do you think I should modify some other structure in rtrlib?

pguibert6WIND · 2019-11-04T11:20:37Z

Hi @mroethke ,
I could use that branch and make it work with associated patch from frr.
so from the operational perspective, this is ok from my side.

mroethke · 2020-01-31T19:50:46Z

Did you install socat?

pguibert6WIND · 2020-01-31T20:46:56Z

socat version 1.7.3.2 on Apr 4 2018 10:06:49

pguibert6WIND · 2020-02-03T09:10:55Z

looks like my ssh settings are incirrect

dut-vm(config)# (2020/02/03 10:04:00:719785): SSH Transport(root@127.0.0.1:22): tr_ssh_init: Wrong hostkey

pguibert6WIND · 2020-02-03T10:28:12Z

ok, the tip was, instead of using the public key directly, (/etc/ssh/ssh_host_rsa_key.pub file ), the expectation is to have /root/.ssh/known_hosts as server public key.

pguibert6WIND · 2020-02-03T13:48:51Z

Hi @mroethke,
so finally I could make a setup and test ssh via local ssh server and socat subsystem procedure.
I made it work with:

rtrlib with initial upstream code
rtrlib with ssh on default vrf (with frr branch Rpki Encapsulation FRRouting/frr#5015)
rtrlib with ssh on custom vrf (with frr branch Rpki Encapsulation FRRouting/frr#5015)

config example used:

rpki
rpki cache 127.0.0.1 22 root /root/.ssh/id_rsa /root/.ssh/id_rsa.pub  /root/.ssh/known_hosts preference 5
exit

thanks to let me know if there are still pending points to discuss, or can this pr be unblocked, now that SSH has been tested.
thanks,

pguibert6WIND · 2020-02-06T08:16:23Z

Hi @mroethke ,
thanks again for the help on this ticket.
now I am waiting for your positive feedback on this PR.
I would really like to go ahead with new FRR RPKI stuff.
thanks.

waehlisch · 2020-02-06T08:59:41Z

ping @mroethke @nbars

mroethke · 2020-02-07T22:27:07Z

I think it looks good now.

But would still like to have another opinion.

pguibert6WIND · 2020-02-11T12:28:19Z

thanks Marcel, I am looking forward to get news on this pr

pguibert6WIND · 2020-03-10T17:36:37Z

Hi,
are there any feedbacks there, please?

waehlisch · 2020-03-10T23:16:09Z

ping @mroethke

mroethke · 2020-03-12T20:55:05Z

coverity pointed out to me, that this breaks the rtrclient. Because the new fields are not initialized. Same for current version of frr.

Fixing this in rtrclient is as simple as this:

diff --git a/tools/rtrclient.c b/tools/rtrclient.c
index 3b89e11..0fb7749 100644
--- a/tools/rtrclient.c
+++ b/tools/rtrclient.c
@@ -755,9 +755,9 @@ static void init_sockets(void)
 {
 	for (size_t i = 0; i < socket_count; ++i) {
 		struct socket_config *config = socket_config[i];
-		struct tr_tcp_config tcp_config;
+		struct tr_tcp_config tcp_config = {};
 #ifdef RTRLIB_HAVE_LIBSSH
-		struct tr_ssh_config ssh_config;
+		struct tr_ssh_config ssh_config = {};
 #endif
 
 		switch (config->type) {

Please add this, than I will merge.

I will also prepare fixes for supported version of frr asap.

the structures tr_tcp_config and tr_ssh_config were not set to 0 at startup, when init_sockets() call is done. handle this by initialising those structures. Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

NetDEF-CI · 2020-03-13T08:03:37Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Failed

Fedora 29 amd64: Failed (click for details)

Fedora 29 amd64: Unknown Log URL: https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/FEDORA29AMD64/ErrorLog/log_rpmlint.txt Fedora 29 amd64: No useful log found

Successful on other platforms/tests

Debian 10 amd64
FreeBSD 11 amd64
CentOS 7 amd64
Debian 9 arm7
Ubuntu 16.04 arm8
Alpine amd64
Ubuntu 18.04 ppc64le
Debian 9 arm8
Debian 9 amd64
CentOS 6 amd64
Debian 8 amd64
Ubuntu 16.04 i386
FreeBSD 12 amd64
Ubuntu 18.04 amd64
NetBSD 8 amd64
Ubuntu 14.04 amd64
OpenBSD 6 amd64
Ubuntu 16.04 amd64
Ubuntu 16.04 arm7

Warnings Generated during build:

Debian 10 amd64: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/DEB10AMD64/ErrorLog/log_lintian.txt)

W: librtr-dbg: priority-extra-is-replaced-by-priority-optional
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional
N: 0 tags overridden; 6 unused overrides

Ubuntu 16.04 arm8: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm8:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/UBUNTU1604ARM8/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 18.04 ppc64le: Successful with additional warnings

Debian Package lintian failed for Ubuntu 18.04 ppc64le:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/U1804PPC64LE/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable
W: librtr-dbg: priority-extra-is-replaced-by-priority-optional
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional

Ubuntu 16.04 i386: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 i386:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/UBUNTU1604I386/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 18.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 18.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/U1804AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable
W: librtr-dbg: priority-extra-is-replaced-by-priority-optional
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional

Ubuntu 14.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 14.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/UBUNTU1404AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/UBUNTU1604AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 arm7: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm7:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-275/artifact/UBUNTU1604ARM7/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

pguibert6WIND · 2020-03-13T09:08:16Z

strange: on https://ci1.netdef.org/browse/RPKI-RTRLIBPR-FEDORA29AMD64-275/log,

13-Mar-2020 01:01:05 | 4 packages and 0 specfiles checked; 4 errors, 2 warnings.
-- | --
13-Mar-2020 01:01:05 | debian package build (debuild) failed

is there a way to relaunch CI ?

pguibert6WIND · 2020-03-13T09:08:24Z

ci:rerun

NetDEF-CI · 2020-03-13T09:17:07Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Failed

Fedora 29 amd64: Failed (click for details)

Fedora 29 amd64: Unknown Log URL: https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/FEDORA29AMD64/ErrorLog/log_rpmlint.txt Fedora 29 amd64: No useful log found

Successful on other platforms/tests

FreeBSD 11 amd64
Debian 9 arm7
CentOS 7 amd64
Ubuntu 14.04 amd64
Ubuntu 16.04 arm8
Alpine amd64
Debian 9 amd64
Debian 9 arm8
Ubuntu 18.04 ppc64le
CentOS 6 amd64
Debian 8 amd64
Debian 10 amd64
FreeBSD 12 amd64
Ubuntu 16.04 i386
NetBSD 8 amd64
Ubuntu 18.04 amd64
Ubuntu 16.04 amd64
Ubuntu 16.04 arm7
OpenBSD 6 amd64

Warnings Generated during build:

Ubuntu 14.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 14.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/UBUNTU1404AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 arm8: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm8:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/UBUNTU1604ARM8/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 18.04 ppc64le: Successful with additional warnings

Debian Package lintian failed for Ubuntu 18.04 ppc64le:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/U1804PPC64LE/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional
W: librtr-dbg: priority-extra-is-replaced-by-priority-optional

Debian 10 amd64: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/DEB10AMD64/ErrorLog/log_lintian.txt)

W: librtr-dbg: priority-extra-is-replaced-by-priority-optional
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional
N: 0 tags overridden; 6 unused overrides

Ubuntu 16.04 i386: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 i386:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/UBUNTU1604I386/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 18.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 18.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/U1804AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable
W: librtr-dbg: priority-extra-is-replaced-by-priority-optional
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional

Ubuntu 16.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/UBUNTU1604AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 arm7: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm7:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-276/artifact/UBUNTU1604ARM7/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

mroethke · 2020-03-16T18:30:25Z

I just merged and pushed this manually.

Thanks!

pguibert6WIND · 2021-09-02T16:30:59Z

Hi @waehlisch , who could help on having an official delivery of rtrlib ?
This pull request is a requisite for FRRouting/frr#5015 that has been reverted because rtrlib was not up to date with distribs.

I would love having an up to date rtrlib version for linux distros:)

waehlisch · 2021-09-02T19:44:58Z

@pguibert6WIND rtrlib is available as an official Debian package (https://packages.debian.org/search?keywords=librtr-dev) -- you mean we need to update the package?

pguibert6WIND · 2021-09-03T06:38:21Z

@waehlisch , that is it. updating the package would be very nice. I know it takes time. I dont know who to contact and how to package to different distros.

waehlisch · 2021-09-03T08:41:40Z

last time @lukas227 helped us. ping. but, librtr-0.7 is already part of stable (see Version widget https://tracker.debian.org/pkg/librtr).

what we need to do is to create a new RTRlib release. this will then be adapted in Debian. if #256 is merged, we can create a new release.

lukas227 · 2021-09-03T15:18:24Z

Happy update the package in Debian once you've created a new release.

This was referenced Oct 21, 2019

rtrlib: support for vrf #249

Closed

Rpki Encapsulation FRRouting/frr#5015

Merged