kohsuke / libpam4j

libpam4j
http://libpam4j.kohsuke.org/
MIT License
44 stars 47 forks source link

libpam4j not compatible with JNA 4.2.2 #16

Closed todd-richmond closed 6 years ago

todd-richmond commented 8 years ago

I attempted to updated to JNA 4.2.2, but libpam4j authentication (simple unix passwd file) fails on Linux (Centos 6.5 and 6.7). OSX seems to still work fine so the issue isn't consistent. JNA 4.1.0 works consistently across OS types

matthiasblaesing commented 8 years ago

I just ran the libpam4j unittests on ubuntu xenial - both test work correctly. I look at this from the perspective of the jna project and can't reproduce the problem.

matthiasblaesing commented 8 years ago

Ok - it is reproducible on centos (6.8+7.1) I used the images from osboxes.org. The resulting exception:

Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 70.679 sec <<< FAILURE!
testConcurrent(org.jvnet.libpam.InteractiveTester)  Time elapsed: 35.35 sec  <<< ERROR!
java.util.concurrent.ExecutionException: org.jvnet.libpam.PAMException: pam_authenticate failed : Authentication failure
    at java.util.concurrent.FutureTask.report(FutureTask.java:122)
    at java.util.concurrent.FutureTask.get(FutureTask.java:188)
    at org.jvnet.libpam.InteractiveTester.testConcurrent(InteractiveTester.java:71)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at junit.framework.TestCase.runTest(TestCase.java:176)
    at junit.framework.TestCase.runBare(TestCase.java:141)
    at junit.framework.TestResult$1.protect(TestResult.java:122)
    at junit.framework.TestResult.runProtected(TestResult.java:142)
    at junit.framework.TestResult.run(TestResult.java:125)
    at junit.framework.TestCase.run(TestCase.java:129)
    at junit.framework.TestSuite.runTest(TestSuite.java:255)
    at junit.framework.TestSuite.run(TestSuite.java:250)
    at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:84)
    at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
    at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
    at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
    at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
    at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)
Caused by: org.jvnet.libpam.PAMException: pam_authenticate failed : Authentication failure
    at org.jvnet.libpam.PAM.check(PAM.java:106)
    at org.jvnet.libpam.PAM.authenticate(PAM.java:124)
    at org.jvnet.libpam.InteractiveTester.testOne(InteractiveTester.java:46)
    at org.jvnet.libpam.InteractiveTester$1.call(InteractiveTester.java:64)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:745)

It could be, that jna in newer releases just exposes undefined behaviour/threading issues. Please note, that I won't investigate further.

todd-richmond commented 8 years ago

I had not seen issues on Ubuntu either, but the vast majority of people using my code are on Centos/Redhat so I would definitely appreciate a resolution. PAM itself is thread safe as long as you use different handles and this fails for a single call in a very simple test case so it isn't due to concurrency or threading

matthiasblaesing commented 8 years ago

So could you provide the testcase? I added the exception, because it at least traces back into the code and might give an idea of the problem.

todd-richmond commented 8 years ago

I'm not doing anything more sophisticated than the test case you have

PAM pam = new PAM(service); UnixUser user = pam.authenticate(id, password);

matthiasblaesing commented 8 years ago

@todd-richmond I pushed a potential fix to the branch reference above. For you the most probably fix is:

https://github.com/matthiasblaesing/libpam4j/commit/afcc12286a2ce0aebb1c5a91293c0fe4ee28a0d3

Without that commit your dependend on the mercy of the GC. I suspect the changes in JNA when moving from 4.1.0 to 4.2.0 allowed a more aggressive GC and that is revealed in this case. Please check if that fixes the issue (for me the test-suite now works ok).

jamshid commented 7 years ago

Yikes just discovered I've been hitting this problem (random pam_authenticate failed : Authentication failure under load) even with jna-4.0.0.jar and libpam4j-1.8.jar. Thanks @matthiasblaesing your branch https://github.com/matthiasblaesing/libpam4j/commits/gc_and_threadsave seems to fix it.

It would be great if that branch could be deployed to maven repo as an official 1.9. The current version 1.8 is not safe.

The test code https://gist.github.com/jamshid/d03bb69f6a9076b178d73e69ee5446c4 shows the problem -- it disposes of a global PAM object periodically. Eventually authenticate() throw a PAMException. With libpam4j 1.8 you'll get this failure after a minute or so in this (single-threaded) test. With patch 1.9X it succeeds indefinitely and performance is consistent, ~12ms.

$ javac -classpath ./jna-4.0.0.jar:./libpam4j-1.8.jar:. PAMTest.java
$ sudo java -classpath ./jna-4.0.0.jar:./libpam4j-1.8.jar:. PAMTest testuser:password
...
DEBUG: pam.authenticate=40ms / 40ms, User admin1 1000 1000 /home/admin1  /bin/bash in [everyone, admin1, admin]
Exception for user 'admin1', wrong password? org.jvnet.libpam.PAMException: pam_authenticate failed : Authentication information cannot be recovered
org.jvnet.libpam.PAMException: pam_authenticate failed : Authentication information cannot be recovered
    at org.jvnet.libpam.PAM.check(PAM.java:106)
    at org.jvnet.libpam.PAM.authenticate(PAM.java:124)
    at PAMTest.main(PAMTest.java:59)

PS: this bug was difficult to see with libpam4j 1.8. If you never reset the global PAM object the test succeeds, but leaks memory. Resetting after every use succeeds, but is a little slow. Resetting after every few uses will eventually fail, I guess it all depends on GC behavior.

todd-richmond commented 7 years ago

I'll have to track down a 6.x system since everything around here was upgrade to 7.3

Pushing a 1.9 package to the standard Maven repo would be a great help though and it seems that jamshid verified it fixes at least one issue

jamshid commented 7 years ago

Let me know if I can help in any way with building or further testing. If you need a centos 6 (java 6?) environment to build the gc_and_threadsave branch in matthiasblaesing/libpam4j, just docker build -t mylibpam4j . this Dockerfile:

# Java 1.6 / centos6 environment that builds a libpam4j patch.
#
FROM centos:centos6

# Does jar need to be built with Java 6 for drop-in backward-compatibility?
RUN yum install -y git java-1.6.0-openjdk-devel 

# Installs maven and apparently JDK 1.7
RUN curl -fL http://repos.fedorapeople.org/repos/dchen/apache-maven/epel-apache-maven.repo -o /etc/yum.repos.d/epel-apache-maven.repo && yum install -y apache-maven

# Java 6 JDK is installed but no the default. Run this if java 6 has to be the
# default and you don't mind breaking maven. Instead specify 1.6 in pom.xml?
#RUN alternatives --set java /usr/lib/jvm/jre-1.6.0-openjdk.x86_64/bin/java
#RUN alternatives --set javac /usr/lib/jvm/java-1.6.0-openjdk.x86_64/bin/javac

RUN git clone https://github.com/matthiasblaesing/libpam4j.git && cd libpam4j && git checkout gc_and_threadsave && mvn install

RUN echo "Deploy these files..." ; find ~/.m2 -name '*libpam4j*' -ls
jamshid commented 7 years ago

Any chance of this thread-safety fix getting deployed to the maven repo, @kohsuke or @matthiasblaesing ? I need it and would rather not add a dependency to a local / fork repo.

todd-richmond commented 7 years ago

I can confirm that the PR fixes my test case after I found a 6.8 system

thejasmn commented 6 years ago

@kohsuke Can we have this fix included in a new version ? Thanks for creating and maintaining this library!