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

Patches to support AUTHTOK for use with OpenVPN #39

Closed
ThomasHabets opened this Issue Oct 10, 2014 · 52 comments

Comments

Projects
None yet
2 participants
@ThomasHabets
Contributor

ThomasHabets commented Oct 10, 2014

Original issue 39 created by fraser.scott on 2011-02-19T23:10:21.000Z:

Hi,

I have created a couple of patches to allow me to use google-authenticator with OpenVPN.

0001-Added-lpam.patch

This simple adds -lpam to the Makefile so OpenVPN can dlopen

0002-supports-AUTHTOK.patch

This adds support for AUTHTOK so that you can enter the 6 digit code immediately followed by the unix password for two-factor auth. Using OpenVPN through Gnome's network manager doesn't allow you to have multiple prompts it seems, so both factors can be entered at once.

Example PAM config for common-auth:

auth required pam_google_authenticator.so
auth required pam_unix.so nullok_secure try_first_pass

Please note, I'm not a C developer and I haven't fully tested my patch yet, but I thought it might be useful.

Cheers,
zeroXten

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #1 originally posted by fraser.scott on 2011-02-22T00:42:01.000Z:

Another patch to my patch, fixes a silly segfault when resp size was wrong (too big etc).

Contributor

ThomasHabets commented Oct 10, 2014

Comment #1 originally posted by fraser.scott on 2011-02-22T00:42:01.000Z:

Another patch to my patch, fixes a silly segfault when resp size was wrong (too big etc).

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #2 originally posted by valient on 2011-02-25T07:44:52.000Z:

Would this be easy to do the other way - unix password followed by code? Other two-factor systems that I use already do it this way, so it would be easier on muscle memory if they worked the same way.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #2 originally posted by valient on 2011-02-25T07:44:52.000Z:

Would this be easy to do the other way - unix password followed by code? Other two-factor systems that I use already do it this way, so it would be easier on muscle memory if they worked the same way.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #3 originally posted by fraser.scott on 2011-02-25T09:50:48.000Z:

yeah I guess so. Don't think it will require much of a change, will take a look this weekend.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #3 originally posted by fraser.scott on 2011-02-25T09:50:48.000Z:

yeah I guess so. Don't think it will require much of a change, will take a look this weekend.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #4 originally posted by fraser.scott on 2011-02-25T17:44:05.000Z:

Attaching two full patches for Makefile (same as 0001* above) and pam_google_authenticator.c which now takes password+code instead of code+password. Ignore the patches above.

One thing is that the pam_google_authenticator_unittest is broken with the following error:

$ make test
./pam_google_authenticator_unittest
Checking base32 encoding
Checking base32 decoding
Loading PAM module
Testing successful login
pam_google_authenticator_unittest: pam_google_authenticator_unittest.c:119: main: Assertion `pam_sm_open_session(((void )0), 0, 0, ((void *)0)) == 0' failed.
make: *
* [test] Aborted (core dumped)

Although the patch itself seems to work ok. I will take a look at the unit test soonish.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #4 originally posted by fraser.scott on 2011-02-25T17:44:05.000Z:

Attaching two full patches for Makefile (same as 0001* above) and pam_google_authenticator.c which now takes password+code instead of code+password. Ignore the patches above.

One thing is that the pam_google_authenticator_unittest is broken with the following error:

$ make test
./pam_google_authenticator_unittest
Checking base32 encoding
Checking base32 decoding
Loading PAM module
Testing successful login
pam_google_authenticator_unittest: pam_google_authenticator_unittest.c:119: main: Assertion `pam_sm_open_session(((void )0), 0, 0, ((void *)0)) == 0' failed.
make: *
* [test] Aborted (core dumped)

Although the patch itself seems to work ok. I will take a look at the unit test soonish.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #5 originally posted by brandon.ewing03 on 2011-03-03T19:33:20.000Z:

Would it be smarter to have this as a separate PAM module, so you could use the original with services that support multiple prompts (SSH), and this modified version for others (OpenVPN, FTP, etc)?

Contributor

ThomasHabets commented Oct 10, 2014

Comment #5 originally posted by brandon.ewing03 on 2011-03-03T19:33:20.000Z:

Would it be smarter to have this as a separate PAM module, so you could use the original with services that support multiple prompts (SSH), and this modified version for others (OpenVPN, FTP, etc)?

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #6 originally posted by fraser.scott on 2011-03-03T21:21:14.000Z:

The patch supports both. If the code length is 6 it won't try to pass on the AUTHTOK.

e.g.
user@hosta:$ ssh hostb
oh, hai
Verification code: [password + code here]
...
Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Thu Mar 3 17:04:10 2011 ...
user@hostb:
$ logout

and

user@hosta:$ ssh hostb
oh, hai
Verification code: [6 digit code]
Password: [password]
...
Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Thu Mar 3 21:14:28 2011 ...
user@hostb:
$
user@hostb:~$ logout

Contributor

ThomasHabets commented Oct 10, 2014

Comment #6 originally posted by fraser.scott on 2011-03-03T21:21:14.000Z:

The patch supports both. If the code length is 6 it won't try to pass on the AUTHTOK.

e.g.
user@hosta:$ ssh hostb
oh, hai
Verification code: [password + code here]
...
Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Thu Mar 3 17:04:10 2011 ...
user@hostb:
$ logout

and

user@hosta:$ ssh hostb
oh, hai
Verification code: [6 digit code]
Password: [password]
...
Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Thu Mar 3 21:14:28 2011 ...
user@hostb:
$
user@hostb:~$ logout

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #7 originally posted by me@aaronryan.com on 2011-03-07T21:44:45.000Z:

Thanks for your patches, I've been wanting to use google authenticator with freeradius, and now I've got it working using freeradius and pam. The password + otp was exactly what I needed.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #7 originally posted by me@aaronryan.com on 2011-03-07T21:44:45.000Z:

Thanks for your patches, I've been wanting to use google authenticator with freeradius, and now I've got it working using freeradius and pam. The password + otp was exactly what I needed.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #8 originally posted by markus@google.com on 2011-03-09T20:34:58.000Z:

Issue 25 has been merged into this issue.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #8 originally posted by markus@google.com on 2011-03-09T20:34:58.000Z:

Issue 25 has been merged into this issue.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #9 originally posted by markus@google.com on 2011-03-09T21:09:04.000Z:

Issue 34 has been merged into this issue.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #9 originally posted by markus@google.com on 2011-03-09T21:09:04.000Z:

Issue 34 has been merged into this issue.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #10 originally posted by markus@google.com on 2011-03-09T21:27:09.000Z:

<empty>

Contributor

ThomasHabets commented Oct 10, 2014

Comment #10 originally posted by markus@google.com on 2011-03-09T21:27:09.000Z:

<empty>

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #11 originally posted by AEGarbutt on 2011-03-23T00:37:12.000Z:

Two things:
a. The current patch completely breaks scratch codes by assuming if the string is longer than six characters that it's "pw"+"token". Further, do you interpret "Password99123456" as "Password" and scratch code "99123456" or "Password99" and token code "123456". I think the best way to support this would be to require a separator character between the password and the code. Then search for the last occurrence of that character to split between the password and token. This eliminates the, albeit unlikely, possibility that a user's password (ending in at least two numbers) and token code would also be a valid scratch code. This would remove the scratch code, but fail authentication because two characters were grabbed from the password.

b. the current unittests allow for tokens that are less than 6 characters long. The tests still pass because they are converted back to the appropriate integer with strtoul(). I believe this is incorrect behavior, as all tokens should be six characters (zero-padded as necessary).

Contributor

ThomasHabets commented Oct 10, 2014

Comment #11 originally posted by AEGarbutt on 2011-03-23T00:37:12.000Z:

Two things:
a. The current patch completely breaks scratch codes by assuming if the string is longer than six characters that it's "pw"+"token". Further, do you interpret "Password99123456" as "Password" and scratch code "99123456" or "Password99" and token code "123456". I think the best way to support this would be to require a separator character between the password and the code. Then search for the last occurrence of that character to split between the password and token. This eliminates the, albeit unlikely, possibility that a user's password (ending in at least two numbers) and token code would also be a valid scratch code. This would remove the scratch code, but fail authentication because two characters were grabbed from the password.

b. the current unittests allow for tokens that are less than 6 characters long. The tests still pass because they are converted back to the appropriate integer with strtoul(). I believe this is incorrect behavior, as all tokens should be six characters (zero-padded as necessary).

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #12 originally posted by fraser.scott on 2011-05-09T11:14:22.000Z:

Ok, I'll take a look at it as ASAP.. been a bit busy lately.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #12 originally posted by fraser.scott on 2011-05-09T11:14:22.000Z:

Ok, I'll take a look at it as ASAP.. been a bit busy lately.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #13 originally posted by jason@masker.net on 2011-06-17T14:58:43.000Z:

The issues described could be addressed by applying the following logic:

  1. strip the last 6 chars as a token
    a. prompt if pass was exactly 6 chars
    b. otherwise, pass what is left of the string as a password
  2. strip 2 additional chars from the end and prepend to the token to test as scratch code
    a. prompt if nothing remains
    b. otherwise, pass as password

I think adding a delimiter is a bad idea just because it would make this different than most all authtok implementations.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #13 originally posted by jason@masker.net on 2011-06-17T14:58:43.000Z:

The issues described could be addressed by applying the following logic:

  1. strip the last 6 chars as a token
    a. prompt if pass was exactly 6 chars
    b. otherwise, pass what is left of the string as a password
  2. strip 2 additional chars from the end and prepend to the token to test as scratch code
    a. prompt if nothing remains
    b. otherwise, pass as password

I think adding a delimiter is a bad idea just because it would make this different than most all authtok implementations.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #14 originally posted by jason@masker.net on 2011-06-21T03:44:27.000Z:

Here is a new patch. Both tokens & scratch codes now work.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #14 originally posted by jason@masker.net on 2011-06-21T03:44:27.000Z:

Here is a new patch. Both tokens & scratch codes now work.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #15 originally posted by jason@masker.net on 2011-06-21T04:33:34.000Z:

Here is a patch that works for regular tokens or authtok and authtok will work both when using a token and when using a scratch code.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #15 originally posted by jason@masker.net on 2011-06-21T04:33:34.000Z:

Here is a patch that works for regular tokens or authtok and authtok will work both when using a token and when using a scratch code.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #16 originally posted by mega.hurts on 2011-06-29T23:29:57.000Z:

This patch is working great for me in conjunction with freeradius. One issue though, is there a way to require password + token and reject just the token?

With the current implementation anyone with the token generator could login, whereas requiring the password+token would be slightly more secure. Although I guess it would require two modules. One for ssh/challenge response and one for legacy apps.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #16 originally posted by mega.hurts on 2011-06-29T23:29:57.000Z:

This patch is working great for me in conjunction with freeradius. One issue though, is there a way to require password + token and reject just the token?

With the current implementation anyone with the token generator could login, whereas requiring the password+token would be slightly more secure. Although I guess it would require two modules. One for ssh/challenge response and one for legacy apps.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #17 originally posted by jason@masker.net on 2011-06-29T23:39:42.000Z:

This patch does work with password+token. That is really the whole point of it. In order to log in with freeradius this way you need these two lines in /etc/pam.d/radiusd:

auth required pam_google_authenticator.so
auth required pam_unix.so nullok_secure try_first_pass

Or, in my case, I have a Mac, so the lines are:

auth required pam_google_authenticator.so
auth required pam_opendirectory.so try_first_pass

The point is, you require the token and then just under that require the password with the 'try_first_pass' option. This patch basically modifies the google authenticator pam module to try the token at the end of the password and pass the first part of the password on to the next module.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #17 originally posted by jason@masker.net on 2011-06-29T23:39:42.000Z:

This patch does work with password+token. That is really the whole point of it. In order to log in with freeradius this way you need these two lines in /etc/pam.d/radiusd:

auth required pam_google_authenticator.so
auth required pam_unix.so nullok_secure try_first_pass

Or, in my case, I have a Mac, so the lines are:

auth required pam_google_authenticator.so
auth required pam_opendirectory.so try_first_pass

The point is, you require the token and then just under that require the password with the 'try_first_pass' option. This patch basically modifies the google authenticator pam module to try the token at the end of the password and pass the first part of the password on to the next module.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #18 originally posted by dan@gzero.org on 2011-10-31T04:38:51.000Z:

Would any one happen to have an update for the newer code. The patch fails a few hunks. If not I guess I can dig into it. Thanks! :)

Contributor

ThomasHabets commented Oct 10, 2014

Comment #18 originally posted by dan@gzero.org on 2011-10-31T04:38:51.000Z:

Would any one happen to have an update for the newer code. The patch fails a few hunks. If not I guess I can dig into it. Thanks! :)

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #19 originally posted by fmcclory on 2011-10-31T07:37:59.000Z:

I'd love to take another look but I recently became a dad so I seem to have very little time for all things geeky :) I haven't even fixed my google auth installation since it broke a few months back (clock problem I think it was).

Contributor

ThomasHabets commented Oct 10, 2014

Comment #19 originally posted by fmcclory on 2011-10-31T07:37:59.000Z:

I'd love to take another look but I recently became a dad so I seem to have very little time for all things geeky :) I haven't even fixed my google auth installation since it broke a few months back (clock problem I think it was).

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #20 originally posted by jason@masker.net on 2011-10-31T11:30:52.000Z:

I made a few minimal edits to get this working again. Could someone test & confirm? It may be a while before I can.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #20 originally posted by jason@masker.net on 2011-10-31T11:30:52.000Z:

I made a few minimal edits to get this working again. Could someone test & confirm? It may be a while before I can.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #21 originally posted by dan@gzero.org on 2011-11-01T03:59:23.000Z:

I had to modify
+static int request_verification_code(pam_handle_t *pamh, char *codeString) {
to
+static int request_verification_code(pam_handle_t *pamh, int echocode, char *codeString) {
in order to get it to compile

Contributor

ThomasHabets commented Oct 10, 2014

Comment #21 originally posted by dan@gzero.org on 2011-11-01T03:59:23.000Z:

I had to modify
+static int request_verification_code(pam_handle_t *pamh, char *codeString) {
to
+static int request_verification_code(pam_handle_t *pamh, int echocode, char *codeString) {
in order to get it to compile

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #22 originally posted by dan@gzero.org on 2011-11-01T04:14:54.000Z:

That was the only change I had to do to make it work. I tested ssh user/code/pass and networkmanager vpn user/pass+otp

Contributor

ThomasHabets commented Oct 10, 2014

Comment #22 originally posted by dan@gzero.org on 2011-11-01T04:14:54.000Z:

That was the only change I had to do to make it work. I tested ssh user/code/pass and networkmanager vpn user/pass+otp

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #23 originally posted by jason@masker.net on 2011-11-02T17:32:55.000Z:

Correct. Sorry about that. I updated the patch.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #23 originally posted by jason@masker.net on 2011-11-02T17:32:55.000Z:

Correct. Sorry about that. I updated the patch.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #24 originally posted by jeff@crowdstrike.com on 2011-11-18T01:21:11.000Z:

Will this patch work with pam_ldap.so?

my /etc/pam.d/radiusd file looks like this:

auth required pam_google_authenticator.so
auth required pam_ldap.so use_first_pass

And the google auth works, however it seems that pam_ldap never gets called:

Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: pam_sm_authenticate()
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Secret filename: /home/jeff/.google_authenticator
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Successfully authenticated
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: pam_sm_authenticate()
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Secret filename: /home/jeff/.google_authenticator
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Successfully authenticated

Contributor

ThomasHabets commented Oct 10, 2014

Comment #24 originally posted by jeff@crowdstrike.com on 2011-11-18T01:21:11.000Z:

Will this patch work with pam_ldap.so?

my /etc/pam.d/radiusd file looks like this:

auth required pam_google_authenticator.so
auth required pam_ldap.so use_first_pass

And the google auth works, however it seems that pam_ldap never gets called:

Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: pam_sm_authenticate()
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Secret filename: /home/jeff/.google_authenticator
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Successfully authenticated
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: pam_sm_authenticate()
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Secret filename: /home/jeff/.google_authenticator
Nov 18 01:16:24 rad01 radiusd(pam_google_authenticator)[14167]: Successfully authenticated

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #25 originally posted by jeff80 on 2011-11-18T02:54:13.000Z:

Will this patch work with pam_ldap.so?

my /etc/pam.d/radiusd file looks like this:

auth required pam_google_authenticator.so
auth required pam_ldap.so use_first_pass

Contributor

ThomasHabets commented Oct 10, 2014

Comment #25 originally posted by jeff80 on 2011-11-18T02:54:13.000Z:

Will this patch work with pam_ldap.so?

my /etc/pam.d/radiusd file looks like this:

auth required pam_google_authenticator.so
auth required pam_ldap.so use_first_pass

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #26 originally posted by jason@masker.net on 2011-11-18T19:36:28.000Z:

Jeff, I could not get 'use_first_pass' to work, I had to use 'try_first_pass.' I need someone more familiar with pam to help troubleshoot and determine why 'use_first_pass' does not work. In the case of radius, only one password prompt is supported, so when 'try_first_pass' reprompts in the case of a bad password, it fails and has the same result as 'use_next_pass.'

Contributor

ThomasHabets commented Oct 10, 2014

Comment #26 originally posted by jason@masker.net on 2011-11-18T19:36:28.000Z:

Jeff, I could not get 'use_first_pass' to work, I had to use 'try_first_pass.' I need someone more familiar with pam to help troubleshoot and determine why 'use_first_pass' does not work. In the case of radius, only one password prompt is supported, so when 'try_first_pass' reprompts in the case of a bad password, it fails and has the same result as 'use_next_pass.'

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #27 originally posted by tore.lonoy on 2011-11-19T18:17:07.000Z:

How do we get this merged into master? I cannot see a reason why this couldn't be a part of the official release

Contributor

ThomasHabets commented Oct 10, 2014

Comment #27 originally posted by tore.lonoy on 2011-11-19T18:17:07.000Z:

How do we get this merged into master? I cannot see a reason why this couldn't be a part of the official release

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #28 originally posted by jason@masker.net on 2011-11-19T18:40:54.000Z:

I'm not sure that anyone who has contributed to this issue is
officially part of the project. We can submit patches, but not modify
the project source.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #28 originally posted by jason@masker.net on 2011-11-19T18:40:54.000Z:

I'm not sure that anyone who has contributed to this issue is
officially part of the project. We can submit patches, but not modify
the project source.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #29 originally posted by jeff80 on 2011-11-20T01:15:47.000Z:

I can confirm that this patch does work with pam_winbind against ActiveDirectory

Contributor

ThomasHabets commented Oct 10, 2014

Comment #29 originally posted by jeff80 on 2011-11-20T01:15:47.000Z:

I can confirm that this patch does work with pam_winbind against ActiveDirectory

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #30 originally posted by rvrignaud on 2011-11-24T10:35:53.000Z:

I don't manage to get this patch working with up-to-date source code checkout and Freeradius.
It's working the libpam-google-authenticator is working just fine with sshd.
My radtest is working fine with just
"auth sufficient pam_ldap.so try_first_pass
auth sufficient pam_unix.so try_first_pass" in the PAM configuration
but if I add "auth required pam_google_authenticator.so" before,
I always get in my auth.log :
"radiusd(pam_google_authenticator)[26576]: Failed to change user id"
and pam_unix(radiusd:auth): authentication failure or same kind of error with ldap user.
I guess the password/OTP crop is not done correctly. Any idea where my configuration could be wrong ?

Thanks in advance

Contributor

ThomasHabets commented Oct 10, 2014

Comment #30 originally posted by rvrignaud on 2011-11-24T10:35:53.000Z:

I don't manage to get this patch working with up-to-date source code checkout and Freeradius.
It's working the libpam-google-authenticator is working just fine with sshd.
My radtest is working fine with just
"auth sufficient pam_ldap.so try_first_pass
auth sufficient pam_unix.so try_first_pass" in the PAM configuration
but if I add "auth required pam_google_authenticator.so" before,
I always get in my auth.log :
"radiusd(pam_google_authenticator)[26576]: Failed to change user id"
and pam_unix(radiusd:auth): authentication failure or same kind of error with ldap user.
I guess the password/OTP crop is not done correctly. Any idea where my configuration could be wrong ?

Thanks in advance

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #31 originally posted by markus@google.com on 2011-12-15T08:50:57.000Z:

The PAM module now supports "forward_pass", "use_first_pass", and "try_first_pass". I suspect, "forward_pass" is the most useful feature and will allow for stacking of other PAM modules.

I believe this fixes the problem that was reported, but please feel free to re-open the issue, if there are remaining problems that need to be addressed.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #31 originally posted by markus@google.com on 2011-12-15T08:50:57.000Z:

The PAM module now supports "forward_pass", "use_first_pass", and "try_first_pass". I suspect, "forward_pass" is the most useful feature and will allow for stacking of other PAM modules.

I believe this fixes the problem that was reported, but please feel free to re-open the issue, if there are remaining problems that need to be addressed.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #32 originally posted by markus@google.com on 2011-12-15T08:53:13.000Z:

Issue 119 has been merged into this issue.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #32 originally posted by markus@google.com on 2011-12-15T08:53:13.000Z:

Issue 119 has been merged into this issue.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #33 originally posted by jason@masker.net on 2011-12-18T03:12:55.000Z:

Hmmm. I'm not sure this solves the problem entirely. Not without modification to other Pam modules or a shim Pam module to sit between the google module and strip the token or otp before passing the remaining string to the password module desired. That is unless forward_pass does this. The goal I think we were all trying to achieve is two factor authentication for services which only support one prompt.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #33 originally posted by jason@masker.net on 2011-12-18T03:12:55.000Z:

Hmmm. I'm not sure this solves the problem entirely. Not without modification to other Pam modules or a shim Pam module to sit between the google module and strip the token or otp before passing the remaining string to the password module desired. That is unless forward_pass does this. The goal I think we were all trying to achieve is two factor authentication for services which only support one prompt.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #34 originally posted by markus@google.com on 2011-12-18T03:56:05.000Z:

Just try it :-)

It does strip the OTP part from the password before forwarding. And that's of course what makes the changelist so complicated. HOTP and TOTP tokens are always six digits, whereas scratch codes are always eight digits. If the authenticator PAM module sees eight or more digits at the end of the user's entry, it can't easily tell whether this is a scratch code or whether it is a password ending in digits followed by an HOTP/TOTP token.

So, your idea of doing the stripping in a "shim" module wouldn't really work anyway. You are quite correct that the authenticator module has to do the stripping.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #34 originally posted by markus@google.com on 2011-12-18T03:56:05.000Z:

Just try it :-)

It does strip the OTP part from the password before forwarding. And that's of course what makes the changelist so complicated. HOTP and TOTP tokens are always six digits, whereas scratch codes are always eight digits. If the authenticator PAM module sees eight or more digits at the end of the user's entry, it can't easily tell whether this is a scratch code or whether it is a password ending in digits followed by an HOTP/TOTP token.

So, your idea of doing the stripping in a "shim" module wouldn't really work anyway. You are quite correct that the authenticator module has to do the stripping.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #35 originally posted by jason@masker.net on 2011-12-28T16:21:23.000Z:

Ok, I'm still missing something to get the module working as I expect.. or at least as my patch worked. I expected this kind of pam config to work for my radius server:

auth required pam_google_authenticator.so forward_pass
auth required pam_opendirectory.so try_first_pass

However, it doesn't... I've tripple checked the password is correct and it works with my patch (though I'm sure my patch broke some internals, it did actually work with both HOTP & TOTP tokens at the end of the string). In fact, as soon as I add the 'forward_pass' directive the pam_google_authenticator module always fails. Even with a pam config that looks like this:

auth required pam_google_authenticator.so forward_pass

Just to be clear I am expecting standard usernames and concatenated passwords in the form of SYSTEMtoken where SYSTEM is the system password and token is the HOTP/TOTP token.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #35 originally posted by jason@masker.net on 2011-12-28T16:21:23.000Z:

Ok, I'm still missing something to get the module working as I expect.. or at least as my patch worked. I expected this kind of pam config to work for my radius server:

auth required pam_google_authenticator.so forward_pass
auth required pam_opendirectory.so try_first_pass

However, it doesn't... I've tripple checked the password is correct and it works with my patch (though I'm sure my patch broke some internals, it did actually work with both HOTP & TOTP tokens at the end of the string). In fact, as soon as I add the 'forward_pass' directive the pam_google_authenticator module always fails. Even with a pam config that looks like this:

auth required pam_google_authenticator.so forward_pass

Just to be clear I am expecting standard usernames and concatenated passwords in the form of SYSTEMtoken where SYSTEM is the system password and token is the HOTP/TOTP token.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #36 originally posted by markus@google.com on 2011-12-28T18:41:31.000Z:

This is all a little puzzling and will need more debugging. Things do seem to be working fine in all the test-cases that I can throw at the code. So, something is obviously different between your environment and the tests that I have devised.

Can you reproduce the problem by running the "demo" application. Running the application is equivalent to invoking the "pam_google_authenticator" PAM module. You can give extra arguments on the command line.

If you pass the "forward_pass" option, you should be able to authenticate both with a) just the plain HOTP/TOTP token, or b) with any arbitrary characters preceding the token. As we only invoke the "pam_google_authenticator" code, the "demo" application doesn't actually verify your system password.

If "demo" works for you, we need to figure out why it then doesn't work as a PAM module (e.g. double-check you actually installed the new code, check the system log, maybe add more logging output, ...). If "demo" doesn't work, then we need to find out why; "gdb" is probably the fasted way to shed some light on what's going on.

Your help in collecting more data is appreciated.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #36 originally posted by markus@google.com on 2011-12-28T18:41:31.000Z:

This is all a little puzzling and will need more debugging. Things do seem to be working fine in all the test-cases that I can throw at the code. So, something is obviously different between your environment and the tests that I have devised.

Can you reproduce the problem by running the "demo" application. Running the application is equivalent to invoking the "pam_google_authenticator" PAM module. You can give extra arguments on the command line.

If you pass the "forward_pass" option, you should be able to authenticate both with a) just the plain HOTP/TOTP token, or b) with any arbitrary characters preceding the token. As we only invoke the "pam_google_authenticator" code, the "demo" application doesn't actually verify your system password.

If "demo" works for you, we need to figure out why it then doesn't work as a PAM module (e.g. double-check you actually installed the new code, check the system log, maybe add more logging output, ...). If "demo" doesn't work, then we need to find out why; "gdb" is probably the fasted way to shed some light on what's going on.

Your help in collecting more data is appreciated.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #37 originally posted by jason@masker.net on 2011-12-30T00:38:14.000Z:

ok, I did some pam debugs and I'm actually getting unexpected return codes when using that module. I didn't pay attention during build, but I'm concerned about some of the warnings. You've updated the includes now so this compiles on Mac, but I'm thinking something might not be right. Back when I wrote the patch I couldn't even compile without modifying the make file. Now I get these warnings:

find: /lib: No such file or directory
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o google-authenticator.o google-authenticator.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o base32.o base32.c
base32.c: In function ‘base32_decode’:
base32.c:22: warning: internal and protected visibility attributes not supported in this configuration; ignored
base32.c: In function ‘base32_encode’:
base32.c:65: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o hmac.o hmac.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o sha1.o sha1.c
sha1.c: In function ‘sha1_init’:
sha1.c:222: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_final’:
sha1.c:302: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_update’:
sha1.c:237: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc -g -o google-authenticator google-authenticator.o base32.o hmac.o sha1.o
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator.so pam_google_authenticator.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o demo.o demo.c
gcc -DDEMO --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_demo.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -g -rdynamic -o demo demo.o pam_google_authenticator_demo.o base32.o hmac.o sha1.o
gcc -DTESTING --std=gnu99 -Wall -O2 -g -fPIC -c
-o pam_google_authenticator_testing.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator_testing.so pam_google_authenticator_testing.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_unittest.o pam_google_authenticator_unittest.c
gcc -g -rdynamic -o pam_google_authenticator_unittest pam_google_authenticator_unittest.o base32.o hmac.o sha1.o -lc

Contributor

ThomasHabets commented Oct 10, 2014

Comment #37 originally posted by jason@masker.net on 2011-12-30T00:38:14.000Z:

ok, I did some pam debugs and I'm actually getting unexpected return codes when using that module. I didn't pay attention during build, but I'm concerned about some of the warnings. You've updated the includes now so this compiles on Mac, but I'm thinking something might not be right. Back when I wrote the patch I couldn't even compile without modifying the make file. Now I get these warnings:

find: /lib: No such file or directory
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o google-authenticator.o google-authenticator.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o base32.o base32.c
base32.c: In function ‘base32_decode’:
base32.c:22: warning: internal and protected visibility attributes not supported in this configuration; ignored
base32.c: In function ‘base32_encode’:
base32.c:65: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o hmac.o hmac.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o sha1.o sha1.c
sha1.c: In function ‘sha1_init’:
sha1.c:222: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_final’:
sha1.c:302: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_update’:
sha1.c:237: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc -g -o google-authenticator google-authenticator.o base32.o hmac.o sha1.o
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator.so pam_google_authenticator.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o demo.o demo.c
gcc -DDEMO --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_demo.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -g -rdynamic -o demo demo.o pam_google_authenticator_demo.o base32.o hmac.o sha1.o
gcc -DTESTING --std=gnu99 -Wall -O2 -g -fPIC -c
-o pam_google_authenticator_testing.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator_testing.so pam_google_authenticator_testing.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_unittest.o pam_google_authenticator_unittest.c
gcc -g -rdynamic -o pam_google_authenticator_unittest pam_google_authenticator_unittest.o base32.o hmac.o sha1.o -lc

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #38 originally posted by markus@google.com on 2011-12-30T01:25:23.000Z:

That could certainly cause problems. It sounds as if your compiler decided to ignore what we told it and to export symbols that shouldn't be exported.

I just made a few changes that should make the code more conservative. Let me know, whether this works better for you.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #38 originally posted by markus@google.com on 2011-12-30T01:25:23.000Z:

That could certainly cause problems. It sounds as if your compiler decided to ignore what we told it and to export symbols that shouldn't be exported.

I just made a few changes that should make the code more conservative. Let me know, whether this works better for you.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #39 originally posted by jason@masker.net on 2011-12-30T01:42:47.000Z:

hmm.. did you commit the changes to the trunk? I just ran 'hg up' but didn't get any files updated.
My compiler is the llvm gcc 4.2 distributed with xcode.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #39 originally posted by jason@masker.net on 2011-12-30T01:42:47.000Z:

hmm.. did you commit the changes to the trunk? I just ran 'hg up' but didn't get any files updated.
My compiler is the llvm gcc 4.2 distributed with xcode.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #40 originally posted by markus@google.com on 2011-12-30T02:33:31.000Z:

I can see the changes at http://code.google.com/p/google-authenticator/source/detail?r=94d90b9f7b38fd54598d72fdbce16b6b62e8be34#

Do you not see them?

Contributor

ThomasHabets commented Oct 10, 2014

Comment #40 originally posted by markus@google.com on 2011-12-30T02:33:31.000Z:

I can see the changes at http://code.google.com/p/google-authenticator/source/detail?r=94d90b9f7b38fd54598d72fdbce16b6b62e8be34#

Do you not see them?

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #41 originally posted by markus@google.com on 2011-12-30T02:35:30.000Z:

Oh, and you probably meant "hg pull" instead of "hg up".

Contributor

ThomasHabets commented Oct 10, 2014

Comment #41 originally posted by markus@google.com on 2011-12-30T02:35:30.000Z:

Oh, and you probably meant "hg pull" instead of "hg up".

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #42 originally posted by jason@masker.net on 2011-12-30T03:23:17.000Z:

See them now. This is building w/ the changes:

find: /lib: No such file or directory
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o google-authenticator.o google-authenticator.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o base32.o base32.c
base32.c: In function ‘base32_decode’:
base32.c:22: warning: internal and protected visibility attributes not supported in this configuration; ignored
base32.c: In function ‘base32_encode’:
base32.c:65: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o hmac.o hmac.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o sha1.o sha1.c
sha1.c: In function ‘sha1_init’:
sha1.c:222: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_final’:
sha1.c:302: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_update’:
sha1.c:237: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc -g -o google-authenticator google-authenticator.o base32.o hmac.o sha1.o
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator.so pam_google_authenticator.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o demo.o demo.c
gcc -DDEMO --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_demo.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -g -rdynamic -o demo demo.o pam_google_authenticator_demo.o base32.o hmac.o sha1.o
gcc -DTESTING --std=gnu99 -Wall -O2 -g -fPIC -c
-o pam_google_authenticator_testing.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator_testing.so pam_google_authenticator_testing.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_unittest.o pam_google_authenticator_unittest.c
gcc -g -rdynamic -o pam_google_authenticator_unittest pam_google_authenticator_unittest.o base32.o hmac.o sha1.o -lc

Contributor

ThomasHabets commented Oct 10, 2014

Comment #42 originally posted by jason@masker.net on 2011-12-30T03:23:17.000Z:

See them now. This is building w/ the changes:

find: /lib: No such file or directory
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o google-authenticator.o google-authenticator.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o base32.o base32.c
base32.c: In function ‘base32_decode’:
base32.c:22: warning: internal and protected visibility attributes not supported in this configuration; ignored
base32.c: In function ‘base32_encode’:
base32.c:65: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o hmac.o hmac.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o sha1.o sha1.c
sha1.c: In function ‘sha1_init’:
sha1.c:222: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_final’:
sha1.c:302: warning: internal and protected visibility attributes not supported in this configuration; ignored
sha1.c: In function ‘sha1_update’:
sha1.c:237: warning: internal and protected visibility attributes not supported in this configuration; ignored
gcc -g -o google-authenticator google-authenticator.o base32.o hmac.o sha1.o
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator.so pam_google_authenticator.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o demo.o demo.c
gcc -DDEMO --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_demo.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -g -rdynamic -o demo demo.o pam_google_authenticator_demo.o base32.o hmac.o sha1.o
gcc -DTESTING --std=gnu99 -Wall -O2 -g -fPIC -c
-o pam_google_authenticator_testing.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:754: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator_testing.so pam_google_authenticator_testing.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -o pam_google_authenticator_unittest.o pam_google_authenticator_unittest.c
gcc -g -rdynamic -o pam_google_authenticator_unittest pam_google_authenticator_unittest.o base32.o hmac.o sha1.o -lc

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #43 originally posted by markus@google.com on 2011-12-30T05:11:34.000Z:

Are you sure you have a clean tree? Some of those error messages look like issues that no longer exist. Can you do me a favor and pull a completely fresh tree (i.e. in a separate directory)? Let's just make sure, we are not chasing a red herring.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #43 originally posted by markus@google.com on 2011-12-30T05:11:34.000Z:

Are you sure you have a clean tree? Some of those error messages look like issues that no longer exist. Can you do me a favor and pull a completely fresh tree (i.e. in a separate directory)? Let's just make sure, we are not chasing a red herring.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #44 originally posted by jason@masker.net on 2011-12-30T13:05:02.000Z:

Yeah, I just removed the whole build tree and pulled it all back down to make sure it was clean.

gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o google-authenticator.o google-authenticator.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o base32.o base32.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o hmac.o hmac.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o sha1.o sha1.c
gcc -g -o google-authenticator google-authenticator.o base32.o hmac.o sha1.o -ldl
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o pam_google_authenticator.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:755: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator.so pam_google_authenticator.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o demo.o demo.c
gcc -DDEMO --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o pam_google_authenticator_demo.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:755: warning: initialization discards qualifiers from pointer target type
gcc -g -rdynamic -o demo demo.o pam_google_authenticator_demo.o base32.o hmac.o sha1.o -ldl
gcc -DTESTING --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden
-o pam_google_authenticator_testing.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:755: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator_testing.so pam_google_authenticator_testing.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o pam_google_authenticator_unittest.o pam_google_authenticator_unittest.c
gcc -g -rdynamic -o pam_google_authenticator_unittest pam_google_authenticator_unittest.o base32.o hmac.o sha1.o -lc -ldl

Contributor

ThomasHabets commented Oct 10, 2014

Comment #44 originally posted by jason@masker.net on 2011-12-30T13:05:02.000Z:

Yeah, I just removed the whole build tree and pulled it all back down to make sure it was clean.

gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o google-authenticator.o google-authenticator.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o base32.o base32.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o hmac.o hmac.c
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o sha1.o sha1.c
gcc -g -o google-authenticator google-authenticator.o base32.o hmac.o sha1.o -ldl
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o pam_google_authenticator.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:755: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator.so pam_google_authenticator.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o demo.o demo.c
gcc -DDEMO --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o pam_google_authenticator_demo.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:755: warning: initialization discards qualifiers from pointer target type
gcc -g -rdynamic -o demo demo.o pam_google_authenticator_demo.o base32.o hmac.o sha1.o -ldl
gcc -DTESTING --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden
-o pam_google_authenticator_testing.o pam_google_authenticator.c
pam_google_authenticator.c: In function ‘request_pass’:
pam_google_authenticator.c:755: warning: initialization discards qualifiers from pointer target type
gcc -shared -g -o pam_google_authenticator_testing.so pam_google_authenticator_testing.o base32.o hmac.o sha1.o -lpam
gcc --std=gnu99 -Wall -O2 -g -fPIC -c -fvisibility=hidden -o pam_google_authenticator_unittest.o pam_google_authenticator_unittest.c
gcc -g -rdynamic -o pam_google_authenticator_unittest pam_google_authenticator_unittest.o base32.o hmac.o sha1.o -lc -ldl

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #45 originally posted by markus@google.com on 2011-12-30T18:16:56.000Z:

That looks a lot more like it.

The one remaining warning is harmless. I'd still like to fix it, but I think it is something specific to MacOS and without carefully going through the system headers I am unlikely to spot what is different here.

Having said all of that, do things work correctly for you now? The "visibility" changes should ideally have taken care of the problem that you were reporting initially.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #45 originally posted by markus@google.com on 2011-12-30T18:16:56.000Z:

That looks a lot more like it.

The one remaining warning is harmless. I'd still like to fix it, but I think it is something specific to MacOS and without carefully going through the system headers I am unlikely to spot what is different here.

Having said all of that, do things work correctly for you now? The "visibility" changes should ideally have taken care of the problem that you were reporting initially.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #46 originally posted by jason@masker.net on 2011-12-31T01:17:14.000Z:

No luck.. I'm just not sure what's up. At first I get an error that the pam module can't be found, which is easy enough to fix. The make script installs to /usr/lib and my modules are located /usr/lib/pam for OSX 10.7. When I move the module, I start getting this unexpected return value whenever it tries to load the pam module:

12/30/11 8:13:58.647 PM radiusd: in _openpam_check_error_code(): pam_sm_authenticate(): unexpected return value 19

The radiusd is just because that's what i'm using to test.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #46 originally posted by jason@masker.net on 2011-12-31T01:17:14.000Z:

No luck.. I'm just not sure what's up. At first I get an error that the pam module can't be found, which is easy enough to fix. The make script installs to /usr/lib and my modules are located /usr/lib/pam for OSX 10.7. When I move the module, I start getting this unexpected return value whenever it tries to load the pam module:

12/30/11 8:13:58.647 PM radiusd: in _openpam_check_error_code(): pam_sm_authenticate(): unexpected return value 19

The radiusd is just because that's what i'm using to test.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #47 originally posted by markus@google.com on 2011-12-31T06:05:16.000Z:

On Linux, "19" would be "PAM_CONV_ERR". No idea whether OSX uses the same values.

But if it is in fact "PAM_CONV_ERR", then I am a little puzzled. I don't see where our PAM module could return this particular value. You might want to add some more logging to see whether our code even gets called, and what it is trying to do.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #47 originally posted by markus@google.com on 2011-12-31T06:05:16.000Z:

On Linux, "19" would be "PAM_CONV_ERR". No idea whether OSX uses the same values.

But if it is in fact "PAM_CONV_ERR", then I am a little puzzled. I don't see where our PAM module could return this particular value. You might want to add some more logging to see whether our code even gets called, and what it is trying to do.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #48 originally posted by dhahn@trialpay.com on 2012-01-09T21:46:23.000Z:

I'm having trouble applying the patch - 3 out 7 hunks fail. I pulled a clean copy of the trunk today and applied the patch version from comment # 23.

This is what I'm getting:

patching file libpam/pam_google_authenticator.c
Hunk # 1 succeeded at 79 (offset 17 lines).
Hunk # 2 FAILED at 646.
Hunk # 3 FAILED at 654.
Hunk # 4 succeeded at 764 with fuzz 1 (offset 101 lines).
Hunk # 5 succeeded at 1256 with fuzz 2 (offset 120 lines).
Hunk # 6 FAILED at 1269.
Hunk # 7 succeeded at 1418 with fuzz 1 (offset 243 lines).
3 out of 7 hunks FAILED -- saving rejects to file libpam/pam_google_authenticator.c.rej

Any ideas?

Contributor

ThomasHabets commented Oct 10, 2014

Comment #48 originally posted by dhahn@trialpay.com on 2012-01-09T21:46:23.000Z:

I'm having trouble applying the patch - 3 out 7 hunks fail. I pulled a clean copy of the trunk today and applied the patch version from comment # 23.

This is what I'm getting:

patching file libpam/pam_google_authenticator.c
Hunk # 1 succeeded at 79 (offset 17 lines).
Hunk # 2 FAILED at 646.
Hunk # 3 FAILED at 654.
Hunk # 4 succeeded at 764 with fuzz 1 (offset 101 lines).
Hunk # 5 succeeded at 1256 with fuzz 2 (offset 120 lines).
Hunk # 6 FAILED at 1269.
Hunk # 7 succeeded at 1418 with fuzz 1 (offset 243 lines).
3 out of 7 hunks FAILED -- saving rejects to file libpam/pam_google_authenticator.c.rej

Any ideas?

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #49 originally posted by dhahn@trialpay.com on 2012-01-09T21:46:54.000Z:

I'm having trouble applying the patch - 3 out of 7 hunks fail. I pulled a clean copy of the trunk today and applied the patch version from comment # 23.

This is what I'm getting:

patching file libpam/pam_google_authenticator.c
Hunk # 1 succeeded at 79 (offset 17 lines).
Hunk # 2 FAILED at 646.
Hunk # 3 FAILED at 654.
Hunk # 4 succeeded at 764 with fuzz 1 (offset 101 lines).
Hunk # 5 succeeded at 1256 with fuzz 2 (offset 120 lines).
Hunk # 6 FAILED at 1269.
Hunk # 7 succeeded at 1418 with fuzz 1 (offset 243 lines).
3 out of 7 hunks FAILED -- saving rejects to file libpam/pam_google_authenticator.c.rej

Any ideas?

Contributor

ThomasHabets commented Oct 10, 2014

Comment #49 originally posted by dhahn@trialpay.com on 2012-01-09T21:46:54.000Z:

I'm having trouble applying the patch - 3 out of 7 hunks fail. I pulled a clean copy of the trunk today and applied the patch version from comment # 23.

This is what I'm getting:

patching file libpam/pam_google_authenticator.c
Hunk # 1 succeeded at 79 (offset 17 lines).
Hunk # 2 FAILED at 646.
Hunk # 3 FAILED at 654.
Hunk # 4 succeeded at 764 with fuzz 1 (offset 101 lines).
Hunk # 5 succeeded at 1256 with fuzz 2 (offset 120 lines).
Hunk # 6 FAILED at 1269.
Hunk # 7 succeeded at 1418 with fuzz 1 (offset 243 lines).
3 out of 7 hunks FAILED -- saving rejects to file libpam/pam_google_authenticator.c.rej

Any ideas?

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #50 originally posted by markus@google.com on 2012-01-09T22:08:28.000Z:

That's not altogether surprising.

The issue has been closed and marked as "fixed"; as such, the original patch is obsolete and clearly can't be applied to the head of the tree any more.

If there are remaining issues that you believe are not currently addressed by the version of the code that you find in the source repository, then please create a new bug description detailing these issues.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #50 originally posted by markus@google.com on 2012-01-09T22:08:28.000Z:

That's not altogether surprising.

The issue has been closed and marked as "fixed"; as such, the original patch is obsolete and clearly can't be applied to the head of the tree any more.

If there are remaining issues that you believe are not currently addressed by the version of the code that you find in the source repository, then please create a new bug description detailing these issues.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #51 originally posted by fraser.scott on 2012-01-24T07:54:41.000Z:

I can (finally) confirm this as working for me.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #51 originally posted by fraser.scott on 2012-01-24T07:54:41.000Z:

I can (finally) confirm this as working for me.

@ThomasHabets

This comment has been minimized.

Show comment
Hide comment
@ThomasHabets

ThomasHabets Oct 10, 2014

Contributor

Comment #52 originally posted by ayella2u on 2013-07-10T05:23:52.000Z:

Anyone has done before!! to config GG with ldap authen

Please advise me.

-Mike

Contributor

ThomasHabets commented Oct 10, 2014

Comment #52 originally posted by ayella2u on 2013-07-10T05:23:52.000Z:

Anyone has done before!! to config GG with ldap authen

Please advise me.

-Mike

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment