Skip to content
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

noisy debugs for getservbyname in pam_radius_auth.c #6

Closed
ssbertilson opened this issue May 22, 2013 · 3 comments
Closed

noisy debugs for getservbyname in pam_radius_auth.c #6

ssbertilson opened this issue May 22, 2013 · 3 comments

Comments

@ssbertilson
Copy link

Seems to me that they're useless but very chatty in their current form.

I'm wondering if you'd consider patching it to:

  1. delete the 3 getservbyname DPRINTs
  2. make them silent unless getservbyname fails

I'd also want to change the message - to not print the pointer value (in decimal of all things) but instead announce that the lookup failed when that happens.

alancarwile referenced this issue in alancarwile/pam_radius Dec 30, 2013
-------------------------
The notes below describe what was done in getting pam_radius_auth.so to the point of supporting IPv6.
In addition to adding IPv6 support, a TRACE facility was added, hardtab/whitespace was cleaned up, and
some bug fixes were made.

Following is a rough breakdown of the changes:
-   Began with 1.3.17 from tar.gz, which did not include a few changes in the github repo.
    o   Those changes were applied at the end of the project (see below).
-   Removed hard tab characters using vim's :se ts=8 and :se ts=4 and :1,.retab in sections
    o   Did not realign all text to use same indent rules.  Just removed hard tabs.
-   Code changes to reach a first snapshot of IPv6 support:
    o   Can now handle radius server names that resolve to IPv6 (or IPv4)
    o   Can still of course handle IPv4 addresses
    o   Does not yet handle IPv6 addresses completely due to bracket parsing work to be completed
    o   Replaced IPv4-only gethostbyname calls with getaddrinfo calls
    o   Switched from sockaddr/sockaddr_in IPv4-only to generic sockaddr_storage which handles IPv4/IPv6
    o   Set IPV6_V6ONLY to off, allowing one IPv6 socket to handle IPv6 and IPv4 communication.
    o   Commented out no-longer needed IPv4-only validation code (comments in source for this change).
    o   Added TRACEPRINT to aid with development, debugging, and learning how pam_radius_auth works.
-   Near final snapshot:
    o   Implemented bracket handling etc. to support [<ipv6-address>]:port form where :port is still optional.
    o   IPv6 address can be abbreviated forms, such as  [fdca:1:2:3::4]:1812
    o   Improved debug output around port lookup to prevent some failures on odd input in server config file
    o   Modify Makefile to build normal shared library and a trace version using -DTRACEON
    o   Add comments in less obvious parts of code especially where I made changes.
-   Merged in changes from latest github master version of this project, current as of 5/19/2013.
-   Trace version of library:
    o   Modified the Makefile to build a second library, pam_radius_auth_trace.so with tracing enabled.
        * The trace version can be renamed at runtime to give more debug/trace output if needed.
-   Testing:
    o   Tested with names, IPv6 addresses, and IPv4 addresses against freeradius 2.2
    o   Tested with names, IPv6 addresses, and IPv4 addresses against Windows 2008 NPS
    o   Tested with names and IPv4 addresses against freeradius 2.1 and Windows 2003 IAS
        * freeradius 2.1 and Windows 2003 IAS do not support IPv6.
-   Contact info:  alan.d.carwile@gmail.com      AC Network Software
                   alan.carwile@hds.com          Hitachi Data Systems
-   Credit also to Kentaro Aoki, whose code changes were referenced and helpful along the way.
    o   http://code.google.com/p/pam-radius-ipv6/
@alancarwile
Copy link

A change consistent with this is part of the changes in: alancarwile@cda81ff
See comment I've just added in that commit

@walterdejong
Copy link

Also fixed by commit d4d97e2

@alandekok
Copy link
Member

This should now be fixed in the HEAD

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

No branches or pull requests

4 participants