Bug 820488

Summary: Review Request: mod_auth_xradius - Apache module that provides authentication against RADIUS Servers
Product: [Fedora] Fedora Reporter: Simone Caronni <negativo17>
Component: Package ReviewAssignee: Gwyn Ciesla <gwync>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dpal, gwync, notting, package-review, sgallagh
Target Milestone: ---Flags: gwync: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-07-27 22:06:13 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
unixd patch
none
Spec file creating shared libxradius
none
SRPM containing new patches to build libxradius
none
Patch libxradius for mozilla-nss support
none
SRPM containing new patches to build libxradius none

Description Simone Caronni 2012-05-10 07:10:09 UTC
Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-3.fc17.src.rpm
Description:
Apache module that provides high performance authentication against
RFC 2865 RADIUS Servers.

Comment 1 Gwyn Ciesla 2012-05-15 16:17:30 UTC
In progress.

Comment 2 Gwyn Ciesla 2012-05-15 16:30:51 UTC
Good:

- rpmlint checks return:

mod_auth_xradius-debuginfo.x86_64: W: spelling-error Summary(en_US) auth -> auto, Ruth, author
The value of this tag appears to be misspelled. Please double-check.

mod_auth_xradius-debuginfo.x86_64: W: spelling-error Summary(en_US) xradius -> radius, x radius, radium
The value of this tag appears to be misspelled. Please double-check.

mod_auth_xradius-debuginfo.x86_64: W: spelling-error %description -l en_US auth -> auto, Ruth, author
The value of this tag appears to be misspelled. Please double-check.

mod_auth_xradius-debuginfo.x86_64: W: spelling-error %description -l en_US xradius -> radius, x radius, radium
The value of this tag appears to be misspelled. Please double-check.

2 packages and 0 specfiles checked; 0 errors, 4 warnings.

Ignore, otherwise clean.

- package meets naming guidelines
- package meets packaging guidelines

X license ( ASL 2.0 ) OK, text in %doc, matches source except for md5.

- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 

In auth_xradius.conf, should line 16 possibly be limited to just localhost by default?  Maybe with what's there present, but commented out?

Bundles md5, which is OK, but needs to Provide bundled(md5-$IMPLEMENTATION).  Not sure what implementation we have here, might be a new one for the list.

Bundles libradius.  Where's it from?  Is there an upstream we can package?

Comment 3 Gwyn Ciesla 2012-05-15 17:04:30 UTC
Builds fine on f16, but needs the following patch to build in mock for rawhide:

Comment 4 Gwyn Ciesla 2012-05-15 17:05:20 UTC
Created attachment 584723 [details]
unixd patch

Comment 5 Simone Caronni 2012-05-15 18:19:00 UTC
Many thanks for the patch and the the time for the review.

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-4.fc17.src.rpm

- Changed default config to point to localhost only.
- Added patch for rawhide.

Regarding md5 and libradius; it seems is none of the md5 implementations listed at:

http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#cite_note-1 

this is the text file that comes inside the source:

"This software was originally written by John Polstra, and donated to the
FreeBSD project by Juniper Networks, Inc. Oleg Semyonov subsequently
added the ability to perform RADIUS accounting.

The following source files were extracted from a FreeBSD 5-STABLE
system as of 20-Sep-2004:

    /usr/src/sys/sys/md5.h
    /usr/src/lib/libmd/md5c.c
    /usr/src/lib/libradius/libradius.3
    /usr/src/lib/libradius/radius.conf.5
    /usr/src/lib/libradius/radlib.c
    /usr/src/lib/libradius/radlib.h
    /usr/src/lib/libradius/radlib_private.h
    /usr/src/lib/libradius/radlib_vs.h"

They're still in FreeBSD CVS and licensed as part of the base BSD archive that's BSD licensed:

http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libradius/

What should I do?

Thanks,
--Simone

Comment 6 Gwyn Ciesla 2012-05-15 18:40:51 UTC
Ok, I've added that md5 implementation, so use bundled(md5-polstra).

Can that libradius build a solib?  Is it updated in the CVS beyond what's in this module?  Are there tarball releases of it?

Comment 7 Simone Caronni 2012-05-15 18:48:49 UTC
The libradius bundled has been updated in recent FreeBSD CVS commits at the previous links.

There are no tarball releases for it, as it is part of the FreeBSD release:

ftp://ftp.freebsd.org/pub/FreeBSD/releases/amd64/9.0-RELEASE/src.txz

bundled(libradius-bsd)?....

Thanks,
--Simone

Comment 8 Gwyn Ciesla 2012-05-15 18:58:49 UTC
Ugh.  Sigh.

https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions

File a Trac for FPC and we'll see.  I'm optimistic.  I'd rather see this packaged separately, so if you can coax a solib out of it that would be better.  If you can get the Trac in soon we can probably look at it at tomorrow's meeting.

Comment 9 Simone Caronni 2012-05-16 06:50:42 UTC
Many thanks, I've added you in CC for the ticket on FPC:

https://fedorahosted.org/fpc/ticket/175

I'll try to do something with the library and see if it can be built as a shared object.

Comment 10 Simone Caronni 2012-05-21 09:50:20 UTC
Changes required as per ticket #175 on fpc track instance:

- Strip out libmd and libradius.
- Remove libradius from sources.
- Added conditional for Fedora >= 18 patch (does not build on Fedora <= 17).
- Added dependency on stripped out libmd and libradius.

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-6.fc17.src.rpm

Comment 11 Simone Caronni 2012-05-30 09:46:56 UTC
Hello,

here is the updated package following scrutiny on libradius; I've updated a couple of patches to make it easier to read and mantain.

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-7.fc17.src.rpm

- Remove libmd BR.
- Reworked memcache patch.
- Updated patch for removing libradius.

Comment 12 Simone Caronni 2012-05-30 10:40:57 UTC
(In reply to comment #8)
> Ugh.  Sigh.
> 
> https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions
> 
> File a Trac for FPC and we'll see.  I'm optimistic.  I'd rather see this
> packaged separately, so if you can coax a solib out of it that would be
> better.  If you can get the Trac in soon we can probably look at it at
> tomorrow's meeting.

Btw, to make it work in the meanwhile on our production systems I've created the following which contains all the changes listed so far in the review but still includes the original md5 and libradius libraries, using:

Provides:      bundled(md5-polstra) - which is approved
Provides:      bundled(libradius-polstra) - which is not approved

http://slaanesh.fedorapeople.org/rhel6/mod_auth_xradius-0.4.6-8.el6.src.rpm

This of course gets back to the original point, so I'm not expecting this to be approved.

Regards,
--Simone

Comment 13 Simone Caronni 2012-05-30 15:24:06 UTC
Nope, I can't make it work with the external libradius library.

I renamed all calls to the original libradius to the external one from xrad_* to rad_*. Fortunately all calls are there but Apache's child crashes upon the request of a Radius protected page:

==> /var/log/httpd/error_log <==
[Wed May 30 17:10:21 2012] [notice] child pid 5984 exit signal Segmentation fault (11)

If someone wants to have a look at it:

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-8.fc17.src.rpm

Comment 14 Stephen Gallagher 2012-06-08 11:32:02 UTC
I think you took the wrong approach there. I don't think we want to try to force mod_auth_xradius to link against libradius from https://bugzilla.redhat.com/show_bug.cgi?id=823446

What I think we really want to do is modify this package (mod_auth_xradius) so that you can build the libradius sources it ships as a sub-package called 'libxradius' (and 'libxradius-devel'), which the main package will depend on.

Then we have eliminated the bundling (without mandating that the sources themselves are stripped out) because we're providing a shared library that other packages can depend on.

This is NOT bundling. Bundling means static linking. So let's see if we can take some clues from libradius in the other BZ and use that to build a shared library from the mod_auth_xradius sources in the same fashion, that mod_auth_xradius can link to.

Comment 15 Gwyn Ciesla 2012-06-08 12:29:04 UTC
Simone, if you can implement Stephen's suggestion, I'm fully supportive of making the libmd and libradius pacakges moot. :)

Comment 16 Simone Caronni 2012-06-09 14:16:01 UTC
I understood that creating a subpackage for libxradius was not correct because it was not connected to any "upstream" project, hence the need to package libradius which was somehow trackable to something.

If that's ok I'll be more than happy to separate the bundled libradius and maybe the patches to remove libmd and use nss/nspr can be re-used as well.

I will try monday morning when I'll get back to the office.

Thanks,
--Simone

Comment 17 Stephen Gallagher 2012-06-10 23:42:11 UTC
(In reply to comment #16)
> I understood that creating a subpackage for libxradius was not correct
> because it was not connected to any "upstream" project, hence the need to
> package libradius which was somehow trackable to something.
> 

Well, the best-case scenario here is to contribute the changes you're making to split the library into a shared object back to upstream. Right now, we're effectively maintaining a fork (albeit one with little risk), but you certainly want upstream to take this on as well.

> If that's ok I'll be more than happy to separate the bundled libradius and
> maybe the patches to remove libmd and use nss/nspr can be re-used as well.
> 

I think it's fine to take this approach. Let me know if you need help converting the mod_auth_xradius sources to use NSS for crypto (like I did for libradius in the other BZ).

The forbidding of bundling is there so that we don't have to patch many packages to fix a bug/vulnerability in multiple places when it's discovered. We just patch the sources that produce the library and any consumer of it gets fixed freely, without having to make an update of their own.

> I will try monday morning when I'll get back to the office.
>

Comment 18 Dmitri Pal 2012-06-11 16:46:53 UTC
Do not forget that libxradius seems to have more capabilities than libradius. How are you planning to deal with that? Are you going to contribute patches that correspond to the diff between two.

IMO it seems better to contribute to the upstream patches that would split mod_auth_xradius and libxradius rather than trying to patch libradius to match libxradius functionality.

Comment 19 Stephen Gallagher 2012-06-11 17:12:06 UTC
(In reply to comment #18)
> Do not forget that libxradius seems to have more capabilities than
> libradius. How are you planning to deal with that? Are you going to
> contribute patches that correspond to the diff between two.
> 
> IMO it seems better to contribute to the upstream patches that would split
> mod_auth_xradius and libxradius rather than trying to patch libradius to
> match libxradius functionality.


I think you misunderstood the proposal above, Dmitri. The idea is that we'll forget about packaging libradius entirely (dropping that BZ) and instead build the portions of the mod_auth_xradius as a subpackage providing a shared object called libxradius.so which is build from the mod_auth_xradius source tarball.

So there's no diff to concern ourselves with, and we'll pick up the complete set of functionality from libxradius.

Is that more clear?

Comment 20 Dmitri Pal 2012-06-11 17:26:10 UTC
I was reacting to the first paragraph in comment #16

>I understood that creating a subpackage for libxradius was not correct because >it was not connected to any "upstream" project, hence the need to package >libradius which was somehow trackable to something.

It talks about libradius not libxradius. If it going to be libxradius as you suggested in comment #14 I am fine. If the idea is to use off shelf libradius then my comment is valid.

Comment 21 Simone Caronni 2012-06-11 17:49:57 UTC
Here's the first attempt in building the bundled libradius as a subpackage.

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-9.fc17.src.rpm

For now, to see it works, I'm trying to build the library as is without NSS; I'll add the patch later when I'll fix the problems.

A few things don't work as expected though; and any help is much appreciated:

- /etc/httpd/modules/mod_auth_xradius.so does not link to libradius, when installed it does not find symbols from libradius.so.0.0.0. During build, -L and -l are passed to the compiler and in fact it doesn't complain.

- libradius is built with a "custom" Makefile in the src.rpm, maybe would be better to modify configure and configure.ac to make it build directly from there?

Thanks,
--Simone

Comment 22 Dmitri Pal 2012-06-11 18:13:56 UTC
Simone,

This is really confusing. When you say libradius do you mean the libxradius extracted from the mod_auth_xradius or libraius as it exists upstream as an independent package that you tied to package earlier? Please clarify. 

Thanks
Dmitri

Comment 23 Stephen Gallagher 2012-06-11 18:18:41 UTC
Simone, to avoid confusion with the libradius package provided by the BSD upstream, please name the subpackage that you are creating here "libxradius".

Comment 24 Stephen Gallagher 2012-06-11 19:40:32 UTC
Created attachment 591005 [details]
Spec file creating shared libxradius

Comment 25 Stephen Gallagher 2012-06-11 19:44:07 UTC
Created attachment 591006 [details]
SRPM containing new patches to build libxradius

I'm attaching a new SRPM that should build the library correctly and link mod_auth_xradius against it properly.

Please test this out. You can extract the srpm with the following command:

rpm2cpio ./mod_auth_xradius-0.4.6-10.fc17.src.rpm | cpio --extract -d


This will dump its contents into the current folder. Please pay close attention to mod_auth_xradius-0.4.6-share_libxradius.patch which fixes the library building.

I also attached the new SPEC file which corrects some issues in building.

Comment 26 Simone Caronni 2012-06-12 09:25:18 UTC
Well, it seems I need to improve my autotools knowledge :)

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-11.fc17.src.rpm

I did some modifications to the memcache and libxradius patch to use newer autotools (the default packages, version 1.11) as RHEL does not have an automake17 package to be used as a BR.

- Update memcache patch
- Update libradius patch
- Swap automake17 BR with libtool
- Use autoreconf instead of the 1.7 patched autogen.sh

Everything works perfectly, I'll try to bring in the NSS patch now.

One side note regarding the library name, there's no package with a name similar to "libradius" in Fedora, only the other review has it; I think could be a better idea to use the source bundle as it was named inside the mod_auth_xradius, i.e. "libradius".

Comment 27 Simone Caronni 2012-06-12 10:07:05 UTC
I did include the nss patch; I need some help on a couple of points (or a link to some good docs) as I'm no programmer.

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-12.fc17.src.rpm

After adapting the previous patch [1] to libxradius there are a few calls to the MD5 library left:

$ cat mod_auth_xradius-0.4.6/libradius/radlib.c | egrep "MD5Init|MD5Update|MD5Final"
	MD5Init(&Context);
	MD5Update(&Context, S, strlen(S));
	MD5Update(&Context, R, LEN_AUTH);
	MD5Final(b, &Context);
			MD5Init(&Context);
			MD5Update(&Context, S, strlen(S));
			MD5Update(&Context, C, 16);
			MD5Final(b, &Context);
	MD5Init(&Context);
	MD5Update(&Context, S, Slen);
	MD5Update(&Context, R, LEN_AUTH);
	MD5Update(&Context, A, SALT_LEN);
	MD5Final(b, &Context);
			MD5Init(&Context);
			MD5Update(&Context, S, Slen);
			MD5Update(&Context, C, 16);
			MD5Final(b, &Context);

[1] http://slaanesh.fedorapeople.org/libradius-libnss.patch

Comment 28 Stephen Gallagher 2012-06-12 18:49:36 UTC
Created attachment 591254 [details]
Patch libxradius for mozilla-nss support

The attached patch should handle the NSS support. As before, I haven't tested it beyond ensuring that it compiles, since I don't have available infrastructure. Please let me know if it fails in any way.

Comment 29 Simone Caronni 2012-06-13 04:52:56 UTC
I will test it tomorrow, I'm currently on a business trip without datacenter access.

Thanks,
--Simone

Comment 30 Simone Caronni 2012-06-14 07:29:37 UTC
Compiled and tested, unfortunately it doesn't work, the module crashes with a Segmentation Fault without any other info in the logs:

==> /var/log/httpd/error_log <==
[Thu Jun 14 09:21:03 2012] [notice] child pid 10708 exit signal Segmentation fault (11)

I re-upped the package, no change to the build number as the spec file has not changed at all, I simply replaced the patch in mod_auth_xradius-0.4.6-12.fc17.src.rpm:

Any hint for debugging it?

btw, bundled(md5-polstra) is approved:
http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#cite_note-1

Comment 31 Simone Caronni 2012-06-14 08:03:01 UTC
A few compile issues, might they be the cause of the segfault?

DEBUG: libradius/radlib.c: In function 'xrad_continue_send_request':
DEBUG: libradius/radlib.c:573: warning: pointer targets in passing argument 6 of 'recvfrom' differ in signedness
DEBUG: libradius/radlib.c: In function 'xrad_demangle':
DEBUG: libradius/radlib.c:1181: warning: pointer targets in passing argument 2 of 'HASH_Update' differ in signedness
DEBUG: libradius/radlib.c:1182: warning: pointer targets in passing argument 2 of 'HASH_Update' differ in signedness
DEBUG: libradius/radlib.c:1183: warning: pointer targets in passing argument 3 of 'HASH_End' differ in signedness
DEBUG: libradius/radlib.c:1196: warning: pointer targets in passing argument 2 of 'HASH_Update' differ in signedness
DEBUG: libradius/radlib.c:1198: warning: pointer targets in passing argument 3 of 'HASH_End' differ in signedness
DEBUG: libradius/radlib.c: In function 'xrad_demangle_mppe_key':
DEBUG: libradius/radlib.c:1242: warning: pointer targets in passing argument 2 of 'HASH_Update' differ in signedness
DEBUG: libradius/radlib.c:1243: warning: pointer targets in passing argument 2 of 'HASH_Update' differ in signedness
DEBUG: libradius/radlib.c:1259: warning: pointer targets in passing argument 2 of 'HASH_Update' differ in signedness
DEBUG: src/xradius_cache.c: In function 'xrad_cache_dbm_post_config':
DEBUG: src/xradius_cache.c:146: warning: ignoring return value of 'chown', declared with attribute warn_unused_result
DEBUG: src/xradius_cache.c:148: warning: ignoring return value of 'chown', declared with attribute warn_unused_result
DEBUG: src/xradius_cache.c: In function 'xrad_cache_dbm_check':
DEBUG: src/xradius_cache.c:200: warning: format '%d' expects type 'int', but argument 7 has type 'apr_size_t'

Updated SPEC file and patch format to make it compile for EPEL 5:

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-13.fc17.src.rpm

Comment 32 Stephen Gallagher 2012-06-14 12:17:21 UTC
Can you install the debuginfo package, then attach to the process with gdb and get a 'bt full' for me? That would be the best solution.

None of those warnings look too problematic to me, but I could always be wrong. But the backtrace will tell me for sure.


Steps:
yum install mod_auth_xradius-debuginfo
Start up apache, find out the PID of the process you want to attach to. (You may need to modify apache to launch only one child process, so you can know to attach to that one)
gdb -p <PID>
'cont'

When it crashes, you'll get the prompt back, type 'bt full' without the quotes and paste the results in this bug, please.

Comment 33 Simone Caronni 2012-06-14 13:13:00 UTC
Hi Stephen, I'm really grateful for the time spent on this.

Here's the output:

(gdb) cont
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x00007f8ff98b3da0 in HASH_Begin () from /usr/lib64/libnss3.so
(gdb) bt full
#0  0x00007f8ff98b3da0 in HASH_Begin () from /usr/lib64/libnss3.so
No symbol table info available.
#1  0x00007f8ff3ae3a69 in insert_request_authenticator (h=0x7f8fff916950, selected=<value optimized out>, fd=0x7fffb73e3a6c, tv=0x7fffb73e3a40)
    at libradius/radlib.c:164
        md5_ctx = 0x0
        len = 4254014728
#2  xrad_continue_send_request (h=0x7f8fff916950, selected=<value optimized out>, fd=0x7fffb73e3a6c, tv=0x7fffb73e3a40) at libradius/radlib.c:600
        n = <value optimized out>
#3  0x00007f8ff3ae3f54 in xrad_init_send_request (h=0x7f8fff916950, fd=0x7fffb73e3a6c, tv=<value optimized out>) at libradius/radlib.c:787
        srv = <value optimized out>
#4  0x00007f8ff3ae412f in xrad_send_request (h=0x7f8fff916950) at libradius/radlib.c:943
        timelimit = {tv_sec = 13, tv_usec = 140256245331052}
        tv = {tv_sec = 140256439548168, tv_usec = 140256444770640}
        fd = <value optimized out>
        n = <value optimized out>
#5  0x00007f8ff3ce84b5 in xrad_run_auth_check (r=0x7f8fff9149e8, user=0x7f8fff929f30 "scaronni", password=0x7f8fff929f19 "scaronni_gmcs")
    at src/mod_auth_xradius.c:168
        rctx = 0x7f8fff916950
        sr = <value optimized out>
        digest = 0x0
        sc = 0x7f8fff460be0
        can_cache = 0
        ret = 401
        dc = 0x7f8fff41b7c0
        i = <value optimized out>
        rc = 0
        md5ctx = {state = {1, 32655, 4254012304, 32655}, count = {4282218200, 32655}, 
          buffer = "\b", '\000' <repeats 15 times>, "\020\237\222\377\217\177\000\000\260W\221\377\217\177\000\000\253\202\r\374\217\177\000\000HTUA\000\000\000\000\002\f\360\374\217\177\000\000 \000\000\000\000\000\000", xlate = 0x1}
#6  xrad_check_pw (r=0x7f8fff9149e8, user=0x7f8fff929f30 "scaronni", password=0x7f8fff929f19 "scaronni_gmcs") at src/mod_auth_xradius.c:249
        arv = <value optimized out>
        rv = 0
#7  0x00007f8ffc0d7ff7 in ?? () from /etc/httpd/modules/mod_auth_basic.so
No symbol table info available.
#8  0x00007f8ffe42a948 in ap_run_check_user_id ()
No symbol table info available.
#9  0x00007f8ffe42cb42 in ap_process_request_internal ()
No symbol table info available.
#10 0x00007f8ffe43ea20 in ap_process_request ()
No symbol table info available.
---Type <return> to continue, or q <return> to quit---
#11 0x00007f8ffe43b8f8 in ?? ()
No symbol table info available.
#12 0x00007f8ffe437608 in ap_run_process_connection ()
No symbol table info available.
#13 0x00007f8ffe443807 in ?? ()
No symbol table info available.
#14 0x00007f8ffe443ad6 in ?? ()
No symbol table info available.
#15 0x00007f8ffe444123 in ap_mpm_run ()
No symbol table info available.
#16 0x00007f8ffe41b900 in main ()
No symbol table info available.
(gdb)

Comment 34 Stephen Gallagher 2012-06-14 19:25:16 UTC
Ok, I know what the problem is, but I'm not going to have time until tomorrow afternoon (EDT) at the earliest to look at it.

Basically, I forgot to initialize and destroy the NSS context using NSS_InitContext() and NSS_ShutdownContext() (notes to myself).

Comment 35 Simone Caronni 2012-07-02 09:14:14 UTC
Hello,

any news regarding this?

Many thanks,
--Simone

Comment 36 Stephen Gallagher 2012-07-03 11:27:51 UTC
Sorry, Simone. A lot of urgent stuff hit me all at once and I haven't had a chance to address this. I'll try to get to it some time this week.

Comment 37 Stephen Gallagher 2012-07-13 13:23:44 UTC
Created attachment 598069 [details]
SRPM containing new patches to build libxradius

Sorry for the long delay on this. Life gets hectic sometimes.

I'm attaching a new SRPM that contains specfile changes and a new patch (you can probably merge it into the previous one, but I wanted you to be able to easily see the differences).

It compiles cleanly, but as before I can't really test it myself.

Also, there are probably better places to start and terminate the NSS context, but since I don't really understand the rest of the code, I opted to just start and terminate before each hash routine to be on the safe side.

Comment 38 Simone Caronni 2012-07-13 14:58:03 UTC
Woah, it works!

Thank you very much.

Here is the updated src.rpm, same version as the only thing I changed compared to yours are:

- merged the 2 patches into one
- fixed typo, first declaration of "xss_init_nss" with "xss_init_nss" as the rest of the patch, or we get an undefined symbol at startup
- inserted the missing changelog line (13.fc17)

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-14.fc17.src.rpm

Comment 39 Stephen Gallagher 2012-07-13 16:06:06 UTC
Please restore the versioned BuildRequires I placed on NSS. That was mostly to make it clear that some massaging would be required to get it to work on old versions of RHEL 5 and maybe RHEL 6, if such backports were planned for EPEL.

Both RHEL 5 and RHEL 6 with their latest updates have a compatible version of NSS, but they did not begin that way.

So re-add
BuildRequires:  nss-devel%{?_isa} >= 3.12.5


Beyond that, this looks good to me. I'm going to leave the final decision to John Ciesla, since I've had too much of a hand in the packaging process to be the formal reviewer.

Comment 40 Simone Caronni 2012-07-13 16:38:41 UTC
Oh, sorry I missed that.

I actually did the testing also on a RHEL 6.2. I checked with my redhat account, only early RHEL 5's have less than that version.

I took the chance to remove also the %{_isa} on BuildRequires, which i forgot to do last time following input from other reviews.

Spec URL: http://slaanesh.fedorapeople.org/mod_auth_xradius.spec
SRPM URL: http://slaanesh.fedorapeople.org/mod_auth_xradius-0.4.6-15.fc17.src.rpm

Thank you very much for everything, I owe you a beer if you come to Italy or Switzerland. :)

Comment 41 Gwyn Ciesla 2012-07-20 17:54:59 UTC
Sorry for the delay, I had vacation followed by a family emergency.

Looks great, one minor change, libxradius-devel actaully depends on mod_auth_xradius, and should instead depend on libxradius.  I trust you'll fix that before importing.

APPROVED.

Thanks to everyone involved in getting this done.

Comment 42 Simone Caronni 2012-07-23 12:32:14 UTC
New Package SCM Request
=======================
Package Name: mod_auth_xradius
Short Description: Apache module that provides authentication against RADIUS Servers
Owners: slaanesh
Branches: f17 f16 el6 el5
InitialCC:

Comment 43 Simone Caronni 2012-07-23 12:34:07 UTC
Fixed libxradius-devel requirement, it will be in git.

Many thanks to everybody!
--Simone

Comment 44 Gwyn Ciesla 2012-07-23 13:20:21 UTC
Git done (by process-git-requests).

Comment 45 Fedora Update System 2012-07-26 14:14:36 UTC
mod_auth_xradius-0.4.6-16.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/mod_auth_xradius-0.4.6-16.el5

Comment 46 Fedora Update System 2012-07-26 14:14:58 UTC
mod_auth_xradius-0.4.6-16.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/mod_auth_xradius-0.4.6-16.el6

Comment 47 Fedora Update System 2012-07-26 14:15:14 UTC
mod_auth_xradius-0.4.6-16.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mod_auth_xradius-0.4.6-16.fc16

Comment 48 Fedora Update System 2012-07-26 14:15:30 UTC
mod_auth_xradius-0.4.6-16.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mod_auth_xradius-0.4.6-16.fc17

Comment 49 Fedora Update System 2012-07-26 17:58:49 UTC
mod_auth_xradius-0.4.6-16.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 50 Fedora Update System 2012-07-27 22:06:13 UTC
mod_auth_xradius-0.4.6-16.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 51 Fedora Update System 2012-07-27 22:07:36 UTC
mod_auth_xradius-0.4.6-16.el5 has been pushed to the Fedora EPEL 5 stable repository.

Comment 52 Fedora Update System 2012-07-28 01:25:12 UTC
mod_auth_xradius-0.4.6-16.fc17 has been pushed to the Fedora 17 stable repository.

Comment 53 Fedora Update System 2012-07-28 01:25:58 UTC
mod_auth_xradius-0.4.6-16.fc16 has been pushed to the Fedora 16 stable repository.