Fix PXB-3141 - fix folder permission on fifo get

[altmannmarcelo]

https://jira.percona.com/browse/PXB-3141

Problem
When creating a folder without executing permission, a user gets limited action on that folder. For a folder, the x privilege means the user can cd into the folder.
Currently, if we are doing a xbcloud get and the folder does not exists, xbcloud will create it as 600. The same problem does no happens with xtrabackup as the thread stream-dir as target-dir, and that is created as 777.

Fix
Adjusted folder creation on ds_fifo.cc and xbcloud to use 777 as permission. The same mask is used on xtrabackup.

[altmannmarcelo]

https://pxb.cd.percona.com/job/percona-xtrabackup-8.0-test-param/369/ - failures are due to centos7 issues on bootstrap (fixed on trunk already) and rhel9 not reporting junit (fixed on trunk)

[satya-bodapati]

https://jira.percona.com/browse/PXB-3141

Problem When creating a folder without executing permission, a user gets limited action on that folder. For a folder, the x privilege means the user can cd into the folder. Currently, if we are doing a xbcloud get and the folder does not exists, xbcloud will create it as 600. The same problem does no happens with xtrabackup as the thread stream-dir as target-dir, and that is created as 777.

Fix Adjusted folder creation on ds_fifo.cc and xbcloud to use 777 as permission. The same mask is used on xtrabackup.

Hi @altmannmarcelo , I need help understanding the problem. Is the problem that the user cannot "cd" to the directory created by xbcloud and the fifo-stream-dir?

777 on stream files seems a bit too generous and can any other users attach to the FIFOs?

You are right that backup creates all directories with 777 and this also seems very liberal. Is xtrabackup mainly used as a root user?

[altmannmarcelo]

@satya-bodapati The key point here is that we are only changing the directory to 777,the files are still being created as 600.
Without execute privilege on the folder, xbcloud cannot create the fifo files inside it.

[satya-bodapati]

@satya-bodapati The key point here is that we are only changing the directory to 777,the files are still being created as 600. Without execute privilege on the folder, xbcloud cannot create the fifo files inside it.

and if I understand correctly, you are changing the permission of fifo_stream_dir only?

[altmannmarcelo]

and if I understand correctly, you are changing the permission of fifo_stream_dir only?

@satya-bodapati correct. Also, adjusting the test to cover this scenario - When we download the files we don't have the folder created.

[satya-bodapati]

@altmannmarcelo ok, thanks for confirming. Just out of curiosity, why did we find this now? It seems a very basic scenario and the test should have failed in the first commit? 🤔

[altmannmarcelo]

Just out of curiosity, why did we find this now? It seems a very basic scenario and the test should have failed in the first commit?

Yeah I had the same thought when I first saw this while doing some unrelated testing. There are two reasons for this not been caught under current testing and probably QA too:

1. For xtrabackup --backup we use set --target-dir with --fifo-dir at https://github.com/percona/percona-xtrabackup/blob/percona-xtrabackup-8.0.34-29/storage/innobase/xtrabackup/src/xtrabackup.cc#L4147 and we follow the normal xtrabackup --target-dir code path to create the directory, which already creates the folder as 777.
2. All tests do a backup then xbcloud get using the same fifo-dir folder. This means the folder is already created by xtrabackup and xbcloud is not creating it.

[satya-bodapati]

Just out of curiosity, why did we find this now? It seems a very basic scenario and the test should have failed in the first commit?

Yeah I had the same thought when I first saw this while doing some unrelated testing. There are two reasons for this not been caught under current testing and probably QA too:

1. For `xtrabackup --backup` we use set --target-dir with --fifo-dir at https://github.com/percona/percona-xtrabackup/blob/percona-xtrabackup-8.0.34-29/storage/innobase/xtrabackup/src/xtrabackup.cc#L4147 and we follow the normal xtrabackup --target-dir code path to create the directory, which already creates the folder as 777.

2. All tests do a backup then `xbcloud get` using the same fifo-dir folder. This means the folder is already created by xtrabackup and xbcloud is not creating it.

Thanks for the detailed explanation. Can you please add a test with different target-dir and --fifo-dir ?

[altmannmarcelo]

Just out of curiosity, why did we find this now? It seems a very basic scenario and the test should have failed in the first commit?

Yeah I had the same thought when I first saw this while doing some unrelated testing. There are two reasons for this not been caught under current testing and probably QA too:

1. For `xtrabackup --backup` we use set --target-dir with --fifo-dir at https://github.com/percona/percona-xtrabackup/blob/percona-xtrabackup-8.0.34-29/storage/innobase/xtrabackup/src/xtrabackup.cc#L4147 and we follow the normal xtrabackup --target-dir code path to create the directory, which already creates the folder as 777.

2. All tests do a backup then `xbcloud get` using the same fifo-dir folder. This means the folder is already created by xtrabackup and xbcloud is not creating it.

Thanks for the detailed explanation. Can you please add a test with different target-dir and --fifo-dir ?

--target-dir gets replaced unconditionally if --fifo-dir is set. It has no effect. I can add the test if you want, but it will not be covering anything :)