[TACACS+]: Add support to specify source address for TACACS+

[venkatmahalingam]

This pull request was cherry picked from "#1238" to resolve the conflicts.

- Why I did it
Add support to specify source address for TACACS+
- How I did it
Add patches for libpam-tacplus and libnss-tacplus. The patches parse the new option 'src_ip' and store the converted addrinfo. Then the addrinfo is used for TACACS+ connection.
Add a attribute 'src_ip' for table "TACPLUS|global" in configDB
Add some code to adapt to the attribute 'src_ip'.
- How to verify it
Config command for source address PR in sonic-utilities
config tacacs src_ip <ip_address>

- Description for the changelog
Add patches to specify source address for the TACACS+ outgoing packets.

- A picture of a cute animal (not mandatory but encouraged)

**UT logs: **

UT_tacacs_source_intf.txt

[renukamanavalan]

Is there a PR for unit test for this change ?

[renukamanavalan]

When we enforce src-IP, should not we enforce the route too?

If the right route not available, the default route might get used. If the default route is not the right one, each TACACS server would take 5 seconds to timeout. Say there are 3 servers, each login attempt would take 15 seconds before it falls back to local.

[venkatmahalingam]

Is there a PR for unit test for this change ?

No, I dont see any UT in the original PR (#1238), may be I'll try to add few test cases, can you share some reference tests to add the UT for the source_ip changes?

[venkatmahalingam]

When we enforce src-IP, should not we enforce the route too?

If the right route not available, the default route might get used. If the default route is not the right one, each TACACS server would take 5 seconds to timeout. Say there are 3 servers, each login attempt would take 15 seconds before it falls back to local.

No, the scope of this PR is to change the source IP of the TACACS+ packet, the actual routing to reach the TACACS+ server is expected to happen based on the route table lookup in the kernel.

[renukamanavalan]

Are we changing code to resolve the comments?

[venkatmahalingam]

Are we changing code to resolve the comments?

Yes Renuka, will change the code.

[renukamanavalan]

From a build failure log:

[2020-06-16T18:40:00.200Z] nss_tacplus.c: In function 'parse_config':
[2020-06-16T18:40:00.200Z] nss_tacplus.c:251:5: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
[2020-06-16T18:40:00.200Z]      if(source_addr)
[2020-06-16T18:40:00.200Z]      ^~
[2020-06-16T18:40:00.200Z] nss_tacplus.c:253:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
[2020-06-16T18:40:00.200Z]          source_addr = NULL;
[2020-06-16T18:40:00.200Z]          ^~~~~~~~~~~

[venkatmahalingam]

From a build failure log:

[2020-06-16T18:40:00.200Z] nss_tacplus.c: In function 'parse_config':
[2020-06-16T18:40:00.200Z] nss_tacplus.c:251:5: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
[2020-06-16T18:40:00.200Z]      if(source_addr)
[2020-06-16T18:40:00.200Z]      ^~
[2020-06-16T18:40:00.200Z] nss_tacplus.c:253:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
[2020-06-16T18:40:00.200Z]          source_addr = NULL;
[2020-06-16T18:40:00.200Z]          ^~~~~~~~~~~

Sorry, this issue has been fixed.

[renukamanavalan]

Hi Venkat,
Has anyone tested this code, with & w/o source-IP configured ?
W/o unit test, this is critical, as we can't afford any regression.

[venkatmahalingam]

Hi Venkat,
Has anyone tested this code, with & w/o source-IP configured ?
W/o unit test, this is critical, as we can't afford any regression.

Yes Renuka, we have tested it, it works fine, we'll have to test again with new changes and post the UT logs, please let me know once you are done with the review.

[renukamanavalan]

I am done with review. Please add the test result, then this PR would be good to get in.

[venkatmahalingam]

I am done with review. Please add the test result, then this PR would be good to get in.

Thanks, will do it soon.

[renukamanavalan]

retest baseimage please