Re: pledge ffmpegthumbnailer
On Tue, Jun 14, 2016 at 06:47:25AM +0200, Sebastien Marie wrote:
> Hi,
>
> First, a bump of library version would be required as you added new
> exported functions.
>
> But I think your changes are too invasive for been patches for OpenBSD
> port tree.
>
> You should first deal with upstream for these changes, else it will be a
> shame for us to deal with future upgrade.
>
I doubt upstream would take it, the early libav calls are probably not
suitable for the library.
Would a simpler patch be acceptable?
This doesn't check for the entire kitchen-sink of protocols that libav
can use but does do a nice pledge for the typical use-cases, and I don't
think the fallback pledge is too bad (still avoids wpath and cpath).
--
Carlin
Index: graphics/ffmpegthumbnailer/Makefile
===
RCS file: /cvs/ports/graphics/ffmpegthumbnailer/Makefile,v
retrieving revision 1.27
diff -u -p -u -r1.27 Makefile
--- graphics/ffmpegthumbnailer/Makefile 11 Mar 2016 19:59:14 - 1.27
+++ graphics/ffmpegthumbnailer/Makefile 14 Jun 2016 08:37:32 -
@@ -3,17 +3,18 @@
COMMENT= lightweight video thumbnailer for file managers
DISTNAME= ffmpegthumbnailer-2.0.8
-REVISION= 3
+REVISION= 4
CATEGORIES=graphics multimedia
MASTER_SITES= https://ffmpegthumbnailer.googlecode.com/files/
-SHARED_LIBS= ffmpegthumbnailer 4.1
+SHARED_LIBS= ffmpegthumbnailer 4.2
HOMEPAGE= https://github.com/dirkvdb/ffmpegthumbnailer
# GPLv2+
PERMIT_PACKAGE_CDROM= Yes
+# uses pledge()
WANTLIB += avcodec avformat avutil c jpeg m png pthread stdc++
WANTLIB += swscale x264
Index:
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
===
RCS file:
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
diff -N
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
--- /dev/null 1 Jan 1970 00:00:00 -
+++
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
14 Jun 2016 08:37:32 -
@@ -0,0 +1,33 @@
+$OpenBSD$
+
+_generateThumbnail() provides a way to call the private
+generateThumbnail() method with a manually created ImageWriter, this is
+needed so pledge() can be called immediately after creating the
+ImageWriter but before the first frame of the video is decoded.
+
+Avoiding the stat() call on non-file protocols helps allow for dropping
+of the rpath promise when using sockets.
+
+--- libffmpegthumbnailer/videothumbnailer.cpp.orig Sun Aug 26 00:07:44 2012
libffmpegthumbnailer/videothumbnailer.cpp Tue Jun 14 19:34:24 2016
+@@ -177,6 +177,11 @@ void VideoThumbnailer::generateSmartThumbnail(MovieDec
+ videoFrame = videoFrames[bestFrame];
+ }
+
++void VideoThumbnailer::_generateThumbnail(const string& videoFile,
ImageWriter& imageWriter)
++{
++generateThumbnail(videoFile, imageWriter, NULL);
++}
++
+ void VideoThumbnailer::generateThumbnail(const string& videoFile,
ThumbnailerImageType type, const string& outputFile, AVFormatContext*
pAvContext)
+ {
+ ImageWriter* imageWriter = ImageWriterFactory::createImageWriter(type, outputFile);
+@@ -194,7 +199,7 @@ void VideoThumbnailer::generateThumbnail(const string&
+
+ void VideoThumbnailer::writeImage(const string& videoFile, ImageWriter&
imageWriter, const VideoFrame& videoFrame, int duration, vector&
rowPointers)
+ {
+-if (videoFile != "-")
++if (videoFile != "-" && (videoFile.find(":") == std::string::npos ||
videoFile.compare(0, 5, "file:") == 0))
+ {
+ struct stat statInfo;
+ if (stat(videoFile.c_str(), ) == 0)
Index:
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_h
===
RCS file:
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_h
diff -N
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_h
--- /dev/null 1 Jan 1970 00:00:00 -
+++
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_h
14 Jun 2016 08:37:32 -
@@ -0,0 +1,14 @@
+$OpenBSD$
+--- libffmpegthumbnailer/videothumbnailer.h.orig Sun Aug 26 00:07:44 2012
libffmpegthumbnailer/videothumbnailer.hTue Jun 14 19:17:04 2016
+@@ -46,6 +46,10 @@ class VideoThumbnailer (public)
+ void generateThumbnail(const std::string& videoFile, ThumbnailerImageType
type, const std::string& outputFile, AVFormatContext* pAvContext = NULL);
+ void generateThumbnail(const std::string& videoFile, ThumbnailerImageType
type, std::vector& buffer, AVFormatContext* pAvContext = NULL);
+
++/* this function is needed for pledge(2) and is not part of the
++ standard libffmpegthumbnailer interface */
++void _generateThumbnail(const std::string& videoFile, ImageWriter&
Re: pledge ffmpegthumbnailer
Hi, First, a bump of library version would be required as you added new exported functions. But I think your changes are too invasive for been patches for OpenBSD port tree. You should first deal with upstream for these changes, else it will be a shame for us to deal with future upgrade. Thanks. -- Sebastien Marie
pledge ffmpegthumbnailer
This thing has been used in the past as a vector to exploit ffmpeg
library vulneraiblities, and it's also unpledgeable without some
changes.
It opens the only file it's going to write early on, so if we could
pledge immediately after that it wouldn't need wpath or cpath, but the
public methods to generate the thumbnail automatically open the file for
writing and then begin decoding the video. We can't add pledge calls
inside those methods as they're part of libffmpegthumbnailer and other
programs might call them.
The input source can use any protocol that libavformat supports (and one
that it doesn't), so depending on the input it can need inet sockets or
unix sockets, or both.
This patch adds a public method to call the private method we need, so
the image can be opened for writing and then pledge happens and then it
goes on to decode the video.
It also adds a method to determine the protocol that libav is going to
use to access the input so an appropriate pledge can be picked. This
method uses libavformat's avio_find_protocol_name() and compares it
against known protocols that were obtained with avio_enum_protocols().
A call to av_register_all() is added early on. This sets up libav's
protocol associations (which is needed for avio_find_protocol_name() to
work) and also loads the codec libraries. This also prevents later calls
to av_register_all() from trying to read in the codec libraries. This
and a fix to a bogus stat(2) call, which would happen even when remote
protocols are being used, prevents rpath being needed for non-file
protocols.
The pledges:
Local files need rpath but no sockets.
Network protocols need dns and inet. Unix sockets need unix. Neither of
those need rpath.
This means, in normal use, if it's reading a file it doesn't get sockets
and if it's accessing sockets it doesn't get to read files. The
unfortunate exceptions being some non-standard protocols that libav can
use such as concat and crypto, which can use any and multiple protocols.
Without these changes the best pledge would be:
stdio rpath wpath cpath dns inet unix
--
Carlin
Index: graphics/ffmpegthumbnailer/Makefile
===
RCS file: /cvs/ports/graphics/ffmpegthumbnailer/Makefile,v
retrieving revision 1.27
diff -u -p -u -r1.27 Makefile
--- graphics/ffmpegthumbnailer/Makefile 11 Mar 2016 19:59:14 - 1.27
+++ graphics/ffmpegthumbnailer/Makefile 13 Jun 2016 16:03:49 -
@@ -3,7 +3,7 @@
COMMENT= lightweight video thumbnailer for file managers
DISTNAME= ffmpegthumbnailer-2.0.8
-REVISION= 3
+REVISION= 4
CATEGORIES=graphics multimedia
MASTER_SITES= https://ffmpegthumbnailer.googlecode.com/files/
@@ -14,6 +14,7 @@ HOMEPAGE= https://github.com/dirkvdb/ffm
# GPLv2+
PERMIT_PACKAGE_CDROM= Yes
+# uses pledge()
WANTLIB += avcodec avformat avutil c jpeg m png pthread stdc++
WANTLIB += swscale x264
Index:
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
===
RCS file:
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
diff -N
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
--- /dev/null 1 Jan 1970 00:00:00 -
+++
graphics/ffmpegthumbnailer/patches/patch-libffmpegthumbnailer_videothumbnailer_cpp
13 Jun 2016 16:03:49 -
@@ -0,0 +1,114 @@
+$OpenBSD$
+
+_generateThumbnail() provides a way to call the private
+generateThumbnail() method with a manually created ImageWriter, this is
+needed so pledge() can be called immediately after creating the
+ImageWriter but before the first frame of the video is decoded.
+
+_getProtocolType() provides a method to determine the pledge()
+requirements of the protocol that is being used.
+
+Avoiding the stat() call on non-file protocols helps allow for dropping
+of the rpath promise when using network protocols.
+
+--- libffmpegthumbnailer/videothumbnailer.cpp.orig Sun Aug 26 00:07:44 2012
libffmpegthumbnailer/videothumbnailer.cpp Tue Jun 14 03:55:54 2016
+@@ -37,6 +37,10 @@
+ #include
+ #include
+
++extern "C" {
++#include
++}
++
+ using namespace std;
+
+ namespace ffmpegthumbnailer
+@@ -177,6 +181,11 @@ void VideoThumbnailer::generateSmartThumbnail(MovieDec
+ videoFrame = videoFrames[bestFrame];
+ }
+
++void VideoThumbnailer::_generateThumbnail(const string& videoFile,
ImageWriter& imageWriter)
++{
++generateThumbnail(videoFile, imageWriter, NULL);
++}
++
+ void VideoThumbnailer::generateThumbnail(const string& videoFile,
ThumbnailerImageType type, const string& outputFile, AVFormatContext*
pAvContext)
+ {
+ ImageWriter* imageWriter = ImageWriterFactory::createImageWriter(type, outputFile);
+@@ -194,7 +203,7 @@ void VideoThumbnailer::generateThumbnail(const string&
+
+ void VideoThumbnailer::writeImage(const string& videoFile, ImageWriter&
imageWriter, const VideoFrame&
