It would be good to have a possibility to enter "empty" passwords for authentication in pppd. Now the PAP/CHAP secrets are regenerated, only if both username and password are enterred. My simple patch reduces this requirement to username only. Verified with my provider who asks for CHAP and without chap-secrets there is a negative response from my pppd. *** Without my patch: pppd[25098]: rcvd [LCP ConfReq id=0x1 <auth chap MD5> <accomp> <pcomp> <asyncmap 0x0> <magic 0xbed58806>] pppd[25098]: sent [LCP ConfReq id=0x1 <asyncmap 0x0> <magic 0x504bb20c> <pcomp> <accomp>] pppd[25098]: No auth is possible pppd[25098]: sent [LCP ConfRej id=0x1 <auth chap MD5>] -> CHAP authentication was rejected *** With my patch: pppd[27533]: rcvd [LCP ConfReq id=0x1 <auth chap MD5> <accomp> <pcomp> <asyncmap 0x0> <magic 0x5f18471e>] pppd[27533]: sent [LCP ConfReq id=0x1 <asyncmap 0x0> <magic 0xbf097802> <pcomp> <accomp>] ... pppd[27533]: rcvd [CHAP Challenge id=0x0 <5995345897d8e2b31cac98dd257b92f63355b12f417c193cc2ab8c775c2a85d4679314882461>, name = "Kermit"] pppd[27533]: sent [CHAP Response id=0x0 <6c342c2d1731523af0ecfba44452968e>, name = "internet"] pppd[27533]: rcvd [CHAP Success id=0x0 "Congratulations!"] pppd[27533]: CHAP authentication succeeded: Congratulations! pppd[27533]: CHAP authentication succeeded -> CHAP succeeded, even with the empty password. The PAP/CHAP secrets files look like this: #client server secret IP addresses "internet" ppp0 ""
Created attachment 87488 [details, diff] Generating PAP/CHAP secrets independently on password
Hmm... Some users prefer to set their passwords manually in *.secrets file. This is accomplished by not setting any password in conf.d/net file. Workaround: - you should modify *.secrets files by hand. Reasons: - your patch break existing setups - using pap/chap without a password is pointless; your ISP should know better.
Currently it doesn't cause that I'm not able to connect. The auth is tried twice and then the startup process continues normally. Well, there can be non-conflicting solution: variable nopassword_{ifvar}="yes", if you really want to have empty password for certain interface, just to make the login process happy :). The variable password_{ifvar} is still checked for the correct password. Another solution would be to have it in pppd_{ifvar} as an option and filter this option out for the pppd command. I'm thinking about "forcesecrets" option, but it could be any better one - suggest some ;) Again, no need to understand or change secrets file for the user, it will be handled by scripts automatically. And I think this is what was planned. See the patches.
Created attachment 87546 [details, diff] Option "forcesecrets" This adds a new option to pppd_{ifvar} called "forcesecrets". It will force the script to generate the line in *-secrets files. I think this is better solution than nopassword_{ifvar}.
Created attachment 87547 [details, diff] Variable nopassword_{ifvar}="yes" This adds a check for the value of the new variable nopassword_{ifvar}. If it contains "yes", one line in *-secrets files will be generated also for no (empty) passwords.
comment #4 : a categoric NO. pppd_${ifname} contains only pppd options. comment #5 : I am inclined to refuse this as well (since is illogical to request chap authentication when you don't have a password), but if Roy want to apply this patch, I wouldn't mind.
OK, I'm not going to apply this one as our current behaviour I think is correct. We should only update secret files if both username and password are supplied. After all, the conf.d/net file maybe world readable and the secrets file not, so just supplying the username means we can still have this :) Re-open if you disagree.
If there is no other way, at lease inform the user about this in net.example. I just want to prevent the confusion that I had when I entered the user name, password="" and no secrets were created (it worked some time ago). But the note should sound like this: If you want an empty password in your chap-secrets or pap-secrets, enter the line yourself. We will not do so. If you think about the note above, you also start thinking - why to do not allow empty passwords? I'm not my ISP and if he is happy with empty password, it is his problem, not mine (or should not be my problem). Just the last try, if you really do not like the idea of empty passwords, just close this bug, add some user-friendly note into net.example and I promise that I will never reopen it again ;)
(In reply to comment #8) > If there is no other way, at lease inform the user about this in net.example. I > just want to prevent the confusion that I had when I entered the user name, > password="" and no secrets were created (it worked some time ago). > > But the note should sound like this: If you want an empty password in your > chap-secrets or pap-secrets, enter the line yourself. We will not do so. We already do :p From current net.example # PPP requires at least a username. You can optionally set a password here too # If you don't, then it will use the password specified in /etc/ppp/*-secrets # against the specified username > If you think about the note above, you also start thinking - why to do not > allow empty passwords? I'm not my ISP and if he is happy with empty password, > it is his problem, not mine (or should not be my problem). Well pppd does support empty passwords - we just don't make entries in the secrets file with empty passwords. > > Just the last try, if you really do not like the idea of empty passwords, just > close this bug, add some user-friendly note into net.example and I promise that > I will never reopen it again ;) At most I would consider a patch that adds the username with a blank password if the username does not exist in the secrets file. Would that make a good compromise?
(In reply to comment #9) > At most I would consider a patch that adds the username with a blank password > if the username does not exist in the secrets file. Would that make a good > compromise? Only if no other {chap,pap}-secrets entries have the same username! It is important not to write such line in case one of those files already have an entry with the same username and remotename is either equal with ${iface} or is an *.
Created attachment 88072 [details, diff] Intelligent empty password updating I like the idea from comment #10, so here is the updated patch. The password is updated (for the particular server) only in those situations: 1) Password is non-empty 2) Password is empty AND there is no password set (either on remotename or on *) In both cases the user is informed (if in VERBOSE mode) about the result.
Created attachment 88077 [details, diff] Simple patch Why such a complex patch? Surely this does what is required.
(In reply to comment #12) > Why such a complex patch? Surely this does what is required. Because your patch doesn't count line "username" * "password" as mentioned in comment #10.
I've just looked in pppd source files and learn that it doesn't stop at the first match. For example, suppose you have a ppp0 link with following 2 entries in your pap-secrets: user * pass1 user ppp0 pass2 In this case, the second entry is used because it doesn't contain the wildcard. As if it isn't complicated enough, * could also be used in username.
(In reply to comment #14) > I've just looked in pppd source files and learn that it doesn't stop at the > first match. For example, suppose you have a ppp0 link with following 2 entries > in your pap-secrets: > user * pass1 > user ppp0 pass2 I assume that the following match as well user * pass user p* pass2 user pp* pass3 user ppp* pass4 user ppp0 pass5 How about this? user [a-z][n-q][Pp][0-3] pass6 If that works then we either 1) Drop this entire idea and just don't use blank passwords - easy! 2) Try and match the regex against our interface to see if we can add a new emtpy one or not.
(In reply to comment #15) > (In reply to comment #14) > > I've just looked in pppd source files and learn that it doesn't stop at the > > first match. For example, suppose you have a ppp0 link with following 2 entries > > in your pap-secrets: > > user * pass1 > > user ppp0 pass2 > > I assume that the following match as well > user * pass > user p* pass2 > user pp* pass3 > user ppp* pass4 > user ppp0 pass5 > > How about this? > > user [a-z][n-q][Pp][0-3] pass6 > > If that works then we either > 1) Drop this entire idea and just don't use blank passwords - easy! > 2) Try and match the regex against our interface to see if we can add a new > emtpy one or not. > From the MAN page of pppd (version 2.4.3-r15) the server name can contain either "*" or a name. No more complicated situations like brackets. From which version of pppd there is pattern matching of the server name?
The wildcard cannot be used like that; you either set the field to * or you set it to the exact value. Basically we have 4 priorities: 0) exact name and exact remotename 1) exact username and remotename = * 2) username = * and exact remotename 3) username = * and remotename = * Entries are evaluated in order; a match cannot replace the previous match unless the new_priority < old_priority. I think it would be best to set the password *only* if password_${iface} is defined (even if it is empty). That way we keep compatibility with previous setups.
(In reply to comment #17) > The wildcard cannot be used like that; you either set the field to * or you set > it to the exact value. > > Basically we have 4 priorities: > 0) exact name and exact remotename > 1) exact username and remotename = * > 2) username = * and exact remotename > 3) username = * and remotename = * > Entries are evaluated in order; a match cannot replace the previous match > unless the new_priority < old_priority. > > I think it would be best to set the password *only* if password_${iface} is > defined (even if it is empty). That way we keep compatibility with previous > setups. > I will try to play with it a little bit in the evening. I found in bash MAN page, that with `declare -p variable` it should be possible to check, if the variable is defined or not. However, I would like to do this (according to the described priority rules): * If the password_${ifvar} is set and there is no match, add new line of the 0) form * If the password_${ifvar} is set and there is a match, add/change the line of the 0) form
Created attachment 88099 [details, diff] Allow blank password if set Something like this?
(In reply to comment #19) > Something like this? Yeah, that's the one :) It should be tested though.
(In reply to comment #19) > Something like this? > That's incredibly cool :-) But please tell me, where is it documented? I cannot find it in bash MAN page.
(In reply to comment #21) > (In reply to comment #19) > > Something like this? > > > That's incredibly cool :-) But please tell me, where is it documented? I cannot > find it in bash MAN page. > It's not in the bash man page - which is full of a few lies itself. Instead I use the BASH ABS as a very good source of reference http://www.tldp.org/LDP/abs/html/parameter-substitution.html
(In reply to comment #22) > http://www.tldp.org/LDP/abs/html/parameter-substitution.html Thanks :-) I will try to do some woodoo with checking the current password to match the prioritization in comment #17. Hopefully it will be as simple as ${!password-x} :-). Results tomorrow.
Well, almost ready with the little-bit-hard-core (TM) sed script (not so hardcore as in bug #134254), but I now have a better idea how to do it, so no results today. I don't know if I will have time during weekend, but definitely on Monday, so look forward to see the results not later than on Tuesday :-)
(In reply to comment #24) > Well, almost ready with the little-bit-hard-core (TM) sed script (not so > hardcore as in bug #134254), but I now have a better idea how to do it, so no > results today. I don't know if I will have time during weekend, but definitely > on Monday, so look forward to see the results not later than on Tuesday :-) What sed script is this? Does my patch not satisfy everyone?
(In reply to comment #25) > What sed script is this? Does my patch not satisfy everyone? Your patch is perfectly ok. I was just thinking about improving the current password recognition (called "old_password") to prevent unnecessary changes to secrets file (if it was edited manually for example). However, I thought about the solution and no hardcore sed scripts are necessary.
Fixed in baselayout-1.12.1
Created attachment 88440 [details] Intelligent password updating This file intelligently checks the current password and makes an update, if the password have been changed. It has all the logic with priorities. It is ready to be merged with pppd.sh. It works with escaped characters (including new-lines '\n'), but not with multiline strings in secrets. I personally think that it is necessary only if the user made more complicated changes (that are not handled by current implementation). Bugs in the current pppd.sh (but fixed in pppd-part.sh) are: * Not able to handle comments on the same line as the secret * Not able to handle single-quotes ' * Not able to handle \s, that is interpreted by pppd as space (maybe has other escape problems also) If you think that this is too complicated for this job, just throw the file away. I'm happy with the solution from comment #19, so if you decide to apply only that patch, I think this bug could be closed then.
(In reply to comment #28) > * Not able to handle comments on the same line as the secret > * Not able to handle single-quotes ' > * Not able to handle \s, that is interpreted by pppd as space (maybe has other > escape problems also) I think it can if you escape things properly - iirc mrness did a thorough job testing this. > If you think that this is too complicated for this job, just throw the file > away. I'm happy with the solution from comment #19, so if you decide to apply > only that patch, I think this bug could be closed then. I think it is too complicated, and the original issue is now fixed. However, your patch can stay here as is in-case any poor soul wants it. Thanks for your input on this :)
(In reply to comment #29) > (In reply to comment #28) > > * Not able to handle comments on the same line as the secret > > * Not able to handle single-quotes ' > > * Not able to handle \s, that is interpreted by pppd as space (maybe has other > > escape problems also) > > I think it can if you escape things properly - iirc mrness did a thorough job > testing this. This works with lines in the format username remotename "password" Where both user name and remote name can be enclosed by double quotes. This completely ignores the single quotes 'username' 'remotename' 'password' The problem can arise, when you have a comment on the same line username remotename "password" # comment There are also some interpretation problems: "pass\sword" pass\ word 'pass\ word' pass\sword All the words are the same, but not for pppd.sh. What has been mentioned already is ignoring lines like username * "password" Current implementation is limited, but can be used for most users I think - if the user doesn't modify the file himself. *** And a bug at the end: I tried a password password_ppp0="pass\" word" And it was added as "internet" ppp0 "pass" word" When I started the ppp0 second time, it was edited to (correct) "internet" ppp0 "pass\" word"
Created attachment 88559 [details, diff] pppd.sh.patch This patch solves the bug exposed in comment #30. I believe we should not trouble ourselves with cases where users edit *-secrets by hand for various reasons: a) Entries with * are unsafe. We cannot automatically update/add such entries, knowing we could change authentication info on some other connection. Our entry key is and will be (username, iface), no * allowed! b) Escapes like \s within passwords are indeed treated different than ' ' even though it is all the same to pppd, but only the first time :). The same goes for simple quoted passwords. The only real issue is that our (username, iface) key may be duplicated, which means we could add an entry that pppd don't consider it. However, it is more like a theoretical problem (face it, whoever set the password in /etc/conf.d/net, don't edit *-secrets files unless it doesn't have any other way).
Correction : pppd.sh doesn't care at all about simple quoted fields. Roy, maybe we should replace \" with ['\"] in regex_filter and all sed command lines?
So where are we wish this then? Could we just use the code on comment #28 to replace what we have if that works with all scenarios?
Oldrich, can you propose a new patch? Remember, we don't want to take into account entries with *, just want to properly detect&update normal entries, with names&remotenames optionally quoted/double-quoted. Quoting _must_ be right - if it is started with ", it must be closed by ". Comments must be ignored.
(In reply to comment #34) > Oldrich, can you propose a new patch? > Remember, we don't want to take into account entries with *, just want to > properly detect&update normal entries, with names&remotenames optionally > quoted/double-quoted. > Quoting _must_ be right - if it is started with ", it must be closed by ". > Comments must be ignored. Ok, I will update the patch to accept (and recognize) priority 0 entries only, with and without single/double quotes. Do you want any backslash magic, or a patch as simple as possible?
We need at least \", \\ and \' recognized as valid escape sequences (to see in pppd source if \" is interpreted as escape even when it isn't within a double-quoted string). I prefer to read the old password using one sed command and update or add the new password using another sed command (just as is done now).
(In reply to comment #36) > We need at least \", \\ and \' recognized as valid escape sequences (to see in > pppd source if \" is interpreted as escape even when it isn't within a > double-quoted string). I prefer to read the old password using one sed command > and update or add the new password using another sed command (just as is done > now). > I on the other hand would rather see all the existing regex stuff stripped out as it's 1) hard to read and understand 2) messes up syntax highlighting on all my editors (minor niggle, but hey) If that involves more calls to sed, grep, awk, tool of choice then I'd much rather have that instead of insane regex :)
(In reply to comment #37) > If that involves more calls to sed, grep, awk, tool of choice then I'd much > rather have that instead of insane regex :) The password extraction can be done in one sed line, but it would be a little bit woodoo, it has to be done on a character-by-character basis to create a complete sed script. It depends on how intelligent the script should be. If you say - I want to have everything - then ok, that is almost my script (which cannot handle multi-line fields only). My motto: I can do impossible now, but miracles in 3 days :-)
Well, we could drop these fancy sed commands and use passwordfd.so plugin instead, but I remember (vaguely) that my attempts to use it were unsuccessfull. How about focusing on this variant instead?
(In reply to comment #39) > Well, we could drop these fancy sed commands and use passwordfd.so plugin > instead, but I remember (vaguely) that my attempts to use it were > unsuccessfull. How about focusing on this variant instead? Looks good. I will also look at it.
Created attachment 89024 [details, diff] Patch using passwordfd plugin for sending password This patch introduces support for passwordfd plugin. The only problem was to open the file descriptor of our choice, I used FIFO file for it. The password is output to the FIFO and then the input from FIFO is redirected to the correct file descriptor of pppd. The file descriptor number is passed to pppd in option "passwordfd". Enjoy :-)
I hope there is a solution using just echo -n, without any password named pipe/file.
(In reply to comment #42) > I hope there is a solution using just echo -n, without any password named > pipe/file. > Should be able to use stdin like so echo -n "${password}" | s-s-d pppd ${opts} plugin passwordfd.so passwordfd 0
(In reply to comment #43) > Should be able to use stdin like so > > echo -n "${password}" | s-s-d pppd ${opts} plugin passwordfd.so passwordfd 0 Not from the source files. If it daemonizes (without option nodetach/updetach), file descriptors 0, 1 and 2 are redirected to /dev/null. You can be sure only about file descriptors greater than 2 - see function detach() in pppd/main.c.
(In reply to comment #42) > I hope there is a solution using just echo -n, without any password named > pipe/file. I spend two hours with playing with exec and redirecting, always stopped with "bad file descriptor" error. I mainly didn't know what I was doing, so you can try it as well :-)
(In reply to comment #44) > (In reply to comment #43) > > Should be able to use stdin like so > > > > echo -n "${password}" | s-s-d pppd ${opts} plugin passwordfd.so passwordfd 0 > > Not from the source files. If it daemonizes (without option nodetach/updetach), > file descriptors 0, 1 and 2 are redirected to /dev/null. You can be sure only > about file descriptors greater than 2 - see function detach() in pppd/main.c. > Can we patch it so that if the passwordfd plugin is used AND the fd is 0 not to redirect it to /dev/null? If not, why not just patch pppd not to redirect fd 0? After all, we are in control of the fd ....
(In reply to comment #46) > Can we patch it so that if the passwordfd plugin is used AND the fd is 0 not to > redirect it to /dev/null? > > If not, why not just patch pppd not to redirect fd 0? After all, we are in > control of the fd .... I do not think that patching of pppd is a good solution. We need one particular change only when our pppd.sh starts pppd, not world-wide change. Anyway, we can use perl/python script (wrapper) to do the right thing - read a password from stdin first, create a pipe, pass the parameter to pppd and let it do it's work. Or we can make our own executable to do it - example is in README.pwfd.
I confirm we cannot use stdin for this purpose. If I remember correctly, we could use something like: 3< $(echo -n "${password}")
(In reply to comment #48) > If I remember correctly, we could use something like: > 3< $(echo -n "${password}") Maybe we can try 3< <(echo -n "${!pasword}") - again it uses one intermediate FIFO, but now it is created by BASH itself, not by the script.
Created attachment 89030 [details, diff] Patch using passwordfd plugin without manual FIFO Now it uses ability of Bash to handle FIFO creation. Please test it as I do not have PPP0 available now.
Created attachment 89032 [details, diff] better passwordfd patch OK, I've tested this and it sees to work just fine. We work out the next free fd to use, incase other scripts want todo similar tricks. I've also cut down on the number of variables used - no need for them really. Unless anyone says otherwise this will be committed later today.
Please give me a chance to test it. I will do it tonight and post here the results.
Created attachment 89034 [details, diff] passwordfd plugin in pppd.sh, small cleanup Small fix for comment #51 - reintroduced variable "i" and removed variables "escaped_password" and "password_redir".
I've tested (on PPPoE) the patch submitted in comment #53. Works like a charm. Good job everyone!
Comitted to our svn repo - will be in baselayout-1.12.2 Thanks for this guys - we've made an ugly but of code much much much nicer :)
Created attachment 89229 [details, diff] net.example.patch Please apply this against net.example. Feel free to tackle with words, but keep the sense of the new comments.
(In reply to comment #56) > Please apply this against net.example. Feel free to tackle with words, but keep > the sense of the new comments. > I've applied the second part (updetach), but not the first part as I feel the NOTE section you added just duplicates the paragraph above it.
I think it should be noted explicitly that we do not use {chap,pap}-secrets files.
(In reply to comment #58) > I think it should be noted explicitly that we do not use {chap,pap}-secrets > files. We don't, but if password_ppp0 is not even set then pppd will use those files to work out a possible password.
(In reply to comment #59) > We don't, but if password_ppp0 is not even set then pppd will use those files > to work out a possible password. Sorry, I'm still sleeping or blind or... The note already in net.example is good.
The current functionality requires an *unset* password_${iface} in order to use other ways to authenticate itself (i.e. secret files). I find my comment highly useful (although the exact phrase may be corrected/improved). Why would you explain to users how to set an empty password? Isn't it obvious what they should set in this case?
(In reply to comment #61) > The current functionality requires an *unset* password_${iface} in order to use > other ways to authenticate itself (i.e. secret files). I find my comment highly > useful (although the exact phrase may be corrected/improved). > > Why would you explain to users how to set an empty password? Isn't it obvious > what they should set in this case? Here's the current text # PPP requires at least a username. You can optionally set a password here too # If you don't, then it will use the password specified in /etc/ppp/*-secrets # against the specified username #username_ppp0='user' #password_ppp0='password' # NOTE: You can set a blank password like so #password_ppp0= I think that's clear enough
If you are waiting for my reply, there you go. Obviously you are right. I should have looked over all comments.
Fixed in baselayout-1.12.2
