Re: [FFmpeg-devel] [PATCH] avformat: implement SChannel SSP TLS protocol

2015-10-28 Thread Hendrik Leppkes 
On Wed, Oct 21, 2015 at 1:52 PM, Hendrik Leppkes  wrote:
> This implementation does not support TLS listen sockets and loading
> CA/Certs from files.
>
> The Windows API does not support loading PEM certs, and would either
> require a manual loader or instead be limited to loading Windows PFX
> certificates
>
> TLS listen sockets would have to be implemented quite separately, as many
> of the APIs are different for server-mode (as opposed to client mode).
> ---
>
> Because of the limitations in comparison to OpenSSL or GnuTLS,
> we might make this implementation not active by default, if someone feels
> like thats a better course of action.
>
>
>  configure  |  20 +-
>  libavformat/Makefile   |   1 +
>  libavformat/allformats.c   |   1 +
>  libavformat/tls.h  |   2 +-
>  libavformat/tls_schannel.c | 601 
> +
>  5 files changed, 621 insertions(+), 4 deletions(-)
>  create mode 100644 libavformat/tls_schannel.c
>
> diff --git a/configure b/configure
> index 5e7ded1..6b9b99c 100755
> --- a/configure
> +++ b/configure
> @@ -280,6 +280,8 @@ External library support:
>--enable-opengl  enable OpenGL rendering [no]
>--enable-openssl enable openssl, needed for https support
> if gnutls is not used [no]
> +  --disable-schannel   disable SChannel SSP, needed for TLS support on
> +   Windows if openssl and gnutls are not used 
> [autodetect]
>--disable-sdldisable sdl [autodetect]
>--disable-securetransport disable Secure Transport, needed for TLS support
> on OSX if openssl and gnutls are not used 
> [autodetect]
> @@ -1465,6 +1467,7 @@ EXTERNAL_LIBRARY_LIST="
>  opencl
>  opengl
>  openssl
> +schannel
>  sdl
>  securetransport
>  x11grab
> @@ -2754,13 +2757,15 @@ sctp_protocol_deps="struct_sctp_event_subscribe"
>  sctp_protocol_select="network"
>  srtp_protocol_select="rtp_protocol"
>  tcp_protocol_select="network"
> -tls_gnutls_protocol_deps="gnutls !tls_securetransport_protocol"
> +tls_gnutls_protocol_deps="gnutls !tls_schannel_protocol 
> !tls_securetransport_protocol"
>  tls_gnutls_protocol_select="tcp_protocol"
> -tls_openssl_protocol_deps="openssl !tls_securetransport_protocol 
> !tls_gnutls_protocol"
> +tls_openssl_protocol_deps="openssl !tls_schannel_protocol 
> !tls_securetransport_protocol !tls_gnutls_protocol"
>  tls_openssl_protocol_select="tcp_protocol"
> +tls_schannel_protocol_deps="schannel"
> +tls_schannel_protocol_select="tcp_protocol"
>  tls_securetransport_protocol_deps="securetransport"
>  tls_securetransport_protocol_select="tcp_protocol"
> -tls_protocol_deps_any="tls_securetransport_protocol tls_gnutls_protocol 
> tls_openssl_protocol"
> +tls_protocol_deps_any="tls_schannel_protocol tls_securetransport_protocol 
> tls_gnutls_protocol tls_openssl_protocol"
>  udp_protocol_select="network"
>  udplite_protocol_select="network"
>  unix_protocol_deps="sys_un_h"
> @@ -5501,6 +5506,15 @@ disabled securetransport || { check_func 
> SecIdentityCreate "-Wl,-framework,CoreF
>  check_lib2 "Security/SecureTransport.h Security/Security.h" 
> "SSLCreateContext SecItemImport" "-Wl,-framework,CoreFoundation 
> -Wl,-framework,Security" &&
>  enable securetransport; }
>
> +if ! disabled schannel; then
> +check_cc < +#define SECURITY_WIN32
> +#include 
> +#include 
> +int main(void) { InitializeSecurityContext(NULL, NULL, NULL, 0, 0, 0, NULL, 
> 0, NULL, NULL, NULL, NULL); return 0; }
> +EOF
> +fi
> +
>  makeinfo --version > /dev/null 2>&1 && enable makeinfo  || disable makeinfo
>  enabled makeinfo \
>  && [ 0$(makeinfo --version | grep "texinfo" | sed 
> 's/.*texinfo[^0-9]*\([0-9]*\)\..*/\1/') -ge 5 ] \
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index ded2d54..b2d6057 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -535,6 +535,7 @@ OBJS-$(CONFIG_SUBFILE_PROTOCOL)  += subfile.o
>  OBJS-$(CONFIG_TCP_PROTOCOL)  += tcp.o
>  OBJS-$(CONFIG_TLS_GNUTLS_PROTOCOL)   += tls_gnutls.o tls.o
>  OBJS-$(CONFIG_TLS_OPENSSL_PROTOCOL)  += tls_openssl.o tls.o
> +OBJS-$(CONFIG_TLS_SCHANNEL_PROTOCOL) += tls_schannel.o tls.o
>  OBJS-$(CONFIG_TLS_SECURETRANSPORT_PROTOCOL) += tls_securetransport.o tls.o
>  OBJS-$(CONFIG_UDP_PROTOCOL)  += udp.o
>  OBJS-$(CONFIG_UDPLITE_PROTOCOL)  += udp.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 40fea8e..dad2644 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -387,6 +387,7 @@ void av_register_all(void)
>  REGISTER_PROTOCOL(SRTP, srtp);
>  REGISTER_PROTOCOL(SUBFILE,  subfile);
>  REGISTER_PROTOCOL(TCP,  tcp);
> +REGISTER_PROTOCOL(TLS_SCHANNEL, tls_schannel);
>  REGISTER_PROTOCOL(TLS_SECURETRANSPORT, tls_securetransport);
>

Re: [FFmpeg-devel] [PATCH] avformat: implement SChannel SSP TLS protocol

2015-10-28 Thread Hendrik Leppkes 
On Wed, Oct 28, 2015 at 6:07 PM, Derek Buitenhuis
 wrote:
> Enjoy my half-assed / useless review.
>
>> +#ifndef SECBUFFER_ALERT
>> +#define SECBUFFER_ALERT17
>> +#endif
>
> Why?

I think it was needed on some configuration, I'll verify.

>
>> +SecPkgContext_StreamSizes Sizes;
>
> Accidental capital?
>
>> +if (c->enc_buf == NULL) {
>> +c->enc_buf_offset = 0;
>> +c->enc_buf_size = SCHANNEL_INITIAL_BUFFER_SIZE;
>> +ret = av_reallocp(>enc_buf, c->enc_buf_size);
>> +if (ret < 0)
>> +goto fail;
>
> I can never remember if _close() is guaranteed to be called. I'll assume yes.
>
>> +while (1)
>> +{
>> +if (c->enc_buf_size - c->enc_buf_offset < 
>> SCHANNEL_FREE_BUFFER_SIZE) {
>
> Can enc_buf_offset ever end up bigger? e.. if data keeps being read, and you 
> keep
> getting SEC_E_INCOMPLETE_MESSAGE.

Bigger than what, buf_size? it should grow that as necessary when data
is received.

>
>> +/* input buffers */
>> +InitSecBuffer([0], SECBUFFER_TOKEN, 
>> av_malloc(c->enc_buf_offset), c->enc_buf_offset);
>
> Is this a unchecked malloc, or is it passed into inbuf[0].pvBuffer?

Yes this ends up in inbuf[0].pvBuffer which is then checked.
I could make it explicit before if people like.

>
>> +
>> +sspi_ret = InitializeSecurityContext(>cred_handle, 
>> >ctxt_handle, s->host, c->request_flags,
>> + 0, 0, _desc, 0, NULL, 
>> _desc, >context_flags,
>> + >ctxt_timestamp);
>> +av_freep([0].pvBuffer);
>
> Double checking: is this safe?

Since this is the buffer we av_malloc'ed before, yes.

>
>
>> +/* SChannel Options */
>> +memset(_cred, 0, sizeof(schannel_cred));
>
> SCHANNEL_CRED schannel_cred = { 0 };
>
>> +cleanup:
>> +size = len < c->dec_buf_offset ? len : c->dec_buf_offset;
>> +if (size) {
>> +memcpy(buf, c->dec_buf, size);
>> +memmove(c->dec_buf, c->dec_buf + size, c->dec_buf_offset - size);
>> +c->dec_buf_offset -= size;
>> +
>> +return size;
>> +}
>> +
>> +if (ret == 0 && !c->connection_closed)
>> +ret = AVERROR(EAGAIN);
>> +
>> +return ret < 0 ? ret : 0;
>> +
>> +fail:
>> +return ret;
>
> No cleanup in case of failure?

fail is only used for serious unrecoverable failures, like socket read
failure or mem allocation failure. It could still call that code, I
suppose, just in case some valid data is stuck in the decrypted
buffer.

>
> - Derek
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat: implement SChannel SSP TLS protocol

2015-10-28 Thread Derek Buitenhuis 
Enjoy my half-assed / useless review.

> +#ifndef SECBUFFER_ALERT
> +#define SECBUFFER_ALERT17
> +#endif

Why?

> +SecPkgContext_StreamSizes Sizes;

Accidental capital?

> +if (c->enc_buf == NULL) {
> +c->enc_buf_offset = 0;
> +c->enc_buf_size = SCHANNEL_INITIAL_BUFFER_SIZE;
> +ret = av_reallocp(>enc_buf, c->enc_buf_size);
> +if (ret < 0)
> +goto fail;

I can never remember if _close() is guaranteed to be called. I'll assume yes.

> +while (1)
> +{
> +if (c->enc_buf_size - c->enc_buf_offset < SCHANNEL_FREE_BUFFER_SIZE) 
> {

Can enc_buf_offset ever end up bigger? e.. if data keeps being read, and you 
keep
getting SEC_E_INCOMPLETE_MESSAGE.

> +/* input buffers */
> +InitSecBuffer([0], SECBUFFER_TOKEN, 
> av_malloc(c->enc_buf_offset), c->enc_buf_offset);

Is this a unchecked malloc, or is it passed into inbuf[0].pvBuffer?

> +
> +sspi_ret = InitializeSecurityContext(>cred_handle, 
> >ctxt_handle, s->host, c->request_flags,
> + 0, 0, _desc, 0, NULL, 
> _desc, >context_flags,
> + >ctxt_timestamp);
> +av_freep([0].pvBuffer);

Double checking: is this safe?


> +/* SChannel Options */
> +memset(_cred, 0, sizeof(schannel_cred));

SCHANNEL_CRED schannel_cred = { 0 };

> +cleanup:
> +size = len < c->dec_buf_offset ? len : c->dec_buf_offset;
> +if (size) {
> +memcpy(buf, c->dec_buf, size);
> +memmove(c->dec_buf, c->dec_buf + size, c->dec_buf_offset - size);
> +c->dec_buf_offset -= size;
> +
> +return size;
> +}
> +
> +if (ret == 0 && !c->connection_closed)
> +ret = AVERROR(EAGAIN);
> +
> +return ret < 0 ? ret : 0;
> +
> +fail:
> +return ret;

No cleanup in case of failure?

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat: implement SChannel SSP TLS protocol

2015-10-21 Thread Hendrik Leppkes 
This implementation does not support TLS listen sockets and loading
CA/Certs from files.

The Windows API does not support loading PEM certs, and would either
require a manual loader or instead be limited to loading Windows PFX
certificates

TLS listen sockets would have to be implemented quite separately, as many
of the APIs are different for server-mode (as opposed to client mode).
---

Because of the limitations in comparison to OpenSSL or GnuTLS,
we might make this implementation not active by default, if someone feels
like thats a better course of action.


 configure  |  20 +-
 libavformat/Makefile   |   1 +
 libavformat/allformats.c   |   1 +
 libavformat/tls.h  |   2 +-
 libavformat/tls_schannel.c | 601 +
 5 files changed, 621 insertions(+), 4 deletions(-)
 create mode 100644 libavformat/tls_schannel.c

diff --git a/configure b/configure
index 5e7ded1..6b9b99c 100755
--- a/configure
+++ b/configure
@@ -280,6 +280,8 @@ External library support:
   --enable-opengl  enable OpenGL rendering [no]
   --enable-openssl enable openssl, needed for https support
if gnutls is not used [no]
+  --disable-schannel   disable SChannel SSP, needed for TLS support on
+   Windows if openssl and gnutls are not used 
[autodetect]
   --disable-sdldisable sdl [autodetect]
   --disable-securetransport disable Secure Transport, needed for TLS support
on OSX if openssl and gnutls are not used 
[autodetect]
@@ -1465,6 +1467,7 @@ EXTERNAL_LIBRARY_LIST="
 opencl
 opengl
 openssl
+schannel
 sdl
 securetransport
 x11grab
@@ -2754,13 +2757,15 @@ sctp_protocol_deps="struct_sctp_event_subscribe"
 sctp_protocol_select="network"
 srtp_protocol_select="rtp_protocol"
 tcp_protocol_select="network"
-tls_gnutls_protocol_deps="gnutls !tls_securetransport_protocol"
+tls_gnutls_protocol_deps="gnutls !tls_schannel_protocol 
!tls_securetransport_protocol"
 tls_gnutls_protocol_select="tcp_protocol"
-tls_openssl_protocol_deps="openssl !tls_securetransport_protocol 
!tls_gnutls_protocol"
+tls_openssl_protocol_deps="openssl !tls_schannel_protocol 
!tls_securetransport_protocol !tls_gnutls_protocol"
 tls_openssl_protocol_select="tcp_protocol"
+tls_schannel_protocol_deps="schannel"
+tls_schannel_protocol_select="tcp_protocol"
 tls_securetransport_protocol_deps="securetransport"
 tls_securetransport_protocol_select="tcp_protocol"
-tls_protocol_deps_any="tls_securetransport_protocol tls_gnutls_protocol 
tls_openssl_protocol"
+tls_protocol_deps_any="tls_schannel_protocol tls_securetransport_protocol 
tls_gnutls_protocol tls_openssl_protocol"
 udp_protocol_select="network"
 udplite_protocol_select="network"
 unix_protocol_deps="sys_un_h"
@@ -5501,6 +5506,15 @@ disabled securetransport || { check_func 
SecIdentityCreate "-Wl,-framework,CoreF
 check_lib2 "Security/SecureTransport.h Security/Security.h" 
"SSLCreateContext SecItemImport" "-Wl,-framework,CoreFoundation 
-Wl,-framework,Security" &&
 enable securetransport; }
 
+if ! disabled schannel; then
+check_cc <
+#include 
+int main(void) { InitializeSecurityContext(NULL, NULL, NULL, 0, 0, 0, NULL, 0, 
NULL, NULL, NULL, NULL); return 0; }
+EOF
+fi
+
 makeinfo --version > /dev/null 2>&1 && enable makeinfo  || disable makeinfo
 enabled makeinfo \
 && [ 0$(makeinfo --version | grep "texinfo" | sed 
's/.*texinfo[^0-9]*\([0-9]*\)\..*/\1/') -ge 5 ] \
diff --git a/libavformat/Makefile b/libavformat/Makefile
index ded2d54..b2d6057 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -535,6 +535,7 @@ OBJS-$(CONFIG_SUBFILE_PROTOCOL)  += subfile.o
 OBJS-$(CONFIG_TCP_PROTOCOL)  += tcp.o
 OBJS-$(CONFIG_TLS_GNUTLS_PROTOCOL)   += tls_gnutls.o tls.o
 OBJS-$(CONFIG_TLS_OPENSSL_PROTOCOL)  += tls_openssl.o tls.o
+OBJS-$(CONFIG_TLS_SCHANNEL_PROTOCOL) += tls_schannel.o tls.o
 OBJS-$(CONFIG_TLS_SECURETRANSPORT_PROTOCOL) += tls_securetransport.o tls.o
 OBJS-$(CONFIG_UDP_PROTOCOL)  += udp.o
 OBJS-$(CONFIG_UDPLITE_PROTOCOL)  += udp.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 40fea8e..dad2644 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -387,6 +387,7 @@ void av_register_all(void)
 REGISTER_PROTOCOL(SRTP, srtp);
 REGISTER_PROTOCOL(SUBFILE,  subfile);
 REGISTER_PROTOCOL(TCP,  tcp);
+REGISTER_PROTOCOL(TLS_SCHANNEL, tls_schannel);
 REGISTER_PROTOCOL(TLS_SECURETRANSPORT, tls_securetransport);
 REGISTER_PROTOCOL(TLS_GNUTLS,   tls_gnutls);
 REGISTER_PROTOCOL(TLS_OPENSSL,  tls_openssl);
diff --git a/libavformat/tls.h b/libavformat/tls.h
index 2a36f34..0326ef7 100644
--- a/libavformat/tls.h
+++ b/libavformat/tls.h
@@ -26,7 +26,7 @@
 #include "url.h"
 #include "libavutil/opt.h"
 
-#define CONFIG_TLS_PROTOCOL