[Home]

Summary:ASTERISK-04739: [patch] option "a" of application app_authenticate stop to work with the MD5 implementation
Reporter:Fernando Romo (el_pop)Labels:
Date Opened:2005-07-30 18:34:57Date Closed:2008-01-15 15:45:07.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_auth.diff
( 1) app_auth-12.diff
( 2) app_auth-12-1.diff
( 3) app_auth-current.diff
( 4) authenticate.diff
( 5) authenticate.patch
Description:I update the app_authencticate.c to the last CVS HEAD version (the file has the 1.10 version) and found a new feature implementing the new option "m" for MD5 hash searchs.

I found the "a" option stop to work, and the "m" option don take the MD5 hash properly.


****** ADDITIONAL INFORMATION ******

For example:

exten => _04455XXXXXXXX,1,Authenticate(/etc/asterisk/auth.conf|m)
exten => _04455XXXXXXXX,2,Dial(${TRUNK}/${EXTEN})
exten => _04455XXXXXXXX,3,Congestion

I generate the MD5 hash with a perl script an test with a online MD5 generator (http://md5-hash-online.waraxe.us/) and put the following in the auth.conf (sample file)

  9006:cdf66a6a7a04d87d865335701790c3e3
  9007:b9d487a30398d42ecff55c228ed5652b

i invert values and don't work, i back to the traditional schema with "a" option and found the option stop to work. if i put

exten => _04455XXXXXXXX,1,Authenticate(4568|a)
exten => _04455XXXXXXXX,2,Dial(${TRUNK}/${EXTEN})
exten => _04455XXXXXXXX,3,Congestion

works fine, reading the code i see the problem associate with the option an file path search (i attach athenticate.diff file comparing v 1.9 and 1.10)

For workaround purposes i back to version 1.9 of app_authenticate.c and recompile and works in the traditional behaivor.

I found the feature comment in the bug  0004764 (http://bugs.digium.com/view.php?id=4764), i don found any info in Wiki, User o Developers List.

Maybe the MD5 hash rutine is diferent to the MD5 hash standard (RFC 1321), but i think is a only logic problem with the options and path file processing.

Maybe is not a bug, but is a lack of documentation.
Comments:By: nick (nick) 2005-07-30 18:37:22

(Read first, _then_ post...)


By: Fernando Romo (el_pop) 2005-07-30 18:47:31

Is not a patch right now, is only a diff file to locate the posible error
By: Fernando Romo (el_pop) 2005-07-30 19:08:19

Off Topic: Is possible to digital sign The disclaimer file? I mean use a PGP/GPG fingerprint or need the "hard copy" of this document.
By: Daniel Hope (daniel) 2005-08-05 15:06:40

Got bit by this today - had a look at the source code. Looks like whoever implemented the MD5 stuff removed the code that cuts the newline character off the end of the line read in from the password file before comparing it to the password the user entered.

It's a one-liner to fix the plaintext auth file - dont know if it fixes the MD5 stuff as I dont use that so cant test it.
By: Mark Spencer (markster) 2005-08-05 17:42:12

Try latest CVS head...  I'm not sure I got it entirely fixed, but this should be an improvement.
By: Sergey Basmanov (sb) 2005-08-07 12:53:49

I'm very sorry.
When I posted code with md5 stuff, I've missed (for unknown to me reasons) this line:
buf[strlen(buf) - 1] = '\0';
Whithout this line, plain text & md5 stuff completely broken.
By: Mark Spencer (markster) 2005-08-07 19:12:25

That should already be in CVS head, can you confirm that it works entirely?
By: Fernando Romo (el_pop) 2005-08-07 21:15:32

The last CVS version 1.12 work in MD5 check but not put the account code properly, test with "m" alone, if i put "ma" put the password, not the account code

umm, where is the account-code loaded?

In the app_authenticate.c in line 130 only load the md5 part with

md5secret = strchr(buf, ':');

but, the info of the account code is not load in any variable, when set the account-code, put "buf" instead and in this moment is a empty string.


By: Sergey Basmanov (sb) 2005-08-08 02:58:44

There is something wrong in 1.12.
Attached diff against 1.11 with newline stripping.
Please use this patch to make plaintext & md5 stuff work.
By: Sergey Basmanov (sb) 2005-08-08 03:18:21

Added patch against 1.12
I've been tested it on my system. It's works with 'ma' and 'a'.
By: SIP:ian@txrx.ca (weezey) 2005-08-10 10:30:41

Last patch works perfectly, except for one thing...  If you have Authenticate(/filename|am) and your file only contains a password (no MD5 hash) then it crashes asterisk completely.

#0  0x4207940a in strcmp () from /lib/tls/libc.so.6
#1  0x40a406e6 in auth_exec (chan=0x808a14f, data=0x40e457dc) at app_authenticate.c:153

app_authenticate.c:
151:                if (!ast_strlen_zero(buf)) {
152:                    if (strchr(opts, 'm')) {
153:                       if (!strcmp(md5passwd, md5secret))
154:                           break;
155:                       } else {
156:                           if (!strcmp(passwd, buf))
157:                              break;
158:                       }
159:                    }


By: Sergey Basmanov (sb) 2005-08-11 03:30:21

Please, check if this patch resolves crash problem.
By: SIP:ian@txrx.ca (weezey) 2005-08-11 06:01:06

Works perfectly now.  Thanks.
By: Sergey Basmanov (sb) 2005-08-16 01:01:48

This needs to be applied to cvs.
By: Michael Jerris (mikej) 2005-08-22 21:55:24

sb-  can you please confirm that you have a disclaimer on file.
By: Sergey Basmanov (sb) 2005-08-23 00:42:41

This issue related to http://bugs.digium.com/view.php?id=4764
Yes, I have disclaimer on file.
By: Kevin P. Fleming (kpfleming) 2005-08-23 11:20:35

Committed to CVS HEAD, thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:45:07.000-0600

Repository: asterisk
Revision: 6380

U   trunk/apps/app_authenticate.c

------------------------------------------------------------------------
r6380 | kpfleming | 2008-01-15 15:45:07 -0600 (Tue, 15 Jan 2008) | 2 lines

fix buglets in new authentication code (issue ASTERISK-4739)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=6380