| Message ID | 1475353239-31384-1-git-send-email-jayridge@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Thanks to the suggestion from Compn on IRC, I set up a test server for this patch. Once the patch is applied it can be tested as follows. The cert is self signed and ( as with TLS ) the hostname is not verified with openssl. wget https://gist.githubusercontent.com/jayridge/c506515d969751610188152cee7ca2b2/raw/3c14ce37380f744393d15bebcca4c1cc80f3f55f/cert.pem -O /tmp/cert.pem ./ffprobe 'rtsps://user:password@rtsps.jayridgeway.com:8554/test' -v debug -ca_file /tmp/cert.pem -tls_verify 1 On Sat, Oct 1, 2016 at 4:20 PM <jayridge@gmail.com> wrote: > From: Jay Ridgeway <jayridge@gmail.com> > > > This patch enables TLS args for RTSPS. This is necessary for client > certificates and cert validation. > > Squash changes from feedback into one patch. > > --- > libavformat/rtsp.c | 19 ++++++++++++++++--- > libavformat/rtsp.h | 8 ++++++++ > libavformat/tls_gnutls.c | 7 +++++++ > libavformat/tls_openssl.c | 7 +++++++ > libavformat/tls_schannel.c | 7 +++++++ > libavformat/tls_securetransport.c | 7 +++++++ > 6 files changed, 52 insertions(+), 3 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index c6292c5..53ecb6c 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -78,6 +78,7 @@ > { "reorder_queue_size", "set number of packets to buffer for handling > of reordered packets", OFFSET(reordering_queue_size), AV_OPT_TYPE_INT, { > .i64 = -1 }, -1, INT_MAX, DEC }, \ > { "buffer_size", "Underlying protocol send/receive buffer > size", OFFSET(buffer_size), AV_OPT_TYPE_INT, { > .i64 = -1 }, -1, INT_MAX, DEC|ENC } \ > > +#define NONNULLSTR(s) (s ? s : "") > > const AVOption ff_rtsp_options[] = { > { "initial_pause", "do not start playing the stream immediately", > OFFSET(initial_pause), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC }, > @@ -97,6 +98,10 @@ const AVOption ff_rtsp_options[] = { > { "stimeout", "set timeout (in microseconds) of socket TCP I/O > operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, > INT_MAX, DEC }, > COMMON_OPTS(), > { "user-agent", "override User-Agent header", OFFSET(user_agent), > AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC }, > + { "ca_file", "Certificate Authority database file", OFFSET(ca_file), > AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > + { "tls_verify", "verify the peer certificate", OFFSET(verify), > AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC|ENC}, > + { "cert_file", "certificate file", OFFSET(cert_file), > AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > + { "key_file", "private key file", OFFSET(key_file), > AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > { NULL }, > }; > > @@ -1812,9 +1817,17 @@ redirect: > } else { > int ret; > /* open the tcp connection */ > - ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > - host, port, > - "?timeout=%d", rt->stimeout); > + if (strcmp("tls", lower_rtsp_proto) == 0) { > + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > + host, port, > + > "?timeout=%d&verify=%d&cafile=%s&cert_file=%s&key_file=%s", > + rt->stimeout, rt->verify, NONNULLSTR(rt->ca_file), > + NONNULLSTR(rt->cert_file), > NONNULLSTR(rt->key_file)); > + } else { > + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > + host, port, > + "?timeout=%d", rt->stimeout); > + } > if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, > AVIO_FLAG_READ_WRITE, > &s->interrupt_callback, NULL, > s->protocol_whitelist, s->protocol_blacklist, NULL)) < 0) { > err = ret; > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h > index 852fd67..fa872a8 100644 > --- a/libavformat/rtsp.h > +++ b/libavformat/rtsp.h > @@ -408,6 +408,14 @@ typedef struct RTSPState { > > char default_lang[4]; > int buffer_size; > + > + /** The following are used for RTSPS streams */ > + //@{ > + char *ca_file; > + int verify; > + char *cert_file; > + char *key_file; > + //@} > } RTSPState; > > #define RTSP_FLAG_FILTER_SRC 0x1 /**< Filter incoming UDP packets - > diff --git a/libavformat/tls_gnutls.c b/libavformat/tls_gnutls.c > index 991b36b..ecc80bf 100644 > --- a/libavformat/tls_gnutls.c > +++ b/libavformat/tls_gnutls.c > @@ -235,6 +235,12 @@ static int tls_write(URLContext *h, const uint8_t > *buf, int size) > return print_tls_error(h, ret); > } > > +static int tls_get_file_handle(URLContext *h) > +{ > + TLSContext *c = h->priv_data; > + return ffurl_get_file_handle(c->tls_shared.tcp); > +} > + > static const AVOption options[] = { > TLS_COMMON_OPTIONS(TLSContext, tls_shared), > { NULL } > @@ -253,6 +259,7 @@ const URLProtocol ff_tls_gnutls_protocol = { > .url_read = tls_read, > .url_write = tls_write, > .url_close = tls_close, > + .url_get_file_handle = tls_get_file_handle, > .priv_data_size = sizeof(TLSContext), > .flags = URL_PROTOCOL_FLAG_NETWORK, > .priv_data_class = &tls_class, > diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c > index 46eb3e6..1455392 100644 > --- a/libavformat/tls_openssl.c > +++ b/libavformat/tls_openssl.c > @@ -283,6 +283,12 @@ static int tls_write(URLContext *h, const uint8_t > *buf, int size) > return print_tls_error(h, ret); > } > > +static int tls_get_file_handle(URLContext *h) > +{ > + TLSContext *c = h->priv_data; > + return ffurl_get_file_handle(c->tls_shared.tcp); > +} > + > static const AVOption options[] = { > TLS_COMMON_OPTIONS(TLSContext, tls_shared), > { NULL } > @@ -301,6 +307,7 @@ const URLProtocol ff_tls_openssl_protocol = { > .url_read = tls_read, > .url_write = tls_write, > .url_close = tls_close, > + .url_get_file_handle = tls_get_file_handle, > .priv_data_size = sizeof(TLSContext), > .flags = URL_PROTOCOL_FLAG_NETWORK, > .priv_data_class = &tls_class, > diff --git a/libavformat/tls_schannel.c b/libavformat/tls_schannel.c > index c11b7d4..065dccb 100644 > --- a/libavformat/tls_schannel.c > +++ b/libavformat/tls_schannel.c > @@ -577,6 +577,12 @@ done: > return ret < 0 ? ret : outbuf[1].cbBuffer; > } > > +static int tls_get_file_handle(URLContext *h) > +{ > + TLSContext *c = h->priv_data; > + return ffurl_get_file_handle(c->tls_shared.tcp); > +} > + > static const AVOption options[] = { > TLS_COMMON_OPTIONS(TLSContext, tls_shared), > { NULL } > @@ -595,6 +601,7 @@ const URLProtocol ff_tls_schannel_protocol = { > .url_read = tls_read, > .url_write = tls_write, > .url_close = tls_close, > + .url_get_file_handle = tls_get_file_handle, > .priv_data_size = sizeof(TLSContext), > .flags = URL_PROTOCOL_FLAG_NETWORK, > .priv_data_class = &tls_class, > diff --git a/libavformat/tls_securetransport.c > b/libavformat/tls_securetransport.c > index 253c89c..bc8a320 100644 > --- a/libavformat/tls_securetransport.c > +++ b/libavformat/tls_securetransport.c > @@ -375,6 +375,12 @@ static int tls_write(URLContext *h, const uint8_t > *buf, int size) > return print_tls_error(h, ret); > } > > +static int tls_get_file_handle(URLContext *h) > +{ > + TLSContext *c = h->priv_data; > + return ffurl_get_file_handle(c->tls_shared.tcp); > +} > + > static const AVOption options[] = { > TLS_COMMON_OPTIONS(TLSContext, tls_shared), > { NULL } > @@ -393,6 +399,7 @@ const URLProtocol ff_tls_securetransport_protocol = { > .url_read = tls_read, > .url_write = tls_write, > .url_close = tls_close, > + .url_get_file_handle = tls_get_file_handle, > .priv_data_size = sizeof(TLSContext), > .flags = URL_PROTOCOL_FLAG_NETWORK, > .priv_data_class = &tls_class, > -- > 2.6.3 > >
On Sat, Oct 01, 2016 at 04:20:39PM -0400, jayridge@gmail.com wrote: > From: Jay Ridgeway <jayridge@gmail.com> > > > This patch enables TLS args for RTSPS. This is necessary for client > certificates and cert validation. > > Squash changes from feedback into one patch. > > --- > libavformat/rtsp.c | 19 ++++++++++++++++--- > libavformat/rtsp.h | 8 ++++++++ > libavformat/tls_gnutls.c | 7 +++++++ > libavformat/tls_openssl.c | 7 +++++++ > libavformat/tls_schannel.c | 7 +++++++ > libavformat/tls_securetransport.c | 7 +++++++ > 6 files changed, 52 insertions(+), 3 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index c6292c5..53ecb6c 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -78,6 +78,7 @@ > { "reorder_queue_size", "set number of packets to buffer for handling of reordered packets", OFFSET(reordering_queue_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, DEC }, \ > { "buffer_size", "Underlying protocol send/receive buffer size", OFFSET(buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, DEC|ENC } \ > > +#define NONNULLSTR(s) (s ? s : "") > > const AVOption ff_rtsp_options[] = { > { "initial_pause", "do not start playing the stream immediately", OFFSET(initial_pause), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC }, > @@ -97,6 +98,10 @@ const AVOption ff_rtsp_options[] = { > { "stimeout", "set timeout (in microseconds) of socket TCP I/O operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC }, > COMMON_OPTS(), > { "user-agent", "override User-Agent header", OFFSET(user_agent), AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC }, > + { "ca_file", "Certificate Authority database file", OFFSET(ca_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > + { "tls_verify", "verify the peer certificate", OFFSET(verify), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC|ENC}, > + { "cert_file", "certificate file", OFFSET(cert_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > + { "key_file", "private key file", OFFSET(key_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > { NULL }, > }; > > @@ -1812,9 +1817,17 @@ redirect: > } else { > int ret; > /* open the tcp connection */ > - ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > - host, port, > - "?timeout=%d", rt->stimeout); > + if (strcmp("tls", lower_rtsp_proto) == 0) { > + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > + host, port, > + "?timeout=%d&verify=%d&cafile=%s&cert_file=%s&key_file=%s", > + rt->stimeout, rt->verify, NONNULLSTR(rt->ca_file), > + NONNULLSTR(rt->cert_file), NONNULLSTR(rt->key_file)); > + } else { > + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > + host, port, > + "?timeout=%d", rt->stimeout); > + } > if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, AVIO_FLAG_READ_WRITE, > &s->interrupt_callback, NULL, s->protocol_whitelist, s->protocol_blacklist, NULL)) < 0) { > err = ret; > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h > index 852fd67..fa872a8 100644 > --- a/libavformat/rtsp.h > +++ b/libavformat/rtsp.h > @@ -408,6 +408,14 @@ typedef struct RTSPState { > > char default_lang[4]; > int buffer_size; > + > + /** The following are used for RTSPS streams */ > + //@{ > + char *ca_file; > + int verify; > + char *cert_file; > + char *key_file; > + //@} > } RTSPState; > > #define RTSP_FLAG_FILTER_SRC 0x1 /**< Filter incoming UDP packets - > diff --git a/libavformat/tls_gnutls.c b/libavformat/tls_gnutls.c > index 991b36b..ecc80bf 100644 > --- a/libavformat/tls_gnutls.c > +++ b/libavformat/tls_gnutls.c > @@ -235,6 +235,12 @@ static int tls_write(URLContext *h, const uint8_t *buf, int size) > return print_tls_error(h, ret); > } > > +static int tls_get_file_handle(URLContext *h) > +{ > + TLSContext *c = h->priv_data; > + return ffurl_get_file_handle(c->tls_shared.tcp); > +} > + > static const AVOption options[] = { > TLS_COMMON_OPTIONS(TLSContext, tls_shared), > { NULL } > @@ -253,6 +259,7 @@ const URLProtocol ff_tls_gnutls_protocol = { > .url_read = tls_read, > .url_write = tls_write, > .url_close = tls_close, > + .url_get_file_handle = tls_get_file_handle, > .priv_data_size = sizeof(TLSContext), > .flags = URL_PROTOCOL_FLAG_NETWORK, > .priv_data_class = &tls_class, > diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c > index 46eb3e6..1455392 100644 > --- a/libavformat/tls_openssl.c > +++ b/libavformat/tls_openssl.c > @@ -283,6 +283,12 @@ static int tls_write(URLContext *h, const uint8_t *buf, int size) > return print_tls_error(h, ret); > } > > +static int tls_get_file_handle(URLContext *h) > +{ > + TLSContext *c = h->priv_data; > + return ffurl_get_file_handle(c->tls_shared.tcp); > +} the get_file_handle extensions should be in a spererate patch than the rtsp changes also is it safe for all to use the input file handle that way ? for example if one used a fifo the input state would not match the relevant output neccessarily [...]
Thank you for the feedback. I have been trying to get RTSPS cert validation incorporated for several weeks. I would greatly appreciate someone on the core team helping me guide this to completion. Please find your questions answered below. > the get_file_handle extensions should be in a spererate patch than > the rtsp changes I am process agnostic, but the RTSP changes are dependent on the TLS changes. There is a check for peer addr in RTSP that is based on the file descriptor. > also is it safe for all to use the input file handle that way ? > for example if one used a fifo the input state would not match the > relevant output neccessarily I do not think the peer addr check is necessary. My goal is a minimal patch, making RTSPS work with basic TLS options. Ideally, RTSPS would work with `rtsp://` scheme by recognizing TLS negotiation. I view this patch as an initial step. Thank you. Jay On Sat, Oct 15, 2016 at 3:04 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Oct 01, 2016 at 04:20:39PM -0400, jayridge@gmail.com wrote: > > > From: Jay Ridgeway <jayridge@gmail.com> > > > > > > > > > This patch enables TLS args for RTSPS. This is necessary for client > > > certificates and cert validation. > > > > > > Squash changes from feedback into one patch. > > > > > > --- > > > libavformat/rtsp.c | 19 ++++++++++++++++--- > > > libavformat/rtsp.h | 8 ++++++++ > > > libavformat/tls_gnutls.c | 7 +++++++ > > > libavformat/tls_openssl.c | 7 +++++++ > > > libavformat/tls_schannel.c | 7 +++++++ > > > libavformat/tls_securetransport.c | 7 +++++++ > > > 6 files changed, 52 insertions(+), 3 deletions(-) > > > > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > > > index c6292c5..53ecb6c 100644 > > > --- a/libavformat/rtsp.c > > > +++ b/libavformat/rtsp.c > > > @@ -78,6 +78,7 @@ > > > { "reorder_queue_size", "set number of packets to buffer for > handling of reordered packets", OFFSET(reordering_queue_size), > AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, DEC }, \ > > > { "buffer_size", "Underlying protocol send/receive buffer > size", OFFSET(buffer_size), AV_OPT_TYPE_INT, { > .i64 = -1 }, -1, INT_MAX, DEC|ENC } \ > > > > > > +#define NONNULLSTR(s) (s ? s : "") > > > > > > const AVOption ff_rtsp_options[] = { > > > { "initial_pause", "do not start playing the stream immediately", > OFFSET(initial_pause), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC }, > > > @@ -97,6 +98,10 @@ const AVOption ff_rtsp_options[] = { > > > { "stimeout", "set timeout (in microseconds) of socket TCP I/O > operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, > INT_MAX, DEC }, > > > COMMON_OPTS(), > > > { "user-agent", "override User-Agent header", OFFSET(user_agent), > AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC }, > > > + { "ca_file", "Certificate Authority database file", > OFFSET(ca_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > > > + { "tls_verify", "verify the peer certificate", OFFSET(verify), > AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC|ENC}, > > > + { "cert_file", "certificate file", OFFSET(cert_file), > AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > > > + { "key_file", "private key file", OFFSET(key_file), > AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, > > > { NULL }, > > > }; > > > > > > @@ -1812,9 +1817,17 @@ redirect: > > > } else { > > > int ret; > > > /* open the tcp connection */ > > > - ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > > > - host, port, > > > - "?timeout=%d", rt->stimeout); > > > + if (strcmp("tls", lower_rtsp_proto) == 0) { > > > + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, > NULL, > > > + host, port, > > > + > "?timeout=%d&verify=%d&cafile=%s&cert_file=%s&key_file=%s", > > > + rt->stimeout, rt->verify, > NONNULLSTR(rt->ca_file), > > > + NONNULLSTR(rt->cert_file), > NONNULLSTR(rt->key_file)); > > > + } else { > > > + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, > NULL, > > > + host, port, > > > + "?timeout=%d", rt->stimeout); > > > + } > > > if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, > AVIO_FLAG_READ_WRITE, > > > &s->interrupt_callback, NULL, > s->protocol_whitelist, s->protocol_blacklist, NULL)) < 0) { > > > err = ret; > > > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h > > > index 852fd67..fa872a8 100644 > > > --- a/libavformat/rtsp.h > > > +++ b/libavformat/rtsp.h > > > @@ -408,6 +408,14 @@ typedef struct RTSPState { > > > > > > char default_lang[4]; > > > int buffer_size; > > > + > > > + /** The following are used for RTSPS streams */ > > > + //@{ > > > + char *ca_file; > > > + int verify; > > > + char *cert_file; > > > + char *key_file; > > > + //@} > > > } RTSPState; > > > > > > #define RTSP_FLAG_FILTER_SRC 0x1 /**< Filter incoming UDP packets - > > > diff --git a/libavformat/tls_gnutls.c b/libavformat/tls_gnutls.c > > > index 991b36b..ecc80bf 100644 > > > --- a/libavformat/tls_gnutls.c > > > +++ b/libavformat/tls_gnutls.c > > > @@ -235,6 +235,12 @@ static int tls_write(URLContext *h, const uint8_t > *buf, int size) > > > return print_tls_error(h, ret); > > > } > > > > > > +static int tls_get_file_handle(URLContext *h) > > > +{ > > > + TLSContext *c = h->priv_data; > > > + return ffurl_get_file_handle(c->tls_shared.tcp); > > > +} > > > + > > > static const AVOption options[] = { > > > TLS_COMMON_OPTIONS(TLSContext, tls_shared), > > > { NULL } > > > @@ -253,6 +259,7 @@ const URLProtocol ff_tls_gnutls_protocol = { > > > .url_read = tls_read, > > > .url_write = tls_write, > > > .url_close = tls_close, > > > + .url_get_file_handle = tls_get_file_handle, > > > .priv_data_size = sizeof(TLSContext), > > > .flags = URL_PROTOCOL_FLAG_NETWORK, > > > .priv_data_class = &tls_class, > > > > > diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c > > > index 46eb3e6..1455392 100644 > > > --- a/libavformat/tls_openssl.c > > > +++ b/libavformat/tls_openssl.c > > > @@ -283,6 +283,12 @@ static int tls_write(URLContext *h, const uint8_t > *buf, int size) > > > return print_tls_error(h, ret); > > > } > > > > > > +static int tls_get_file_handle(URLContext *h) > > > +{ > > > + TLSContext *c = h->priv_data; > > > + return ffurl_get_file_handle(c->tls_shared.tcp); > > > +} > > > > the get_file_handle extensions should be in a spererate patch than > > the rtsp changes > > > > also is it safe for all to use the input file handle that way ? > > for example if one used a fifo the input state would not match the > > relevant output neccessarily > > > > [...] > > -- > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > > > Let us carefully observe those good qualities wherein our enemies excel us > > and endeavor to excel them, by avoiding what is faulty, and imitating what > > is excellent in them. -- Plutarch > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
On Sat, Oct 15, 2016 at 08:31:23PM +0000, Jay wrote: > Thank you for the feedback. I have been trying to get RTSPS cert validation > incorporated for several weeks. I would greatly appreciate someone on the > core team helping me guide this to completion. Please find your questions > answered below. > > > the get_file_handle extensions should be in a spererate patch than > > the rtsp changes > > I am process agnostic, but the RTSP changes are dependent on the TLS > changes. There is a check for peer addr in RTSP that is based on the file > descriptor. The TLS changes do not depend on the RTSP changes, they can be in a seperate patch, applied before. TLS and RTSP are seperate "modules" changes to them are cleaner if split > > > also is it safe for all to use the input file handle that way ? > > for example if one used a fifo the input state would not match the > > relevant output neccessarily > > I do not think the peer addr check is necessary. My goal is a minimal > patch, making RTSPS work with basic TLS options. Is that the only use of the file handle ? [...]
On Sat, Oct 15, 2016 at 10:44 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Oct 15, 2016 at 08:31:23PM +0000, Jay wrote: > > Thank you for the feedback. I have been trying to get RTSPS cert > validation > > incorporated for several weeks. I would greatly appreciate someone on the > > core team helping me guide this to completion. Please find your questions > > answered below. > > > > > the get_file_handle extensions should be in a spererate patch than > > > the rtsp changes > > > > I am process agnostic, but the RTSP changes are dependent on the TLS > > changes. There is a check for peer addr in RTSP that is based on the file > > descriptor. > > The TLS changes do not depend on the RTSP changes, they can be in a > seperate patch, applied before. TLS and RTSP are seperate "modules" > changes to them are cleaner if split > > OK. > > > > > also is it safe for all to use the input file handle that way ? > > > for example if one used a fifo the input state would not match the > > > relevant output neccessarily > > > > I do not think the peer addr check is necessary. My goal is a minimal > > patch, making RTSPS work with basic TLS options. > > Is that the only use of the file handle ? > > I took another look. The peer addr is used later to setup output streams and in the setup request. File handle is required for TLS+RTSP. To fully answer the previous question, getpeername will not work on a fifo. It will fail with ENOTSOCK. In practice this should not be a problem. Named pipes are not supported by RTSP to my knowledge. > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Complexity theory is the science of finding the exact solution to an > approximation. Benchmarking OTOH is finding an approximation of the exact > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Sun, Oct 16, 2016 at 01:43:24PM +0000, Jay wrote: > On Sat, Oct 15, 2016 at 10:44 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Sat, Oct 15, 2016 at 08:31:23PM +0000, Jay wrote: > > > Thank you for the feedback. I have been trying to get RTSPS cert > > validation > > > incorporated for several weeks. I would greatly appreciate someone on the > > > core team helping me guide this to completion. Please find your questions > > > answered below. > > > > > > > the get_file_handle extensions should be in a spererate patch than > > > > the rtsp changes > > > > > > I am process agnostic, but the RTSP changes are dependent on the TLS > > > changes. There is a check for peer addr in RTSP that is based on the file > > > descriptor. > > > > The TLS changes do not depend on the RTSP changes, they can be in a > > seperate patch, applied before. TLS and RTSP are seperate "modules" > > changes to them are cleaner if split > > > > > OK. > > > > > > > > > also is it safe for all to use the input file handle that way ? > > > > for example if one used a fifo the input state would not match the > > > > relevant output neccessarily > > > > > > I do not think the peer addr check is necessary. My goal is a minimal > > > patch, making RTSPS work with basic TLS options. > > > > Is that the only use of the file handle ? > > > > > I took another look. The peer addr is used later to setup output streams > and in the setup request. File handle is required for TLS+RTSP. > > To fully answer the previous question, getpeername will not work on a fifo. > It will fail with ENOTSOCK. In practice this should not be a problem. Named > pipes are not supported by RTSP to my knowledge. the code would never even know theres a fifo if there is one just that there will be data in the fifo while the tcp socket it sees has no new data for example. The code cannot know of a fifo inside a TLS lib See our UDP code for example, it uses a fifo and a background thread if a tls lib does something similar then the data availability on the tcp socket will not 1:1 match the data availability at the libs decrypted output this was my concern about the design, but i did not check if there are any pathes where such data availability races can occur in relation to this patchset ... (thats why iam asking ...) [...]
OK. I understand. I do not see anything like that in tls. On Sun, Oct 16, 2016 at 11:06 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sun, Oct 16, 2016 at 01:43:24PM +0000, Jay wrote: > > On Sat, Oct 15, 2016 at 10:44 PM Michael Niedermayer > <michael@niedermayer.cc> > > wrote: > > > > > On Sat, Oct 15, 2016 at 08:31:23PM +0000, Jay wrote: > > > > Thank you for the feedback. I have been trying to get RTSPS cert > > > validation > > > > incorporated for several weeks. I would greatly appreciate someone > on the > > > > core team helping me guide this to completion. Please find your > questions > > > > answered below. > > > > > > > > > the get_file_handle extensions should be in a spererate patch than > > > > > the rtsp changes > > > > > > > > I am process agnostic, but the RTSP changes are dependent on the TLS > > > > changes. There is a check for peer addr in RTSP that is based on the > file > > > > descriptor. > > > > > > The TLS changes do not depend on the RTSP changes, they can be in a > > > seperate patch, applied before. TLS and RTSP are seperate "modules" > > > changes to them are cleaner if split > > > > > > > > OK. > > > > > > > > > > > > > > also is it safe for all to use the input file handle that way ? > > > > > for example if one used a fifo the input state would not match the > > > > > relevant output neccessarily > > > > > > > > I do not think the peer addr check is necessary. My goal is a minimal > > > > patch, making RTSPS work with basic TLS options. > > > > > > Is that the only use of the file handle ? > > > > > > > > I took another look. The peer addr is used later to setup output streams > > and in the setup request. File handle is required for TLS+RTSP. > > > > To fully answer the previous question, getpeername will not work on a > fifo. > > It will fail with ENOTSOCK. In practice this should not be a problem. > Named > > pipes are not supported by RTSP to my knowledge. > > the code would never even know theres a fifo if there is one > just that there will be data in the fifo while the tcp socket it sees > has no new data for example. > The code cannot know of a fifo inside a TLS lib > > See our UDP code for example, it uses a fifo and a background thread > if a tls lib does something similar then the data availability on > the tcp socket will not 1:1 match the data availability at the libs > decrypted output > > this was my concern about the design, but i did not check if there are > any pathes where such data availability races can occur in relation to > this patchset ... (thats why iam asking ...) > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > In a rich man's house there is no place to spit but his face. > -- Diogenes of Sinope > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index c6292c5..53ecb6c 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -78,6 +78,7 @@ { "reorder_queue_size", "set number of packets to buffer for handling of reordered packets", OFFSET(reordering_queue_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, DEC }, \ { "buffer_size", "Underlying protocol send/receive buffer size", OFFSET(buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, DEC|ENC } \ +#define NONNULLSTR(s) (s ? s : "") const AVOption ff_rtsp_options[] = { { "initial_pause", "do not start playing the stream immediately", OFFSET(initial_pause), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC }, @@ -97,6 +98,10 @@ const AVOption ff_rtsp_options[] = { { "stimeout", "set timeout (in microseconds) of socket TCP I/O operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC }, COMMON_OPTS(), { "user-agent", "override User-Agent header", OFFSET(user_agent), AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC }, + { "ca_file", "Certificate Authority database file", OFFSET(ca_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, + { "tls_verify", "verify the peer certificate", OFFSET(verify), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC|ENC}, + { "cert_file", "certificate file", OFFSET(cert_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, + { "key_file", "private key file", OFFSET(key_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC|ENC }, { NULL }, }; @@ -1812,9 +1817,17 @@ redirect: } else { int ret; /* open the tcp connection */ - ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, - host, port, - "?timeout=%d", rt->stimeout); + if (strcmp("tls", lower_rtsp_proto) == 0) { + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, + host, port, + "?timeout=%d&verify=%d&cafile=%s&cert_file=%s&key_file=%s", + rt->stimeout, rt->verify, NONNULLSTR(rt->ca_file), + NONNULLSTR(rt->cert_file), NONNULLSTR(rt->key_file)); + } else { + ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, + host, port, + "?timeout=%d", rt->stimeout); + } if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, AVIO_FLAG_READ_WRITE, &s->interrupt_callback, NULL, s->protocol_whitelist, s->protocol_blacklist, NULL)) < 0) { err = ret; diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h index 852fd67..fa872a8 100644 --- a/libavformat/rtsp.h +++ b/libavformat/rtsp.h @@ -408,6 +408,14 @@ typedef struct RTSPState { char default_lang[4]; int buffer_size; + + /** The following are used for RTSPS streams */ + //@{ + char *ca_file; + int verify; + char *cert_file; + char *key_file; + //@} } RTSPState; #define RTSP_FLAG_FILTER_SRC 0x1 /**< Filter incoming UDP packets - diff --git a/libavformat/tls_gnutls.c b/libavformat/tls_gnutls.c index 991b36b..ecc80bf 100644 --- a/libavformat/tls_gnutls.c +++ b/libavformat/tls_gnutls.c @@ -235,6 +235,12 @@ static int tls_write(URLContext *h, const uint8_t *buf, int size) return print_tls_error(h, ret); } +static int tls_get_file_handle(URLContext *h) +{ + TLSContext *c = h->priv_data; + return ffurl_get_file_handle(c->tls_shared.tcp); +} + static const AVOption options[] = { TLS_COMMON_OPTIONS(TLSContext, tls_shared), { NULL } @@ -253,6 +259,7 @@ const URLProtocol ff_tls_gnutls_protocol = { .url_read = tls_read, .url_write = tls_write, .url_close = tls_close, + .url_get_file_handle = tls_get_file_handle, .priv_data_size = sizeof(TLSContext), .flags = URL_PROTOCOL_FLAG_NETWORK, .priv_data_class = &tls_class, diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c index 46eb3e6..1455392 100644 --- a/libavformat/tls_openssl.c +++ b/libavformat/tls_openssl.c @@ -283,6 +283,12 @@ static int tls_write(URLContext *h, const uint8_t *buf, int size) return print_tls_error(h, ret); } +static int tls_get_file_handle(URLContext *h) +{ + TLSContext *c = h->priv_data; + return ffurl_get_file_handle(c->tls_shared.tcp); +} + static const AVOption options[] = { TLS_COMMON_OPTIONS(TLSContext, tls_shared), { NULL } @@ -301,6 +307,7 @@ const URLProtocol ff_tls_openssl_protocol = { .url_read = tls_read, .url_write = tls_write, .url_close = tls_close, + .url_get_file_handle = tls_get_file_handle, .priv_data_size = sizeof(TLSContext), .flags = URL_PROTOCOL_FLAG_NETWORK, .priv_data_class = &tls_class, diff --git a/libavformat/tls_schannel.c b/libavformat/tls_schannel.c index c11b7d4..065dccb 100644 --- a/libavformat/tls_schannel.c +++ b/libavformat/tls_schannel.c @@ -577,6 +577,12 @@ done: return ret < 0 ? ret : outbuf[1].cbBuffer; } +static int tls_get_file_handle(URLContext *h) +{ + TLSContext *c = h->priv_data; + return ffurl_get_file_handle(c->tls_shared.tcp); +} + static const AVOption options[] = { TLS_COMMON_OPTIONS(TLSContext, tls_shared), { NULL } @@ -595,6 +601,7 @@ const URLProtocol ff_tls_schannel_protocol = { .url_read = tls_read, .url_write = tls_write, .url_close = tls_close, + .url_get_file_handle = tls_get_file_handle, .priv_data_size = sizeof(TLSContext), .flags = URL_PROTOCOL_FLAG_NETWORK, .priv_data_class = &tls_class, diff --git a/libavformat/tls_securetransport.c b/libavformat/tls_securetransport.c index 253c89c..bc8a320 100644 --- a/libavformat/tls_securetransport.c +++ b/libavformat/tls_securetransport.c @@ -375,6 +375,12 @@ static int tls_write(URLContext *h, const uint8_t *buf, int size) return print_tls_error(h, ret); } +static int tls_get_file_handle(URLContext *h) +{ + TLSContext *c = h->priv_data; + return ffurl_get_file_handle(c->tls_shared.tcp); +} + static const AVOption options[] = { TLS_COMMON_OPTIONS(TLSContext, tls_shared), { NULL } @@ -393,6 +399,7 @@ const URLProtocol ff_tls_securetransport_protocol = { .url_read = tls_read, .url_write = tls_write, .url_close = tls_close, + .url_get_file_handle = tls_get_file_handle, .priv_data_size = sizeof(TLSContext), .flags = URL_PROTOCOL_FLAG_NETWORK, .priv_data_class = &tls_class,
From: Jay Ridgeway <jayridge@gmail.com> This patch enables TLS args for RTSPS. This is necessary for client certificates and cert validation. Squash changes from feedback into one patch. --- libavformat/rtsp.c | 19 ++++++++++++++++--- libavformat/rtsp.h | 8 ++++++++ libavformat/tls_gnutls.c | 7 +++++++ libavformat/tls_openssl.c | 7 +++++++ libavformat/tls_schannel.c | 7 +++++++ libavformat/tls_securetransport.c | 7 +++++++ 6 files changed, 52 insertions(+), 3 deletions(-)