[vlc-devel] [PATCH 1/2] dvdnav: Improve user information about insufficient permissions

[David Fuhrmann]

Hi,

We need the #ifdef in the header anyhow (due to the usage of builtin_available), so I would probably prefer to keep it as it is.

And for the module, with the current approach the compiler will probably optimise out the if block anyhow on other platforms.

BR. David

> Am 05.05.2020 um 10:07 schrieb Alexandre Janniaux <ajanni at videolabs.io>:
> 
> Hi,
> 
> I would probably put #ifdef around DVDProbeMacOSPermission
> instead of inside as the function seems to depicts a MacOS
> specificity.
> 
> Otherwise LGTM.
> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> 
> On Mon, May 04, 2020 at 08:09:13PM +0200, david.fuhrmann at gmail.com wrote:
>> From: David Fuhrmann <dfuhrmann at videolan.org>
>> 
>> The mentioned security setting is only relevant for accessing
>> RAW block devices (/dev/xxx), which is one of the main use cases
>> for this module (accessing an optical drive).
>> It is not relevant for file or folder access.
>> 
>> Probe for this case explicitly to inform the user only there.
>> 
>> This is only relevant starting with macOS Catalina.
>> ---
>> modules/access/Makefile.am  |  2 +-
>> modules/access/dvd_helper.h | 57 +++++++++++++++++++++++++++++++++++++
>> modules/access/dvdnav.c     | 19 ++++++++-----
>> 3 files changed, 70 insertions(+), 8 deletions(-)
>> create mode 100644 modules/access/dvd_helper.h
>> 
>> diff --git a/modules/access/Makefile.am b/modules/access/Makefile.am
>> index b73fda5f4c..265683c3f8 100644
>> --- a/modules/access/Makefile.am
>> +++ b/modules/access/Makefile.am
>> @@ -242,7 +242,7 @@ endif
>> EXTRA_LTLIBRARIES += libvcd_plugin.la
>> access_LTLIBRARIES += $(LTLIBvcd)
>> 
>> -libdvdnav_plugin_la_SOURCES = access/dvdnav.c demux/mpeg/ps.h demux/mpeg/pes.h
>> +libdvdnav_plugin_la_SOURCES = access/dvd_helper.h access/dvdnav.c demux/mpeg/ps.h demux/mpeg/pes.h
>> libdvdnav_plugin_la_CFLAGS = $(AM_CFLAGS) $(DVDNAV_CFLAGS)
>> libdvdnav_plugin_la_LIBADD = $(DVDNAV_LIBS)
>> libdvdnav_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(accessdir)'
>> diff --git a/modules/access/dvd_helper.h b/modules/access/dvd_helper.h
>> new file mode 100644
>> index 0000000000..403b44c542
>> --- /dev/null
>> +++ b/modules/access/dvd_helper.h
>> @@ -0,0 +1,57 @@
>> +/*****************************************************************************
>> + * dvd_helper.h: dvdnav / dvdread helper functions
>> + *****************************************************************************
>> + * Copyright (C) 2020 VLC authors and VideoLAN
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU Lesser General Public License as published by
>> + * the Free Software Foundation; either version 2.1 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public License
>> + * along with this program; if not, write to the Free Software Foundation,
>> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>> + *****************************************************************************/
>> +
>> +#include <sys/stat.h>
>> +#include <unistd.h>
>> +
>> +#include <vlc_fs.h>
>> +
>> +
>> +inline static int DVDProbeMacOSPermission( const char *psz_file )
>> +{
>> +#ifdef __APPLE__
>> +    /* Check is only relevant starting macOS Catalina */
>> +    if( __builtin_available( macOS 10.15, * ) )
>> +    {
>> +        /* Continue. The check above cannot be negated. */
>> +    }
>> +    else
>> +    {
>> +        return VLC_SUCCESS;
>> +    }
>> +
>> +    /* This is only relevant when accessing RAW block devices */
>> +    struct stat stat_buf;
>> +    if( vlc_stat( psz_file, &stat_buf ) != 0 )
>> +        return VLC_SUCCESS; // Continue with probing to be on the safe side
>> +
>> +    if( !S_ISBLK( stat_buf.st_mode ) && !S_ISCHR( stat_buf.st_mode ) )
>> +        return VLC_SUCCESS;
>> +
>> +    /* Check that device access in fact fails with EPERM error */
>> +    int retVal = access( psz_file, R_OK );
>> +    if( retVal == -1 && errno == EPERM )
>> +        return VLC_EGENERIC;
>> +
>> +    return VLC_SUCCESS;
>> +#else
>> +    return VLC_SUCCESS;
>> +#endif
>> +}
>> diff --git a/modules/access/dvdnav.c b/modules/access/dvdnav.c
>> index f082f1021b..a62950c605 100644
>> --- a/modules/access/dvdnav.c
>> +++ b/modules/access/dvdnav.c
>> @@ -65,6 +65,8 @@ dvdnav_status_t dvdnav_jump_to_sector_by_time(dvdnav_t *, uint64_t, int32_t);
>> #include "../demux/mpeg/ps.h"
>> #include "../demux/timestamps_filter.h"
>> 
>> +#include "dvd_helper.h"
>> +
>> /*****************************************************************************
>>  * Module descriptor
>>  *****************************************************************************/
>> @@ -364,6 +366,16 @@ static int AccessDemuxOpen ( vlc_object_t *p_this )
>>     if( !forced && ProbeDVD( psz_file ) != VLC_SUCCESS )
>>         goto bailout;
>> 
>> +    if( forced && DVDProbeMacOSPermission( psz_file ) != VLC_SUCCESS )
>> +    {
>> +        msg_Err( p_demux, "Path %s cannot be opened due to unsufficient permissions", psz_file );
>> +        vlc_dialog_display_error( p_demux, _("Problem accessing a system resource"),
>> +                                 _("Potentially, macOS blocks access to your disc. "
>> +                                   "Please open \"System Preferences\" -> \"Security & Privacy\" "
>> +                                   "and allow VLC to access your external media in \"Files and Folders\" section."));
>> +        goto bailout;
>> +    }
>> +
>>     /* Open dvdnav */
>>     psz_path = ToLocale( psz_file );
>> #if DVDNAV_VERSION >= 60100
>> @@ -375,13 +387,6 @@ static int AccessDemuxOpen ( vlc_object_t *p_this )
>> #endif
>>     {
>>         msg_Warn( p_demux, "cannot open DVD (%s)", psz_file);
>> -
>> -#ifdef __APPLE__
>> -        vlc_dialog_display_error( p_demux, _("Problem accessing a system resource"),
>> -            _("Potentially, macOS blocks access to your disc. "
>> -              "Please open \"System Preferences\" -> \"Security & Privacy\" "
>> -              "and allow VLC to access your external media in \"Files and Folders\" section."));
>> -#endif
>>         goto bailout;
>>     }
>> 
>> --
>> 2.21.1 (Apple Git-122.3)
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel