Re: #10481: Hardening LyX against potential misuse
On Sun, Apr 02, 2017 at 02:57:58PM +0200, Tommaso Cucinotta wrote: > On 01/04/2017 07:12, Scott Kostyshak wrote: > > Tommaso, any thoughts? Even if you don't have time to implement > > something yourself, your opinion would be useful. > > Hi, > > AFAICR, boundary checks on existing fields of that form, like the > converter file cache expiry time, are all independent/orthogonal of > thehardening patch. I assume it's all good to check the values > and don't allow to save if the inserted values are wrong. > > Actually, I just confirmed the weird behavior in setting a negative > value and getting back a non-sense positive one, so I just pushed > a fix for this [1]. > > Thanks, > > T. Thanks, Tommaso. Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
On 01/04/2017 07:12, Scott Kostyshak wrote: Tommaso, any thoughts? Even if you don't have time to implement something yourself, your opinion would be useful. Hi, AFAICR, boundary checks on existing fields of that form, like the converter file cache expiry time, are all independent/orthogonal of thehardening patch. I assume it's all good to check the values and don't allow to save if the inserted values are wrong. Actually, I just confirmed the weird behavior in setting a negative value and getting back a non-sense positive one, so I just pushed a fix for this [1]. Thanks, T. [1] commit 621ccc5e Author: Tommaso Cucinotta Date: Sun Apr 2 14:55:33 2017 +0200 Restrict file cache expiry time to positive values. diff --git a/src/frontends/qt4/GuiPrefs.cpp b/src/frontends/qt4/GuiPrefs.cpp index 1d8bb201..69877e38 100644 --- a/src/frontends/qt4/GuiPrefs.cpp +++ b/src/frontends/qt4/GuiPrefs.cpp @@ -72,6 +72,7 @@ #include #include #include +#include using namespace Ui; @@ -1683,7 +1684,7 @@ PrefConverters::PrefConverters(GuiPreferences * form) converterED->setValidator(new NoNewLineValidator(converterED)); converterFlagED->setValidator(new NoNewLineValidator(converterFlagED)); - maxAgeLE->setValidator(new QDoubleValidator(maxAgeLE)); + maxAgeLE->setValidator(new QDoubleValidator(0, HUGE_VAL, 6, maxAgeLE)); //converterDefGB->setFocusProxy(convertersLW); }
Re: #10481: Hardening LyX against potential misuse
On Sat, Jan 21, 2017 at 11:35:37PM -0500, Scott Kostyshak wrote: > On Sun, Dec 04, 2016 at 12:52:13PM -0500, Scott Kostyshak wrote: > > On Sun, Dec 04, 2016 at 06:06:57PM +0100, Tommaso Cucinotta wrote: > > > On 28/11/2016 00:42, Tommaso Cucinotta wrote: > > > > On 27/11/2016 13:52, Guillaume Munch wrote: > > > > > * Converters>Security is located below the converter configuration, > > > > > which leads to think that they are converter properties instead of > > > > > global settings. What about placing it above the converter list? > > > > > > > > Same problem with the Converter Cache option already, isn't it? > > > > > > > > Let me propose the attached fix for both at once. > > > > > > just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm > > > it shows up as intended ... > > > > I did not follow this conversation closely, but here are my comments: > > > > Keep in mind that most LyX users do not know what a file cache is. Can > > you describe exactly what's being cached in the tooltip? Maybe you could > > use the word "remembered" instead of "cached" in the tooltip because it > > is a more user-friendly word? > > > > A tooltip for maximum age would also be useful. e.g. "after this amount > > of days, ..." > > > > I wonder if the cache should expire after e.g. 60 days, or after 60 days > > since the last use of the file. For example, I (think I) would want it > > to be 60 days since the last use of the file, but I don't know if that's > > what would be best for the general user. > > > > I think you want some kind of validator for the maximum age. I put in a > > negative number and after saving it turned it into 49651.3. I'm guessing > > you want to restrict to only non-negative integers? Look at other places > > we use validators for similar text boxes. > > Also what would happen if I put in 0? Does that effectively mean the > > converter file cache is disabled? Or is it a special value that means > > "never expire"? > > > > Thanks for your work on this! > > Tommaso, what are your thoughts on the above? Tommaso, any thoughts? Even if you don't have time to implement something yourself, your opinion would be useful. Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
On 22/01/2017 05:13, Scott Kostyshak wrote: But I still think that the dialog should be an error, not a warning. There is 0% chance that the export was correct. To me this suggests an error. Tommaso, what are your thoughts? agree, see [eaa3ddaf/lyxgit]. Also, highlighted that it's a preference settings (not a document settings) that forbids using the converter. Thanks, T.
Re: #10481: Hardening LyX against potential misuse
On Sun, Dec 04, 2016 at 12:52:13PM -0500, Scott Kostyshak wrote: > On Sun, Dec 04, 2016 at 06:06:57PM +0100, Tommaso Cucinotta wrote: > > On 28/11/2016 00:42, Tommaso Cucinotta wrote: > > > On 27/11/2016 13:52, Guillaume Munch wrote: > > > > * Converters>Security is located below the converter configuration, > > > > which leads to think that they are converter properties instead of > > > > global settings. What about placing it above the converter list? > > > > > > Same problem with the Converter Cache option already, isn't it? > > > > > > Let me propose the attached fix for both at once. > > > > just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm it > > shows up as intended ... > > I did not follow this conversation closely, but here are my comments: > > Keep in mind that most LyX users do not know what a file cache is. Can > you describe exactly what's being cached in the tooltip? Maybe you could > use the word "remembered" instead of "cached" in the tooltip because it > is a more user-friendly word? > > A tooltip for maximum age would also be useful. e.g. "after this amount > of days, ..." > > I wonder if the cache should expire after e.g. 60 days, or after 60 days > since the last use of the file. For example, I (think I) would want it > to be 60 days since the last use of the file, but I don't know if that's > what would be best for the general user. > > I think you want some kind of validator for the maximum age. I put in a > negative number and after saving it turned it into 49651.3. I'm guessing > you want to restrict to only non-negative integers? Look at other places > we use validators for similar text boxes. > Also what would happen if I put in 0? Does that effectively mean the > converter file cache is disabled? Or is it a special value that means > "never expire"? > > Thanks for your work on this! Tommaso, what are your thoughts on the above? Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
On Sun, Nov 27, 2016 at 01:16:13PM +0100, Guillaume Munch wrote: > > Also regarding the "first" warning (the "Launch of external converter is > > forbidden" warning, which is the one I'm referring to above), although I > > have not closely followed the conversation I have the following comment: > > > > I think it should be converted to an error and/or I don't think there > > should be a checkbox "Do not show this warning again". > > I agree. Note that this also fixes the the line breaking issue since > QMessageBox is used instead of GuiToggleWarningDialog in that case. (But it > would still be good to fix GuiToggleWarningDialog.) On current master, I do not see the checkbox anymore of "Do not show this warning again", which I think is good. But I still think that the dialog should be an error, not a warning. There is 0% chance that the export was correct. To me this suggests an error. Tommaso, what are your thoughts? Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
On 05/01/2017 20:15, Pavel Sanda wrote: Helge Hafting wrote: According to http://stackoverflow.com/questions/10937597/security-risks-of-gnuplot-web-interface , gnuplot can be built "safer" by disabling pipe operations. That leaves the unsafe commands "shell", "system" and "!", but a simple shellscript using "grep" can check for these 3 commands and refuse to run gnuplot on a file that contains any of them. Is that safe enough? Opening doors for hacks like s\ystem or whatever weird might be possible with gnuplot syntax. Homebrew solutions are at the end just waiting for someone who has enough twisted mind to see things you don't catch. I can understand that devs might not want to create a "safe gnuplot" because pipes and "shell/system" are useful commands. indeed, those are very handy & helpful constructs in everyday use of gnuplot Apparently they are also against having --safe-mode switch, even though it wouldn't impact those not using this switch. As I understand it from these 13/10 years old threads http://comp.graphics.apps.gnuplot.narkive.com/89H4smZk/does-gnuplot-have-a-safe-mode http://comp.graphics.apps.gnuplot.narkive.com/uho0K7uC/embedding-gnuplot-safe-mode the problem is mainly that it's not clear what exactly to disable, under such a safe-mode operation, e.g., starting from the quite standard "set output " that allows to overwrite any file in the user's home directory. For example, the simple "System call and file readability is removed from original gnuplot." from https://github.com/hletrd/gnuplot-safemode looks like largely insufficient, but couldn't look through that code yet (perhaps the 1-liner is largely imprecise & old). --safe-mode would be solution I would consider 'safe' and as I infer from your mail gnuplots devs did not change their opinion about it... perhaps a properly worked out safe/restricted-mode patch might have a chance of being seriouslyconsidered by gnuplot devels, as it's been asked by many mostly in the context of exposinggnuplot via web interfaces/php & the likes. T.
Re: #10481: Hardening LyX against potential misuse
Helge Hafting wrote: > According to > http://stackoverflow.com/questions/10937597/security-risks-of-gnuplot-web-interface > > , > gnuplot can be built "safer" by disabling pipe operations. That leaves the > unsafe commands "shell", "system" and "!", but a simple shellscript using > "grep" can check for these 3 commands and refuse to run gnuplot on a file > that contains any of them. Is that safe enough? Opening doors for hacks like s\ystem or whatever weird might be possible with gnuplot syntax. Homebrew solutions are at the end just waiting for someone who has enough twisted mind to see things you don't catch. > I can understand that devs might not want to create a "safe gnuplot" > because pipes and "shell/system" are useful commands. Apparently they are > also against having --safe-mode switch, even though it wouldn't impact > those not using this switch. --safe-mode would be solution I would consider 'safe' and as I infer from your mail gnuplots devs did not change their opinion about it... I just read that alpha go was smugled into online Go platforms and won 50 games in row over number of world champions, some stronger than Lee Sedol last year. I hope some 'unknown' developer appears in gnuplot list and let it pass. Alpha, do you hear me? Pavel
Re: #10481: Hardening LyX against potential misuse
Den 17. des. 2016 00:14, skrev Pavel Sanda: Helge Hafting wrote: Protection will not be achieved in most cases, because users are used to While I agree with what you write in general about security, I do not think this is how things were implemented, so in 'most cases' you are wrong. 1. Unless you do informed decision and go to the prefs and allow dangerous mode you will never be asked and nothing will ever run. This covers 99% of lyx users and usecases. Nice, if nearly nobody uses the unsafe features. 3. Chrooting is nice idea and practically hard to achieve across platforms. I see - unix only. Many years back when I checked gnuplot devs were against including 'safe mode' so the disable-write18-in-LaTeX alternative for gnuplot is not in our reach either. According to http://stackoverflow.com/questions/10937597/security-risks-of-gnuplot-web-interface , gnuplot can be built "safer" by disabling pipe operations. That leaves the unsafe commands "shell", "system" and "!", but a simple shellscript using "grep" can check for these 3 commands and refuse to run gnuplot on a file that contains any of them. Is that safe enough? I can understand that devs might not want to create a "safe gnuplot" because pipes and "shell/system" are useful commands. Apparently they are also against having --safe-mode switch, even though it wouldn't impact those not using this switch. There is a safe fork of gnuplot: https://github.com/hletrd/gnuplot-safemode Helge Hafting
Re: #10481: Hardening LyX against potential misuse
Den 15. des. 2016 21:13, skrev Tommaso Cucinotta: About chroot-ing, albeit seems doable to copy what a converter needs in the restricted root, eg, ldd gives you what dynlibs are needed, the problem stays with additional data the program might need, plus you need additional privileges to chroot(), overall making the chroot() way quite impractical [ we don't want to have any suid exe in lyx, do we :-) ? ] A SUID lyx might be dangerous indeed. If necessary, the way to do it is a small SUID utility that creates a sandbox. ( bind-mount readonly /bin, /lib, possibly /home and whatever else we need in the tmp directory, chroot, drop privileges & launch the converter.) Helge Hafting
Re: #10481: Hardening LyX against potential misuse
On Sun, Dec 18, 2016 at 12:06:02PM +0100, Kornel Benko wrote: > Am Sonntag, 18. Dezember 2016 um 05:39:05, schrieb Scott Kostyshak > > > On Mon, Nov 28, 2016 at 12:42:31AM +0100, Tommaso Cucinotta wrote: > > > > > what are further calls to converters? > > > > After 244de5d2, LyX crashes for me on "paste from LaTeX" and "paste from > > HTML". To test, copy some plain text (don't copy from inside LyX because > > then "paste from LaTeX" will still do a paste using LyX's internal > > clipboard), then go to Edit > Paste Special > Paste from LaTeX. > > > > Can others reproduce? > > > > Scott > > Confirmed. I think this was fixed at a95385ab. Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
Am Sonntag, 18. Dezember 2016 um 05:39:05, schrieb Scott Kostyshak > On Mon, Nov 28, 2016 at 12:42:31AM +0100, Tommaso Cucinotta wrote: > > > what are further calls to converters? > > After 244de5d2, LyX crashes for me on "paste from LaTeX" and "paste from > HTML". To test, copy some plain text (don't copy from inside LyX because > then "paste from LaTeX" will still do a paste using LyX's internal > clipboard), then go to Edit > Paste Special > Paste from LaTeX. > > Can others reproduce? > > Scott Confirmed. Kornel signature.asc Description: This is a digitally signed message part.
Re: #10481: Hardening LyX against potential misuse
On Mon, Nov 28, 2016 at 12:42:31AM +0100, Tommaso Cucinotta wrote: > what are further calls to converters? After 244de5d2, LyX crashes for me on "paste from LaTeX" and "paste from HTML". To test, copy some plain text (don't copy from inside LyX because then "paste from LaTeX" will still do a paste using LyX's internal clipboard), then go to Edit > Paste Special > Paste from LaTeX. Can others reproduce? Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
Helge Hafting wrote: > Protection will not be achieved in most cases, because users are used to While I agree with what you write in general about security, I do not think this is how things were implemented, so in 'most cases' you are wrong. 1. Unless you do informed decision and go to the prefs and allow dangerous mode you will never be asked and nothing will ever run. This covers 99% of lyx users and usecases. 2. If you are in special need for knitr/gnuplot you can allow it personally for yourself or ask your colleagues to do that as well for shared document. Here your concern applies and here we might differ - I think that from this point it's basically your responsibility to check what you are opening and do not suggest colleagues who don't understand what they are doing to use this feature. If people think that (2.) is still dangerous we might hide it even more so only hackers can use it, but personally I do not see need for it. 3. Chrooting is nice idea and practically hard to achieve across platforms. Many years back when I checked gnuplot devs were against including 'safe mode' so the disable-write18-in-LaTeX alternative for gnuplot is not in our reach either. Pavel
Re: #10481: Hardening LyX against potential misuse
Le 15/12/16 à 21:13, Tommaso Cucinotta a écrit : On 13/12/2016 11:25, Helge Hafting wrote: that's why I'm looking into AppArmor instead, which is essentially a Seems like a good thing - especially the ability to prevent networking. No network - no LyX-based virus at least. we need both, file-system confinement and no networking, otherwise e.g., an R script in a LyX doc might overwrite ~/.Rprofile, which might getexecuted next time the user runs R independently (outside of LyX, where it's not confined). On the other hand, allowing only to read ~/.Rprofile (without writing) seems useful and not harmful. So allow read-only of everything and read/write in the temp file. JMarc
Re: #10481: Hardening LyX against potential misuse
On 13/12/2016 11:25, Helge Hafting wrote: that's why I'm looking into AppArmor instead, which is essentially a Seems like a good thing - especially the ability to prevent networking. No network - no LyX-based virus at least. we need both, file-system confinement and no networking, otherwise e.g., an R script in a LyX doc might overwrite ~/.Rprofile, which might getexecuted next time the user runs R independently (outside of LyX, where it's not confined). On the other hand, allowing only to read ~/.Rprofile (without writing) seems useful and not harmful. About chroot-ing, albeit seems doable to copy what a converter needs in the restricted root, eg, ldd gives you what dynlibs are needed, the problem stays with additional data the program might need, plus you need additional privileges to chroot(), overall making the chroot() way quite impractical [ we don't want to have any suid exe in lyx, do we :-) ? ] T.
Re: #10481: Hardening LyX against potential misuse
Den 13. des. 2016 00:06, skrev Tommaso Cucinotta: On 12/12/2016 12:04, Helge Hafting wrote: In the general case, make a script (or utility program) that runs the dangerous converter in a chroot, where nothing dangerous can be done. No need for questions then. LyX already puts the document files in a temp directory so the cleanup after a latex run will be easier. chrooting before running a converter means the converter can't overwrite files outside the chroot, which helps quite a bit security-wise. unfortunately, chroot-ing also makes the system inaccessible/invisible, Yes. Necessary software must be available in a chroot, possibly in the form of "bin" and "lib" folders containing hardlinks to software the "dangerous" utility is allowed to use. Most software can be made available this way, since the user has no right to overwrite the binaries - and the binaries can't be used to overwrite random user files because of the chroot. that's why I'm looking into AppArmor instead, which is essentially a Seems like a good thing - especially the ability to prevent networking. No network - no LyX-based virus at least. Trashing files in a chroot is less of a problem. "chroot on steroids". However, AA is more difficult to get right, and it will work "out of the box" only on a limited set of distros. On the other hand, the immediate, always working across any OS and portable security mitigation (to possible threats/viruses starting to spread as lyx docs), is the one to show up a dialog and ask the user -- I know, it's reminding all of us of smth, but also please remember that it won't show up always, rather only if you're using a fewselected converters. As for batch conversions, we could have an option (even a LyX-wide option) saying --assume-yes or--assume-no or similar, that would actually prevent any question to be asked. If it really is only a few select converters that are not likely to be widely used - then the few users are probably experts who know when to say "yes". Unfortunately - if a LyX virus becomes possible for "those that say yes", then such a virus can be launched against people with no interest in those few converters. They'll have no idea what's going on, and they'll click through the warnings 'as usual' and spread the virus. Crisis will be averted mostly due to low LyX market share. I wonder, if AppArmor solves this problem for distros that have it - then perhaps AA should be a mandatory dependency for these few converters? If you want that converter - you get a working AA setup first? I hope future LyX won't be asking security questions most people can't answer with any confidence. I might be able to answer such questions; but only if I review the sw in question, which I certainly won't have time for. thanks for your honest opinion, Helge. Please, remember you can disable these security settings from the already added preferences options, if you feel these are just productivity stoppers. The problem I see, is that the game is lost already if security questions have to be asked. The questions can be turned off for convenience, but that is just as dangerous as blindly clicking through. (Unless you only ever handle your own documents.) A system where the warnings can be turned off permanently can become a virus server. And if I made the virus, it would turn that setting off as the first 'bad thing done' when the user clicks to allow the virus for the first time. A 'disable security question' feature is dangerous - and so is having a security question that can be answered wrongly by users without security knowledge. This covers most users who aren't programmers. So I wonder, is it better to simply demand AA for such converters? Users who need them will either need to get a distro with working AA, or put pressure on their sw developers to create a 'harmless' version of the converter they need. I would guess that a harmless version is possible in many cases, but so far nobody saw a need. (A harmless mode might restrict itself to not open files for writing, except for files specified on the command line and perhaps a few well-documented others. Also, disable the running of other executables except 'safe' ones.) Now, you're just raising the rightful point on whether we can really ship with these options ON by default, the first time, namely whether it's worthwhile to see users annoyed due to enabling them. If it only was about annoying - then turning the warning off (or simply not implementing it) is good. But the risk is real. While the decision isn't mine to make, I belive we need a solution that doesn't depend on the user's (lack of) understanding of computer security. At least for anything distributed as a part of LyX, via lyx.org A user who wants an unsafe converter can always install his own unofficial one, if he can't be bothered with AA. Those that wants unsafe converters to work out of the box, can use known good distros.
Re: #10481: Hardening LyX against potential misuse
On 12/12/2016 12:04, Helge Hafting wrote: In the general case, make a script (or utility program) that runs the dangerous converter in a chroot, where nothing dangerous can be done. No need for questions then. LyX already puts the document files in a temp directory so the cleanup after a latex run will be easier. chrooting before running a converter means the converter can't overwrite files outside the chroot, which helps quite a bit security-wise. unfortunately, chroot-ing also makes the system inaccessible/invisible, that's why I'm looking into AppArmor instead, which is essentially a "chroot on steroids". However, AA is more difficult to get right, and it will work "out of the box" only on a limited set of distros. On the other hand, the immediate, always working across any OS and portable security mitigation (to possible threats/viruses starting to spread as lyx docs), is the one to show up a dialog and ask the user -- I know, it's reminding all of us of smth, but also please remember that it won't show up always, rather only if you're using a fewselected converters. As for batch conversions, we could have an option (even a LyX-wide option) saying --assume-yes or--assume-no or similar, that would actually prevent any question to be asked. I hope future LyX won't be asking security questions most people can't answer with any confidence. I might be able to answer such questions; but only if I review the sw in question, which I certainly won't have time for. thanks for your honest opinion, Helge. Please, remember you can disable these security settings from the already added preferences options, if you feel these are just productivity stoppers. Now, you're just raising the rightful point on whether we can really ship with these options ON by default, the first time, namely whether it's worthwhile to see users annoyed due to enabling them. Thanks, T.
Re: #10481: Hardening LyX against potential misuse
On 13/12/2016 12:04 a.m., Helge Hafting wrote: I see a problem with this: Den 06. nov. 2016 20:57, skrev Tommaso Cucinotta: Converters marked with the new "needauth" option won't be run unless the user gives explicit authorization, which is asked on-demand when the converter is about to be run (question is not asked if the file is cached and the converter is not needed). The idea (to protect against misuses) is good! Unfortunately, I don't think protection actually will be achieved this way. Also, getting prompts/popups of any kind is annoying - (except for stuff I ask for myself, such as the search/replace dialog.) When I start a time-consuming job, I want to go for lunch and come back to a finished job. Not a prompt telling me the time was wasted because I must make a decision before anything more happens. Protection will not be achieved in most cases, because users are used to "clicking through various annoying popups without even reading them". They want to print a document or something, they are not prepared to make a security decision. The vast majority of writers aren't competent to make such a decision either. They have no idea what converter is about to run, or what the security implications are. If they got the document in the mail, they may not know if the question is to be expected because the co-author is using a special package, or if this is likely to be a hacking attempt. The exceptionally careful might try answering "no". When that doesn't work, they answer "yes" the next time. From then on, the question is nothing more than an annoyance - they will always answer "yes", and only because they know it is necessary to get their work done. They won't be contemplating security issues. You may avoid asking the question over and over in a single LyX session, but it will come back the next time they restart LyX and work on such a document. It'd be nice if we could avoid this "stopping of the proceedings to ask a question." That is bad enough in its own right, and even worse when people can't be expected to give an answer based on knowledge. (The answer will be based on convenience instead - the user want his pdf NOW. Having LyX stop production to ask questions is only irritating.) A security question is useful in cases where the user can be expected to decide from knowledge. Otherwise, it is a waste of time. How about solving the problem instead? In some cases, this might mean working on the converter software, adding a mode/parameter so it won't do anything "dangerous". Similiar to how you can disable write18 in LaTeX. Only possible for open source though. In the general case, make a script (or utility program) that runs the dangerous converter in a chroot, where nothing dangerous can be done. No need for questions then. LyX already puts the document files in a temp directory so the cleanup after a latex run will be easier. chrooting before running a converter means the converter can't overwrite files outside the chroot, which helps quite a bit security-wise. I hope future LyX won't be asking security questions most people can't answer with any confidence. I might be able to answer such questions; but only if I review the sw in question, which I certainly won't have time for. Helge Hafting The psychology of LyX use that Helge describes is "spot on". The protection afforded by need-authorisation popups/dialogues is likely to be illusory, an irritant to be clicked through. Andrew --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Re: #10481: Hardening LyX against potential misuse
I see a problem with this: Den 06. nov. 2016 20:57, skrev Tommaso Cucinotta: Converters marked with the new "needauth" option won't be run unless the user gives explicit authorization, which is asked on-demand when the converter is about to be run (question is not asked if the file is cached and the converter is not needed). The idea (to protect against misuses) is good! Unfortunately, I don't think protection actually will be achieved this way. Also, getting prompts/popups of any kind is annoying - (except for stuff I ask for myself, such as the search/replace dialog.) When I start a time-consuming job, I want to go for lunch and come back to a finished job. Not a prompt telling me the time was wasted because I must make a decision before anything more happens. Protection will not be achieved in most cases, because users are used to "clicking through various annoying popups without even reading them". They want to print a document or something, they are not prepared to make a security decision. The vast majority of writers aren't competent to make such a decision either. They have no idea what converter is about to run, or what the security implications are. If they got the document in the mail, they may not know if the question is to be expected because the co-author is using a special package, or if this is likely to be a hacking attempt. The exceptionally careful might try answering "no". When that doesn't work, they answer "yes" the next time. From then on, the question is nothing more than an annoyance - they will always answer "yes", and only because they know it is necessary to get their work done. They won't be contemplating security issues. You may avoid asking the question over and over in a single LyX session, but it will come back the next time they restart LyX and work on such a document. It'd be nice if we could avoid this "stopping of the proceedings to ask a question." That is bad enough in its own right, and even worse when people can't be expected to give an answer based on knowledge. (The answer will be based on convenience instead - the user want his pdf NOW. Having LyX stop production to ask questions is only irritating.) A security question is useful in cases where the user can be expected to decide from knowledge. Otherwise, it is a waste of time. How about solving the problem instead? In some cases, this might mean working on the converter software, adding a mode/parameter so it won't do anything "dangerous". Similiar to how you can disable write18 in LaTeX. Only possible for open source though. In the general case, make a script (or utility program) that runs the dangerous converter in a chroot, where nothing dangerous can be done. No need for questions then. LyX already puts the document files in a temp directory so the cleanup after a latex run will be easier. chrooting before running a converter means the converter can't overwrite files outside the chroot, which helps quite a bit security-wise. I hope future LyX won't be asking security questions most people can't answer with any confidence. I might be able to answer such questions; but only if I review the sw in question, which I certainly won't have time for. Helge Hafting
Re: #10481: Hardening LyX against potential misuse
On 10/12/2016 19:58, Tommaso Cucinotta wrote:
thanks! The most clear seems to me the 2nd one ("enrico-proposal") :-)!
and, looking forward, the security box will likely add 2 items inside:
-) the number of days authorizations granted by the user are going to last for
(as discussed, to be done by accompanying each authorized file in the
session with a timestamp)
-) another checkbox for the
hardening/apparmor(Linux)/sandbox(Mac)/whatever(Win) option
(refined patch almost there for AA on Linux)
T.
Re: #10481: Hardening LyX against potential misuse
On 10/12/2016 11:31, Enrico Forestieri wrote:
On Sat, Dec 10, 2016 at 12:40:15AM +0100, Tommaso Cucinotta wrote:
I confess I'm lost as to which layouts have been proposed for the converters
prefs pane.
Probably, having a few pics/snapshots comparing the options might be the best
way to gather feedback from others.
Here you go.
thanks! The most clear seems to me the 2nd one ("enrico-proposal") :-)!
T.
Re: #10481: Hardening LyX against potential misuse
I confess I'm lost as to which layouts have been proposed for the converters prefs pane. Probably, having a few pics/snapshots comparing the options might be the best way to gather feedback from others. Thanks, T. On 07/12/2016 21:00, Enrico Forestieri wrote: On Wed, Dec 07, 2016 at 06:15:02PM +0100, Jean-Marc Lasgouttes wrote: Yes, your patch make the separation clearer (although I am not sure that I like the oval rect). I thought it would have helped in distingushing the sections, but this is a mere detail. But the fact that this converter pane is already crowed means that we cannot implement a proper UI for converter flags. It might be that such an UI would be much too big anyway. Why would you think that a proper implementation (if and when that will be performed) would need more space? However, please have a look at the attached patch, which leaves more space to the converters definitions. No need to revert f0f555b5, as this time I used the designer and the changes would have been anyway extensive.
Re: #10481: Hardening LyX against potential misuse
On Fri, Dec 09, 2016 at 10:39:14AM +0100, Jean-Marc Lasgouttes wrote:
> Le 07/12/2016 à 21:00, Enrico Forestieri a écrit :
> > Why would you think that a proper implementation (if and when that will
> > be performed) would need more space? However, please have a look at the
> > attached patch, which leaves more space to the converters definitions.
> > No need to revert f0f555b5, as this time I used the designer and the
> > changes would have been anyway extensive.
>
> I like it better. I am not even sure that the vertical separator is needed.
> If it is kept, is it possible to have it continue higher up to the bold
> titles?
See attached.
> Concerning space, removing the raw flags field would require at least
>
> - a flavor menu that combines latex, latex_flavor and xml (these variables
> do not need to be separate IIUC).
> - 3 check boxes for need_aux, need_auth and nice
> - 3 text fields for result_dir, result_file and parse_log (I suspect that
> the two first ones could be combined)
>
> That's a lot of space IMO.
Well, I don't think the the raw flags are that a big issue. They are
very flexible and allow to add options or flavors without the need for
redesigning the gui. Going that direction we would have an ever increasing
space requirement. The most annoying behavior is that you have to remember
to hit "modify" for registering a change, or start modifying an existing
converter for adding one (!).
--
Enrico
diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui
b/src/frontends/qt4/ui/PrefConvertersUi.ui
index f9c02a8..8120e7d 100644
--- a/src/frontends/qt4/ui/PrefConvertersUi.ui
+++ b/src/frontends/qt4/ui/PrefConvertersUi.ui
@@ -1,96 +1,97 @@
-
+
PrefConvertersUi
-
-
+
+
0
0
-438
-466
+596
+498
-
+
-
-
+
+
9
-
+
6
-
-
-
- Converter Defi&nitions
+
+
+
+ QGroupBox{border:1px solid
gray;margin-top:0.5ex;}
-
-
+
+
+
+
+
9
-
+
6
-
-
-
-
- 7
- 7
+
+
+
+
0
0
-
-
-
+
+
+
0
-
+
6
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
+
-
+
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
-
+
+
C&onverter:
-
+
converterED
-
-
+
+
E&xtra flag:
-
+
converterFlagED
@@ -99,38 +100,36 @@
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
-
- 0
-
-
+
+
6
+
+ 0
+
-
-
+
+
&From format:
-
+
converterFromCO
-
-
-
- 3
- 0
+
+
+
0
0
@@ -140,29 +139,27 @@
-
-
- 0
-
-
+
+
6
+
+ 0
+
-
-
+
+
&To format:
-
+
converterToCO
-
-
-
- 3
- 0
+
+
+
0
0
@@ -173,49 +170,47 @@
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
-
+
+
A&dd
-
-
+
+
&Modify
-
-
-
-1
-
Re: #10481: Hardening LyX against potential misuse
Le 07/12/2016 à 21:00, Enrico Forestieri a écrit : Why would you think that a proper implementation (if and when that will be performed) would need more space? However, please have a look at the attached patch, which leaves more space to the converters definitions. No need to revert f0f555b5, as this time I used the designer and the changes would have been anyway extensive. I like it better. I am not even sure that the vertical separator is needed. If it is kept, is it possible to have it continue higher up to the bold titles? Concerning space, removing the raw flags field would require at least - a flavor menu that combines latex, latex_flavor and xml (these variables do not need to be separate IIUC). - 3 check boxes for need_aux, need_auth and nice - 3 text fields for result_dir, result_file and parse_log (I suspect that the two first ones could be combined) That's a lot of space IMO. JMarc
Re: #10481: Hardening LyX against potential misuse
Le 05/12/2016 à 08:53, Tommaso Cucinotta a écrit : On 04/12/2016 18:51, Guillaume Munch wrote: Le 04/12/2016 à 18:06, Tommaso Cucinotta a écrit : On 28/11/2016 00:42, Tommaso Cucinotta wrote: On 27/11/2016 13:52, Guillaume Munch wrote: * Converters>Security is located below the converter configuration, which leads to think that they are converter properties instead of global settings. What about placing it above the converter list? Same problem with the Converter Cache option already, isn't it? Let me propose the attached fix for both at once. just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm it shows up as intended ... How is it supposed to show up? Can you send a screenshot? Here only the font of "Converters Definition" changed, and I still find it unclear. Here you go: screenshots before and after the patch attached. With the "Converter Definitions" label now at the same highlight/logical level as "Converter File Cache" and "Security" ones, I think there is no more the confusionabout which options in the dialog are specific to a single converter. Thanks. I see the same. I agree that it is already better.
Re: #10481: Hardening LyX against potential misuse
Le 05/12/2016 à 08:36, Tommaso Cucinotta a écrit : On 04/12/2016 18:37, Guillaume Munch wrote: If there are n graphics, then are there n dialogs when opening the file for the first time? it asks as many times as there are (uncached) graphics needing 'needauth' converters,unless you hit the "Run, and don't ask again for the same doc" button. I guess this is better than nothing. Is this why you wanted the "don't warn again" checkbox on the other dialog?
Re: #10481: Hardening LyX against potential misuse
On Wed, Dec 07, 2016 at 06:15:02PM +0100, Jean-Marc Lasgouttes wrote:
>
> Yes, your patch make the separation clearer (although I am not sure that I
> like the oval rect).
I thought it would have helped in distingushing the sections, but this is
a mere detail.
> But the fact that this converter pane is already crowed means that we cannot
> implement a proper UI for converter flags. It might be that such an UI would
> be much too big anyway.
Why would you think that a proper implementation (if and when that will
be performed) would need more space? However, please have a look at the
attached patch, which leaves more space to the converters definitions.
No need to revert f0f555b5, as this time I used the designer and the
changes would have been anyway extensive.
--
Enrico
diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui
b/src/frontends/qt4/ui/PrefConvertersUi.ui
index f9c02a8..3497644 100644
--- a/src/frontends/qt4/ui/PrefConvertersUi.ui
+++ b/src/frontends/qt4/ui/PrefConvertersUi.ui
@@ -1,7 +1,7 @@
-
+
PrefConvertersUi
-
-
+
+
0
0
@@ -9,88 +9,89 @@
466
-
+
-
-
+
+
9
-
+
6
-
-
-
- Converter Defi&nitions
+
+
+
+ QGroupBox{border:1px solid
gray;margin-top:0.5ex;}
-
-
+
+
+
+
+
9
-
+
6
-
-
-
-
- 7
- 7
+
+
+
+
0
0
-
-
-
+
+
+
0
-
+
6
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
+
-
+
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
-
+
+
C&onverter:
-
+
converterED
-
-
+
+
E&xtra flag:
-
+
converterFlagED
@@ -99,38 +100,36 @@
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
-
- 0
-
-
+
+
6
+
+ 0
+
-
-
+
+
&From format:
-
+
converterFromCO
-
-
-
- 3
- 0
+
+
+
0
0
@@ -140,29 +139,27 @@
-
-
- 0
-
-
+
+
6
+
+ 0
+
-
-
+
+
&To format:
-
+
converterToCO
-
-
-
- 3
- 0
+
+
+
0
0
@@ -173,49 +170,47 @@
-
-
-
- 0
-
-
+
+
+
6
+
+ 0
+
-
-
+
+
A&dd
-
-
+
+
&Modify
-
-
-
-1
-0
+
+
+
0
0
-
+
Remo&ve
-
+
Qt::Vertical
-
+
75
16
@@ -225,109 +220,178 @@
+
+
+
+
+ 0
+ 0
+
+
+
+ font-weight: bold;
+
+
+ Converter Defi&nitions
+
+
+ convertersLW
+
+
+
-
-
-
-
Re: #10481: Hardening LyX against potential misuse
Le 07/12/2016 à 17:01, Enrico Forestieri a écrit : On Wed, Dec 07, 2016 at 12:06:41PM +0100, Jean-Marc Lasgouttes wrote: I did not mean a security tab, but a "general" one that could contain: - converter file cache - security - overwrite on export (from Output|general) I don't like this further fragmentation. I like the fact that we have a single tab related to converters. Please, try the patch(es) I attached in another email in this thread that clearly separate the 3 sections. Yes, your patch make the separation clearer (although I am not sure that I like the oval rect). But the fact that this converter pane is already crowed means that we cannot implement a proper UI for converter flags. It might be that such an UI would be much too big anyway. JMarc
Re: #10481: Hardening LyX against potential misuse
On Wed, Dec 07, 2016 at 12:06:41PM +0100, Jean-Marc Lasgouttes wrote: > > I did not mean a security tab, but a "general" one that could contain: > > - converter file cache > - security > - overwrite on export (from Output|general) I don't like this further fragmentation. I like the fact that we have a single tab related to converters. Please, try the patch(es) I attached in another email in this thread that clearly separate the 3 sections. -- Enrico
Re: #10481: Hardening LyX against potential misuse
Le 06/12/2016 à 17:37, Tommaso Cucinotta a écrit : Creating a separate prefs tab might be meaningful if we had more security settings not related exclusively to converters, but keeping things together within a single converters pane has the advantage of linking clearly the security checkboxes tooltips to the 'needauth' options that can be set within each converter's individual settings mask. So I'd be in favor of leaving all into a single converters prefs pane as it is now. I did not mean a security tab, but a "general" one that could contain: - converter file cache - security - overwrite on export (from Output|general) Concerning the UI, I use it rarely (obviously) but I encounter nonintuitiveness every time :) I see several points - for converters , "Add" is enabled only when selecting a new from/to pair that does not have a converter yet. Nothing in the UI indicates that. - The big question is whether or not it is required to use Apply at some point. - Now if I go to files formats, I have New.../Remove instead of Add/Modify/Remove. How is it supposed to work? Why is it different? - For format, I have nice check-boxes, but for converters I have to type a list of cryptic keywords. Nobody should have to know what 'needauth' means. I do think that there is room for improvement. But since I am not the one who is going to do it, I'll stay quiet ! JMarc
Re: #10481: Hardening LyX against potential misuse
On Tue, Dec 06, 2016 at 10:57:56PM +0100, Tommaso Cucinotta wrote:
> On 06/12/2016 20:44, Enrico Forestieri wrote:
> > > From 8f157f2d3beb48ae87f9dcd07d59ee062a7a7da0 Mon Sep 17 00:00:00 2001
> > > From: Tommaso Cucinotta
> > > Date: Mon, 28 Nov 2016 00:31:46 +0100
> > > Subject: [PATCH] Converters Prefs UI layout clarification.
> > [...]
> > > -
> > > - Converter Defi&nitions
> > > -
> > > -
> > > - convertersLW
> > > -
> >
> > Please, note that you removed this "buddy", so that the Converters
> > Definitions section is not accessible by shortcut anymore.
>
> right ... couldn't find a way to restore the shortcut, other than ...
> rollback :(, and apply a different UI clarification fix, would look like
> the attached, how would u see that?
What about the attached, instead?
--
Enrico
>From bb1d89aafaf9c8aefdafeac3f638f2e9097cfaea Mon Sep 17 00:00:00 2001
From: Enrico Forestieri
Date: Wed, 7 Dec 2016 00:46:00 +0100
Subject: [PATCH 1/2] Revert "Converters Prefs UI layout clarification."
This reverts commit f0f555b5be8cf5eefcb716a53a5632c7c608b216.
---
src/frontends/qt4/ui/PrefConvertersUi.ui | 28 +++-
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui
b/src/frontends/qt4/ui/PrefConvertersUi.ui
index f9c02a8..7e4a1b3 100644
--- a/src/frontends/qt4/ui/PrefConvertersUi.ui
+++ b/src/frontends/qt4/ui/PrefConvertersUi.ui
@@ -22,7 +22,7 @@
- Converter Defi&nitions
+
@@ -31,7 +31,7 @@
6
-
+
@@ -43,7 +43,7 @@
-
+
0
@@ -99,7 +99,7 @@
-
+
0
@@ -173,7 +173,7 @@
-
+
0
@@ -225,6 +225,24 @@
+
+
+
+
+ 1
+ 5
+ 0
+ 0
+
+
+
+ Converter Defi&nitions
+
+
+ convertersLW
+
+
+
--
2.8.3
>From 8e080433031e34f7341b81c37846a1930f692961 Mon Sep 17 00:00:00 2001
From: Enrico Forestieri
Date: Wed, 7 Dec 2016 01:07:46 +0100
Subject: [PATCH 2/2] Real Converters Prefs UI clarification
---
src/frontends/qt4/ui/PrefConvertersUi.ui | 54 ++--
1 file changed, 51 insertions(+), 3 deletions(-)
diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui
b/src/frontends/qt4/ui/PrefConvertersUi.ui
index 7e4a1b3..6c0490e 100644
--- a/src/frontends/qt4/ui/PrefConvertersUi.ui
+++ b/src/frontends/qt4/ui/PrefConvertersUi.ui
@@ -21,6 +21,9 @@
+
+ QGroupBox{border:1px solid
gray;border-radius:5px;margin-top:0.5ex;}
+
@@ -226,7 +229,7 @@
-
+
1
@@ -235,6 +238,9 @@
0
+
+ font-weight: bold;
+
Converter Defi&nitions
@@ -248,8 +254,11 @@
+
+ QGroupBox{border:1px solid
gray;border-radius:5px;margin-top:0.5ex;}
+
- Converter File Cache
+
@@ -259,6 +268,24 @@
6
+
+
+
+ 1
+ 5
+ 0
+ 0
+
+
+
+ font-weight: bold;
+
+
+ Converter File Cache
+
+
+
+
0
@@ -306,8 +333,11 @@
+
+ QGroupBox{border:1px solid
gray;border-radius:5px;margin-top:0.5ex;}
+
- Security
+
@@ -317,6 +347,24 @@
6
+
+
+
+ 1
+ 5
+ 0
+ 0
+
+
+
+ font-weight: bold;
+
+
+ Security
+
+
+
+
0
--
2.8.3
Re: #10481: Hardening LyX against potential misuse
On 06/12/2016 22:57, Tommaso Cucinotta wrote: right ... couldn't find a way to restore the shortcut, other than ... rollback :(, and apply a different UI clarification fix, would look like the attached, how would u see that? as we're at it, there was a clash in shortcuts for the new two security checkboxes, could be removed by this diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui b/src/frontends/qt4/ui/PrefConvertersUi.ui index 87abef57..efd23abd 100644 --- a/src/frontends/qt4/ui/PrefConvertersUi.ui +++ b/src/frontends/qt4/ui/PrefConvertersUi.ui @@ -327,7 +327,7 @@ - &Forbid use of needauth converters + Forbid &use of needauth converters When enabled, use of converters with the 'needauth' option is forbidden. @@ -337,7 +337,7 @@ - Use need&auth option + Use needaut&h option When enabled, ask user before launching any external converter with the 'needauth' option. objections? Thanks, T.
Re: #10481: Hardening LyX against potential misuse
On 06/12/2016 20:44, Enrico Forestieri wrote: From 8f157f2d3beb48ae87f9dcd07d59ee062a7a7da0 Mon Sep 17 00:00:00 2001 From: Tommaso Cucinotta Date: Mon, 28 Nov 2016 00:31:46 +0100 Subject: [PATCH] Converters Prefs UI layout clarification. [...] - - Converter Defi&nitions - - - convertersLW - Please, note that you removed this "buddy", so that the Converters Definitions section is not accessible by shortcut anymore. right ... couldn't find a way to restore the shortcut, other than ... rollback :(, and apply a different UI clarification fix, would look like the attached, how would u see that? Thanks, T. >From 0fbddf986305cd8a8c6e056bc4ebef024cc83aa8 Mon Sep 17 00:00:00 2001 From: Tommaso Cucinotta Date: Tue, 6 Dec 2016 22:50:45 +0100 Subject: [PATCH 1/2] Revert "Converters Prefs UI layout clarification." This reverts commit f0f555b5be8cf5eefcb716a53a5632c7c608b216. --- src/frontends/qt4/ui/PrefConvertersUi.ui | 28 +++- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui b/src/frontends/qt4/ui/PrefConvertersUi.ui index f9c02a86..7e4a1b32 100644 --- a/src/frontends/qt4/ui/PrefConvertersUi.ui +++ b/src/frontends/qt4/ui/PrefConvertersUi.ui @@ -22,7 +22,7 @@ - Converter Defi&nitions + @@ -31,7 +31,7 @@ 6 - + @@ -43,7 +43,7 @@ - + 0 @@ -99,7 +99,7 @@ - + 0 @@ -173,7 +173,7 @@ - + 0 @@ -225,6 +225,24 @@ + + + + + 1 + 5 + 0 + 0 + + + + Converter Defi&nitions + + + convertersLW + + + -- 2.9.3 >From 5685d8357864658aeae5163ab9e827bc10c81b79 Mon Sep 17 00:00:00 2001 From: Tommaso Cucinotta Date: Tue, 6 Dec 2016 22:54:21 +0100 Subject: [PATCH 2/2] Real Converters Prefs UI clarification. --- src/frontends/qt4/ui/PrefConvertersUi.ui | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui b/src/frontends/qt4/ui/PrefConvertersUi.ui index 7e4a1b32..87abef57 100644 --- a/src/frontends/qt4/ui/PrefConvertersUi.ui +++ b/src/frontends/qt4/ui/PrefConvertersUi.ui @@ -22,7 +22,7 @@ - + Edit converters -- 2.9.3
Re: #10481: Hardening LyX against potential misuse
On Mon, Nov 28, 2016 at 12:42:31AM +0100, Tommaso Cucinotta wrote: > From 8f157f2d3beb48ae87f9dcd07d59ee062a7a7da0 Mon Sep 17 00:00:00 2001 > From: Tommaso Cucinotta > Date: Mon, 28 Nov 2016 00:31:46 +0100 > Subject: [PATCH] Converters Prefs UI layout clarification. [...] > - > - Converter Defi&nitions > - > - > - convertersLW > - Please, note that you removed this "buddy", so that the Converters Definitions section is not accessible by shortcut anymore. -- Enrico
Re: #10481: Hardening LyX against potential misuse
On Tue, Dec 06, 2016 at 06:10:52PM +0100, Enrico Forestieri wrote: > On Tue, Dec 06, 2016 at 05:37:40PM +0100, Tommaso Cucinotta wrote: > > > > Creating a separate prefs tab might be meaningful if we had more security > > settings not related exclusively to converters, but keeping things > > together within a single converters pane has the advantage of linking > > clearly the security checkboxes tooltips to the 'needauth' options that > > can be set within each converter's individual settings mask. So I'd be in > > favor of leaving all into a single converters prefs pane as it is now. > > I concur with you. This preference has been like that since version 1.5 > and nobody complained that it is confusing (apart from its quirks, which > are unrelated to this issue). Until this recent addition, at least. True, I don't remember any complaints either. We should always look for opportunities to improve clarity though. I think the idea bout a thicker border is good. Perhaps we could also have a label just stating "global options". Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
On Tue, Dec 06, 2016 at 05:37:40PM +0100, Tommaso Cucinotta wrote: > > Creating a separate prefs tab might be meaningful if we had more security > settings not related exclusively to converters, but keeping things > together within a single converters pane has the advantage of linking > clearly the security checkboxes tooltips to the 'needauth' options that > can be set within each converter's individual settings mask. So I'd be in > favor of leaving all into a single converters prefs pane as it is now. I concur with you. This preference has been like that since version 1.5 and nobody complained that it is confusing (apart from its quirks, which are unrelated to this issue). Until this recent addition, at least. -- Enrico
Re: #10481: Hardening LyX against potential misuse
On 05/12/2016 23:44, Scott Kostyshak wrote: With the "Converter Definitions" label now at the same highlight/logical level as "Converter File Cache" and "Security" ones, I think there is no more the confusionabout which options in the dialog are specific to a single converter. I still think it is confusing. What do you think about Guillaume's idea of placing the global options above the converter definitions? I tried that as well first, but then realized that the actual problem was the "Converter Definitions" text showing up as a QLabel within the content of the converters definitions section; "promoting" it to becoming the title of the section fixed the issue, because now with my style the "Converters Definition" shows up left-aligned, boldface and unindented, identically to "Converter File Cache"and "Security",and in Enrico's Qt style, those 3 show up as framing the corresponding sections. The contents of said 3 sections show up as indented in my style, and inside the corresponding frames in Enrico's, so it seems quite clear that the cache and security settings won't refer to the selected converter inside the first section. That said, I don't have objections to reordering these 3 sections. Creating a separate prefs tab might be meaningful if we had more security settings not related exclusively to converters, but keeping things together within a single converters pane has the advantage of linking clearly the security checkboxes tooltips to the 'needauth' options that can be set within each converter's individual settings mask. So I'd be in favor of leaving all into a single converters prefs pane as it is now. Thanks, T.
Re: #10481: Hardening LyX against potential misuse
Le 06/12/2016 à 14:16, Enrico Forestieri a écrit : I still think it is confusing. What do you think about Guillaume's idea of placing the global options above the converter definitions? Note that the appearance also depends on the used Qt style. In the attached you can see that a frame is drawn around the options, clearly separating them. I don't think placing the global options above makes it any clearer. Rather I think it would be more confusing. Maybe a border frame can be placed around the options whatever the used Qt style? I would propose to add a new "General" pane that contains these options. JMarc
Re: #10481: Hardening LyX against potential misuse
On Mon, Dec 05, 2016 at 05:44:46PM -0500, Scott Kostyshak wrote: > On Mon, Dec 05, 2016 at 08:53:58AM +0100, Tommaso Cucinotta wrote: > > > With the "Converter Definitions" label now at the same highlight/logical > > level > > as "Converter File Cache" and "Security" ones, I think there is no more the > > confusionabout which options in the dialog are specific to a single > > converter. > > I still think it is confusing. What do you think about Guillaume's idea > of placing the global options above the converter definitions? Note that the appearance also depends on the used Qt style. In the attached you can see that a frame is drawn around the options, clearly separating them. I don't think placing the global options above makes it any clearer. Rather I think it would be more confusing. Maybe a border frame can be placed around the options whatever the used Qt style? -- Enrico
Re: #10481: Hardening LyX against potential misuse
On Mon, Dec 05, 2016 at 08:53:58AM +0100, Tommaso Cucinotta wrote: > With the "Converter Definitions" label now at the same highlight/logical level > as "Converter File Cache" and "Security" ones, I think there is no more the > confusionabout which options in the dialog are specific to a single converter. I still think it is confusing. What do you think about Guillaume's idea of placing the global options above the converter definitions? Thanks again for your persistence and follow-up commits to improve things. Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
On 04/12/2016 18:37, Guillaume Munch wrote: If there are n graphics, then are there n dialogs when opening the file for the first time? it asks as many times as there are (uncached) graphics needing 'needauth' converters,unless you hit the "Run, and don't ask again for the same doc" button. T.
Re: #10481: Hardening LyX against potential misuse
Am Sonntag, 4. Dezember 2016 um 18:51:15, schrieb Guillaume Munch > Le 04/12/2016 à 18:06, Tommaso Cucinotta a écrit : > > On 28/11/2016 00:42, Tommaso Cucinotta wrote: > >> On 27/11/2016 13:52, Guillaume Munch wrote: > >>> * Converters>Security is located below the converter configuration, > >>> which leads to think that they are converter properties instead of > >>> global settings. What about placing it above the converter list? > >> > >> Same problem with the Converter Cache option already, isn't it? > >> > >> Let me propose the attached fix for both at once. > > > > just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm > > it shows up as intended ... > > > > How is it supposed to show up? Can you send a screenshot? Here only the > font of "Converters Definition" changed, and I still find it unclear. Dialog looks good. There seems to be missing in "The requested operation requires the use of a converter from %2$s to %3$s:" "%1$sThis external program can execute " "arbitrary commands on your system, including dangerous ones, if instructed " "to do so by a maliciously crafted .lyx document." Kornel signature.asc Description: This is a digitally signed message part.
Re: #10481: Hardening LyX against potential misuse
On Sun, Dec 04, 2016 at 06:06:57PM +0100, Tommaso Cucinotta wrote: > On 28/11/2016 00:42, Tommaso Cucinotta wrote: > > On 27/11/2016 13:52, Guillaume Munch wrote: > > > * Converters>Security is located below the converter configuration, > > > which leads to think that they are converter properties instead of > > > global settings. What about placing it above the converter list? > > > > Same problem with the Converter Cache option already, isn't it? > > > > Let me propose the attached fix for both at once. > > just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm it > shows up as intended ... I did not follow this conversation closely, but here are my comments: Keep in mind that most LyX users do not know what a file cache is. Can you describe exactly what's being cached in the tooltip? Maybe you could use the word "remembered" instead of "cached" in the tooltip because it is a more user-friendly word? A tooltip for maximum age would also be useful. e.g. "after this amount of days, ..." I wonder if the cache should expire after e.g. 60 days, or after 60 days since the last use of the file. For example, I (think I) would want it to be 60 days since the last use of the file, but I don't know if that's what would be best for the general user. I think you want some kind of validator for the maximum age. I put in a negative number and after saving it turned it into 49651.3. I'm guessing you want to restrict to only non-negative integers? Look at other places we use validators for similar text boxes. Also what would happen if I put in 0? Does that effectively mean the converter file cache is disabled? Or is it a special value that means "never expire"? Thanks for your work on this! Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
Le 04/12/2016 à 18:06, Tommaso Cucinotta a écrit : On 28/11/2016 00:42, Tommaso Cucinotta wrote: On 27/11/2016 13:52, Guillaume Munch wrote: * Converters>Security is located below the converter configuration, which leads to think that they are converter properties instead of global settings. What about placing it above the converter list? Same problem with the Converter Cache option already, isn't it? Let me propose the attached fix for both at once. just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm it shows up as intended ... How is it supposed to show up? Can you send a screenshot? Here only the font of "Converters Definition" changed, and I still find it unclear.
Re: #10481: Hardening LyX against potential misuse
Le 28/11/2016 à 00:42, Tommaso Cucinotta a écrit : eh eh, what about remembering 'needauth' (as well as cursor pos) only for those files in the recent files list :-), and collapse the 3 lists into a single one, and a single session section ? Two problems I see with this idea is that migrating the user session is going to require some work which can be avoided, and also that the three lists may have different age/length requirements, for instance the recent files is limited to what you see in the menu. * Please see the attached patch for a few suggestions looks great, HTML provides a much better graphics :-), feel free to push ! Done * Converters>Security is located below the converter configuration, which leads to think that they are converter properties instead of global settings. What about placing it above the converter list? Same problem with the Converter Cache option already, isn't it? Let me propose the attached fix for both at once. You made me realise that the converter cache option is indeed global. So it's good to fix both, thanks. browse to the Sweave converters, and you'll see the needauth option in the extraflags, guess we can edit as well from the dialog, can't we? Ok, I did not realise. 1. graphics on-screen conversion & display, used with my (local) gnuplot patch, so I insert a .gnuplot image file, and I see its produced output pic on screen, ..., after a few warnings about running needauth converters :-)... If there are n graphics, then are there n dialogs when opening the file for the first time? what else, what are further calls to converters? Thanks for the explanations, I don't see other calls to converters. Guillaume
Re: #10481: Hardening LyX against potential misuse
On 28/11/2016 00:42, Tommaso Cucinotta wrote: On 27/11/2016 13:52, Guillaume Munch wrote: * Converters>Security is located below the converter configuration, which leads to think that they are converter properties instead of global settings. What about placing it above the converter list? Same problem with the Converter Cache option already, isn't it? Let me propose the attached fix for both at once. just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm it shows up as intended ... Thx, T.
Re: #10481: Hardening LyX against potential misuse
On 27/11/2016 13:52, Guillaume Munch wrote: Hi Tommaso, Hi, [...] Making AppArmor work would be great too, but I suspect that it is going to be hard to have a configuration which is both secure and without hassle to the user, right, security comes often at a usability cost, hopefully we can keep hassle at the very bare minimum, and showing up only for advanced/particular usage scenarios. Le 23/11/2016 à 22:27, Tommaso Cucinotta a écrit : One note: one way to avoid the [auth session] section growing unbounded, might be to have an expiry timestamp, so that e.g., authorizations would expire in ~1y or so. This might be done with a section syntax like: instead of the simple we have now. Could that be used to fix #10310 as well? See https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg195888.html eh eh, what about remembering 'needauth' (as well as cursor pos) only for those files in the recent files list :-), and collapse the 3 lists into a single one, and a single session section ? * Please see the attached patch for a few suggestions looks great, HTML provides a much better graphics :-), feel free to push ! As for the type of possible damages, it's likely beef for the User Manual, and probably we need to redirect the user there, to know more if needed. few corrections, html formatting, no checkbox). (Using html much improves readability, but then the html appears in the console output. I have a solution for this if you choose this route.) and feel free to apply the corresponding workaround... * Converters>Security is located below the converter configuration, which leads to think that they are converter properties instead of global settings. What about placing it above the converter list? Same problem with the Converter Cache option already, isn't it? Let me propose the attached fix for both at once. * It would be nice to be able to edit the needauth flag from the converters settings. you can already, e.g., browse to the Sweave converters, and you'll see the needauth option in the extraflags, guess we can edit as well from the dialog, can't we? * What happens if a needauth converter is called during instant preview? Could that happen? And during graphics display? I've been testing with 2 cases: 1. graphics on-screen conversion & display, used with my (local) gnuplot patch, so I insert a .gnuplot image file, and I see its produced output pic on screen, ..., after a few warnings about running needauth converters :-)... 2. formatted PDF when previewing/rendering PDF The 1. and 2. cases correspond to 2 different converters call paradigms, one is from Converters.cpp, the other from GrpahicsConverter.cpp. AFAIK, instant preview is used for maths, processed through standard (secure) latex (no --shell-escape option), what else, what are further calls to converters? Thanks, T. >From 8f157f2d3beb48ae87f9dcd07d59ee062a7a7da0 Mon Sep 17 00:00:00 2001 From: Tommaso Cucinotta Date: Mon, 28 Nov 2016 00:31:46 +0100 Subject: [PATCH] Converters Prefs UI layout clarification. --- src/frontends/qt4/ui/PrefConvertersUi.ui | 28 +--- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/src/frontends/qt4/ui/PrefConvertersUi.ui b/src/frontends/qt4/ui/PrefConvertersUi.ui index 7e4a1b32..f9c02a86 100644 --- a/src/frontends/qt4/ui/PrefConvertersUi.ui +++ b/src/frontends/qt4/ui/PrefConvertersUi.ui @@ -22,7 +22,7 @@ - + Converter Defi&nitions @@ -31,7 +31,7 @@ 6 - + @@ -43,7 +43,7 @@ - + 0 @@ -99,7 +99,7 @@ - + 0 @@ -173,7 +173,7 @@ - + 0 @@ -225,24 +225,6 @@ - - - - - 1 - 5 - 0 - 0 - - - - Converter Defi&nitions - - - convertersLW - - - -- 2.9.3
Re: #10481: Hardening LyX against potential misuse
On Sun, Nov 27, 2016 at 01:52:20PM +0100, Guillaume Munch wrote: > * Converters>Security is located below the converter configuration, > which leads to think that they are converter properties instead of > global settings. What about placing it above the converter list? I noticed this also. Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
Hi Tommaso,
I have been following your work on this issue with interest. Thank you,
this is something that was much needed. Making AppArmor work would be
great too, but I suspect that it is going to be hard to have a
configuration which is both secure and without hassle to the user,
especially since with security there can be no half-measure, and
unrestricted read access can be an issue too. Still any experiment with
it is good. Further comments below.
Le 23/11/2016 à 22:27, Tommaso Cucinotta a écrit :
One note: one way to avoid the [auth session] section growing unbounded,
might be to have an expiry timestamp, so that e.g., authorizations would
expire in ~1y or so. This might be done with a section syntax like:
instead of the simple we have now.
Could that be used to fix #10310 as well?
See https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg195888.html
Any further comment welcome, thanks!
* Please see the attached patch for a few suggestions (more concise, a
few corrections, html formatting, no checkbox). (Using html much
improves readability, but then the html appears in the console output. I
have a solution for this if you choose this route.)
* Please try to avoid lines longer than 80 characters.
* "dangerous, including deleting files": it has been a while that
computer virus are created to do much worse than deleting files...
* Converters>Security is located below the converter configuration,
which leads to think that they are converter properties instead of
global settings. What about placing it above the converter list?
* It would be nice to be able to edit the needauth flag from the
converters settings.
* What happens if a needauth converter is called during instant preview?
Could that happen? And during graphics display?
Thanks again.
Guillaume
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 14b18dd..e596b3d 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -283,29 +283,40 @@ bool Converters::checkAuth(Converter const & conv, string const & doc_fname)
{
if (!conv.need_auth())
return true;
- const docstring security_warning = bformat(_("Requested operation needs use of converter '%1$s' from %2$s to %3$s, "
- "which is tagged with the 'needauth' option. This is an external program normally acting as a picture/format "
- "converter, but which is known to be able to execute arbitrary actions on the system on behalf of the user, "
- "including dangerous ones such as deleting files, if instructed to do so by a maliciously crafted .lyx document."),
- from_utf8(conv.command()), from_utf8(conv.from()), from_utf8(conv.to()));
+ const docstring security_warning = bformat(
+ _("The requested operation requires the use of a converter from "
+ "%2$s to %3$s:"
+ "%1$s"
+ "This external program can execute arbitrary commands on your "
+ "system, including dangerous ones, if instructed to do so by a "
+ "maliciously crafted .lyx document."),
+ from_utf8(conv.command()), from_utf8(conv.from()),
+ from_utf8(conv.to()));
if (lyxrc.use_converter_needauth_forbidden) {
- frontend::Alert::warning(_("Launch of external converter is forbidden"), security_warning + _("\n\n"
- "This is forbidden by default. In order to unlock execution of these converters, please, go to\n"
- "Preferences->File Handling->Converters and uncheck Security->Forbid needauth converters."), true);
+ frontend::Alert::warning(
+ _("An external converter is disabled for security reasons"),
+ security_warning + _(
+ "Your current settings forbid its execution."
+ "(To change this setting, go to Preferences ▹ File "
+ "Handling ▹ Converters and uncheck Security ▹ "
+ "Forbid needauth converters.)"), false);
return false;
}
if (!lyxrc.use_converter_needauth)
return true;
- static const docstring security_title = _("Launch of external converter needs user authorization");
+ static const docstring security_title =
+ _("An external converter requires your authorization");
int choice;
- const docstring security_warning2 = security_warning + _("\n\nWould you like to run the converter?\n\n"
- "ANSWER RUN ONLY IF YOU TRUST THE ORIGIN/SENDER OF THE LYX DOCUMENT!");
+ const docstring security_warning2 = security_warning +
+ _("Would you like to run this converter?"
+ "Only run if you trust the origin/sender of the LyX "
+ "document!");
if (!doc_fname.empty()) {
LYXERR(Debug::FILES, "looking up: " << doc_fname);
std::set & auth_files = theSession().authFiles().authFiles();
if (auth_files.find(doc_fname) == auth_files.end()) {
choice = frontend::Alert::prompt(security_title, security_warning2,
-0, 0, _("Do &NOT run"), _("&Run"), _("&Always run for this document"));
+0, 0, _("Do ¬ run"), _("&Run"), _("&Always run for this document"));
if (choice == 2)
auth_files.insert(doc_fname);
} else {
@@ -313,7 +324,7 @@ bool Converters::checkAuth(Converter const & conv, s
Re: #10481: Hardening LyX against potential misuse
Le 25/11/2016 à 20:50, Scott Kostyshak a écrit : On Fri, Nov 25, 2016 at 02:32:37PM -0500, Scott Kostyshak wrote: I think the line-breaking in the warning dialog should be improved. The horizontal width is larger than my 13 in. screen. See attached. Note that the linebreaking on the *other* warning (the Run/ Do Not run) is fine. This dialog is custom (GuiToggleWarningDialog). All other messages dialogs use QMessageBox. GuiToggleWarningDialog is essentially a QMessageBox::warning with an added checkbox. Starting from Qt5.2, QMessageBox supports adding a checkbox (QMessageBox::setCheckBox). I suggest reimplementing GuiToggleWarningDialog as a QMessageBox, for consistency and to fix the issue. One will need an #if QT_VERSION >=... directive though. Also regarding the "first" warning (the "Launch of external converter is forbidden" warning, which is the one I'm referring to above), although I have not closely followed the conversation I have the following comment: I think it should be converted to an error and/or I don't think there should be a checkbox "Do not show this warning again". I agree. Note that this also fixes the the line breaking issue since QMessageBox is used instead of GuiToggleWarningDialog in that case. (But it would still be good to fix GuiToggleWarningDialog.) Guillaume
Re: #10481: Hardening LyX against potential misuse
On 11/25/2016 05:49 PM, Tommaso Cucinotta wrote: > On 23/11/2016 01:11, Enrico Forestieri wrote: >> On Tue, Nov 22, 2016 at 11:59:37PM +0100, Tommaso Cucinotta wrote: >>> >>> There's a couple of TODOs left: >>> 1. versioning of the preferences file, I don't know much in this >>> area, the >>> patch adds a couple of preferences options, what else is needed? >> >> Have a look at c2a18fc1 to get an idea. > > Looks straightforward, so I just came up with the attached patch, > anyone willing to review before pushing ? Looks fine to me. rh
Re: #10481: Hardening LyX against potential misuse
On 23/11/2016 01:11, Enrico Forestieri wrote:
On Tue, Nov 22, 2016 at 11:59:37PM +0100, Tommaso Cucinotta wrote:
There's a couple of TODOs left:
1. versioning of the preferences file, I don't know much in this area, the
patch adds a couple of preferences options, what else is needed?
Have a look at c2a18fc1 to get an idea.
Looks straightforward, so I just came up with the attached patch, anyone
willing to review before pushing ?
Thanks,
T.
>From cc653feb605bcbfdbbad436f23547dc0728a175a Mon Sep 17 00:00:00 2001
From: Tommaso Cucinotta
Date: Fri, 25 Nov 2016 23:33:08 +0100
Subject: [PATCH] Bump up RC fileformat after [244de5d2/lyxgit] for the new
'needauth' options.
Addressing #10481.
---
lib/configure.py | 2 +-
lib/scripts/prefs2prefs_prefs.py | 7 ++-
src/LyXRC.cpp| 2 +-
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/lib/configure.py b/lib/configure.py
index b282a284..fb1abd35 100644
--- a/lib/configure.py
+++ b/lib/configure.py
@@ -1551,7 +1551,7 @@ if __name__ == '__main__':
lyx_check_config = True
lyx_kpsewhich = True
outfile = 'lyxrc.defaults'
-lyxrc_fileformat = 19
+lyxrc_fileformat = 20
rc_entries = ''
lyx_keep_temps = False
version_suffix = ''
diff --git a/lib/scripts/prefs2prefs_prefs.py b/lib/scripts/prefs2prefs_prefs.py
index 85fa90e0..ff318369 100644
--- a/lib/scripts/prefs2prefs_prefs.py
+++ b/lib/scripts/prefs2prefs_prefs.py
@@ -80,6 +80,10 @@
# Incremented to format 19, by rgh
# remove print support
+# Incremented to format 20, by tommaso
+# Add options to forbid/ignore 'needauth' option
+# No conversion necessary.
+
# NOTE: The format should also be updated in LYXRC.cpp and
# in configure.py.
@@ -375,5 +379,6 @@ conversions = [
[ 16, [remove_force_paint_single_char]],
[ 17, [remove_rtl]],
[ 18, []],
- [ 19, [remove_print_support]]
+ [ 19, [remove_print_support]],
+ [ 20, []]
]
diff --git a/src/LyXRC.cpp b/src/LyXRC.cpp
index 355e0651..587e1e7c 100644
--- a/src/LyXRC.cpp
+++ b/src/LyXRC.cpp
@@ -59,7 +59,7 @@ namespace {
// The format should also be updated in configure.py, and conversion code
// should be added to prefs2prefs_prefs.py.
-static unsigned int const LYXRC_FILEFORMAT = 19; // rgh: remove print support
+static unsigned int const LYXRC_FILEFORMAT = 20; // tommaso: 'needauth' options
// when adding something to this array keep it sorted!
LexerKeyword lyxrcTags[] = {
--
2.9.3
Re: #10481: Hardening LyX against potential misuse
On Fri, Nov 25, 2016 at 02:32:37PM -0500, Scott Kostyshak wrote: > On Wed, Nov 23, 2016 at 10:27:03PM +0100, Tommaso Cucinotta wrote: > > > Any further comment welcome, thanks! > > I think the line-breaking in the warning dialog should be improved. The > horizontal width is larger than my 13 in. screen. See attached. Note that the linebreaking on the *other* warning (the Run/ Do Not run) is fine. Also regarding the "first" warning (the "Launch of external converter is forbidden" warning, which is the one I'm referring to above), although I have not closely followed the conversation I have the following comment: I think it should be converted to an error and/or I don't think there should be a checkbox "Do not show this warning again". The reason is that if the user blindly clicks the box and clicks OK (yes, users often ignore warnings without reading them...), then the document will export with an error and the user won't know why and they won't be able to go back to the warning. In my opinion, there is 0% chance that the document exports correctly since the converter could not be run. Thus, an error should be used. Scott signature.asc Description: PGP signature
Re: #10481: Hardening LyX against potential misuse
On Wed, Nov 23, 2016 at 10:27:03PM +0100, Tommaso Cucinotta wrote: > > One note: one way to avoid the [auth session] section growing unbounded, > might be to have an expiry timestamp, so that e.g., authorizations would > expire in ~1y or so. This might be done with a section syntax like: > > > > instead of the simple we have now. That seems like a good idea. But you now should also account for correctly parsing a line missing the timestamp on reading (and then add a timestamp on writing), otherwise mysterious crashes could occur for your guinea pigs already testing this new feature. http://tinyurl.com/hptwlqh La gatta frettolosa fa i gattini ciechi :) -- Enrico
Re: #10481: Hardening LyX against potential misuse
On 11/23/2016 04:27 PM, Tommaso Cucinotta wrote: > On 23/11/2016 01:11, Enrico Forestieri wrote: >>> 1. versioning of the preferences file...what else is needed? >> >> Have a look at c2a18fc1 to get an idea. > > ok, thanks, do we need also anything else on the prefs2prefs side? If > not, do we actually need to bump up the prefs file version? What if a > newer prefs file is open with an older LyX ? Yes, you should bump the prefs file version. We don't provide backwards conversions of preferences files. Note that this would be an issue only if someone downgraded from, e.g., 2.2.x to 2.1.x. Nice to see you back at work on LyX. This is stuff we really needed. Richard
Re: #10481: Hardening LyX against potential misuse
On 23/11/2016 01:11, Enrico Forestieri wrote: 1. versioning of the preferences file...what else is needed? Have a look at c2a18fc1 to get an idea. ok, thanks, do we need also anything else on the prefs2prefs side? If not, do we actually need to bump up the prefs file version? What if a newer prefs file is open with an older LyX ? 2. persistent storage of the auth_files strings set; my first pick would be a simple text file in the user's .lyx/ folder with one file-name per line. We already have .lyx/session. See src/Session.cpp. Cool, all the infrastructure was already there, just pushed a change along this line, so now we have a new SessionSection :-)! (LastFiles and AuthFiles have quite many similarities, but AuthFiles is even easier -- for now they don't share code) One note: one way to avoid the [auth session] section growing unbounded, might be to have an expiry timestamp, so that e.g., authorizations would expire in ~1y or so. This might be done with a section syntax like: instead of the simple we have now. Any further comment welcome, thanks! T.
Re: #10481: Hardening LyX against potential misuse
On Tue, Nov 22, 2016 at 11:59:37PM +0100, Tommaso Cucinotta wrote: > > There's a couple of TODOs left: > 1. versioning of the preferences file, I don't know much in this area, the > patch adds a couple of preferences options, what else is needed? Have a look at c2a18fc1 to get an idea. > 2. persistent storage of the auth_files strings set; my first pick would be a > simple text file in the user's .lyx/ folder with one file-name per line. We already have .lyx/session. See src/Session.cpp. -- Enrico
Re: #10481: Hardening LyX against potential misuse
Hi, thanks Enrico & Pavel, hopefully all comments integrated in the pushed version. There's a couple of TODOs left: 1. versioning of the preferences file, I don't know much in this area, the patch adds a couple of preferences options, what else is needed? 2. persistent storage of the auth_files strings set; my first pick would be a simple text file in the user's .lyx/ folder with one file-name per line. Any comments to the above? Thanks, T.
Re: #10481: Hardening LyX against potential misuse
On Mon, Nov 21, 2016 at 05:50:45PM -0800, Pavel Sanda wrote: > Tommaso Cucinotta wrote: > > On 21/11/2016 01:49, LyX Ticket Tracker wrote: > >> Comment (by t.cucinotta): > >> > >> Just worked out new separate patch-set for the cross-OS needauth security > >> option for converters (asking users if they really know what they're > >> about > >> to be doing). Added further global option that forbids use anyway, unless > >> user goes and unchecks it in the prefs. > >> > >> New patch is available in my user repo tommaso, branch features/needauth- > >> converters > > > > Please, find attached the pseudo-final patch. I would push it to master, > > unless there are objections / further comments for improvements. > > I quickly skimmed through the patch and have only minor comments: > - is it just visually misleading because of patches are stacked on top of > each other > or is there (almost) the same long warning string used repeatedly? > - IIRCS we have file pref version, do we increase it with new rc vars or no? > - was just curious why file name is needed in loader classes, got to > authFiles routine > and voila no comment :) could you be more specific than "///" when > introducing > new methods? :) > > Apart from these minors I would suggest you commit to master, it improves > the unfortunate situation with knitr a lot. > > > The new pref might also give us chance to reintroduce opening of hyperlink > insets. And perhaps the new idea of launching web browser for the citations > could be bound to unhardened lyx option as well. Some more minor comments: - Please, don't use "Yes" or "No" type of answers in requesters. Rather, formulate the question such that the answers are "Run", "Don't run", "Always run for this document", or the like. - static char suggestion[] = "Please, don't use very long strings, " "but exploit the compiler ability of " "concatenating adjacent string literals."; - Please, don't repeatedly use the same long string but find a way to have it appear only once in the source. This can be easily achieved by having it defined as a global static in a compilation unit. -- Enrico
Re: #10481: Hardening LyX against potential misuse
Tommaso Cucinotta wrote: > On 21/11/2016 01:49, LyX Ticket Tracker wrote: >> Comment (by t.cucinotta): >> >> Just worked out new separate patch-set for the cross-OS needauth security >> option for converters (asking users if they really know what they're >> about >> to be doing). Added further global option that forbids use anyway, unless >> user goes and unchecks it in the prefs. >> >> New patch is available in my user repo tommaso, branch features/needauth- >> converters > > Please, find attached the pseudo-final patch. I would push it to master, > unless there are objections / further comments for improvements. I quickly skimmed through the patch and have only minor comments: - is it just visually misleading because of patches are stacked on top of each other or is there (almost) the same long warning string used repeatedly? - IIRCS we have file pref version, do we increase it with new rc vars or no? - was just curious why file name is needed in loader classes, got to authFiles routine and voila no comment :) could you be more specific than "///" when introducing new methods? :) Apart from these minors I would suggest you commit to master, it improves the unfortunate situation with knitr a lot. The new pref might also give us chance to reintroduce opening of hyperlink insets. And perhaps the new idea of launching web browser for the citations could be bound to unhardened lyx option as well. Pavel
Re: #10481: Hardening LyX against potential misuse
On 21/11/2016 01:49, LyX Ticket Tracker wrote:
Comment (by t.cucinotta):
Just worked out new separate patch-set for the cross-OS needauth security
option for converters (asking users if they really know what they're about
to be doing). Added further global option that forbids use anyway, unless
user goes and unchecks it in the prefs.
New patch is available in my user repo tommaso, branch features/needauth-
converters
Please, find attached the pseudo-final patch. I would push it to master, unless
there are objections / further comments for improvements.
Note: the AppArmor stuff would come afterwards as a separate patch-set.
Thanks,
T.
>From cf0addda455879609aa42995d7c48327bcaf8f76 Mon Sep 17 00:00:00 2001
From: Tommaso Cucinotta
Date: Sat, 5 Nov 2016 01:00:44 +0100
Subject: [PATCH 1/4] Add needauth option to converters that need explicit user
authorization before being run.
Addressing #10481.
Converters marked with the new "needauth" option won't be run unless
the user gives explicit authorization, which is asked on-demand when
the converter is about to be run (question is not asked if the file is
cached and the converter is not needed).
The user prompt has a 3rd button (yes to all for document) so that
he/she's not prompted again for the same converter over the same document
(identified through buffer->filePath()).
---
lib/configure.py | 28 ++--
src/Converter.cpp | 27 ++-
src/Converter.h| 8
src/graphics/GraphicsConverter.cpp | 10 ++
4 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/lib/configure.py b/lib/configure.py
index a11050fb..b282a284 100644
--- a/lib/configure.py
+++ b/lib/configure.py
@@ -749,8 +749,8 @@ def checkConverterEntries():
rc_entry = [r'''\converter latex lyx"%% -f $$i $$o" ""
\converter latexclipboard lyx"%% -fixedenc utf8 -f $$i $$o" ""
\converter literate lyx"%% -n -m noweb -f $$i $$o" ""
-\converter sweave lyx"%% -n -m sweave -f $$i $$o" ""
-\converter knitr lyx"%% -n -m knitr -f $$i $$o" ""'''], not_found = 'tex2lyx')
+\converter sweave lyx"%% -n -m sweave -f $$i $$o" "needauth"
+\converter knitr lyx"%% -n -m knitr -f $$i $$o" "needauth"'''], not_found = 'tex2lyx')
if path == '':
logger.warning("Failed to find tex2lyx on your system.")
@@ -763,24 +763,24 @@ def checkConverterEntries():
\converter literate dviluatex "%%" ""'''])
#
checkProg('a Sweave -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxsweave.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter sweave latex "%%" ""
-\converter sweave pdflatex "%%" ""
-\converter sweave xetex "%%" ""
-\converter sweave luatex "%%" ""
-\converter sweave dviluatex "%%" ""'''])
+rc_entry = [r'''\converter sweave latex "%%" "needauth"
+\converter sweave pdflatex "%%" "needauth"
+\converter sweave xetex "%%" "needauth"
+\converter sweave luatex "%%" "needauth"
+\converter sweave dviluatex "%%" "needauth"'''])
#
checkProg('a knitr -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter knitr latex "%%" ""
-\converter knitr pdflatex "%%" ""
-\converter knitr xetex "%%" ""
-\converter knitr luatex "%%" ""
-\converter knitr dviluatex "%%" ""'''])
+rc_entry = [r'''\converter knitr latex "%%" "needauth"
+\converter knitr pdflatex "%%" "needauth"
+\converter knitr xetex "%%" "needauth"
+\converter knitr luatex "%%" "needauth"
+\converter knitr dviluatex "%%" "needauth"'''])
#
checkProg('a Sweave -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxstangle.R $$i $$e $$r'],
-rc_entry = [ r'\converter sweave r "%%"""' ])
+rc_entry = [ r'\converter sweave r "%%""needauth"' ])
#
checkProg('a knitr -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r tangle'],
-rc_entry = [ r'\converter knitr r "%%"""' ])
+rc_entry = [ r'\converter knitr r "%%""needauth"' ])
#
checkProg('an HTML -> LaTeX converter', ['html2latex $$i', 'gnuhtml2latex',
'htmltolatex -input $$i -output $$o', 'htmltolatex.jar -input $$i -output $$o'],
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 58e486e6..02631ca4 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -100,7 +100,7 @@ Converter::Converter(string const & f, string const & t,
string const & c, string const & l)
: from_(f), to_(t), command_(c), flags_(l),
From_(0), To_(0), latex_(false), xml_(false),
- need_aux_(false), nice_(false)
+ need_aux_
Re: #10481: Hardening LyX against potential misuse
On 07/11/2016 00:19, Richard Heck wrote: Questions: We're not supposed just to use "Yes" and "No", right? still missing the global option to suppress any questions (assume always yes or always no), and, I think also the persisting the chosen yes/no to disk (where? any hint? what about $HOME/.lyx/auth-files.txt containing a simple list of abs filenames where the user said "yes, and don't ask for this doc" ?) This changes the format of some file or other. Which one? Is this a format change in the relevant sense of "master only"? there's a new option among the flags/options of converters, without file format change; new shipped format would have those additional "needauth" flags set on known potentially harmful converters. + static const char securityWarning[] = This can just be a string yes? I don't think we usually use char[]. I'm ok with anything, is there a usual place where to place a shared string across 2 modules, like this? (noticed it got twice into the final lyx executable, perhaps as I'm compiling debug-mode) Thanks, T.
Re: #10481: Hardening LyX against potential misuse
Questions:
We're not supposed just to use "Yes" and "No", right?
This changes the format of some file or other. Which one? Is this a
format change in the relevant sense of "master only"?
> diff --git a/src/Converter.cpp b/src/Converter.cpp
> index 58e486e6..02631ca4 100644
> --- a/src/Converter.cpp
> +++ b/src/Converter.cpp
>
> + if (conv.need_auth()) {
> + static const char securityWarning[] =
This can just be a string yes? I don't think we usually use char[].
> diff --git a/src/graphics/GraphicsConverter.cpp
> b/src/graphics/GraphicsConverter.cpp
> index 67b1580f..6017073f 100644
> --- a/src/graphics/GraphicsConverter.cpp
> +++ b/src/graphics/GraphicsConverter.cpp
> @@ -15,6 +15,7 @@
> #include "Converter.h"
> #include "Format.h"
>
> +#include "frontends/alert.h"
> #include "support/lassert.h"
> #include "support/convert.h"
> #include "support/debug.h"
> @@ -372,6 +373,15 @@ static void build_script(string const & from_file,
> outfile = tempfile.name().toFilesystemEncoding();
> }
>
> + if (conv.need_auth()) {
> + static const char securityWarning[] =
Same here?
Richard
Re: #10481: Hardening LyX against potential misuse
last patch attached.
T.
On 06/11/2016 20:56, LyX Ticket Tracker wrote:
#10481: Hardening LyX against potential misuse
-+--
Reporter: t.cucinotta | Owner: lasgouttes
Type: enhancement | Status: new
Priority: highest | Milestone:
Component: general | Version: unspecified
Severity: major| Resolution:
Keywords: |
-+--
Comment (by t.cucinotta):
Just fixed minor things and the wording in the attached .2.patch file,
however there's the major issue that in GraphicsConverter there's no info
whatsoever about buffer name, so it won't be possible to have the "yes,
don't ask for the doc" option in that case, that case being display
on-screen, which is probably the most common/needed (the one that works ok
is when you export to PDF or other formats).
Tried to add the missing context in a separate patch, but got to nothing
but ... segfaults!
commit c5c02d37
Author: Tommaso Cucinotta
Date: Sat Nov 5 01:00:44 2016 +0100
Add needauth option to converters that need explicit user authorization before being run.
Addressing #10481.
Converters marked with the new "needauth" option won't be run unless
the user gives explicit authorization, which is asked on-demand when
the converter is about to be run (question is not asked if the file is
cached and the converter is not needed).
The user prompt has a 3rd button (yes to all for document) so that
he/she's not prompted again for the same converter over the same document
(identified through buffer->filePath()).
diff --git a/lib/configure.py b/lib/configure.py
index 09d053e9..705aed6c 100644
--- a/lib/configure.py
+++ b/lib/configure.py
@@ -749,8 +749,8 @@ def checkConverterEntries():
rc_entry = [r'''\converter latex lyx"%% -f $$i $$o" ""
\converter latexclipboard lyx"%% -fixedenc utf8 -f $$i $$o" ""
\converter literate lyx"%% -n -m noweb -f $$i $$o" ""
-\converter sweave lyx"%% -n -m sweave -f $$i $$o" ""
-\converter knitr lyx"%% -n -m knitr -f $$i $$o" ""'''], not_found = 'tex2lyx')
+\converter sweave lyx"%% -n -m sweave -f $$i $$o" "needauth"
+\converter knitr lyx"%% -n -m knitr -f $$i $$o" "needauth"'''], not_found = 'tex2lyx')
if path == '':
logger.warning("Failed to find tex2lyx on your system.")
@@ -763,24 +763,24 @@ def checkConverterEntries():
\converter literate dviluatex "%%" ""'''])
#
checkProg('a Sweave -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxsweave.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter sweave latex "%%" ""
-\converter sweave pdflatex "%%" ""
-\converter sweave xetex "%%" ""
-\converter sweave luatex "%%" ""
-\converter sweave dviluatex "%%" ""'''])
+rc_entry = [r'''\converter sweave latex "%%" "needauth"
+\converter sweave pdflatex "%%" "needauth"
+\converter sweave xetex "%%" "needauth"
+\converter sweave luatex "%%" "needauth"
+\converter sweave dviluatex "%%" "needauth"'''])
#
checkProg('a knitr -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter knitr latex "%%" ""
-\converter knitr pdflatex "%%" ""
-\converter knitr xetex "%%" ""
-\converter knitr luatex "%%" ""
-\converter knitr dviluatex "%%" ""'''])
+rc_entry = [r'''\converter knitr latex "%%" "needauth"
+\converter knitr pdflatex "%%" "needauth"
+\converter knitr xetex "%%" "needauth"
+\converter knitr luatex "%%" "needauth"
+\converter knitr dviluatex "%%" "needauth"'''])
#
checkProg('a Sweave -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxstangle.R $$i $$e $$r'],
-rc_entry = [ r'\converter sweave r "%%"""' ])
+rc_entry = [ r'\converter sweave r "%%""needauth"' ])
#
checkProg('a knitr -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r tangle'],
-rc_entry = [ r'\converter knitr r "%%"""' ])
+rc_entry = [ r'\converter knitr r "%%""needauth"' ])
#
checkProg('an HTML -> LaTeX converter', ['html2latex $$i', 'gnuhtml2latex',
'htmltolatex -input $$i -output $$o', 'htmltolatex.jar -input $$i -output $$o'],
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 58e486e6..02631ca4 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -100,7 +100,7 @@ Converter::Converter(string const & f, string const & t,
string const & c, string const & l)
: from_(f), to_(t), command_(c), flags_(l),
From_(0), To_(0), latex_(false), xml_(fals
Re: #10481: Hardening LyX against potential misuse
forgot to attach the patch.
T.
On 05/11/2016 02:34, LyX Ticket Tracker wrote:
#10481: Hardening LyX against potential misuse
-+--
Reporter: t.cucinotta | Owner: lasgouttes
Type: enhancement | Status: new
Priority: highest | Milestone:
Component: general | Version: unspecified
Severity: major| Resolution:
Keywords: |
-+--
Comment (by t.cucinotta):
I just uploaded 2nd revision of patch that solves my prior issue of the
question not being posted for on-screen display -- found the code in
GraphicsConverter.cpp duplicating behavior of Converter.cpp who knows why
(!) -- the only difference seems that GraphicsConverter likes Python
wrapping!
I had to repeat the same code snippet there as well, but it's in a point
where I have no context about the document buffer/file, so can't show the
"Yes to all for document" button in this case :-(!
Any comment welcome, thanks!
commit c05e9a52
Author: Tommaso Cucinotta
Date: Sat Nov 5 01:00:44 2016 +0100
Add needauth option to converters that need explicit user authorization before being run.
Addressing #10481.
Converters marked with the new "needauth" option won't be run unless
the user gives explicit authorization, which is asked on-demand when
the converter is about to be run (question is not asked if the file is
cached and the converter is not needed).
The user prompt has a 3rd button (yes to all for document) so that
he/she's not prompted again for the same converter over the same document
(identified through buffer->filePath()).
diff --git a/lib/configure.py b/lib/configure.py
index 09d053e9..705aed6c 100644
--- a/lib/configure.py
+++ b/lib/configure.py
@@ -749,8 +749,8 @@ def checkConverterEntries():
rc_entry = [r'''\converter latex lyx"%% -f $$i $$o" ""
\converter latexclipboard lyx"%% -fixedenc utf8 -f $$i $$o" ""
\converter literate lyx"%% -n -m noweb -f $$i $$o" ""
-\converter sweave lyx"%% -n -m sweave -f $$i $$o" ""
-\converter knitr lyx"%% -n -m knitr -f $$i $$o" ""'''], not_found = 'tex2lyx')
+\converter sweave lyx"%% -n -m sweave -f $$i $$o" "needauth"
+\converter knitr lyx"%% -n -m knitr -f $$i $$o" "needauth"'''], not_found = 'tex2lyx')
if path == '':
logger.warning("Failed to find tex2lyx on your system.")
@@ -763,24 +763,24 @@ def checkConverterEntries():
\converter literate dviluatex "%%" ""'''])
#
checkProg('a Sweave -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxsweave.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter sweave latex "%%" ""
-\converter sweave pdflatex "%%" ""
-\converter sweave xetex "%%" ""
-\converter sweave luatex "%%" ""
-\converter sweave dviluatex "%%" ""'''])
+rc_entry = [r'''\converter sweave latex "%%" "needauth"
+\converter sweave pdflatex "%%" "needauth"
+\converter sweave xetex "%%" "needauth"
+\converter sweave luatex "%%" "needauth"
+\converter sweave dviluatex "%%" "needauth"'''])
#
checkProg('a knitr -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter knitr latex "%%" ""
-\converter knitr pdflatex "%%" ""
-\converter knitr xetex "%%" ""
-\converter knitr luatex "%%" ""
-\converter knitr dviluatex "%%" ""'''])
+rc_entry = [r'''\converter knitr latex "%%" "needauth"
+\converter knitr pdflatex "%%" "needauth"
+\converter knitr xetex "%%" "needauth"
+\converter knitr luatex "%%" "needauth"
+\converter knitr dviluatex "%%" "needauth"'''])
#
checkProg('a Sweave -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxstangle.R $$i $$e $$r'],
-rc_entry = [ r'\converter sweave r "%%"""' ])
+rc_entry = [ r'\converter sweave r "%%""needauth"' ])
#
checkProg('a knitr -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r tangle'],
-rc_entry = [ r'\converter knitr r "%%"""' ])
+rc_entry = [ r'\converter knitr r "%%""needauth"' ])
#
checkProg('an HTML -> LaTeX converter', ['html2latex $$i', 'gnuhtml2latex',
'htmltolatex -input $$i -output $$o', 'htmltolatex.jar -input $$i -output $$o'],
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 58e486e6..e7687837 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -100,7 +100,7 @@ Converter::Converter(string const & f, string const & t,
string const & c, string const & l)
: from_(f), to_(t), command_(c), flags_(l),
From_(0), To
Re: #10481: Hardening LyX against potential misuse
On 11/04/2016 08:20 PM, Tommaso Cucinotta wrote:
> On 05/11/2016 01:16, LyX Ticket Tracker wrote:
>> Attached a first attempt at JMarc's proposed user prompt.
>
> here's the patch, for on-list discussion if preferred.
> @@ -402,6 +405,18 @@ bool Converters::convert(Buffer const * buffer,
> "tmpfile.out"));
> }
>
> + if (conv.need_auth()) {
> + std::string const & fname = buffer->filePath();
> + if (auth_files.find(fname) == auth_files.end()) {
> + int choice = frontend::Alert::prompt(
> + _("Confirm use of converter"),
> bformat(_("LyX is about to call converter '%1$s', which is potentially
> dangerous. Continue?"), from_utf8(conv.command())),
> + 0, 0, _("&No"), _("&Yes"), _("Yes to
> &all for document"));
I think this would normally be phrased as: "Yes, and do not ask again
for this document".
rh
Re: #10481: Hardening LyX against potential misuse
On 05/11/2016 01:16, LyX Ticket Tracker wrote:
Attached a first attempt at JMarc's proposed user prompt.
here's the patch, for on-list discussion if preferred.
T.
commit 2c24f0e0
Author: Tommaso Cucinotta
Date: Sat Nov 5 01:00:44 2016 +0100
Add needauth option to converters that need explicit user authorization before being run.
Addressing #10481.
diff --git a/lib/configure.py b/lib/configure.py
index 09d053e9..705aed6c 100644
--- a/lib/configure.py
+++ b/lib/configure.py
@@ -749,8 +749,8 @@ def checkConverterEntries():
rc_entry = [r'''\converter latex lyx"%% -f $$i $$o" ""
\converter latexclipboard lyx"%% -fixedenc utf8 -f $$i $$o" ""
\converter literate lyx"%% -n -m noweb -f $$i $$o" ""
-\converter sweave lyx"%% -n -m sweave -f $$i $$o" ""
-\converter knitr lyx"%% -n -m knitr -f $$i $$o" ""'''], not_found = 'tex2lyx')
+\converter sweave lyx"%% -n -m sweave -f $$i $$o" "needauth"
+\converter knitr lyx"%% -n -m knitr -f $$i $$o" "needauth"'''], not_found = 'tex2lyx')
if path == '':
logger.warning("Failed to find tex2lyx on your system.")
@@ -763,24 +763,24 @@ def checkConverterEntries():
\converter literate dviluatex "%%" ""'''])
#
checkProg('a Sweave -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxsweave.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter sweave latex "%%" ""
-\converter sweave pdflatex "%%" ""
-\converter sweave xetex "%%" ""
-\converter sweave luatex "%%" ""
-\converter sweave dviluatex "%%" ""'''])
+rc_entry = [r'''\converter sweave latex "%%" "needauth"
+\converter sweave pdflatex "%%" "needauth"
+\converter sweave xetex "%%" "needauth"
+\converter sweave luatex "%%" "needauth"
+\converter sweave dviluatex "%%" "needauth"'''])
#
checkProg('a knitr -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r'],
-rc_entry = [r'''\converter knitr latex "%%" ""
-\converter knitr pdflatex "%%" ""
-\converter knitr xetex "%%" ""
-\converter knitr luatex "%%" ""
-\converter knitr dviluatex "%%" ""'''])
+rc_entry = [r'''\converter knitr latex "%%" "needauth"
+\converter knitr pdflatex "%%" "needauth"
+\converter knitr xetex "%%" "needauth"
+\converter knitr luatex "%%" "needauth"
+\converter knitr dviluatex "%%" "needauth"'''])
#
checkProg('a Sweave -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxstangle.R $$i $$e $$r'],
-rc_entry = [ r'\converter sweave r "%%"""' ])
+rc_entry = [ r'\converter sweave r "%%""needauth"' ])
#
checkProg('a knitr -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r tangle'],
-rc_entry = [ r'\converter knitr r "%%"""' ])
+rc_entry = [ r'\converter knitr r "%%""needauth"' ])
#
checkProg('an HTML -> LaTeX converter', ['html2latex $$i', 'gnuhtml2latex',
'htmltolatex -input $$i -output $$o', 'htmltolatex.jar -input $$i -output $$o'],
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 58e486e6..e7687837 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -100,7 +100,7 @@ Converter::Converter(string const & f, string const & t,
string const & c, string const & l)
: from_(f), to_(t), command_(c), flags_(l),
From_(0), To_(0), latex_(false), xml_(false),
- need_aux_(false), nice_(false)
+ need_aux_(false), nice_(false), need_auth_(false)
{}
@@ -128,6 +128,8 @@ void Converter::readFlags()
parselog_ = flag_value;
else if (flag_name == "nice")
nice_ = true;
+ else if (flag_name == "needauth")
+ need_auth_ = true;
}
if (!result_dir_.empty() && result_file_.empty())
result_file_ = "index." + formats.extension(to_);
@@ -177,6 +179,7 @@ void Converters::add(string const & from, string const & to,
converter.setFlags(flags);
}
converter.readFlags();
+ lyxerr << "converter " << from << " " << to << " " << command << " " << flags << " needauth=" << converter.need_auth() << std::endl;
// The latex_command is used to update the .aux file when running
// a converter that uses it.
@@ -402,6 +405,18 @@ bool Converters::convert(Buffer const * buffer,
"tmpfile.out"));
}
+ if (conv.need_auth()) {
+ std::string const & fname = buffer->filePath();
+ if (auth_files.find(fname) == auth_files.end()) {
+int choice = frontend::Alert::prompt(
+ _("Confirm use of converter"), bformat(_("LyX is about to call converter '%1$s', which is potentially dangerous. Continue?"), from_utf8(conv.command())),
+ 0, 0, _("&No"), _("&Yes"), _("Yes to &all for document"));
+if (choice == 0)
+ return false;
+else if (choic
