Skip to content
This repository has been archived by the owner on Feb 28, 2024. It is now read-only.

source ip interface/address #81

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

stanAtAtl
Copy link
Contributor

This patch adds code in _pam_parse() to parse new option "source_ip" and stores the converted address information in static memory. The address info is then used when calling tac_connect_single().

This patch adds code in _pam_parse() to parse new option "source_ip" and stores
the converted address information in static memory. The address info is then
used when calling tac_connect_single().
    
Note: at this moment, the "source_ip" is an ipv4 address in dot-decimal notation.
It could be expanded to accept ip interface name and support ipv6.
Copy link
Collaborator

@pprindeville pprindeville left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit should be rebased as a 'fixup' onto the first commit, and then force pushed.

@pprindeville
Copy link
Collaborator

In general, I'm opposed to this change.

We're seeing an increase of session parameters (not a bad thing) but they're being added as globals (which is a bad thing).

Please look at PR #78 and consider making this a session parameter instead.

@stanAtAtl
Copy link
Contributor Author

stanAtAtl commented Dec 15, 2016

Having had a look at PR #78, it may not be a very bad idea to make srcaddr a member of struct tac_session, but I failed to see any real benefit from doing so. My understanding is that the srcaddr is used only in and by tac_connect_single() and it is already there but not used (NULL was passed in always). There is no much similarity to putting fd to the session.

@pprindeville
Copy link
Collaborator

pprindeville commented Dec 15, 2016

@stanAtAtl:

Having had a look at PR #78, it may not be a very bad idea to make srcaddr a member of struct tac_session, but I failed to see any real benefit from doing so. My understanding is that the srcaddr is used only in and by tac_connect_single() and it is already there but not used (NULL was passed in always).

A sockaddr is 16 bytes. A sockaddr_in6 is 28 bytes long. A sockaddr_in is 16 also, but only 8 bytes are used (the rest are padding to blow it out to the same length as a sockaddr). It's not an excessive trade-off for a cleaner API versus 28 bytes more of overhead per structure.

What do you mean "NULL was passed in always"?

It's passed in as NULL by tacc.c and pam_tacplus.c, but there are other programs out there which use it (I know because mine is one of them).

@mbennett2212
Copy link

@pprindeville We can rework this change based on PR #78, but is PR #78 going to be merged?

@pprindeville
Copy link
Collaborator

We can rework this change based on PR #78, but is PR #78 going to be merged?

@mattbennett2 I'm trying to find the time to kick out a final release of 1.5.0 (or whatever) and then start a separate fork of 2.0 since this will be fairly disruptive (and definitely not ABI compatible).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants