Repository: bluenviron/gortsplib
Pull request #272 (2023-05-07 17:28:41)

auth: add Validate() and deprecate Validator{}

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/bluenviron/gortsplib/pull/272?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) Report
> Merging [#272](https://app.codecov.io/gh/bluenviron/gortsplib/pull/272?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) (72eb116) into [main](https://app.codecov.io/gh/bluenviron/gortsplib/commit/2170ef4b006bf09225bf173afe7398da211a31ef?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) (2170ef4) will **decrease** coverage by `0.58%`.
> The diff coverage is `93.63%`.

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron)

```diff
@@            Coverage Diff             @@
##             main     #272      +/-   ##
==========================================
- Coverage   86.23%   85.66%   -0.58%     
==========================================
  Files          93       93              
  Lines        9553     9640      +87     
==========================================
+ Hits         8238     8258      +20     
- Misses       1024     1090      +66     
- Partials      291      292       +1     
```


| [Impacted Files](https://app.codecov.io/gh/bluenviron/gortsplib/pull/272?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) | Coverage Δ | |
|---|---|---|
| [pkg/auth/validator.go](https://app.codecov.io/gh/bluenviron/gortsplib/pull/272?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron#diff-cGtnL2F1dGgvdmFsaWRhdG9yLmdv) | `42.27% <55.55%> (-57.73%)` | :arrow_down: |
| [pkg/auth/validate.go](https://app.codecov.io/gh/bluenviron/gortsplib/pull/272?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron#diff-cGtnL2F1dGgvdmFsaWRhdGUuZ28=) | `96.77% <96.77%> (ø)` | |
| [pkg/auth/sender.go](https://app.codecov.io/gh/bluenviron/gortsplib/pull/272?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron#diff-cGtnL2F1dGgvc2VuZGVyLmdv) | `100.00% <100.00%> (ø)` | |

... and [3 files with indirect coverage changes](https://app.codecov.io/gh/bluenviron/gortsplib/pull/272/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron)


Patch:
diff --git a/pkg/auth/sender.go b/pkg/auth/sender.go
index c5e22c5d..1efcd7e7 100644
--- a/pkg/auth/sender.go
+++ b/pkg/auth/sender.go
@@ -8,7 +8,16 @@ import (
 	"github.com/bluenviron/gortsplib/v3/pkg/headers"
 )
 
-// Sender allows to generate credentials for a Validator.
+func findHeader(v base.HeaderValue, prefix string) string {
+	for _, vi := range v {
+		if strings.HasPrefix(vi, prefix) {
+			return vi
+		}
+	}
+	return ""
+}
+
+// Sender allows to send credentials.
 type Sender struct {
 	user   string
 	pass   string
@@ -17,18 +26,12 @@ type Sender struct {
 	nonce  string
 }
 
-// NewSender allocates a Sender with the WWW-Authenticate header provided by
-// a Validator and a set of credentials.
+// NewSender allocates a Sender.
+// It requires a WWW-Authenticate header (provided by the server)
+// and a set of credentials.
 func NewSender(v base.HeaderValue, user string, pass string) (*Sender, error) {
 	// prefer digest
-	if v0 := func() string {
-		for _, vi := range v {
-			if strings.HasPrefix(vi, "Digest") {
-				return vi
-			}
-		}
-		return ""
-	}(); v0 != "" {
+	if v0 := findHeader(v, "Digest"); v0 != "" {
 		var auth headers.Authenticate
 		err := auth.Unmarshal(base.HeaderValue{v0})
 		if err != nil {
@@ -52,14 +55,7 @@ func NewSender(v base.HeaderValue, user string, pass string) (*Sender, error) {
 		}, nil
 	}
 
-	if v0 := func() string {
-		for _, vi := range v {
-			if strings.HasPrefix(vi, "Basic") {
-				return vi
-			}
-		}
-		return ""
-	}(); v0 != "" {
+	if v0 := findHeader(v, "Basic"); v0 != "" {
 		var auth headers.Authenticate
 		err := auth.Unmarshal(base.HeaderValue{v0})
 		if err != nil {
diff --git a/pkg/auth/utils.go b/pkg/auth/utils.go
deleted file mode 100644
index 29383cd6..00000000
--- a/pkg/auth/utils.go
+++ /dev/null
@@ -1,20 +0,0 @@
-package auth
-
-import (
-	"crypto/md5"
-	"crypto/sha256"
-	"encoding/base64"
-	"encoding/hex"
-)
-
-func md5Hex(in string) string {
-	h := md5.New()
-	h.Write([]byte(in))
-	return hex.EncodeToString(h.Sum(nil))
-}
-
-func sha256Base64(in string) string {
-	h := sha256.New()
-	h.Write([]byte(in))
-	return base64.StdEncoding.EncodeToString(h.Sum(nil))
-}
diff --git a/pkg/auth/validate.go b/pkg/auth/validate.go
new file mode 100644
index 00000000..6bbedd58
--- /dev/null
+++ b/pkg/auth/validate.go
@@ -0,0 +1,143 @@
+package auth
+
+import (
+	"crypto/rand"
+	"encoding/hex"
+	"fmt"
+
+	"github.com/bluenviron/gortsplib/v3/pkg/base"
+	"github.com/bluenviron/gortsplib/v3/pkg/headers"
+	"github.com/bluenviron/gortsplib/v3/pkg/url"
+)
+
+// GenerateNonce generates a nonce that can be used in Validate().
+func GenerateNonce() string {
+	byts := make([]byte, 16)
+	rand.Read(byts)
+	return hex.EncodeToString(byts)
+}
+
+// GenerateWWWAuthenticate generates a WWW-Authenticate header.
+func GenerateWWWAuthenticate(methods []headers.AuthMethod, realm string, nonce string) base.HeaderValue {
+	if methods == nil {
+		methods = []headers.AuthMethod{headers.AuthBasic, headers.AuthDigest}
+	}
+
+	var ret base.HeaderValue
+	for _, m := range methods {
+		switch m {
+		case headers.AuthBasic:
+			ret = append(ret, (&headers.Authenticate{
+				Method: headers.AuthBasic,
+				Realm:  &realm,
+			}).Marshal()...)
+
+		case headers.AuthDigest:
+			ret = append(ret, headers.Authenticate{
+				Method: headers.AuthDigest,
+				Realm:  &realm,
+				Nonce:  &nonce,
+			}.Marshal()...)
+		}
+	}
+	return ret
+}
+
+func contains(list []headers.AuthMethod, item headers.AuthMethod) bool {
+	for _, i := range list {
+		if i == item {
+			return true
+		}
+	}
+	return false
+}
+
+// Validate validates a request sent by a client.
+func Validate(
+	req *base.Request,
+	user string,
+	pass string,
+	baseURL *url.URL,
+	methods []headers.AuthMethod,
+	realm string,
+	nonce string,
+) error {
+	if methods == nil {
+		methods = []headers.AuthMethod{headers.AuthBasic, headers.AuthDigest}
+	}
+
+	var auth headers.Authorization
+	err := auth.Unmarshal(req.Header["Authorization"])
+	if err != nil {
+		return err
+	}
+
+	switch {
+	case auth.Method == headers.AuthBasic && contains(methods, headers.AuthBasic):
+		if auth.BasicUser != user {
+			return fmt.Errorf("authentication failed")
+		}
+
+		if auth.BasicPass != pass {
+			return fmt.Errorf("authentication failed")
+		}
+	case auth.Method == headers.AuthDigest && contains(methods, headers.AuthDigest):
+		if auth.DigestValues.Realm == nil {
+			return fmt.Errorf("realm is missing")
+		}
+
+		if auth.DigestValues.Nonce == nil {
+			return fmt.Errorf("nonce is missing")
+		}
+
+		if auth.DigestValues.Username == nil {
+			return fmt.Errorf("username is missing")
+		}
+
+		if auth.DigestValues.URI == nil {
+			return fmt.Errorf("uri is missing")
+		}
+
+		if auth.DigestValues.Response == nil {
+			return fmt.Errorf("response is missing")
+		}
+
+		if *auth.DigestValues.Nonce != nonce {
+			return fmt.Errorf("wrong nonce")
+		}
+
+		if *auth.DigestValues.Realm != realm {
+			return fmt.Errorf("wrong realm")
+		}
+
+		if *auth.DigestValues.Username != user {
+			return fmt.Errorf("authentication failed")
+		}
+
+		ur := req.URL
+
+		if *auth.DigestValues.URI != ur.String() {
+			// in SETUP requests, VLC strips the control attribute.
+			// try again with the base URL.
+			if baseURL != nil {
+				ur = baseURL
+				if *auth.DigestValues.URI != ur.String() {
+					return fmt.Errorf("wrong URL")
+				}
+			} else {
+				return fmt.Errorf("wrong URL")
+			}
+		}
+
+		response := md5Hex(md5Hex(user+":"+realm+":"+pass) +
+			":" + nonce + ":" + md5Hex(string(req.Method)+":"+ur.String()))
+
+		if *auth.DigestValues.Response != response {
+			return fmt.Errorf("authentication failed")
+		}
+	default:
+		return fmt.Errorf("no supported authentication methods found")
+	}
+
+	return nil
+}
diff --git a/pkg/auth/validator.go b/pkg/auth/validator.go
index a4a2f7a2..bb5fbdfa 100644
--- a/pkg/auth/validator.go
+++ b/pkg/auth/validator.go
@@ -1,7 +1,10 @@
 package auth
 
 import (
+	"crypto/md5"
 	"crypto/rand"
+	"crypto/sha256"
+	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"strings"
@@ -11,7 +14,20 @@ import (
 	"github.com/bluenviron/gortsplib/v3/pkg/url"
 )
 
+func md5Hex(in string) string {
+	h := md5.New()
+	h.Write([]byte(in))
+	return hex.EncodeToString(h.Sum(nil))
+}
+
+func sha256Base64(in string) string {
+	h := sha256.New()
+	h.Write([]byte(in))
+	return base64.StdEncoding.EncodeToString(h.Sum(nil))
+}
+
 // Validator allows to validate credentials generated by a Sender.
+// Deprecated: Validator{} has been replaced by Validate()
 type Validator struct {
 	user       string
 	userHashed bool
@@ -24,6 +40,7 @@ type Validator struct {
 
 // NewValidator allocates a Validator.
 // If methods is nil, the Basic and Digest methods are used.
+// Deprecated: Validator{} has been replaced by Validate()
 func NewValidator(user string, pass string, methods []headers.AuthMethod) *Validator {
 	if methods == nil {
 		methods = []headers.AuthMethod{headers.AuthBasic, headers.AuthDigest}
@@ -63,6 +80,7 @@ func NewValidator(user string, pass string, methods []headers.AuthMethod) *Valid
 
 // Header generates the WWW-Authenticate header needed by a client to
 // authenticate.
+// Deprecated: Validator{} has been replaced by Validate()
 func (va *Validator) Header() base.HeaderValue {
 	var ret base.HeaderValue
 	for _, m := range va.methods {
@@ -84,8 +102,9 @@ func (va *Validator) Header() base.HeaderValue {
 	return ret
 }
 
-// ValidateRequest validates a request sent by a client.
-func (va *Validator) ValidateRequest(req *base.Request, baseURL *url.URL) error {
+// Validate validates a request sent by a client.
+// Deprecated: Validator{} has been replaced by Validate()
+func (va *Validator) Validate(req *base.Request, baseURL *url.URL) error {
 	var auth headers.Authorization
 	err := auth.Unmarshal(req.Header["Authorization"])
 	if err != nil {


Test patch:
diff --git a/client_play_test.go b/client_play_test.go
index 01460311..83b96fa0 100644
--- a/client_play_test.go
+++ b/client_play_test.go
@@ -1286,12 +1286,12 @@ func TestClientPlayAutomaticProtocol(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, base.Describe, req.Method)
 
-				v := auth.NewValidator("myuser", "mypass", nil)
+				nonce := auth.GenerateNonce()
 
 				err = conn.WriteResponse(&base.Response{
 					StatusCode: base.StatusUnauthorized,
 					Header: base.Header{
-						"WWW-Authenticate": v.Header(),
+						"WWW-Authenticate": auth.GenerateWWWAuthenticate(nil, "IPCAM", nonce),
 					},
 				})
 				require.NoError(t, err)
@@ -1300,7 +1300,7 @@ func TestClientPlayAutomaticProtocol(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, base.Describe, req.Method)
 
-				err = v.ValidateRequest(req, nil)
+				err = auth.Validate(req, "myuser", "mypass", nil, nil, "IPCAM", nonce)
 				require.NoError(t, err)
 
 				err = conn.WriteResponse(&base.Response{
@@ -1399,12 +1399,12 @@ func TestClientPlayAutomaticProtocol(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, base.Setup, req.Method)
 
-				v := auth.NewValidator("myuser", "mypass", nil)
+				nonce := auth.GenerateNonce()
 
 				err = conn.WriteResponse(&base.Response{
 					StatusCode: base.StatusUnauthorized,
 					Header: base.Header{
-						"WWW-Authenticate": v.Header(),
+						"WWW-Authenticate": auth.GenerateWWWAuthenticate(nil, "IPCAM", nonce),
 					},
 				})
 				require.NoError(t, err)
@@ -1414,7 +1414,7 @@ func TestClientPlayAutomaticProtocol(t *testing.T) {
 				require.Equal(t, base.Setup, req.Method)
 				require.Equal(t, mustParseURL("rtsp://localhost:8554/teststream/"+medias[0].Control), req.URL)
 
-				err = v.ValidateRequest(req, nil)
+				err = auth.Validate(req, "myuser", "mypass", nil, nil, "IPCAM", nonce)
 				require.NoError(t, err)
 
 				var inTH headers.Transport
diff --git a/client_test.go b/client_test.go
index a8143fbf..e34f1163 100644
--- a/client_test.go
+++ b/client_test.go
@@ -165,12 +165,12 @@ func TestClientAuth(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, base.Describe, req.Method)
 
-		v := auth.NewValidator("myuser", "mypass", nil)
+		nonce := auth.GenerateNonce()
 
 		err = conn.WriteResponse(&base.Response{
 			StatusCode: base.StatusUnauthorized,
 			Header: base.Header{
-				"WWW-Authenticate": v.Header(),
+				"WWW-Authenticate": auth.GenerateWWWAuthenticate(nil, "IPCAM", nonce),
 			},
 		})
 		require.NoError(t, err)
@@ -179,7 +179,7 @@ func TestClientAuth(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, base.Describe, req.Method)
 
-		err = v.ValidateRequest(req, nil)
+		err = auth.Validate(req, "myuser", "mypass", nil, nil, "IPCAM", nonce)
 		require.NoError(t, err)
 
 		medias := media.Medias{testH264Media}
diff --git a/pkg/auth/package.go b/pkg/auth/auth.go
similarity index 100%
rename from pkg/auth/package.go
rename to pkg/auth/auth.go
diff --git a/pkg/auth/package_test.go b/pkg/auth/auth_test.go
similarity index 59%
rename from pkg/auth/package_test.go
rename to pkg/auth/auth_test.go
index 112caabe..a2731a88 100644
--- a/pkg/auth/package_test.go
+++ b/pkg/auth/auth_test.go
@@ -47,10 +47,10 @@ func TestAuth(t *testing.T) {
 			}
 
 			t.Run(c1.name+"_"+conf, func(t *testing.T) {
-				va := NewValidator("testuser", "testpass", c1.methods)
-				wwwAuthenticate := va.Header()
+				nonce := GenerateNonce()
 
-				se, err := NewSender(wwwAuthenticate,
+				se, err := NewSender(
+					GenerateWWWAuthenticate(c1.methods, "IPCAM", nonce),
 					func() string {
 						if conf == "wronguser" {
 							return "test1user"
@@ -78,7 +78,7 @@ func TestAuth(t *testing.T) {
 
 				req.URL = mustParseURL("rtsp://myhost/mypath")
 
-				err = va.ValidateRequest(req, nil)
+				err = Validate(req, "testuser", "testpass", nil, c1.methods, "IPCAM", nonce)
 
 				if conf != "nofail" {
 					require.Error(t, err)
@@ -104,10 +104,12 @@ func TestAuthVLC(t *testing.T) {
 			"rtsp://myhost/mypath/test?testing/trackID=0",
 		},
 	} {
-		va := NewValidator("testuser", "testpass",
-			[]headers.AuthMethod{headers.AuthBasic, headers.AuthDigest})
+		nonce := GenerateNonce()
 
-		se, err := NewSender(va.Header(), "testuser", "testpass")
+		se, err := NewSender(
+			GenerateWWWAuthenticate(nil, "IPCAM", nonce),
+			"testuser",
+			"testpass")
 		require.NoError(t, err)
 
 		req := &base.Request{
@@ -117,53 +119,10 @@ func TestAuthVLC(t *testing.T) {
 		se.AddAuthorization(req)
 		req.URL = mustParseURL(ca.mediaURL)
 
-		err = va.ValidateRequest(req, mustParseURL(ca.clientURL))
+		err = Validate(req, "testuser", "testpass", mustParseURL(ca.clientURL), nil, "IPCAM", nonce)
 		require.NoError(t, err)
 
-		err = va.ValidateRequest(req, mustParseURL("rtsp://invalid"))
+		err = Validate(req, "testuser", "testpass", mustParseURL("rtsp://invalid"), nil, "IPCAM", nonce)
 		require.Error(t, err)
 	}
 }
-
-func TestAuthHashed(t *testing.T) {
-	for _, conf := range []string{
-		"nofail",
-		"wronguser",
-		"wrongpass",
-	} {
-		t.Run(conf, func(t *testing.T) {
-			se := NewValidator("sha256:rl3rgi4NcZkpAEcacZnQ2VuOfJ0FxAqCRaKB/SwdZoQ=",
-				"sha256:E9JJ8stBJ7QM+nV4ZoUCeHk/gU3tPFh/5YieiJp6n2w=",
-				[]headers.AuthMethod{headers.AuthBasic, headers.AuthDigest})
-
-			va, err := NewSender(se.Header(),
-				func() string {
-					if conf == "wronguser" {
-						return "test1user"
-					}
-					return "testuser"
-				}(),
-				func() string {
-					if conf == "wrongpass" {
-						return "test1pass"
-					}
-					return "testpass"
-				}())
-			require.NoError(t, err)
-
-			req := &base.Request{
-				Method: base.Announce,
-				URL:    mustParseURL("rtsp://myhost/mypath"),
-			}
-			va.AddAuthorization(req)
-
-			err = se.ValidateRequest(req, nil)
-
-			if conf != "nofail" {
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-			}
-		})
-	}
-}
diff --git a/pkg/auth/validate_test.go b/pkg/auth/validate_test.go
new file mode 100644
index 00000000..b8003665
--- /dev/null
+++ b/pkg/auth/validate_test.go
@@ -0,0 +1,77 @@
+package auth
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/bluenviron/gortsplib/v3/pkg/base"
+)
+
+func TestValidateErrors(t *testing.T) {
+	for _, ca := range []struct {
+		name string
+		hv   base.HeaderValue
+		err  string
+	}{
+		{
+			"invalid auth",
+			base.HeaderValue{`Invalid`},
+			"invalid authorization header",
+		},
+		{
+			"digest missing realm",
+			base.HeaderValue{`Digest `},
+			"realm is missing",
+		},
+		{
+			"digest missing nonce",
+			base.HeaderValue{`Digest realm=123`},
+			"nonce is missing",
+		},
+		{
+			"digest missing username",
+			base.HeaderValue{`Digest realm=123,nonce=123`},
+			"username is missing",
+		},
+		{
+			"digest missing uri",
+			base.HeaderValue{`Digest realm=123,nonce=123,username=123`},
+			"uri is missing",
+		},
+		{
+			"digest missing response",
+			base.HeaderValue{`Digest realm=123,nonce=123,username=123,uri=123`},
+			"response is missing",
+		},
+		{
+			"digest wrong nonce",
+			base.HeaderValue{`Digest realm=123,nonce=123,username=123,uri=123,response=123`},
+			"wrong nonce",
+		},
+		{
+			"digest wrong realm",
+			base.HeaderValue{`Digest realm=123,nonce=abcde,username=123,uri=123,response=123`},
+			"wrong realm",
+		},
+	} {
+		t.Run(ca.name, func(t *testing.T) {
+			err := Validate(
+				&base.Request{
+					Method: base.Describe,
+					URL:    nil,
+					Header: base.Header{
+						"Authorization": ca.hv,
+					},
+				},
+				"myuser",
+				"mypass",
+				nil,
+				nil,
+				"IPCAM",
+				"abcde",
+			)
+			require.EqualError(t, err, ca.err)
+		})
+	}
+}
diff --git a/pkg/auth/validator_test.go b/pkg/auth/validator_test.go
index 0b5bad1f..d8210af2 100644
--- a/pkg/auth/validator_test.go
+++ b/pkg/auth/validator_test.go
@@ -58,7 +58,7 @@ func TestValidatorErrors(t *testing.T) {
 		t.Run(ca.name, func(t *testing.T) {
 			va := NewValidator("myuser", "mypass", nil)
 			va.nonce = "abcde"
-			err := va.ValidateRequest(&base.Request{
+			err := va.Validate(&base.Request{
 				Method: base.Describe,
 				URL:    nil,
 				Header: base.Header{
diff --git a/server_test.go b/server_test.go
index 16b36fa6..349332e0 100644
--- a/server_test.go
+++ b/server_test.go
@@ -1045,17 +1045,17 @@ func TestServerSessionTeardown(t *testing.T) {
 }
 
 func TestServerAuth(t *testing.T) {
-	authValidator := auth.NewValidator("myuser", "mypass", nil)
+	nonce := auth.GenerateNonce()
 
 	s := &Server{
 		Handler: &testServerHandler{
 			onAnnounce: func(ctx *ServerHandlerOnAnnounceCtx) (*base.Response, error) {
-				err := authValidator.ValidateRequest(ctx.Request, nil)
+				err := auth.Validate(ctx.Request, "myuser", "mypass", nil, nil, "IPCAM", nonce)
 				if err != nil {
 					return &base.Response{
 						StatusCode: base.StatusUnauthorized,
 						Header: base.Header{
-							"WWW-Authenticate": authValidator.Header(),
+							"WWW-Authenticate": auth.GenerateWWWAuthenticate(nil, "IPCAM", nonce),
 						},
 					}, nil
 				}