scottlamb / retina

High-level RTSP multimedia streaming library, in Rust
https://crates.io/crates/retina
Apache License 2.0
244 stars 48 forks source link

support basic authentication #25

Closed scottlamb closed 3 years ago

scottlamb commented 3 years ago

From https://github.com/scottlamb/moonfire-nvr/discussions/151. When configured with username and password, Anpviz IPC-D250 reports:

W20210823 05:40:40.871 s-ChuckRear-sub moonfire_nvr::streamer] ChuckRear-sub: sleeping for Duration { secs: 1, nanos: 0 } after error: [192.168.254.254:39730(me)->192.168.254.5:554@2021-08-23T05:40:40, 0@2021-08-23T05:40:40] Unauthorized response to DESCRIBE CSeq=1: Non-digest authentication requested: Basic realm="/"
scottlamb commented 3 years ago

Hmm, servers are allowed to specify more than one challenge as a comma-separated list (or as multiple headers, which rtsp-types converts to the comma-separated form here). Ideally we would support that and pick out digest if it's present, then basic if it's present, then err out otherwise.

Splitting the WWW-Authenticate header into challenges is more than header_value.split(", ") because , can occur in a quoted string. See the grammar from RFC 7235 Appendix C:

   WWW-Authenticate = *( "," OWS ) challenge *( OWS "," [ OWS challenge
    ] )

   auth-param = token BWS "=" BWS ( token / quoted-string )
   auth-scheme = token

   challenge = auth-scheme [ 1*SP ( token68 / [ ( "," / auth-param ) *(
    OWS "," [ OWS auth-param ] ) ] ) ]
   credentials = auth-scheme [ 1*SP ( token68 / [ ( "," / auth-param )
    *( OWS "," [ OWS auth-param ] ) ] ) ]

   quoted-string = <quoted-string, see [RFC7230], Section 3.2.6>

   token = <token, see [RFC7230], Section 3.2.6>
   token68 = 1*( ALPHA / DIGIT / "-" / "." / "_" / "~" / "+" / "/" )
    *"="

We could parse to split it properly, then give the digest part to digest_auth to parse again, but that's annoying to parse twice.

@lucaszanella you were working to polish up www-authenticate (KeenS/www-authenticate#4). Do you want to finish that? It'd parse everything in one go.

scottlamb commented 3 years ago

I'm assuming from the name digest_auth that its author isn't interested in expanding it to parse other challenge types like basic.

scottlamb commented 3 years ago

The Anpviz IPC-D250 is just sending this:

WWW-Authenticate: Basic realm="/"

as mentioned above. It wouldn't be hard to directly hack support for this simple case into Retina, but I'm trying to look forward a bit to make sure we're doing the right thing on cameras which may offer multiple challenges. The www-authenticate crate (with your changes to get rid of those unsound-looking transmutes) is probably not far off from doing this properly.

Eg my Hikvision cameras support this:

WWW-Authenticate: Digest realm="8ce748da948f", nonce="9d6f3162baa241e62a3c134c0acf678b", stale="FALSE"
WWW-Authenticate: Basic realm="8ce748da948f"

which I believe rtsp-types transforms into the equivalent:

WWW-Authenticate: Digest realm="8ce748da948f", nonce="9d6f3162baa241e62a3c134c0acf678b", stale="FALSE", Basic realm="8ce748da948f"

Retina currently works correctly with this, but will probably fail if the headers were in the reverse order. It doesn't support Basic, and it doesn't know how to parse multiple challenges or that it should prefer the stronger Digest.

lattice0 commented 3 years ago

I'm confused. Shouldn't these 2 WWW-Authenticate be treated separately and sent to www-authenticate?

Also,

    fn test_parse_crazy_answer() {
        let b = b"Basic realm=\"/\"";
        let stream = Stream::new(b);
        match stream.challenge().unwrap() {
            (_, RawChallenge::Token68(_)) => panic!(),
            (scheme, RawChallenge::Fields(fields)) => {
                assert_eq!(scheme, "Basic");
            }
        }
        assert!(stream.is_end());
    }

works on my crate

scottlamb commented 3 years ago

I'm confused. Shouldn't these 2 WWW-Authenticate be treated separately and sent to www-authenticate?

RFC 2636 section 4.2 refers to RFC 2616 section 4.2, which says the following:

   Multiple message-header fields with the same field-name MAY be
   present in a message if and only if the entire field-value for that
   header field is defined as a comma-separated list [i.e., #(values)].
   It MUST be possible to combine the multiple header fields into one
   "field-name: field-value" pair, without changing the semantics of the
   message, by appending each subsequent field-value to the first, each
   separated by a comma. The order in which header fields with the same
   field-name are received is therefore significant to the
   interpretation of the combined field value, and thus a proxy MUST NOT
   change the order of these field values when a message is forwarded.

and rtsp-types combines it that way, with the code I linked above.