* [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random()
@ 2023-07-04 20:41 James Almer
2023-07-04 20:41 ` [FFmpeg-devel] [PATCH v2 2/2] avutil/random_seed: add support for gcrypt and OpenSSL as source of randomness James Almer
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: James Almer @ 2023-07-04 20:41 UTC (permalink / raw)
To: ffmpeg-devel
Uses the existing code for av_get_random_seed() to return a buffer with
cryptographically secure random data, or an error if none could be generated.
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavutil/random_seed.c | 54 ++++++++++++++++++++++++++++-------------
libavutil/random_seed.h | 12 +++++++++
2 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
index 66dd504ef0..0ed8f89cc6 100644
--- a/libavutil/random_seed.c
+++ b/libavutil/random_seed.c
@@ -46,20 +46,22 @@
#define TEST 0
#endif
-static int read_random(uint32_t *dst, const char *file)
-{
#if HAVE_UNISTD_H
+static int read_random(uint8_t *dst, size_t len, const char *file)
+{
int fd = avpriv_open(file, O_RDONLY);
- int err = -1;
+ ssize_t err = -1;
+ if (len > SSIZE_MAX)
+ return -1;
if (fd == -1)
return -1;
- err = read(fd, dst, sizeof(*dst));
+ err = read(fd, dst, len);
close(fd);
+ if (err == -1)
+ return AVERROR(errno);
- return err;
-#else
- return -1;
+ return err == len;
#endif
}
@@ -118,29 +120,47 @@ static uint32_t get_generic_seed(void)
return AV_RB32(digest) + AV_RB32(digest + 16);
}
-uint32_t av_get_random_seed(void)
+int av_random(uint8_t* buf, size_t len)
{
- uint32_t seed;
+ int err = AVERROR_UNKNOWN;
#if HAVE_BCRYPT
BCRYPT_ALG_HANDLE algo_handle;
NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
MS_PRIMITIVE_PROVIDER, 0);
if (BCRYPT_SUCCESS(ret)) {
- NTSTATUS ret = BCryptGenRandom(algo_handle, (UCHAR*)&seed, sizeof(seed), 0);
+ NTSTATUS ret = BCryptGenRandom(algo_handle, (PUCHAR)buf, len, 0);
BCryptCloseAlgorithmProvider(algo_handle, 0);
if (BCRYPT_SUCCESS(ret))
- return seed;
+ return 0;
}
#endif
#if HAVE_ARC4RANDOM
- return arc4random();
+ arc4random_buf(buf, len);
+ return 0;
+#endif
+
+#if HAVE_UNISTD_H
+ err = read_random(buf, len, "/dev/urandom");
+ if (err == 1)
+ return 0;
+ err = read_random(buf, len, "/dev/random");
+ if (err == 1)
+ return 0;
+ if (err == 0)
+ err = AVERROR_UNKNOWN;
#endif
- if (read_random(&seed, "/dev/urandom") == sizeof(seed))
- return seed;
- if (read_random(&seed, "/dev/random") == sizeof(seed))
- return seed;
- return get_generic_seed();
+ return err;
+}
+
+uint32_t av_get_random_seed(void)
+{
+ uint32_t seed;
+
+ if (av_random((uint8_t *)&seed, sizeof(seed)) < 0)
+ return get_generic_seed();
+
+ return seed;
}
diff --git a/libavutil/random_seed.h b/libavutil/random_seed.h
index 0462a048e0..ce982bb82f 100644
--- a/libavutil/random_seed.h
+++ b/libavutil/random_seed.h
@@ -36,6 +36,18 @@
*/
uint32_t av_get_random_seed(void);
+/**
+ * Generate cryptographically secure random data, i.e. suitable for use as
+ * encryption keys and similar.
+ *
+ * @param buf buffer into which the random data will be written
+ * @param len size of buf in bytes
+ *
+ * @retval 0 success, and len bytes of random data was written into buf, or
+ * a negative AVERROR code if random data could not be generated.
+ */
+int av_random(uint8_t* buf, size_t len);
+
/**
* @}
*/
--
2.41.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* [FFmpeg-devel] [PATCH v2 2/2] avutil/random_seed: add support for gcrypt and OpenSSL as source of randomness
2023-07-04 20:41 [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
@ 2023-07-04 20:41 ` James Almer
2023-07-04 20:47 ` [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: James Almer @ 2023-07-04 20:41 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
configure | 2 +-
libavutil/random_seed.c | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 107d533b3e..d6e78297fe 100755
--- a/configure
+++ b/configure
@@ -3892,7 +3892,7 @@ avfilter_deps="avutil"
avfilter_suggest="libm stdatomic"
avformat_deps="avcodec avutil"
avformat_suggest="libm network zlib stdatomic"
-avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
+avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx opencl openssl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
postproc_deps="avutil gpl"
postproc_suggest="libm stdatomic"
swresample_deps="avutil"
diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
index 0ed8f89cc6..363297bae8 100644
--- a/libavutil/random_seed.c
+++ b/libavutil/random_seed.c
@@ -30,6 +30,11 @@
#include <windows.h>
#include <bcrypt.h>
#endif
+#if CONFIG_GCRYPT
+#include <gcrypt.h>
+#elif CONFIG_OPENSSL
+#include <openssl/rand.h>
+#endif
#include <fcntl.h>
#include <math.h>
#include <time.h>
@@ -152,6 +157,15 @@ int av_random(uint8_t* buf, size_t len)
err = AVERROR_UNKNOWN;
#endif
+#if CONFIG_GCRYPT
+ gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
+ return 0;
+#elif CONFIG_OPENSSL
+ if (RAND_bytes(buf, len) == 1)
+ return 0;
+ err = AVERROR_EXTERNAL;
+#endif
+
return err;
}
--
2.41.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random()
2023-07-04 20:41 [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
2023-07-04 20:41 ` [FFmpeg-devel] [PATCH v2 2/2] avutil/random_seed: add support for gcrypt and OpenSSL as source of randomness James Almer
@ 2023-07-04 20:47 ` James Almer
2023-07-04 20:54 ` Hendrik Leppkes
2023-07-04 22:18 ` Marton Balint
3 siblings, 0 replies; 7+ messages in thread
From: James Almer @ 2023-07-04 20:47 UTC (permalink / raw)
To: ffmpeg-devel
On 7/4/2023 5:41 PM, James Almer wrote:
> Uses the existing code for av_get_random_seed() to return a buffer with
> cryptographically secure random data, or an error if none could be generated.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavutil/random_seed.c | 54 ++++++++++++++++++++++++++++-------------
> libavutil/random_seed.h | 12 +++++++++
> 2 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
> index 66dd504ef0..0ed8f89cc6 100644
> --- a/libavutil/random_seed.c
> +++ b/libavutil/random_seed.c
> @@ -46,20 +46,22 @@
> #define TEST 0
> #endif
>
> -static int read_random(uint32_t *dst, const char *file)
> -{
> #if HAVE_UNISTD_H
> +static int read_random(uint8_t *dst, size_t len, const char *file)
> +{
> int fd = avpriv_open(file, O_RDONLY);
> - int err = -1;
> + ssize_t err = -1;
>
> + if (len > SSIZE_MAX)
> + return -1;
> if (fd == -1)
> return -1;
> - err = read(fd, dst, sizeof(*dst));
> + err = read(fd, dst, len);
> close(fd);
> + if (err == -1)
> + return AVERROR(errno);
>
> - return err;
> -#else
> - return -1;
> + return err == len;
> #endif
> }
>
> @@ -118,29 +120,47 @@ static uint32_t get_generic_seed(void)
> return AV_RB32(digest) + AV_RB32(digest + 16);
> }
>
> -uint32_t av_get_random_seed(void)
> +int av_random(uint8_t* buf, size_t len)
> {
> - uint32_t seed;
> + int err = AVERROR_UNKNOWN;
Changed this locally to AVERROR(ENOSYS), which is the more correct error
value if nothing below is available.
Will send a v3 with that change after this one is reviewed anyway since
this code is delicate and i want to push exactly what was approved.
>
> #if HAVE_BCRYPT
> BCRYPT_ALG_HANDLE algo_handle;
> NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
> MS_PRIMITIVE_PROVIDER, 0);
> if (BCRYPT_SUCCESS(ret)) {
> - NTSTATUS ret = BCryptGenRandom(algo_handle, (UCHAR*)&seed, sizeof(seed), 0);
> + NTSTATUS ret = BCryptGenRandom(algo_handle, (PUCHAR)buf, len, 0);
> BCryptCloseAlgorithmProvider(algo_handle, 0);
> if (BCRYPT_SUCCESS(ret))
> - return seed;
> + return 0;
> }
> #endif
>
> #if HAVE_ARC4RANDOM
> - return arc4random();
> + arc4random_buf(buf, len);
> + return 0;
> +#endif
> +
> +#if HAVE_UNISTD_H
> + err = read_random(buf, len, "/dev/urandom");
> + if (err == 1)
> + return 0;
> + err = read_random(buf, len, "/dev/random");
> + if (err == 1)
> + return 0;
> + if (err == 0)
> + err = AVERROR_UNKNOWN;
> #endif
>
> - if (read_random(&seed, "/dev/urandom") == sizeof(seed))
> - return seed;
> - if (read_random(&seed, "/dev/random") == sizeof(seed))
> - return seed;
> - return get_generic_seed();
> + return err;
> +}
> +
> +uint32_t av_get_random_seed(void)
> +{
> + uint32_t seed;
> +
> + if (av_random((uint8_t *)&seed, sizeof(seed)) < 0)
> + return get_generic_seed();
> +
> + return seed;
> }
> diff --git a/libavutil/random_seed.h b/libavutil/random_seed.h
> index 0462a048e0..ce982bb82f 100644
> --- a/libavutil/random_seed.h
> +++ b/libavutil/random_seed.h
> @@ -36,6 +36,18 @@
> */
> uint32_t av_get_random_seed(void);
>
> +/**
> + * Generate cryptographically secure random data, i.e. suitable for use as
> + * encryption keys and similar.
> + *
> + * @param buf buffer into which the random data will be written
> + * @param len size of buf in bytes
> + *
> + * @retval 0 success, and len bytes of random data was written into buf, or
> + * a negative AVERROR code if random data could not be generated.
> + */
> +int av_random(uint8_t* buf, size_t len);
> +
> /**
> * @}
> */
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random()
2023-07-04 20:41 [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
2023-07-04 20:41 ` [FFmpeg-devel] [PATCH v2 2/2] avutil/random_seed: add support for gcrypt and OpenSSL as source of randomness James Almer
2023-07-04 20:47 ` [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
@ 2023-07-04 20:54 ` Hendrik Leppkes
2023-07-04 21:35 ` James Almer
2023-07-04 22:18 ` Marton Balint
3 siblings, 1 reply; 7+ messages in thread
From: Hendrik Leppkes @ 2023-07-04 20:54 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Tue, Jul 4, 2023 at 10:41 PM James Almer <jamrial@gmail.com> wrote:
>
> Uses the existing code for av_get_random_seed() to return a buffer with
> cryptographically secure random data, or an error if none could be generated.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavutil/random_seed.c | 54 ++++++++++++++++++++++++++++-------------
> libavutil/random_seed.h | 12 +++++++++
> 2 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
> index 66dd504ef0..0ed8f89cc6 100644
> --- a/libavutil/random_seed.c
> +++ b/libavutil/random_seed.c
> @@ -46,20 +46,22 @@
> #define TEST 0
> #endif
>
> -static int read_random(uint32_t *dst, const char *file)
> -{
> #if HAVE_UNISTD_H
> +static int read_random(uint8_t *dst, size_t len, const char *file)
> +{
> int fd = avpriv_open(file, O_RDONLY);
> - int err = -1;
> + ssize_t err = -1;
>
> + if (len > SSIZE_MAX)
> + return -1;
> if (fd == -1)
> return -1;
> - err = read(fd, dst, sizeof(*dst));
> + err = read(fd, dst, len);
> close(fd);
> + if (err == -1)
> + return AVERROR(errno);
>
> - return err;
> -#else
> - return -1;
> + return err == len;
> #endif
> }
>
> @@ -118,29 +120,47 @@ static uint32_t get_generic_seed(void)
> return AV_RB32(digest) + AV_RB32(digest + 16);
> }
>
> -uint32_t av_get_random_seed(void)
> +int av_random(uint8_t* buf, size_t len)
> {
> - uint32_t seed;
> + int err = AVERROR_UNKNOWN;
>
> #if HAVE_BCRYPT
> BCRYPT_ALG_HANDLE algo_handle;
> NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
> MS_PRIMITIVE_PROVIDER, 0);
> if (BCRYPT_SUCCESS(ret)) {
> - NTSTATUS ret = BCryptGenRandom(algo_handle, (UCHAR*)&seed, sizeof(seed), 0);
> + NTSTATUS ret = BCryptGenRandom(algo_handle, (PUCHAR)buf, len, 0);
> BCryptCloseAlgorithmProvider(algo_handle, 0);
> if (BCRYPT_SUCCESS(ret))
> - return seed;
> + return 0;
> }
> #endif
>
> #if HAVE_ARC4RANDOM
> - return arc4random();
> + arc4random_buf(buf, len);
> + return 0;
> +#endif
> +
> +#if HAVE_UNISTD_H
> + err = read_random(buf, len, "/dev/urandom");
> + if (err == 1)
> + return 0;
> + err = read_random(buf, len, "/dev/random");
> + if (err == 1)
> + return 0;
> + if (err == 0)
> + err = AVERROR_UNKNOWN;
> #endif
>
> - if (read_random(&seed, "/dev/urandom") == sizeof(seed))
> - return seed;
> - if (read_random(&seed, "/dev/random") == sizeof(seed))
> - return seed;
> - return get_generic_seed();
> + return err;
> +}
> +
> +uint32_t av_get_random_seed(void)
> +{
> + uint32_t seed;
> +
> + if (av_random((uint8_t *)&seed, sizeof(seed)) < 0)
> + return get_generic_seed();
> +
> + return seed;
> }
> diff --git a/libavutil/random_seed.h b/libavutil/random_seed.h
> index 0462a048e0..ce982bb82f 100644
> --- a/libavutil/random_seed.h
> +++ b/libavutil/random_seed.h
> @@ -36,6 +36,18 @@
> */
> uint32_t av_get_random_seed(void);
>
> +/**
> + * Generate cryptographically secure random data, i.e. suitable for use as
> + * encryption keys and similar.
> + *
> + * @param buf buffer into which the random data will be written
> + * @param len size of buf in bytes
> + *
> + * @retval 0 success, and len bytes of random data was written into buf, or
> + * a negative AVERROR code if random data could not be generated.
> + */
> +int av_random(uint8_t* buf, size_t len);
av_random seems like a pretty generic name for something thats
requiring to be cryptographically secure and otherwise fail. I would
expect a more specific name for that purpose. There is plenty other
uses of randomness in multimedia, noise, dithering, etc, which don't
need crypto security. The function doesn't have to handle those, but
maybe it should be specific in what it does handle?
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random()
2023-07-04 20:54 ` Hendrik Leppkes
@ 2023-07-04 21:35 ` James Almer
2023-07-04 23:32 ` Michael Niedermayer
0 siblings, 1 reply; 7+ messages in thread
From: James Almer @ 2023-07-04 21:35 UTC (permalink / raw)
To: ffmpeg-devel
On 7/4/2023 5:54 PM, Hendrik Leppkes wrote:
> On Tue, Jul 4, 2023 at 10:41 PM James Almer <jamrial@gmail.com> wrote:
>>
>> Uses the existing code for av_get_random_seed() to return a buffer with
>> cryptographically secure random data, or an error if none could be generated.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavutil/random_seed.c | 54 ++++++++++++++++++++++++++++-------------
>> libavutil/random_seed.h | 12 +++++++++
>> 2 files changed, 49 insertions(+), 17 deletions(-)
>>
>> diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
>> index 66dd504ef0..0ed8f89cc6 100644
>> --- a/libavutil/random_seed.c
>> +++ b/libavutil/random_seed.c
>> @@ -46,20 +46,22 @@
>> #define TEST 0
>> #endif
>>
>> -static int read_random(uint32_t *dst, const char *file)
>> -{
>> #if HAVE_UNISTD_H
>> +static int read_random(uint8_t *dst, size_t len, const char *file)
>> +{
>> int fd = avpriv_open(file, O_RDONLY);
>> - int err = -1;
>> + ssize_t err = -1;
>>
>> + if (len > SSIZE_MAX)
>> + return -1;
>> if (fd == -1)
>> return -1;
>> - err = read(fd, dst, sizeof(*dst));
>> + err = read(fd, dst, len);
>> close(fd);
>> + if (err == -1)
>> + return AVERROR(errno);
>>
>> - return err;
>> -#else
>> - return -1;
>> + return err == len;
>> #endif
>> }
>>
>> @@ -118,29 +120,47 @@ static uint32_t get_generic_seed(void)
>> return AV_RB32(digest) + AV_RB32(digest + 16);
>> }
>>
>> -uint32_t av_get_random_seed(void)
>> +int av_random(uint8_t* buf, size_t len)
>> {
>> - uint32_t seed;
>> + int err = AVERROR_UNKNOWN;
>>
>> #if HAVE_BCRYPT
>> BCRYPT_ALG_HANDLE algo_handle;
>> NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
>> MS_PRIMITIVE_PROVIDER, 0);
>> if (BCRYPT_SUCCESS(ret)) {
>> - NTSTATUS ret = BCryptGenRandom(algo_handle, (UCHAR*)&seed, sizeof(seed), 0);
>> + NTSTATUS ret = BCryptGenRandom(algo_handle, (PUCHAR)buf, len, 0);
>> BCryptCloseAlgorithmProvider(algo_handle, 0);
>> if (BCRYPT_SUCCESS(ret))
>> - return seed;
>> + return 0;
>> }
>> #endif
>>
>> #if HAVE_ARC4RANDOM
>> - return arc4random();
>> + arc4random_buf(buf, len);
>> + return 0;
>> +#endif
>> +
>> +#if HAVE_UNISTD_H
>> + err = read_random(buf, len, "/dev/urandom");
>> + if (err == 1)
>> + return 0;
>> + err = read_random(buf, len, "/dev/random");
>> + if (err == 1)
>> + return 0;
>> + if (err == 0)
>> + err = AVERROR_UNKNOWN;
>> #endif
>>
>> - if (read_random(&seed, "/dev/urandom") == sizeof(seed))
>> - return seed;
>> - if (read_random(&seed, "/dev/random") == sizeof(seed))
>> - return seed;
>> - return get_generic_seed();
>> + return err;
>> +}
>> +
>> +uint32_t av_get_random_seed(void)
>> +{
>> + uint32_t seed;
>> +
>> + if (av_random((uint8_t *)&seed, sizeof(seed)) < 0)
>> + return get_generic_seed();
>> +
>> + return seed;
>> }
>> diff --git a/libavutil/random_seed.h b/libavutil/random_seed.h
>> index 0462a048e0..ce982bb82f 100644
>> --- a/libavutil/random_seed.h
>> +++ b/libavutil/random_seed.h
>> @@ -36,6 +36,18 @@
>> */
>> uint32_t av_get_random_seed(void);
>>
>> +/**
>> + * Generate cryptographically secure random data, i.e. suitable for use as
>> + * encryption keys and similar.
>> + *
>> + * @param buf buffer into which the random data will be written
>> + * @param len size of buf in bytes
>> + *
>> + * @retval 0 success, and len bytes of random data was written into buf, or
>> + * a negative AVERROR code if random data could not be generated.
>> + */
>> +int av_random(uint8_t* buf, size_t len);
>
> av_random seems like a pretty generic name for something thats
> requiring to be cryptographically secure and otherwise fail. I would
> expect a more specific name for that purpose. There is plenty other
> uses of randomness in multimedia, noise, dithering, etc, which don't
> need crypto security. The function doesn't have to handle those, but
> maybe it should be specific in what it does handle?
Maybe av_random_buf()? I don't want too much bikeshedding on the name.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random()
2023-07-04 20:41 [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
` (2 preceding siblings ...)
2023-07-04 20:54 ` Hendrik Leppkes
@ 2023-07-04 22:18 ` Marton Balint
3 siblings, 0 replies; 7+ messages in thread
From: Marton Balint @ 2023-07-04 22:18 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Tue, 4 Jul 2023, James Almer wrote:
> Uses the existing code for av_get_random_seed() to return a buffer with
> cryptographically secure random data, or an error if none could be generated.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavutil/random_seed.c | 54 ++++++++++++++++++++++++++++-------------
> libavutil/random_seed.h | 12 +++++++++
> 2 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
> index 66dd504ef0..0ed8f89cc6 100644
> --- a/libavutil/random_seed.c
> +++ b/libavutil/random_seed.c
> @@ -46,20 +46,22 @@
> #define TEST 0
> #endif
>
> -static int read_random(uint32_t *dst, const char *file)
> -{
> #if HAVE_UNISTD_H
> +static int read_random(uint8_t *dst, size_t len, const char *file)
Maybe it is cleaner if you also use ffmpeg error codes for this function,
and no special -1, 1 or 0 return value.
> +{
> int fd = avpriv_open(file, O_RDONLY);
> - int err = -1;
> + ssize_t err = -1;
>
> + if (len > SSIZE_MAX)
> + return -1;
> if (fd == -1)
> return -1;
> - err = read(fd, dst, sizeof(*dst));
> + err = read(fd, dst, len);
As Anton pointed out, this can read less than requested. I suggest using
avpriv_fopen_utf8() and fread(), that should loop reading internally
until the whole length is filled.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random()
2023-07-04 21:35 ` James Almer
@ 2023-07-04 23:32 ` Michael Niedermayer
0 siblings, 0 replies; 7+ messages in thread
From: Michael Niedermayer @ 2023-07-04 23:32 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 5286 bytes --]
On Tue, Jul 04, 2023 at 06:35:06PM -0300, James Almer wrote:
> On 7/4/2023 5:54 PM, Hendrik Leppkes wrote:
> > On Tue, Jul 4, 2023 at 10:41 PM James Almer <jamrial@gmail.com> wrote:
> > >
> > > Uses the existing code for av_get_random_seed() to return a buffer with
> > > cryptographically secure random data, or an error if none could be generated.
> > >
> > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > ---
> > > libavutil/random_seed.c | 54 ++++++++++++++++++++++++++++-------------
> > > libavutil/random_seed.h | 12 +++++++++
> > > 2 files changed, 49 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
> > > index 66dd504ef0..0ed8f89cc6 100644
> > > --- a/libavutil/random_seed.c
> > > +++ b/libavutil/random_seed.c
> > > @@ -46,20 +46,22 @@
> > > #define TEST 0
> > > #endif
> > >
> > > -static int read_random(uint32_t *dst, const char *file)
> > > -{
> > > #if HAVE_UNISTD_H
> > > +static int read_random(uint8_t *dst, size_t len, const char *file)
> > > +{
> > > int fd = avpriv_open(file, O_RDONLY);
> > > - int err = -1;
> > > + ssize_t err = -1;
> > >
> > > + if (len > SSIZE_MAX)
> > > + return -1;
> > > if (fd == -1)
> > > return -1;
> > > - err = read(fd, dst, sizeof(*dst));
> > > + err = read(fd, dst, len);
> > > close(fd);
> > > + if (err == -1)
> > > + return AVERROR(errno);
> > >
> > > - return err;
> > > -#else
> > > - return -1;
> > > + return err == len;
> > > #endif
> > > }
> > >
> > > @@ -118,29 +120,47 @@ static uint32_t get_generic_seed(void)
> > > return AV_RB32(digest) + AV_RB32(digest + 16);
> > > }
> > >
> > > -uint32_t av_get_random_seed(void)
> > > +int av_random(uint8_t* buf, size_t len)
> > > {
> > > - uint32_t seed;
> > > + int err = AVERROR_UNKNOWN;
> > >
> > > #if HAVE_BCRYPT
> > > BCRYPT_ALG_HANDLE algo_handle;
> > > NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
> > > MS_PRIMITIVE_PROVIDER, 0);
> > > if (BCRYPT_SUCCESS(ret)) {
> > > - NTSTATUS ret = BCryptGenRandom(algo_handle, (UCHAR*)&seed, sizeof(seed), 0);
> > > + NTSTATUS ret = BCryptGenRandom(algo_handle, (PUCHAR)buf, len, 0);
> > > BCryptCloseAlgorithmProvider(algo_handle, 0);
> > > if (BCRYPT_SUCCESS(ret))
> > > - return seed;
> > > + return 0;
> > > }
> > > #endif
> > >
> > > #if HAVE_ARC4RANDOM
> > > - return arc4random();
> > > + arc4random_buf(buf, len);
> > > + return 0;
> > > +#endif
> > > +
> > > +#if HAVE_UNISTD_H
> > > + err = read_random(buf, len, "/dev/urandom");
> > > + if (err == 1)
> > > + return 0;
> > > + err = read_random(buf, len, "/dev/random");
> > > + if (err == 1)
> > > + return 0;
> > > + if (err == 0)
> > > + err = AVERROR_UNKNOWN;
> > > #endif
> > >
> > > - if (read_random(&seed, "/dev/urandom") == sizeof(seed))
> > > - return seed;
> > > - if (read_random(&seed, "/dev/random") == sizeof(seed))
> > > - return seed;
> > > - return get_generic_seed();
> > > + return err;
> > > +}
> > > +
> > > +uint32_t av_get_random_seed(void)
> > > +{
> > > + uint32_t seed;
> > > +
> > > + if (av_random((uint8_t *)&seed, sizeof(seed)) < 0)
> > > + return get_generic_seed();
> > > +
> > > + return seed;
> > > }
> > > diff --git a/libavutil/random_seed.h b/libavutil/random_seed.h
> > > index 0462a048e0..ce982bb82f 100644
> > > --- a/libavutil/random_seed.h
> > > +++ b/libavutil/random_seed.h
> > > @@ -36,6 +36,18 @@
> > > */
> > > uint32_t av_get_random_seed(void);
> > >
> > > +/**
> > > + * Generate cryptographically secure random data, i.e. suitable for use as
> > > + * encryption keys and similar.
> > > + *
> > > + * @param buf buffer into which the random data will be written
> > > + * @param len size of buf in bytes
> > > + *
> > > + * @retval 0 success, and len bytes of random data was written into buf, or
> > > + * a negative AVERROR code if random data could not be generated.
> > > + */
> > > +int av_random(uint8_t* buf, size_t len);
> >
> > av_random seems like a pretty generic name for something thats
> > requiring to be cryptographically secure and otherwise fail. I would
> > expect a more specific name for that purpose. There is plenty other
> > uses of randomness in multimedia, noise, dithering, etc, which don't
> > need crypto security. The function doesn't have to handle those, but
> > maybe it should be specific in what it does handle?
>
> Maybe av_random_buf()? I don't want too much bikeshedding on the name.
if the intend is that this is ONLY "cryptographically secure" PRNGs then
maybe av_csprng_buf()
would be an idea for a name
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-04 23:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-04 20:41 [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
2023-07-04 20:41 ` [FFmpeg-devel] [PATCH v2 2/2] avutil/random_seed: add support for gcrypt and OpenSSL as source of randomness James Almer
2023-07-04 20:47 ` [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random() James Almer
2023-07-04 20:54 ` Hendrik Leppkes
2023-07-04 21:35 ` James Almer
2023-07-04 23:32 ` Michael Niedermayer
2023-07-04 22:18 ` Marton Balint
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git