[FFmpeg-devel] [PATCH] fix rtmp handshake for some streams [v2]

[Michael Niedermayer]

On Thu, Aug 29, 2019 at 03:54:21PM -0700, William Martin wrote:
> From: Will Martin <will.martin at verizondigitalmedia.com>
> 
> Some rtmp streamers (i.e. AWS Elemental Encoder, Wirecast) send C0 and C1 together and expect S0 and S1 returned together. When sent in different packets, this results in a C2 handshake. This patch fixes that error.
> Note that the patch is based off of a fix proposed by rubensanchez in https://trac.ffmpeg.org/ticket/6453.
> The only difference between that propsed fix and this patch is that dummy_unit is declared as a uint32_t instead of unit8_8 (this addresses a crash in debug builds).
> This patch being submitted in a [v2] so that these commit messages could be added for clarity.
> ---
>  libavformat/rtmpproto.c | 103 +++++++++++++++++++++++-----------------
>  1 file changed, 59 insertions(+), 44 deletions(-)
> 
> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
> index b741e421af..24070ba0f5 100644
> --- a/libavformat/rtmpproto.c
> +++ b/libavformat/rtmpproto.c
> @@ -1416,71 +1416,86 @@ static int rtmp_send_hs_packet(RTMPContext* rt, uint32_t first_int,
>   */
>  static int rtmp_server_handshake(URLContext *s, RTMPContext *rt)
>  {
> -    uint8_t buffer[RTMP_HANDSHAKE_PACKET_SIZE];
> -    uint32_t hs_epoch;
> +    uint8_t hs_s0s1[RTMP_HANDSHAKE_PACKET_SIZE + 1];
> +    uint8_t hs_c0c1[RTMP_HANDSHAKE_PACKET_SIZE + 1];
> +    uint8_t hs_c2[RTMP_HANDSHAKE_PACKET_SIZE + 1];
> +    uint8_t hs_s2[RTMP_HANDSHAKE_PACKET_SIZE];
> +    uint32_t dummy_uint;
>      uint32_t hs_my_epoch;
> -    uint8_t hs_c1[RTMP_HANDSHAKE_PACKET_SIZE];
> -    uint8_t hs_s1[RTMP_HANDSHAKE_PACKET_SIZE];
> -    uint32_t zeroes;
>      uint32_t temp       = 0;
>      int randomidx       = 0;
>      int inoutsize       = 0;
>      int ret;
>  
> -    inoutsize = ffurl_read_complete(rt->stream, buffer, 1);       // Receive C0
> -    if (inoutsize <= 0) {
> -        av_log(s, AV_LOG_ERROR, "Unable to read handshake\n");
> -        return AVERROR(EIO);
> +    /****************
> +     * Receive C0+C1
> +     ***************/
> +    ret = rtmp_receive_hs_packet(rt, &dummy_uint, &dummy_uint, hs_c0c1,
> +                                 RTMP_HANDSHAKE_PACKET_SIZE + 1);
> +    if (ret) {
> +        av_log(s, AV_LOG_ERROR, "RTMP Handshake C1 Error %d\n", ret);
> +        return ret;
>      }
>      // Check Version
> -    if (buffer[0] != 3) {
> -        av_log(s, AV_LOG_ERROR, "RTMP protocol version mismatch\n");
> +    if (hs_c0c1[0] != 3) {
> +        av_log(s, AV_LOG_ERROR, "RTMP protocol version mismatch. Expected 0x03 received %02x\n", hs_c0c1[0]);
>          return AVERROR(EIO);
>      }
> -    if (ffurl_write(rt->stream, buffer, 1) <= 0) {                 // Send S0
> -        av_log(s, AV_LOG_ERROR,
> -               "Unable to write answer - RTMP S0\n");
> +    // Get client epoch and set our with the same value
> +    hs_my_epoch = AV_RB32(hs_c0c1 + 1);
> +
> +    /*************
> +     * Send S0+S1
> +     ************/
> +    // Generate random data to send it on S0+S1
> +    for (randomidx = 9; randomidx < (RTMP_HANDSHAKE_PACKET_SIZE + 1);
> +         randomidx += 4)
> +        AV_WB32(hs_s0s1 + randomidx, av_get_random_seed());
> +    // Set the RTMP protocol code on S0+S1 (First byte)
> +    hs_s0s1[0] = 0x03;
> +    // Copy the random data from C1 to S1
> +    memcpy(hs_s0s1 + 1, hs_c0c1 + 1, RTMP_HANDSHAKE_PACKET_SIZE);
> +    AV_WB32(hs_s0s1 + 1, hs_my_epoch);
> +    AV_WB32(hs_s0s1 + 5, 0);
> +    inoutsize = ffurl_write(rt->stream, hs_s0s1,
> +                            RTMP_HANDSHAKE_PACKET_SIZE + 1);
> +    if (inoutsize != RTMP_HANDSHAKE_PACKET_SIZE + 1) {
> +        av_log(s, AV_LOG_ERROR, "RTMP Handshake S1 Error %d\n", ret);
>          return AVERROR(EIO);
>      }
> -    /* Receive C1 */
> -    ret = rtmp_receive_hs_packet(rt, &hs_epoch, &zeroes, hs_c1,
> -                                 RTMP_HANDSHAKE_PACKET_SIZE);
> -    if (ret) {
> -        av_log(s, AV_LOG_ERROR, "RTMP Handshake C1 Error\n");
> -        return ret;
> -    }
> -    /* Send S1 */
> -    /* By now same epoch will be sent */
> -    hs_my_epoch = hs_epoch;
> -    /* Generate random */
> -    for (randomidx = 8; randomidx < (RTMP_HANDSHAKE_PACKET_SIZE);
> -         randomidx += 4)
> -        AV_WB32(hs_s1 + randomidx, av_get_random_seed());
>  
> -    ret = rtmp_send_hs_packet(rt, hs_my_epoch, 0, hs_s1,
> -                              RTMP_HANDSHAKE_PACKET_SIZE);
> -    if (ret) {
> -        av_log(s, AV_LOG_ERROR, "RTMP Handshake S1 Error\n");
> -        return ret;
> -    }

> -    /* Send S2 */
> +    /***********
> +     * Send S2
> +     **********/
> +    // Get the S2 random data from C0+C1
> +    memcpy(hs_s2, hs_c0c1, RTMP_HANDSHAKE_PACKET_SIZE);

> -    ret = rtmp_send_hs_packet(rt, hs_epoch, 0, hs_c1,
> +    ret = rtmp_send_hs_packet(rt, hs_my_epoch, 0, hs_s2,

>                                RTMP_HANDSHAKE_PACKET_SIZE);
>      if (ret) {
>          av_log(s, AV_LOG_ERROR, "RTMP Handshake S2 Error\n");
>          return ret;
>      }

> -    /* Receive C2 */
> +
> +    /*************
> +     * Receive C2
> +     ************/

this patch is mixing cosmetics and functional changes and is quite
messy, its unlikely that this will be applied
and its not easy to review because of this as well

If you could try to make it look more tidy overall so its easy
to review then its more likely to receice usefull review comments

generally, if you need to rename, move or reformat something do it in 
a seperate patch. This should make this more readable

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190921/d43feaa4/attachment.sig>