Fix msfdb init command failure in systems that use the 'pg_ctl.rb' msfdb_helper #16094
Conversation
|
Instead of analyzing the configuration file to determine if the platform is Windows or Linux, can you just use I'm looking at PostgreSQL on Windows right now and it looks like |
|
Okay! Thats a really good idea. I'll work on it and make the commit! |
|
This idea really simplified the code, and works well in both the systems! Thanks! :D |
|
Any updates on this @smcintyre-r7 ? |
|
A note for anyone testing this, change need to be tested on macOS for x64 as |
|
I wasn't able to reproduce the original issue. From what you reported it sounds like it should be as simple as using a fresh install on Linux and running Can you provide some additional steps to reproduce the issue that this is intended to fix to demonstrate that the changes do so? |
|
Did you use the nightly-installers to install MSF or did you git clone the repo? |
|
It was from a git clone. I attached the docker file I used and the output. DockerfileWhat version of Linux were you using when you ran into the issue originally? Is it possible it was broken because the file system permissions on the /run directory are incorrect? |
|
Hello @smcintyre-r7 , I'm running Arch linux as my main machine and I am still able to reproduce this issue. For re-checking, I opened up my old laptop running Arch Linux as well, and this issue still persists there. (I'm using the latest version of MSF & linux both) Console output in old computer: Do you think this is something local to my computer? I guess gwillcox-r7 was also able to produce this in ubuntu.. (https://metasploit.slack.com/archives/CCN8L75UN/p1642017073011000?thread_ts=1642010086.009600&cid=CCN8L75UN) |
|
For what it's worth, I'm on Arch as well (well, Artix, but close enough). I installed MSF for the first time on it and was able to reproduce the issue. Modifying the files in |
Well possible, I'll try to reproduce it on ubuntu today then.. |
|
Some added context |
|
@lilithium-hydride thanks alot for chiming in. It's always super helpful to confirm an issue can reproduced by multiple people and I'll definitely take a much closer look at Arch now. |
|
I am also checking to reproduce this issue in an ubuntu VM.. If it happens to be an issue just local to Arch I guess we wouldn't need to make specific adjustments 😁 |
|
Hello, so I tried to replicate this issue in ubuntu, but failed to do so. I also just found why this issue isn't showing up in ubuntu.
run_cmd("pg_createcluster --user=$(whoami) -l #{@db.shellescape}/log -d #{@db.shellescape} -s /tmp --encoding=UTF8 #{@pg_version} #{options[:msf_db_name].shellescape} -- --username=$(whoami) --auth-host=trust --auth-local=trust")
run_cmd("initdb --auth-host=trust --auth-local=trust -E UTF8 #{@db.shellescape}")
The above point can be verified by running Ubuntu Output in debug mode:./msfdb init Running the 'init' command for the database: The database cluster will be initialized with locale "en_IN". Data page checksums are disabled. fixing permissions on existing directory /home/beleswar/.msf4/db ... ok Success. You can now start the database server using: Warning: The parent /var/run/postgresql of the selected success Arch Output in debug mode:./msfdb init Running the 'init' command for the database: The database cluster will be initialized with locale "en_US.UTF-8". Data page checksums are disabled. fixing permissions on existing directory /home/beleswar/.msf4/db ... ok Success. You can now start the database server using: run_cmd: cmd=pg_ctl -o "-p 5433" -D /home/beleswar/.msf4/db status, input=, env={} Notice this block in the Ubuntu output, and the cmd = pg_createcluster & -s /tmp flag here This verifies that Ubuntu uses the pg_ctlcluster.rb file which creates the socket at /tmp instead of the default location. Whereas in Arch, notice this block, cmd = initdb This verifies that Arch uses the pg_ctl.rb helper which does not change the default socket location, hence the error. Finally, As these changes are done in pg_ctlcluster.rb already, shouldn't we make them implemented in pg_ctl.rb as well (this PR) ? That would ensure that this issue isn't found again in any of the systems as we would have these 2 correct helpers only! |
|
As a short summary of the above message, the logs can be examined for clear insight. Ubuntu's postgresql log ( Arch's postgresql log ( So, Ubuntu's But for systems that use the |
3V3RYONE
changed the title
|
Another note about this I recently came across, some hardened environments mount |
|
Alright, I was able to confirm the original issue and that this patch fixes it. I'll approve it. @jmartin-r7 are there any outstanding items that need to be tested now before this is merged?
|
As you said, The @smcintyre-r7 , do you suggest replacing the |
|
Okay so if we could determine that /tmp isn't going to work because the dev object can't be created there are we thinking the script should still fail, or that we should come up with some other location automatically through some means to place the dev object? |
|
Hello!.. Sorry for being so late on this, I just finished my college exams and back after a long. yes, so first it checks if
I have implemented point 1 in the latest commit. The 2nd one may seem more optimal. However it comes with it's one drawbacks :
So I just wanted to ask your opinions. Which one should we implement? or any other suggestions? |
|
IMHO, most users would not specifically partition a So for users that specifically partition their |
|
I am not a fan of inspecting the My thought here would be to test if we can create a device in I believe |
|
Sure, thats a good idea, I'll work on it |
|
Hello, |
Does running msfconsole ask for the database service? I haven't encountered it yet. Can you please share your msfconsole output or a snapshot showing the steps! Thanks :) |
|
Hey @jmartin-r7 , I tried to look on the commands you said like Infact, the commands executed successfully to create a device in the Snapshot of output in Arch VM: Do you have any suggestions? or other specific commands? |
|
@jmartin-r7 Any update on this? This PR is currently blocked on your feedback. |
|
I am looking for a good pattern for detection, as is the pattern here is unreliable. Per my previously stated feedback. |
|
Can we move this forward with the partial fix for the most common cases since this issue is still affecting people while we continue to investigate a solution for all of the edge cases? |
|
The change here could impact working environments. Just adding the path to initial config startup is somewhat reasonable however the original issue is already an edge case for One issue here is usage of the database helpers when not in a supported OS. Landing as is could expand the issue and introduce impact to various supported platforms. Looking at possible testing patterns since this is performed only on db creation maybe the path forward is to create a test executable file and a test devices and interact with them as part for the decision processing for selecting location. This checks out for testing the I will see if there is more we can add to the |
|
Hey, thank you so much for the suggestion above. I was also looking for a good pattern to get this one landed. So, we can attempt to execute a write file for checking the |
|
Hello, I've added the commit to check if the Testing result in Arch Linux when Testing result in Arch Linux when Testing result in Ubuntu when |
lib/msfdb_helpers/pg_ctl.rb
Outdated
| def test_executable_file(path) | ||
| File.open("#{path}/msfdb_testfile", 'w') do |f| | ||
| f.puts "#!/bin/bash\necho exec" | ||
| end | ||
| File.chmod(0744, "#{path}/msfdb_testfile") | ||
|
|
||
| if run_cmd("#{path}/msfdb_testfile") | ||
| File.open("#{@db}/postgresql.conf", 'a') do |f| | ||
| f.puts "unix_socket_directories = \'#{path}\'" | ||
| end | ||
| @socket_directory = path | ||
| puts "Creating db socket file at #{path}" | ||
| end | ||
| end |
There was a problem hiding this comment.
Instead of using a side-effect to set @socket_directory consider adjusting this method to return a boolean result.
Also an ensure block should be added to remove the temporary file.
| def test_executable_file(path) | |
| File.open("#{path}/msfdb_testfile", 'w') do |f| | |
| f.puts "#!/bin/bash\necho exec" | |
| end | |
| File.chmod(0744, "#{path}/msfdb_testfile") | |
| if run_cmd("#{path}/msfdb_testfile") | |
| File.open("#{@db}/postgresql.conf", 'a') do |f| | |
| f.puts "unix_socket_directories = \'#{path}\'" | |
| end | |
| @socket_directory = path | |
| puts "Creating db socket file at #{path}" | |
| end | |
| end | |
| def test_executable_path(path) | |
| file_name = File.join(path, 'msfdb_testfile') | |
| File.open(file_name, 'w') do |f| | |
| f.puts "#!/bin/bash\necho exec" | |
| end | |
| File.chmod(0744, file_name) | |
| if run_cmd(file_name) | |
| File.open("#{@db}/postgresql.conf", 'a') do |f| | |
| f.puts "unix_socket_directories = \'#{path}\'" | |
| end | |
| @socket_directory = path | |
| puts "Creating db socket file at #{path}" | |
| end | |
| return true | |
| rescue => e | |
| return false | |
| ensure | |
| begin | |
| File.delete(file_name) | |
| rescue | |
| print_error("Unable to delete test file #{file_name}") | |
| end | |
| end |
There was a problem hiding this comment.
Thanks for the suggestions! Helped me make the code more normalized!
I've added the commit and tested again on Arch (with NOEXEC and without NOEXEC) and Ubuntu. It works fine!
|
Before Patch:
|
|
After patch:
|
|
LGTM will land this as soon as tests pass 👍 |
Release NotesA bug has been fixed in the |
This PR fixes #16086
Cause of bug
The cause of this issue is because of the default path of
unix_socket_directoriesin thepostgresql.conffile. The default path is set to/run/postgresql, and this is where the pg tries to create the socket file for starting the server. But the current user who runs./msfdb init(and notsudo ./msfdb init) does not have the privileges to access the/rundirectory. Hence, the socket file is not created and the server start up fails.Approach
There were two approaches to solve this issue. One was to run the commands as the super user, by changing the internal commands from
pg_ctl -D -l starttosudo pg_ctl -D -l start. This way, the super user could access the/run/postgresqldirectory and the socket could be created.However, the other and the better approach was to change the location of
unix_socket_directoriesfrom/run/postgresqlto/tmp. This way, the normal user can create the socket files and make the server started. So, I created an additional parameter@options[:unix_socket_directories]in the msfdb file which would contain the new socket path along with other db confs. and appended this updated directory path (along with the updated port 5433) in thepostgresql.conffile. Thedb_interface.rbfile was also updated, where I added the-h '/tmp'argument in thepsqlcommands to specify it the updated socket directory.This worked pretty well in linux but failed in windows. This was because there is no
/tmplocation in windows. So, to solve this, I first checked in thepostgresql.conffile, that whether the default location is set to/run/postgresqlor''(for windows), using a regex. Only in the case of a regex match, thepg_ctl.rbfile appends both updated path as well as updated port to thepostgresql.conffile, else it only appends the updated port. This seems to work well in both systems now!Before
After
Verification
msfdb,lib/msfdb_helpers/pg_ctl.rb,lib/msfdb_helpers/db_interface.rbfiles with the files in this PR.Note
The above changes are also tested in windows environment. The server startup failure was already present in windows environment.
Log: