List:Commits« Previous MessageNext Message »
From:Ingo Strüwing Date:February 9 2009 2:50pm
Subject:Re: bzr commit into mysql-6.0-backup branch (charles.bell:2758)
Bug#39581
View as plain text  
Hi Chuck,

State: Approved

Suggestions:
[1] While this is good information for reviewers, it becomes much less
useful when included into the repository forever. I strongly suggest to
remove it from the patch. It is sufficient to have it in the email to
reviewers, which could have CC commits@stripped.
[2] And the information is already present in the "renamed:" section.
[3] Please keep lines <= 80 columns. Multiple places.

Notes:
[4] Just for the records: IMHO this change gives the user a chance to
improve security. But it leaves room to screw the configuration up. IMHO
it is not helpful that "backupdir" can be configured outside of
"secure-backup-file-priv". And we allow to configure "secure-file-priv"
and "secure-backup-file-priv" the same.

Chuck Bell, 06.02.2009 17:43:
...

>  2758 Chuck Bell	2009-02-06
>       BUG#39581 : BACKUP file restrictions should be decoupled from FILE file
> restrictions
>       
>       This patch creeates a new --secure-backup-file-priv startup option and 
>       secure_backup_file_priv read only variable. This replaces the original 
>       use of the --secure-file-priv and associated variable. This change was 
>       needed to prevent exploitation of a security vulnerability by giving too 
>       much access to backup and restore. The new --secure-backup-file-priv 
>       allows administrators to restrict backup and restore to/from a specific 
>       directory.
>       
>       Attention: This patch contains three file moves. To apply this patch
>       you must first execute the following commands from the tree root:
>       
>       bzr mv ./mysql-test/suite/backup/t/backup_securefilepriv.test
> ./mysql-test/suite/backup/t/backup_securebackup.test
>       
>       bzr mv ./mysql-test/suite/backup/r/backup_securefilepriv.result
> ./mysql-test/suite/backup/r/backup_securebackup.result
>       
>       bzr mv ./mysql-test/suite/backup/t/backup_securefilepriv-master.opt
> ./mysql-test/suite/backup/t/backup_securebackup-master.opt


[1] While this is good information for reviewers, it becomes much less
useful when included into the repository forever. I strongly suggest to
remove it from the patch. It is sufficient to have it in the email to
reviewers, which could have CC commits@stripped.

> renamed:
>   mysql-test/suite/backup/r/backup_securefilepriv.result =>
> mysql-test/suite/backup/r/backup_securebackup.result
>   mysql-test/suite/backup/t/backup_securefilepriv-master.opt =>
> mysql-test/suite/backup/t/backup_securebackup-master.opt
>   mysql-test/suite/backup/t/backup_securefilepriv.test =>
> mysql-test/suite/backup/t/backup_securebackup.test


[2] And the information is already present in the "renamed:" section.

...
> --- a/mysql-test/suite/backup/t/backup_securefilepriv.test	2008-10-07 17:15:44 +0000
> +++ b/mysql-test/suite/backup/t/backup_securebackup.test	2009-02-06 16:43:10 +0000
> @@ -1,11 +1,12 @@
>  #
>  # Purpose: Backup images should only be allowed to be written to the
> -# path specified by --secure-file-priv option or a sub-path of it.
> +# path specified by --secure-backup-file-priv option or a sub-path of it.
>  #
> -# See backup_securefilepriv-master.opt for --secure-file-priv command line option
> +# See backup_securefilepriv-master.opt for --secure-backup-file-priv command line
> option


[3] Please keep lines <= 80 columns. Multiple places.

...
[4] Just for the records: This change gives the user a chance to improve
security. But it leaves room to screw the configuration up. IMHO it is
not helpful that "backupdir" can be configured outside of
"secure-backup-file-priv". And we allow to configure "secure-file-priv"
and "secure-backup-file-priv" the same.

Regards
Ingo
-- 
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Dr. Roland Bömer
Vorsitzender des Aufsichtsrates: Martin Häring   HRB München 161028

Thread
bzr commit into mysql-6.0-backup branch (charles.bell:2758) Bug#39581Chuck Bell6 Feb
  • Re: bzr commit into mysql-6.0-backup branch (charles.bell:2758)Bug#39581Ingo Strüwing9 Feb