Maximum length of command line reached error when saving job settings

[mauromol]

When I try to save the settings of my backup job, I'm getting the following error:

net.launchpad.backintime.LimitExceeded: Maximum length of command line reached

Here is a screenshot:

This is a SSH profile for which I never had problems saving settings in BackInTime < 1.2.

[mauromol]

Still having this problem :-( This must be a regression because I could successfully set up and edit that same backup job in the past. Also, the backup jobs, when running, works perfectly.

[mauromol]

Today I tried to recreate that backup profile from scratch, on two different systems. On a system with BIT 1.1.12 (BIT packaged with Linux Mint 19/Ubuntu 18.04), all works fine. On another system (Linux Mint 18/Ubuntu 16.04) having BIT 1.2.1 installed from your PPA, I get the "Maximum length of command line reached" error. Tried many combinations of settings (for example, I tried to empty the exclusion list, tried to play with backup profile settings, etc.), but could not find a solution. As I said, I even tried to create a new profile from scratch, settings just the SSH host/path settings and the inclusion of the home directory, still I get this error with BIT 1.2.1.

[mauromol]

I fixed this by changing https://github.com/bit-team/backintime/blob/7d24d1d07bf5ecb47990348dc9f291add52a6a49/qt/serviceHelper.py#L108

Setting max_cmd_len to 1000 fixes the problem.

If in your backup job you enable both ionice and nice, you break the 100 characters limit for the cron command (I don't know why and by which criteria this was set to just 100...). In fact, the cron command becomes something like this:

/usr/bin/nice -n19 /usr/bin/ionice -c2 -n7 /usr/bin/backintime --profile-id 1 backup-job >/dev/null 2>&1

which is 104 characters long.

In Ubuntu, that file is at /usr/share/backintime/qt/serviceHelper.py.

[buhtz]

Just documenting my own thoughts: Why does that hard coded limit self.max_cmd_len = 100 exist?

I see no reason against using ssMaxArgs.py to figuring out the real limit.

[aryoda]

The serviceHelper.py D-Bus system service has nothing to do with ssh and the restriction was introduced when the Suse programmer Matthias Gerstner fixed a CVE (!) in this D-Bus service (see Git blame and the last commit message):

limit maximum udev rule command line length

one more limit for hardening the serviceHelper.py: don't allow arbitrarily long command lines to be added.

But I agree: This length could be increased a little bit...

[buhtz]

There is fog in my head but maybe I have touched this topic somehow in the past? Isn't it fixed somehow? 👴

Dear @mauromol , can you reproduce this with the latest BIT version? If so can you please post the output of backintime --diagnostics.

[aryoda]

There is fog in my head but maybe I have touched this topic somehow in the past? Isn't it fixed somehow?

I felt the same but it isn't (I have just checked the code in the dev branch):

https://github.com/bit-team/backintime/blob/d645bee63a96503e0b9aa4b7b6f927baf367c713/qt/serviceHelper.py#L108

Let' fix it (1000 is too much and risky, the serviceHelper is running as root). How about 200?

[mauromol]

Sorry for the delay. I didn't have the chance to test, but I believe that the problem is still there if the limit is still at 100. What may make a too long limit risky? How was 100 been chosen (probably still unknown I guess)?

[mauromol]

Oh well, I installed 1.3.3 from the PPA and, guess what? It doesn't give me the error when I save settings. However, there's a difference. Previously, BIT was writing this to the crontab: /usr/bin/nice -n19 /usr/bin/ionice -c2 -n7 /usr/bin/backintime --profile-id 1 backup-job >/dev/null 2>&1 which is 104 characters long. Now it's writing this: /usr/bin/nice -n19 /usr/bin/ionice -c2 -n7 /usr/bin/backintime backup-job >/dev/null 2>&1 which is 89 characters. So, perhaps the --profile-id 1 is now smartly omitted because it's implicit. However, I would expect this to break as soon as my SSH backup is not the first one in the list of backup profiles...

By the way: the README here on GitHub still says 1.3.3 has not been released yet, when it talks about the incompatibility with recent rsync versions.

[buhtz]

About the README: I assume you saw the "old" version in the main branch. The newer and latest version is in the dev branch. We discussed this in the team and will make the dev branch the default. Then the landing page should show the latest README from the dev branch.

And thanks for checking the new version. Not sure how to interpret this. Maybe Germar as the most experienced BIT maintainer has a good explanation about it?

[mauromol]

About the README: I'm talking about the version of the README that is shown when you go to the code tab of this GitHub repository, hence at: https://github.com/bit-team/backintime So yes, I guess it's the one in the main branch, however consider that it's the default one shown when a user comes here and possibly the only official source of information for a user, since there's no BiT website any more (is there?).

[buhtz]

About the README: I'm talking about the version of the README that is shown when you go to the code tab of this GitHub repository, hence at: https://github.com/bit-team/backintime

Correct, that is the "landing page" and also BITs "official" project website.

So yes, I guess it's the one in the main branch

Currently it is, yes. We will change the default branch to "develop". Then you will see a more up to date README on the landing page.

[buhtz]

The missing --profile-id 1 is also no problem because it is used as default.

Closing because the problem seems to be solved or not reproducible with the latest BIT version.

Feel free to re-open or ask further questions.

[mauromol]

Hmmm... not sure this is really fixed. I guess it's just incidental that my backup profile is the first one and hence --profile-id 1 is omitted now, but if the maximum command line length is still 100, it will break for any profile != 1. 100 is really a too low limit here.

[buhtz]

Dear mauromol, thanks for reporting back.

Can you give us the paramters how do you setup the snapshot profile to reproduce this problem.

According to the code this limit seems to exists only in the context of Udev?

There must be a better solution instead of hard codening such a limit.

[mauromol]

Well, indeed, looking at the command-line above produced in the crontab, to break the 100 limit you just need to:

• setup a profile beyond the first one (the second or third, for instance)
• you have to schedule this to be executed at some time, so that it's added to the crontab
• you have to enable "run rsync with nice" as cron job and "run rsync with ionice" as cron job
• you have to enable redirect stdout to /dev/null in cronjob
• you have to enable redirect stderr to /dev/null in cronjob
• you have to try this in Ubuntu, so that nice, ionice and backtime executables are all in /usr/bin

The command line to be put into the crontab should then eventually become something like:

/usr/bin/nice -n19 /usr/bin/ionice -c2 -n7 /usr/bin/backintime --profile-id 2 backup-job >/dev/null 2>&1

which is 104 characters long. I don't think udev fits into the picture, at least it didn't when I opened this bug report. Honestly, I didn't check the code to see if something has changed meanwhile that causes this limit to be enforced only in the context of udev.

[buhtz]

I don't think udev fits into the picture, at least it didn't when I opened this bug report.

That is an important info. Then there might be a problem with the naming in the code because that limit is set in a class named UdevRules.

https://github.com/bit-team/backintime/blob/d645bee63a96503e0b9aa4b7b6f927baf367c713/qt/serviceHelper.py#L91-L108

We need further investigation on this.

[mauromol]

When I fixed the code myself locally, as described in https://github.com/bit-team/backintime/issues/1027#issuecomment-748616261, the limit was set in serviceHelper.py though.

[buhtz]

Now I'm getting confused.

When I fixed the code myself locally, as described in #1027 (comment), the limit was set in serviceHelper.py though.

You mean you modified the line in UdefRules.__init__()?

And how do you know and validated that "the limit was set in serviceHelper.py though"?

I still don't know what kind of snapshot profile you tried to setup: SSH, SSH encrypted, local, local encrypted? Source & destination path? Schedule options?

Can you for sure reproduce this? Then please do it with backintime-qt --debug (with the latest 1.3.3 version if possible) and show us that terminal output.

[Germar]

The exception net.launchpad.backintime.LimitExceeded is raised while checking/saving all profiles. Not just the current selected profile. So the screenshot in OPs first post is missleading. Not the current profile with mode SSH raised that exception but an other local or local_encfs profile.

If you would configure a profile with mode SSH and schedule When drive get connected (udev) you would get an error message Schedule udev doesn't work with mode SSH.

So the solution is to alter self.max_cmd_len = 100 to a value little higher (maybe 110).

[mauromol]

And how do you know and validated that "the limit was set in serviceHelper.py though"?

@buhtz I'm not quite sure what is still unclear. My original report mentioned the symptom, an exception thrown when I try to save my profile. I looked at the code to seek where that exception was thrown, I'm not even a Python developer but I found out the exact line I had to change to increase the limit that is causing the exception to be thrown. I also even highlighted where the problems lies, that is the length of the command line that Back in Time is trying to write in crontab. I even stated this is a regression in 1.2 compared to previous versions. I validated my change because Back in Time started to work again after I changed that line, as described in my previous comments. Given this information I also described how I think you should set up your BiT profiles to reproduce the problem. Isn't this information enough yet to determine what is going on?

Thanks to Germar to point out that all profiles are saved on confirmation and hence the problem might not be caused by the current profile but possibly by another one.

[mauromol]

So the solution is to alter self.max_cmd_len = 100 to a value little higher (maybe 110).

Isn't 110 still a bit strict? I was thinking about the case in which BiT is installed in a path longer than /usr/bin, you may hit again the limit (considering we are at 104 characters already).

[buhtz]

@buhtz I'm not quite sure what is still unclear.

That is why we are talking about. :smiley:

My original report mentioned the symptom, an exception thrown when I try to save my profile.

And my job as a maintainer is to try to reproduce it. I wasn't able to reproduce.

I validated my change because Back in Time started to work again after I changed that line

This was a misunderstanding on my site based on small weaknesses in English. That is why I ask back in a way that might look kind of stupid somehow. I try to rephrase things.

Thanks to Germar to point out that all profiles are saved on confirmation and hence the problem might not be caused by the current profile but possibly by another one.

That was an important key fact making things clearer.

I think it is obvious that I'm not the one who should implement something here. With my restricted knowledge about DBus, security and DBus integration in BIT my proposal would be to eliminate the max length restriction and validate the command syntactically. Check if there are only allowed commands (e.g. ionice) in it and if the rest are valid paths.

IMHO hard coded limits are not a good design decision and it wouldn't prevent injecting bad code. Using a syntactically validation of the command would make it harder (not impossible) to inject bad code no matter how long the cmd is.

[aryoda]

Isn't 110 still a bit strict? I was thinking about the case in which BiT is installed in a path longer than /usr/bin, you may hit again the limit (considering we are at 104 characters already).

I think it is time to agree on a number now and implement it ;-)

Given the fact that this is highly sensitive code (serviceHelper.py is running as root) it is important to be as restrictive as it is possible.

AFAIK we did not have other reports of problems with this limit so I suggest to increase the value from 100 to 120.

I could change this for the upcoming 1.4.0 release.

[buhtz]

Fine for me.

[emtiu]

I agree as well.

[aryoda]

Fixed by increasing the max cmd line from 100 to 120.

I close this issue now. Please open a new issue if things still go south.