diff --git a/CHANGELOG.md b/CHANGELOG.md index dfeae3b1..ed20426c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,41 @@ and this project adheres to ### Security +#### `SameSite` Policy + +The `SameSite` policy on the AdGuard Home session cookies is now set to `Lax`. +Which means that the only cross-site HTTP request for which the browser is +allowed to send the session cookie is navigating to the AdGuard Home domain. + +**Users are strongly advised to log out, clear browser cache, and log in again +after updating.** + +#### Removal Of Plain-Text APIs (BREAKING API CHANGE) + +A Cross-Site Request Forgery (CSRF) vulnerability has been discovered. We have +implemented several measures to prevent such vulnerabilities in the future, but +some of these measures break backwards compatibility for the sake of better +protection. + +The following APIs, which previously accepted or returned `text/plain` data, +now accept or return data as JSON. All new formats for the request and response +bodies are documented in `openapi/openapi.yaml` and `openapi/CHANGELOG.md`. + +- `GET /control/i18n/current_language`; +- `POST /control/dhcp/find_active_dhcp`; +- `POST /control/filtering/set_rules`; +- `POST /control/i18n/change_language`. + +The CVE number is to be assigned. We thank Daniel Elkabes from Mend.io for +reporting this vulnerability to us. + +#### Stricter Content-Type Checks (BREAKING API CHANGE) + +All JSON APIs now check if the request actually has the `application/json` +content-type. + +#### Other Security Changes + - Weaker cipher suites that use the CBC (cipher block chaining) mode of operation have been disabled ([#2993]). diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..2fa3a67b --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,18 @@ + # Security Policy + +## Reporting a Vulnerability + +Please send your vulnerability reports to . To make sure +that your report reaches us, please: + +1. Include the words “AdGuard Home” and “vulnerability” to the subject line as + well as a short description of the vulnerability. For example: + + > AdGuard Home API vulnerability: possible XSS attack + +2. Make sure that the message body contains a clear description of the + vulnerability. + +If you have not received a reply to your email within 7 days, please make sure +to follow up with us again at . Once again, make sure +that the word “vulnerability” is in the subject line. diff --git a/client/src/actions/filtering.js b/client/src/actions/filtering.js index 02bc97f8..6039fdcc 100644 --- a/client/src/actions/filtering.js +++ b/client/src/actions/filtering.js @@ -31,7 +31,9 @@ export const setRulesSuccess = createAction('SET_RULES_SUCCESS'); export const setRules = (rules) => async (dispatch) => { dispatch(setRulesRequest()); try { - const normalizedRules = normalizeRulesTextarea(rules); + const normalizedRules = { + rules: normalizeRulesTextarea(rules)?.split('\n'), + }; await apiClient.setRules(normalizedRules); dispatch(addSuccessToast('updated_custom_filtering_toast')); dispatch(setRulesSuccess()); diff --git a/client/src/actions/index.js b/client/src/actions/index.js index e1b4e96c..2242490b 100644 --- a/client/src/actions/index.js +++ b/client/src/actions/index.js @@ -355,7 +355,7 @@ export const changeLanguageSuccess = createAction('CHANGE_LANGUAGE_SUCCESS'); export const changeLanguage = (lang) => async (dispatch) => { dispatch(changeLanguageRequest()); try { - await apiClient.changeLanguage(lang); + await apiClient.changeLanguage({ language: lang }); dispatch(changeLanguageSuccess()); } catch (error) { dispatch(addErrorToast({ error })); @@ -370,8 +370,8 @@ export const getLanguageSuccess = createAction('GET_LANGUAGE_SUCCESS'); export const getLanguage = () => async (dispatch) => { dispatch(getLanguageRequest()); try { - const language = await apiClient.getCurrentLanguage(); - dispatch(getLanguageSuccess(language)); + const langSettings = await apiClient.getCurrentLanguage(); + dispatch(getLanguageSuccess(langSettings.language)); } catch (error) { dispatch(addErrorToast({ error })); dispatch(getLanguageFailure()); @@ -421,7 +421,10 @@ export const findActiveDhcpFailure = createAction('FIND_ACTIVE_DHCP_FAILURE'); export const findActiveDhcp = (name) => async (dispatch, getState) => { dispatch(findActiveDhcpRequest()); try { - const activeDhcp = await apiClient.findActiveDhcp(name); + const req = { + interface: name, + }; + const activeDhcp = await apiClient.findActiveDhcp(req); dispatch(findActiveDhcpSuccess(activeDhcp)); const { check, interface_name, interfaces } = getState().dhcp; const selectedInterface = getState().form[FORM_NAME.DHCP_INTERFACES].values.interface_name; diff --git a/client/src/api/Api.js b/client/src/api/Api.js index d5693bfe..113c2c00 100644 --- a/client/src/api/Api.js +++ b/client/src/api/Api.js @@ -130,7 +130,7 @@ class Api { const { path, method } = this.FILTERING_SET_RULES; const parameters = { data: rules, - headers: { 'Content-Type': 'text/plain' }, + headers: { 'Content-Type': 'application/json' }, }; return this.makeRequest(path, method, parameters); } @@ -173,12 +173,7 @@ class Api { enableParentalControl() { const { path, method } = this.PARENTAL_ENABLE; - const parameter = 'sensitivity=TEEN'; // this parameter TEEN is hardcoded - const config = { - data: parameter, - headers: { 'Content-Type': 'text/plain' }, - }; - return this.makeRequest(path, method, config); + return this.makeRequest(path, method); } disableParentalControl() { @@ -240,11 +235,11 @@ class Api { return this.makeRequest(path, method); } - changeLanguage(lang) { + changeLanguage(config) { const { path, method } = this.CHANGE_LANGUAGE; const parameters = { - data: lang, - headers: { 'Content-Type': 'text/plain' }, + data: config, + headers: { 'Content-Type': 'application/json' }, }; return this.makeRequest(path, method, parameters); } @@ -285,11 +280,11 @@ class Api { return this.makeRequest(path, method, parameters); } - findActiveDhcp(name) { + findActiveDhcp(req) { const { path, method } = this.DHCP_FIND_ACTIVE; const parameters = { - data: name, - headers: { 'Content-Type': 'text/plain' }, + data: req, + headers: { 'Content-Type': 'application/json' }, }; return this.makeRequest(path, method, parameters); } diff --git a/internal/aghhttp/aghhttp.go b/internal/aghhttp/aghhttp.go index 23f9f5d3..8f786749 100644 --- a/internal/aghhttp/aghhttp.go +++ b/internal/aghhttp/aghhttp.go @@ -2,10 +2,12 @@ package aghhttp import ( + "encoding/json" "fmt" "io" "net/http" + "github.com/AdguardTeam/AdGuardHome/internal/version" "github.com/AdguardTeam/golibs/log" ) @@ -34,3 +36,40 @@ func Error(r *http.Request, w http.ResponseWriter, code int, format string, args log.Error("%s %s: %s", r.Method, r.URL, text) http.Error(w, text, code) } + +// UserAgent returns the ID of the service as a User-Agent string. It can also +// be used as the value of the Server HTTP header. +func UserAgent() (ua string) { + return fmt.Sprintf("AdGuardHome/%s", version.Version()) +} + +// textPlainDeprMsg is the message returned to API users when they try to use +// an API that used to accept "text/plain" but doesn't anymore. +const textPlainDeprMsg = `using this api with the text/plain content-type is deprecated; ` + + `use application/json` + +// WriteTextPlainDeprecated responds to the request with a message about +// deprecation and removal of a plain-text API if the request is made with the +// "text/plain" content-type. +func WriteTextPlainDeprecated(w http.ResponseWriter, r *http.Request) (isPlainText bool) { + if r.Header.Get(HdrNameContentType) != HdrValTextPlain { + return false + } + + Error(r, w, http.StatusUnsupportedMediaType, textPlainDeprMsg) + + return true +} + +// WriteJSONResponse sets the content-type header in w.Header() to +// "application/json", encodes resp to w, calls Error on any returned error, and +// returns it as well. +func WriteJSONResponse(w http.ResponseWriter, r *http.Request, resp any) (err error) { + w.Header().Set(HdrNameContentType, HdrValApplicationJSON) + err = json.NewEncoder(w).Encode(resp) + if err != nil { + Error(r, w, http.StatusInternalServerError, "encoding resp: %s", err) + } + + return err +} diff --git a/internal/aghhttp/header.go b/internal/aghhttp/header.go new file mode 100644 index 00000000..1509a7e0 --- /dev/null +++ b/internal/aghhttp/header.go @@ -0,0 +1,22 @@ +package aghhttp + +// HTTP Headers + +// HTTP header name constants. +// +// TODO(a.garipov): Remove unused. +const ( + HdrNameAcceptEncoding = "Accept-Encoding" + HdrNameAccessControlAllowOrigin = "Access-Control-Allow-Origin" + HdrNameContentType = "Content-Type" + HdrNameContentEncoding = "Content-Encoding" + HdrNameServer = "Server" + HdrNameTrailer = "Trailer" + HdrNameUserAgent = "User-Agent" +) + +// HTTP header value constants. +const ( + HdrValApplicationJSON = "application/json" + HdrValTextPlain = "text/plain" +) diff --git a/internal/dhcpd/http_unix.go b/internal/dhcpd/http_unix.go index 8a32dab6..de06431f 100644 --- a/internal/dhcpd/http_unix.go +++ b/internal/dhcpd/http_unix.go @@ -5,11 +5,9 @@ package dhcpd import ( "encoding/json" "fmt" - "io" "net" "net/http" "os" - "strings" "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" @@ -410,31 +408,37 @@ type dhcpSearchResult struct { V6 dhcpSearchV6Result `json:"v6"` } -// Perform the following tasks: -// . Search for another DHCP server running -// . Check if a static IP is configured for the network interface -// Respond with results +// findActiveServerReq is the JSON structure for the request to find active DHCP +// servers. +type findActiveServerReq struct { + Interface string `json:"interface"` +} + +// handleDHCPFindActiveServer performs the following tasks: +// 1. searches for another DHCP server in the network; +// 2. check if a static IP is configured for the network interface; +// 3. responds with the results. func (s *server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Request) { - // This use of ReadAll is safe, because request's body is now limited. - body, err := io.ReadAll(r.Body) + if aghhttp.WriteTextPlainDeprecated(w, r) { + return + } + + req := &findActiveServerReq{} + err := json.NewDecoder(r.Body).Decode(req) if err != nil { - msg := fmt.Sprintf("failed to read request body: %s", err) - log.Error(msg) - http.Error(w, msg, http.StatusBadRequest) + aghhttp.Error(r, w, http.StatusBadRequest, "reading req: %s", err) return } - ifaceName := strings.TrimSpace(string(body)) + ifaceName := req.Interface if ifaceName == "" { - msg := "empty interface name specified" - log.Error(msg) - http.Error(w, msg, http.StatusBadRequest) + aghhttp.Error(r, w, http.StatusBadRequest, "empty interface name") return } - result := dhcpSearchResult{ + result := &dhcpSearchResult{ V4: dhcpSearchV4Result{ OtherServer: dhcpSearchOtherResult{ Found: "no", @@ -459,6 +463,14 @@ func (s *server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Reque result.V4.StaticIP.IP = aghnet.GetSubnet(ifaceName).String() } + setOtherDHCPResult(ifaceName, result) + + _ = aghhttp.WriteJSONResponse(w, r, result) +} + +// setOtherDHCPResult sets the results of the check for another DHCP server in +// result. +func setOtherDHCPResult(ifaceName string, result *dhcpSearchResult) { found4, found6, err4, err6 := aghnet.CheckOtherDHCP(ifaceName) if err4 != nil { result.V4.OtherServer.Found = "error" @@ -466,24 +478,13 @@ func (s *server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Reque } else if found4 { result.V4.OtherServer.Found = "yes" } + if err6 != nil { result.V6.OtherServer.Found = "error" result.V6.OtherServer.Error = err6.Error() } else if found6 { result.V6.OtherServer.Found = "yes" } - - w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(result) - if err != nil { - aghhttp.Error( - r, - w, - http.StatusInternalServerError, - "Failed to marshal DHCP found json: %s", - err, - ) - } } func (s *server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request) { diff --git a/internal/filtering/controlfiltering.go b/internal/filtering/http.go similarity index 97% rename from internal/filtering/controlfiltering.go rename to internal/filtering/http.go index 1cce8ded..50890f93 100644 --- a/internal/filtering/controlfiltering.go +++ b/internal/filtering/http.go @@ -3,13 +3,11 @@ package filtering import ( "encoding/json" "fmt" - "io" "net" "net/http" "net/url" "os" "path/filepath" - "strings" "time" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" @@ -249,16 +247,25 @@ func (d *DNSFilter) handleFilteringSetURL(w http.ResponseWriter, r *http.Request } } +// filteringRulesReq is the JSON structure for settings custom filtering rules. +type filteringRulesReq struct { + Rules []string `json:"rules"` +} + func (d *DNSFilter) handleFilteringSetRules(w http.ResponseWriter, r *http.Request) { - // This use of ReadAll is safe, because request's body is now limited. - body, err := io.ReadAll(r.Body) + if aghhttp.WriteTextPlainDeprecated(w, r) { + return + } + + req := &filteringRulesReq{} + err := json.NewDecoder(r.Body).Decode(req) if err != nil { - aghhttp.Error(r, w, http.StatusBadRequest, "Failed to read request body: %s", err) + aghhttp.Error(r, w, http.StatusBadRequest, "reading req: %s", err) return } - d.UserRules = strings.Split(string(body), "\n") + d.UserRules = req.Rules d.ConfigModified() d.EnableFilters(true) } diff --git a/internal/home/auth.go b/internal/home/auth.go index ca708f4a..14d2f52b 100644 --- a/internal/home/auth.go +++ b/internal/home/auth.go @@ -8,12 +8,14 @@ import ( "fmt" "net" "net/http" + "path" "strconv" "strings" "sync" "time" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" + "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/timeutil" @@ -32,7 +34,8 @@ const sessionTokenSize = 16 type session struct { userName string - expire uint32 // expiration time (in seconds) + // expire is the expiration time, in seconds. + expire uint32 } func (s *session) serialize() []byte { @@ -64,29 +67,29 @@ func (s *session) deserialize(data []byte) bool { // Auth - global object type Auth struct { - db *bbolt.DB - blocker *authRateLimiter - sessions map[string]*session - users []User - lock sync.Mutex - sessionTTL uint32 + db *bbolt.DB + raleLimiter *authRateLimiter + sessions map[string]*session + users []webUser + lock sync.Mutex + sessionTTL uint32 } -// User object -type User struct { +// webUser represents a user of the Web UI. +type webUser struct { Name string `yaml:"name"` - PasswordHash string `yaml:"password"` // bcrypt hash + PasswordHash string `yaml:"password"` } // InitAuth - create a global object -func InitAuth(dbFilename string, users []User, sessionTTL uint32, blocker *authRateLimiter) *Auth { +func InitAuth(dbFilename string, users []webUser, sessionTTL uint32, rateLimiter *authRateLimiter) *Auth { log.Info("Initializing auth module: %s", dbFilename) a := &Auth{ - sessionTTL: sessionTTL, - blocker: blocker, - sessions: make(map[string]*session), - users: users, + sessionTTL: sessionTTL, + raleLimiter: rateLimiter, + sessions: make(map[string]*session), + users: users, } var err error a.db, err = bbolt.Open(dbFilename, 0o644, nil) @@ -326,35 +329,25 @@ func newSessionToken() (data []byte, err error) { return randData, nil } -// cookieTimeFormat is the format to be used in (time.Time).Format for cookie's -// expiry field. -const cookieTimeFormat = "Mon, 02 Jan 2006 15:04:05 GMT" - -// cookieExpiryFormat returns the formatted exp to be used in cookie string. -// It's quite simple for now, but probably will be expanded in the future. -func cookieExpiryFormat(exp time.Time) (formatted string) { - return exp.Format(cookieTimeFormat) -} - -func (a *Auth) httpCookie(req loginJSON, addr string) (cookie string, err error) { - blocker := a.blocker - u := a.UserFind(req.Name, req.Password) - if len(u.Name) == 0 { - if blocker != nil { - blocker.inc(addr) +// newCookie creates a new authentication cookie. +func (a *Auth) newCookie(req loginJSON, addr string) (c *http.Cookie, err error) { + rateLimiter := a.raleLimiter + u, ok := a.findUser(req.Name, req.Password) + if !ok { + if rateLimiter != nil { + rateLimiter.inc(addr) } - return "", err + return nil, errors.Error("invalid username or password") } - if blocker != nil { - blocker.remove(addr) + if rateLimiter != nil { + rateLimiter.remove(addr) } - var sess []byte - sess, err = newSessionToken() + sess, err := newSessionToken() if err != nil { - return "", err + return nil, fmt.Errorf("generating token: %w", err) } now := time.Now().UTC() @@ -364,11 +357,15 @@ func (a *Auth) httpCookie(req loginJSON, addr string) (cookie string, err error) expire: uint32(now.Unix()) + a.sessionTTL, }) - return fmt.Sprintf( - "%s=%s; Path=/; HttpOnly; Expires=%s", - sessionCookieName, hex.EncodeToString(sess), - cookieExpiryFormat(now.Add(cookieTTL)), - ), nil + return &http.Cookie{ + Name: sessionCookieName, + Value: hex.EncodeToString(sess), + Path: "/", + Expires: now.Add(cookieTTL), + + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }, nil } // realIP extracts the real IP address of the client from an HTTP request using @@ -436,8 +433,8 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { return } - if blocker := Context.auth.blocker; blocker != nil { - if left := blocker.check(remoteAddr); left > 0 { + if rateLimiter := Context.auth.raleLimiter; rateLimiter != nil { + if left := rateLimiter.check(remoteAddr); left > 0 { w.Header().Set("Retry-After", strconv.Itoa(int(left.Seconds()))) aghhttp.Error(r, w, http.StatusTooManyRequests, "auth: blocked for %s", left) @@ -445,10 +442,9 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { } } - var cookie string - cookie, err = Context.auth.httpCookie(req, remoteAddr) + cookie, err := Context.auth.newCookie(req, remoteAddr) if err != nil { - aghhttp.Error(r, w, http.StatusBadRequest, "crypto rand reader: %s", err) + aghhttp.Error(r, w, http.StatusForbidden, "%s", err) return } @@ -462,20 +458,11 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { log.Error("auth: unknown ip") } - if len(cookie) == 0 { - log.Info("auth: failed to login user %q from ip %v", req.Name, ip) - - time.Sleep(1 * time.Second) - - http.Error(w, "invalid username or password", http.StatusBadRequest) - - return - } - log.Info("auth: user %q successfully logged in from ip %v", req.Name, ip) + http.SetCookie(w, cookie) + h := w.Header() - h.Set("Set-Cookie", cookie) h.Set("Cache-Control", "no-store, no-cache, must-revalidate, proxy-revalidate") h.Set("Pragma", "no-cache") h.Set("Expires", "0") @@ -484,17 +471,31 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { } func handleLogout(w http.ResponseWriter, r *http.Request) { - cookie := r.Header.Get("Cookie") - sess := parseCookie(cookie) + respHdr := w.Header() + c, err := r.Cookie(sessionCookieName) + if err != nil { + // The only error that is returned from r.Cookie is [http.ErrNoCookie]. + // The user is already logged out. + respHdr.Set("Location", "/login.html") + w.WriteHeader(http.StatusFound) - Context.auth.RemoveSession(sess) + return + } - w.Header().Set("Location", "/login.html") + Context.auth.RemoveSession(c.Value) - s := fmt.Sprintf("%s=; Path=/; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:00 GMT", - sessionCookieName) - w.Header().Set("Set-Cookie", s) + c = &http.Cookie{ + Name: sessionCookieName, + Value: "", + Path: "/", + Expires: time.Unix(0, 0), + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + } + + respHdr.Set("Location", "/login.html") + respHdr.Set("Set-Cookie", c.String()) w.WriteHeader(http.StatusFound) } @@ -504,101 +505,108 @@ func RegisterAuthHandlers() { httpRegister(http.MethodGet, "/control/logout", handleLogout) } -func parseCookie(cookie string) string { - pairs := strings.Split(cookie, ";") - for _, pair := range pairs { - pair = strings.TrimSpace(pair) - kv := strings.SplitN(pair, "=", 2) - if len(kv) != 2 { - continue - } - if kv[0] == sessionCookieName { - return kv[1] - } - } - return "" -} - // optionalAuthThird return true if user should authenticate first. -func optionalAuthThird(w http.ResponseWriter, r *http.Request) (authFirst bool) { - authFirst = false +func optionalAuthThird(w http.ResponseWriter, r *http.Request) (mustAuth bool) { + if glProcessCookie(r) { + log.Debug("auth: authentication is handled by GL-Inet submodule") + + return false + } // redirect to login page if not authenticated - ok := false + isAuthenticated := false cookie, err := r.Cookie(sessionCookieName) - - if glProcessCookie(r) { - log.Debug("auth: authentication was handled by GL-Inet submodule") - ok = true - } else if err == nil { - r := Context.auth.checkSession(cookie.Value) - if r == checkSessionOK { - ok = true - } else if r < 0 { - log.Debug("auth: invalid cookie value: %s", cookie) - } - } else { - // there's no Cookie, check Basic authentication - user, pass, ok2 := r.BasicAuth() - if ok2 { - u := Context.auth.UserFind(user, pass) - if len(u.Name) != 0 { - ok = true - } else { + if err != nil { + // The only error that is returned from r.Cookie is [http.ErrNoCookie]. + // Check Basic authentication. + user, pass, hasBasic := r.BasicAuth() + if hasBasic { + _, isAuthenticated = Context.auth.findUser(user, pass) + if !isAuthenticated { log.Info("auth: invalid Basic Authorization value") } } - } - if !ok { - if r.URL.Path == "/" || r.URL.Path == "/index.html" { - if glProcessRedirect(w, r) { - log.Debug("auth: redirected to login page by GL-Inet submodule") - } else { - w.Header().Set("Location", "/login.html") - w.WriteHeader(http.StatusFound) - } - } else { - w.WriteHeader(http.StatusForbidden) - _, _ = w.Write([]byte("Forbidden")) + } else { + res := Context.auth.checkSession(cookie.Value) + isAuthenticated = res == checkSessionOK + if !isAuthenticated { + log.Debug("auth: invalid cookie value: %s", cookie) } - authFirst = true } - return authFirst + if isAuthenticated { + return false + } + + if p := r.URL.Path; p == "/" || p == "/index.html" { + if glProcessRedirect(w, r) { + log.Debug("auth: redirected to login page by GL-Inet submodule") + } else { + log.Debug("auth: redirected to login page") + w.Header().Set("Location", "/login.html") + w.WriteHeader(http.StatusFound) + } + } else { + log.Debug("auth: responded with forbidden to %s %s", r.Method, p) + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte("Forbidden")) + } + + return true } -func optionalAuth(handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) { +// TODO(a.garipov): Use [http.Handler] consistently everywhere throughout the +// project. +func optionalAuth( + h func(http.ResponseWriter, *http.Request), +) (wrapped func(http.ResponseWriter, *http.Request)) { return func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/login.html" { - // redirect to dashboard if already authenticated - authRequired := Context.auth != nil && Context.auth.AuthRequired() + p := r.URL.Path + authRequired := Context.auth != nil && Context.auth.AuthRequired() + if p == "/login.html" { cookie, err := r.Cookie(sessionCookieName) if authRequired && err == nil { - r := Context.auth.checkSession(cookie.Value) - if r == checkSessionOK { + // Redirect to the dashboard if already authenticated. + res := Context.auth.checkSession(cookie.Value) + if res == checkSessionOK { w.Header().Set("Location", "/") w.WriteHeader(http.StatusFound) return - } else if r == checkSessionNotFound { - log.Debug("auth: invalid cookie value: %s", cookie) } - } - } else if strings.HasPrefix(r.URL.Path, "/assets/") || - strings.HasPrefix(r.URL.Path, "/login.") { - // process as usual - // no additional auth requirements - } else if Context.auth != nil && Context.auth.AuthRequired() { + log.Debug("auth: invalid cookie value: %s", cookie) + } + } else if isPublicResource(p) { + // Process as usual, no additional auth requirements. + } else if authRequired { if optionalAuthThird(w, r) { return } } - handler(w, r) + h(w, r) } } +// isPublicResource returns true if p is a path to a public resource. +func isPublicResource(p string) (ok bool) { + isAsset, err := path.Match("/assets/*", p) + if err != nil { + // The only error that is returned from path.Match is + // [path.ErrBadPattern]. This is a programmer error. + panic(fmt.Errorf("bad asset pattern: %w", err)) + } + + isLogin, err := path.Match("/login.*", p) + if err != nil { + // Same as above. + panic(fmt.Errorf("bad login pattern: %w", err)) + } + + return isAsset || isLogin +} + type authHandler struct { handler http.Handler } @@ -612,7 +620,7 @@ func optionalAuthHandler(handler http.Handler) http.Handler { } // UserAdd - add new user -func (a *Auth) UserAdd(u *User, password string) { +func (a *Auth) UserAdd(u *webUser, password string) { if len(password) == 0 { return } @@ -631,31 +639,35 @@ func (a *Auth) UserAdd(u *User, password string) { log.Debug("auth: added user: %s", u.Name) } -// UserFind - find a user -func (a *Auth) UserFind(login, password string) User { +// findUser returns a user if there is one. +func (a *Auth) findUser(login, password string) (u webUser, ok bool) { a.lock.Lock() defer a.lock.Unlock() - for _, u := range a.users { + + for _, u = range a.users { if u.Name == login && bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte(password)) == nil { - return u + return u, true } } - return User{} + + return webUser{}, false } // getCurrentUser returns the current user. It returns an empty User if the // user is not found. -func (a *Auth) getCurrentUser(r *http.Request) User { +func (a *Auth) getCurrentUser(r *http.Request) (u webUser) { cookie, err := r.Cookie(sessionCookieName) if err != nil { // There's no Cookie, check Basic authentication. user, pass, ok := r.BasicAuth() if ok { - return Context.auth.UserFind(user, pass) + u, _ = Context.auth.findUser(user, pass) + + return u } - return User{} + return webUser{} } a.lock.Lock() @@ -663,20 +675,20 @@ func (a *Auth) getCurrentUser(r *http.Request) User { s, ok := a.sessions[cookie.Value] if !ok { - return User{} + return webUser{} } - for _, u := range a.users { + for _, u = range a.users { if u.Name == s.userName { return u } } - return User{} + return webUser{} } // GetUsers - get users -func (a *Auth) GetUsers() []User { +func (a *Auth) GetUsers() []webUser { a.lock.Lock() users := a.users a.lock.Unlock() diff --git a/internal/home/auth_test.go b/internal/home/auth_test.go index 6a2ebea7..1bf38753 100644 --- a/internal/home/auth_test.go +++ b/internal/home/auth_test.go @@ -43,14 +43,14 @@ func TestAuth(t *testing.T) { dir := t.TempDir() fn := filepath.Join(dir, "sessions.db") - users := []User{{ + users := []webUser{{ Name: "name", PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2", }} a := InitAuth(fn, nil, 60, nil) s := session{} - user := User{Name: "name"} + user := webUser{Name: "name"} a.UserAdd(&user, "password") assert.Equal(t, checkSessionNotFound, a.checkSession("notfound")) @@ -84,7 +84,8 @@ func TestAuth(t *testing.T) { a.storeSession(sess, &s) a.Close() - u := a.UserFind("name", "password") + u, ok := a.findUser("name", "password") + assert.True(t, ok) assert.NotEmpty(t, u.Name) time.Sleep(3 * time.Second) @@ -118,7 +119,7 @@ func TestAuthHTTP(t *testing.T) { dir := t.TempDir() fn := filepath.Join(dir, "sessions.db") - users := []User{ + users := []webUser{ {Name: "name", PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2"}, } Context.auth = InitAuth(fn, users, 60, nil) @@ -150,18 +151,19 @@ func TestAuthHTTP(t *testing.T) { assert.True(t, handlerCalled) // perform login - cookie, err := Context.auth.httpCookie(loginJSON{Name: "name", Password: "password"}, "") + cookie, err := Context.auth.newCookie(loginJSON{Name: "name", Password: "password"}, "") require.NoError(t, err) - assert.NotEmpty(t, cookie) + require.NotNil(t, cookie) // get / handler2 = optionalAuth(handler) w.hdr = make(http.Header) - r.Header.Set("Cookie", cookie) + r.Header.Set("Cookie", cookie.String()) r.URL = &url.URL{Path: "/"} handlerCalled = false handler2(&w, &r) assert.True(t, handlerCalled) + r.Header.Del("Cookie") // get / with basic auth @@ -177,7 +179,7 @@ func TestAuthHTTP(t *testing.T) { // get login page with a valid cookie - we're redirected to / handler2 = optionalAuth(handler) w.hdr = make(http.Header) - r.Header.Set("Cookie", cookie) + r.Header.Set("Cookie", cookie.String()) r.URL = &url.URL{Path: loginURL} handlerCalled = false handler2(&w, &r) diff --git a/internal/home/clientshttp.go b/internal/home/clientshttp.go index 5f10ccbe..59821d0a 100644 --- a/internal/home/clientshttp.go +++ b/internal/home/clientshttp.go @@ -93,13 +93,7 @@ func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http data.Tags = clientTags - w.Header().Set("Content-Type", "application/json") - e := json.NewEncoder(w).Encode(data) - if e != nil { - aghhttp.Error(r, w, http.StatusInternalServerError, "failed to encode to json: %v", e) - - return - } + _ = aghhttp.WriteJSONResponse(w, r, data) } // Convert JSON object to Client object @@ -249,11 +243,7 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http }) } - w.Header().Set("Content-Type", "application/json") - err := json.NewEncoder(w).Encode(data) - if err != nil { - aghhttp.Error(r, w, http.StatusInternalServerError, "Couldn't write response: %s", err) - } + _ = aghhttp.WriteJSONResponse(w, r, data) } // findRuntime looks up the IP in runtime and temporary storages, like diff --git a/internal/home/config.go b/internal/home/config.go index ff597761..ed337c86 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -85,10 +85,10 @@ type configuration struct { // It's reset after config is parsed fileData []byte - BindHost net.IP `yaml:"bind_host"` // BindHost is the IP address of the HTTP server to bind to - BindPort int `yaml:"bind_port"` // BindPort is the port the HTTP server - BetaBindPort int `yaml:"beta_bind_port"` // BetaBindPort is the port for new client - Users []User `yaml:"users"` // Users that can access HTTP server + BindHost net.IP `yaml:"bind_host"` // BindHost is the IP address of the HTTP server to bind to + BindPort int `yaml:"bind_port"` // BindPort is the port the HTTP server + BetaBindPort int `yaml:"beta_bind_port"` // BetaBindPort is the port for new client + Users []webUser `yaml:"users"` // Users that can access HTTP server // AuthAttempts is the maximum number of failed login attempts a user // can do before being blocked. AuthAttempts uint `yaml:"auth_attempts"` diff --git a/internal/home/control.go b/internal/home/control.go index 829063e9..881e957a 100644 --- a/internal/home/control.go +++ b/internal/home/control.go @@ -146,13 +146,7 @@ func handleStatus(w http.ResponseWriter, r *http.Request) { resp.IsDHCPAvailable = Context.dhcpServer != nil } - w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(resp) - if err != nil { - aghhttp.Error(r, w, http.StatusInternalServerError, "Unable to write response json: %s", err) - - return - } + _ = aghhttp.WriteJSONResponse(w, r, resp) } type profileJSON struct { @@ -162,13 +156,16 @@ type profileJSON struct { func handleGetProfile(w http.ResponseWriter, r *http.Request) { pj := profileJSON{} u := Context.auth.getCurrentUser(r) + pj.Name = u.Name data, err := json.Marshal(pj) if err != nil { aghhttp.Error(r, w, http.StatusInternalServerError, "json.Marshal: %s", err) + return } + _, _ = w.Write(data) } @@ -207,11 +204,24 @@ func ensure(method string, handler func(http.ResponseWriter, *http.Request)) fun log.Debug("%s %v", r.Method, r.URL) if r.Method != method { - http.Error(w, "This request must be "+method, http.StatusMethodNotAllowed) + aghhttp.Error(r, w, http.StatusMethodNotAllowed, "only %s is allowed", method) + return } if method == http.MethodPost || method == http.MethodPut || method == http.MethodDelete { + if r.Header.Get(aghhttp.HdrNameContentType) != aghhttp.HdrValApplicationJSON { + aghhttp.Error( + r, + w, + http.StatusUnsupportedMediaType, + "only %s is allowed", + aghhttp.HdrValApplicationJSON, + ) + + return + } + Context.controlLock.Lock() defer Context.controlLock.Unlock() } diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go index c46f3459..78de64f7 100644 --- a/internal/home/controlinstall.go +++ b/internal/home/controlinstall.go @@ -59,19 +59,7 @@ func (web *Web) handleInstallGetAddresses(w http.ResponseWriter, r *http.Request data.Interfaces[iface.Name] = iface } - w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(data) - if err != nil { - aghhttp.Error( - r, - w, - http.StatusInternalServerError, - "Unable to marshal default addresses to json: %s", - err, - ) - - return - } + _ = aghhttp.WriteJSONResponse(w, r, data) } type checkConfReqEnt struct { @@ -201,13 +189,7 @@ func (web *Web) handleInstallCheckConfig(w http.ResponseWriter, r *http.Request) resp.StaticIP = handleStaticIP(req.DNS.IP, req.SetStaticIP) } - w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(resp) - if err != nil { - aghhttp.Error(r, w, http.StatusInternalServerError, "encoding the response: %s", err) - - return - } + _ = aghhttp.WriteJSONResponse(w, r, resp) } // handleStaticIP - handles static IP request @@ -424,7 +406,7 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) { return } - u := &User{ + u := &webUser{ Name: req.Username, } Context.auth.UserAdd(u, req.Password) @@ -688,19 +670,7 @@ func (web *Web) handleInstallGetAddressesBeta(w http.ResponseWriter, r *http.Req data.Interfaces = ifaces - w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(data) - if err != nil { - aghhttp.Error( - r, - w, - http.StatusInternalServerError, - "Unable to marshal default addresses to json: %s", - err, - ) - - return - } + _ = aghhttp.WriteJSONResponse(w, r, data) } // registerBetaInstallHandlers registers the install handlers for new client diff --git a/internal/home/controlupdate.go b/internal/home/controlupdate.go index 91164696..ef4f0659 100644 --- a/internal/home/controlupdate.go +++ b/internal/home/controlupdate.go @@ -28,8 +28,6 @@ type temporaryError interface { // Get the latest available version from the Internet func handleGetVersionJSON(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - resp := &versionResponse{} if Context.disableUpdate { resp.Disabled = true @@ -71,10 +69,7 @@ func handleGetVersionJSON(w http.ResponseWriter, r *http.Request) { return } - err = json.NewEncoder(w).Encode(resp) - if err != nil { - aghhttp.Error(r, w, http.StatusInternalServerError, "writing body: %s", err) - } + _ = aghhttp.WriteJSONResponse(w, r, resp) } // requestVersionInfo sets the VersionInfo field of resp if it can reach the diff --git a/internal/home/home.go b/internal/home/home.go index 0f88f57b..f917b6a5 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -409,7 +409,7 @@ func run(args options, clientBuildFS fs.FS) { configureLogger(args) // Print the first message after logger is configured. - log.Println(version.Full()) + log.Info(version.Full()) log.Debug("current working directory is %s", Context.workDir) if args.runningAsService { log.Info("AdGuard Home is running as a service") @@ -455,9 +455,9 @@ func run(args options, clientBuildFS fs.FS) { sessFilename := filepath.Join(Context.getDataDir(), "sessions.db") GLMode = args.glinetMode - var arl *authRateLimiter + var rateLimiter *authRateLimiter if config.AuthAttempts > 0 && config.AuthBlockMin > 0 { - arl = newAuthRateLimiter( + rateLimiter = newAuthRateLimiter( time.Duration(config.AuthBlockMin)*time.Minute, config.AuthAttempts, ) @@ -469,7 +469,7 @@ func run(args options, clientBuildFS fs.FS) { sessFilename, config.Users, config.WebSessionTTLHours*60*60, - arl, + rateLimiter, ) if Context.auth == nil { log.Fatalf("Couldn't initialize Auth module") diff --git a/internal/home/i18n.go b/internal/home/i18n.go index d58dfcf3..cc8da8fe 100644 --- a/internal/home/i18n.go +++ b/internal/home/i18n.go @@ -1,10 +1,8 @@ package home import ( - "fmt" - "io" + "encoding/json" "net/http" - "strings" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/golibs/log" @@ -51,43 +49,35 @@ var allowedLanguages = stringutil.NewSet( "zh-tw", ) -func handleI18nCurrentLanguage(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "text/plain") - log.Printf("config.Language is %s", config.Language) - _, err := fmt.Fprintf(w, "%s\n", config.Language) - if err != nil { - msg := fmt.Sprintf("Unable to write response json: %s", err) - log.Println(msg) - http.Error(w, msg, http.StatusInternalServerError) +// languageJSON is the JSON structure for language requests and responses. +type languageJSON struct { + Language string `json:"language"` +} - return - } +func handleI18nCurrentLanguage(w http.ResponseWriter, r *http.Request) { + log.Printf("home: language is %s", config.Language) + + _ = aghhttp.WriteJSONResponse(w, r, &languageJSON{ + Language: config.Language, + }) } func handleI18nChangeLanguage(w http.ResponseWriter, r *http.Request) { - // This use of ReadAll is safe, because request's body is now limited. - body, err := io.ReadAll(r.Body) + if aghhttp.WriteTextPlainDeprecated(w, r) { + return + } + + langReq := &languageJSON{} + err := json.NewDecoder(r.Body).Decode(langReq) if err != nil { - msg := fmt.Sprintf("failed to read request body: %s", err) - log.Println(msg) - http.Error(w, msg, http.StatusBadRequest) + aghhttp.Error(r, w, http.StatusInternalServerError, "reading req: %s", err) return } - language := strings.TrimSpace(string(body)) - if language == "" { - msg := "empty language specified" - log.Println(msg) - http.Error(w, msg, http.StatusBadRequest) - - return - } - - if !allowedLanguages.Has(language) { - msg := fmt.Sprintf("unknown language specified: %s", language) - log.Println(msg) - http.Error(w, msg, http.StatusBadRequest) + lang := langReq.Language + if !allowedLanguages.Has(lang) { + aghhttp.Error(r, w, http.StatusBadRequest, "unknown language: %q", lang) return } @@ -96,7 +86,8 @@ func handleI18nChangeLanguage(w http.ResponseWriter, r *http.Request) { config.Lock() defer config.Unlock() - config.Language = language + config.Language = lang + log.Printf("home: language is set to %s", lang) }() onConfigModified() diff --git a/internal/home/service.go b/internal/home/service.go index c670ebe2..e52f9799 100644 --- a/internal/home/service.go +++ b/internal/home/service.go @@ -176,7 +176,8 @@ func handleServiceControlAction(opts options, clientBuildFS fs.FS) { chooseSystem() action := opts.serviceControlAction - log.Printf("service: control action: %s", action) + log.Info(version.Full()) + log.Info("service: control action: %s", action) if action == "reload" { sendSigReload() diff --git a/internal/home/tls.go b/internal/home/tls.go index b454e152..359a7a9d 100644 --- a/internal/home/tls.go +++ b/internal/home/tls.go @@ -680,8 +680,6 @@ func unmarshalTLS(r *http.Request) (tlsConfigSettingsExt, error) { } func marshalTLS(w http.ResponseWriter, r *http.Request, data tlsConfig) { - w.Header().Set("Content-Type", "application/json") - if data.CertificateChain != "" { encoded := base64.StdEncoding.EncodeToString([]byte(data.CertificateChain)) data.CertificateChain = encoded @@ -692,16 +690,7 @@ func marshalTLS(w http.ResponseWriter, r *http.Request, data tlsConfig) { data.PrivateKey = "" } - err := json.NewEncoder(w).Encode(data) - if err != nil { - aghhttp.Error( - r, - w, - http.StatusInternalServerError, - "Failed to marshal json with TLS status: %s", - err, - ) - } + _ = aghhttp.WriteJSONResponse(w, r, data) } // registerWebHandlers registers HTTP handlers for TLS configuration diff --git a/internal/home/upgrade.go b/internal/home/upgrade.go index 63e17030..132a82af 100644 --- a/internal/home/upgrade.go +++ b/internal/home/upgrade.go @@ -278,11 +278,11 @@ func upgradeSchema4to5(diskConf yobj) error { log.Fatalf("Can't use password \"%s\": bcrypt.GenerateFromPassword: %s", passStr, err) return nil } - u := User{ + u := webUser{ Name: nameStr, PasswordHash: string(hash), } - users := []User{u} + users := []webUser{u} diskConf["users"] = users return nil } diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index fbbe7169..d9946115 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -4,6 +4,64 @@ ## v0.108.0: API changes + + +## v0.107.14: BREAKING API CHANGES + +A Cross-Site Request Forgery (CSRF) vulnerability has been discovered. We have +implemented several measures to prevent such vulnerabilities in the future, but +some of these measures break backwards compatibility for the sake of better +protection. + +All new formats for the request and response bodies are documented in +`openapi.yaml`. + +### `POST /control/filtering/set_rules` And Other Plain-Text APIs + +The following APIs, which previously accepted or returned `text/plain` data, +now accept or return data as JSON. + +#### `POST /control/filtering/set_rules` + +Previously, the API accepted a raw list of filters as a plain-text file. Now, +the filters must be presented in a JSON object with the following format: + +```json +{ + "rules": + [ + "||example.com^", + "# comment", + "@@||www.example.com^" + ] +} +``` + +#### `GET /control/i18n/current_language` And `POST /control/i18n/change_language` + +Previously, these APIs accepted and returned the language code in plain text. +Now, they accept and return them in a JSON object with the following format: + +```json +{ + "language": "en" +} +``` + +#### `POST /control/dhcp/find_active_dhcp` + +Previously, the API accepted the name of the network interface as a plain-text +string. Now, it must be contained within a JSON object with the following +format: + +```json +{ + "interface": "eth0" +} +``` + + + ## v0.107.12: API changes ### `GET /control/blocked_services/services` @@ -11,6 +69,8 @@ * The new `GET /control/blocked_services/services` HTTP API allows inspecting all available services. + + ## v0.107.7: API changes ### The new optional field `"ecs"` in `QueryLogItem` @@ -24,6 +84,8 @@ `POST /install/configure` which means that the specified password does not meet the strength requirements. + + ## v0.107.3: API changes ### The new field `"version"` in `AddressesInfo` @@ -31,6 +93,8 @@ * The new field `"version"` in `GET /install/get_addresses` is the version of the AdGuard Home instance. + + ## v0.107.0: API changes ### The new field `"cached"` in `QueryLogItem` diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index ad57d807..c6451fa7 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -413,6 +413,11 @@ - 'dhcp' 'operationId': 'checkActiveDhcp' 'summary': 'Searches for an active DHCP server on the network' + 'requestBody': + 'content': + 'application/json': + 'schema': + '$ref': '#/components/schemas/DhcpFindActiveReq' 'responses': '200': 'description': 'OK.' @@ -667,24 +672,6 @@ - 'parental' 'operationId': 'parentalEnable' 'summary': 'Enable parental filtering' - 'requestBody': - 'content': - 'text/plain': - 'schema': - 'type': 'string' - 'enum': - - 'EARLY_CHILDHOOD' - - 'YOUNG' - - 'TEEN' - - 'MATURE' - 'example': 'sensitivity=TEEN' - 'description': | - Age sensitivity for parental filtering, - EARLY_CHILDHOOD is 3 - YOUNG is 10 - TEEN is 13 - MATURE is 17 - 'required': true 'responses': '200': 'description': 'OK.' @@ -958,10 +945,9 @@ Change current language. Argument must be an ISO 639-1 two-letter code. 'requestBody': 'content': - 'text/plain': + 'application/json': 'schema': - 'type': 'string' - 'example': 'en' + '$ref': '#/components/schemas/LanguageSettings' 'description': > New language. It must be known to the server and must be an ISO 639-1 two-letter code. @@ -980,10 +966,9 @@ '200': 'description': 'OK.' 'content': - 'text/plain': - 'examples': - 'response': - 'value': 'en' + 'application/json': + 'schema': + '$ref': '#/components/schemas/LanguageSettings' '/install/get_addresses_beta': 'get': 'tags': @@ -1777,6 +1762,16 @@ 'additionalProperties': '$ref': '#/components/schemas/NetInterface' + 'DhcpFindActiveReq': + 'description': > + Request for checking for other DHCP servers in the network. + 'properties': + 'interface': + 'description': 'The name of the network interface' + 'example': 'eth0' + 'type': 'string' + 'type': 'object' + 'DhcpSearchResult': 'type': 'object' 'description': > @@ -2692,6 +2687,15 @@ 'description': 'The error message, an opaque string.' 'type': 'string' 'type': 'object' + 'LanguageSettings': + 'description': 'Language settings object.' + 'properties': + 'language': + 'description': 'The current language or the language to set.' + 'type': 'string' + 'required': + - 'language' + 'type': 'object' 'securitySchemes': 'basicAuth': 'type': 'http'