mirror of
https://github.com/AdguardTeam/AdGuardHome.git
synced 2024-11-24 22:15:45 +03:00
Pull request: 4970-error-415
Updates #4970. Squashed commit of the following: commit 10365d9c8474e9d9735f581fb32b2892b2153cc4 Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Fri Sep 30 14:23:06 2022 +0300 all: imp docs, names commit cff1103a0618a6430dc91e7e018febbf313c12ba Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Fri Sep 30 14:02:38 2022 +0300 home: imp content-type check
This commit is contained in:
Ainar Garipov
parent
7b48863041
commit
4d404b887f
7 changed files with 122 additions and 60 deletions
17
CHANGELOG.md
17
CHANGELOG.md
|
|
@ -15,6 +15,19 @@ and this project adheres to
|
|||
## [v0.108.0] - TBA (APPROX.)
|
||||
-->
|
||||
|
||||
### Security
|
||||
|
||||
- As an additional CSRF protection measure, AdGuard Home now ensures that
|
||||
requests that change its state but have no body (such as `POST
|
||||
/control/stats_reset` requests) do not have a `Content-Type` header set on
|
||||
them ([#4970]).
|
||||
|
||||
### Fixed
|
||||
|
||||
- `only application/json is allowed` errors in various APIs ([#4970]).
|
||||
|
||||
[#4970]: https://github.com/AdguardTeam/AdGuardHome/issues/4970
|
||||
|
||||
|
||||
|
||||
<!--
|
||||
|
|
@ -63,8 +76,8 @@ bodies are documented in `openapi/openapi.yaml` and `openapi/CHANGELOG.md`.
|
|||
|
||||
#### Stricter Content-Type Checks (BREAKING API CHANGE)
|
||||
|
||||
All JSON APIs now check if the request actually has the `application/json`
|
||||
content-type.
|
||||
All JSON APIs that expect a body now check if the request actually has
|
||||
`Content-Type` set to `application/json`.
|
||||
|
||||
#### Other Security Changes
|
||||
|
||||
|
|
|
|||
|
|
@ -10,11 +10,17 @@ class Api {
|
|||
async makeRequest(path, method = 'POST', config) {
|
||||
const url = `${this.baseUrl}/${path}`;
|
||||
|
||||
const axiosConfig = config || {};
|
||||
if (method !== 'GET' && axiosConfig.data) {
|
||||
axiosConfig.headers = axiosConfig.headers || {};
|
||||
axiosConfig.headers['Content-Type'] = axiosConfig.headers['Content-Type'] || 'application/json';
|
||||
}
|
||||
|
||||
try {
|
||||
const response = await axios({
|
||||
url,
|
||||
method,
|
||||
...config,
|
||||
...axiosConfig,
|
||||
});
|
||||
return response.data;
|
||||
} catch (error) {
|
||||
|
|
@ -55,7 +61,6 @@ class Api {
|
|||
const { path, method } = this.GLOBAL_TEST_UPSTREAM_DNS;
|
||||
const config = {
|
||||
data: servers,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, config);
|
||||
}
|
||||
|
|
@ -64,7 +69,6 @@ class Api {
|
|||
const { path, method } = this.GLOBAL_VERSION;
|
||||
const config = {
|
||||
data,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, config);
|
||||
}
|
||||
|
|
@ -100,7 +104,6 @@ class Api {
|
|||
const { path, method } = this.FILTERING_REFRESH;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
|
||||
return this.makeRequest(path, method, parameters);
|
||||
|
|
@ -110,7 +113,6 @@ class Api {
|
|||
const { path, method } = this.FILTERING_ADD_FILTER;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
|
||||
return this.makeRequest(path, method, parameters);
|
||||
|
|
@ -120,7 +122,6 @@ class Api {
|
|||
const { path, method } = this.FILTERING_REMOVE_FILTER;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
|
||||
return this.makeRequest(path, method, parameters);
|
||||
|
|
@ -130,7 +131,6 @@ class Api {
|
|||
const { path, method } = this.FILTERING_SET_RULES;
|
||||
const parameters = {
|
||||
data: rules,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -139,7 +139,6 @@ class Api {
|
|||
const { path, method } = this.FILTERING_CONFIG;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -148,7 +147,6 @@ class Api {
|
|||
const { path, method } = this.FILTERING_SET_URL;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -239,7 +237,6 @@ class Api {
|
|||
const { path, method } = this.CHANGE_LANGUAGE;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -275,7 +272,6 @@ class Api {
|
|||
const { path, method } = this.DHCP_SET_CONFIG;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -284,7 +280,6 @@ class Api {
|
|||
const { path, method } = this.DHCP_FIND_ACTIVE;
|
||||
const parameters = {
|
||||
data: req,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -293,7 +288,6 @@ class Api {
|
|||
const { path, method } = this.DHCP_ADD_STATIC_LEASE;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -302,7 +296,6 @@ class Api {
|
|||
const { path, method } = this.DHCP_REMOVE_STATIC_LEASE;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -333,7 +326,6 @@ class Api {
|
|||
const { path, method } = this.INSTALL_CONFIGURE;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -342,7 +334,6 @@ class Api {
|
|||
const { path, method } = this.INSTALL_CHECK_CONFIG;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -363,7 +354,6 @@ class Api {
|
|||
const { path, method } = this.TLS_CONFIG;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -372,7 +362,6 @@ class Api {
|
|||
const { path, method } = this.TLS_VALIDATE;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -397,7 +386,6 @@ class Api {
|
|||
const { path, method } = this.ADD_CLIENT;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -406,7 +394,6 @@ class Api {
|
|||
const { path, method } = this.DELETE_CLIENT;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -415,7 +402,6 @@ class Api {
|
|||
const { path, method } = this.UPDATE_CLIENT;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -440,7 +426,6 @@ class Api {
|
|||
const { path, method } = this.ACCESS_SET;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -461,7 +446,6 @@ class Api {
|
|||
const { path, method } = this.REWRITE_ADD;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -470,7 +454,6 @@ class Api {
|
|||
const { path, method } = this.REWRITE_DELETE;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -496,7 +479,6 @@ class Api {
|
|||
const { path, method } = this.BLOCKED_SERVICES_SET;
|
||||
const parameters = {
|
||||
data: config,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, parameters);
|
||||
}
|
||||
|
|
@ -524,7 +506,6 @@ class Api {
|
|||
const { path, method } = this.STATS_CONFIG;
|
||||
const config = {
|
||||
data,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, config);
|
||||
}
|
||||
|
|
@ -560,7 +541,6 @@ class Api {
|
|||
const { path, method } = this.QUERY_LOG_CONFIG;
|
||||
const config = {
|
||||
data,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, config);
|
||||
}
|
||||
|
|
@ -577,7 +557,6 @@ class Api {
|
|||
const { path, method } = this.LOGIN;
|
||||
const config = {
|
||||
data,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, config);
|
||||
}
|
||||
|
|
@ -604,7 +583,6 @@ class Api {
|
|||
const { path, method } = this.SET_DNS_CONFIG;
|
||||
const config = {
|
||||
data,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
};
|
||||
return this.makeRequest(path, method, config);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ func OK(w http.ResponseWriter) {
|
|||
// Error writes formatted message to w and also logs it.
|
||||
func Error(r *http.Request, w http.ResponseWriter, code int, format string, args ...any) {
|
||||
text := fmt.Sprintf(format, args...)
|
||||
log.Error("%s %s: %s", r.Method, r.URL, text)
|
||||
log.Error("%s %s %s: %s", r.Method, r.Host, r.URL, text)
|
||||
http.Error(w, text, code)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -8,8 +8,8 @@ package aghhttp
|
|||
const (
|
||||
HdrNameAcceptEncoding = "Accept-Encoding"
|
||||
HdrNameAccessControlAllowOrigin = "Access-Control-Allow-Origin"
|
||||
HdrNameContentType = "Content-Type"
|
||||
HdrNameContentEncoding = "Content-Encoding"
|
||||
HdrNameContentType = "Content-Type"
|
||||
HdrNameServer = "Server"
|
||||
HdrNameTrailer = "Trailer"
|
||||
HdrNameUserAgent = "User-Agent"
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import (
|
|||
"net/url"
|
||||
"runtime"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
|
||||
"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
|
||||
|
|
@ -97,16 +98,16 @@ func collectDNSAddresses() (addrs []string, err error) {
|
|||
|
||||
// statusResponse is a response for /control/status endpoint.
|
||||
type statusResponse struct {
|
||||
Version string `json:"version"`
|
||||
Language string `json:"language"`
|
||||
DNSAddrs []string `json:"dns_addresses"`
|
||||
DNSPort int `json:"dns_port"`
|
||||
HTTPPort int `json:"http_port"`
|
||||
IsProtectionEnabled bool `json:"protection_enabled"`
|
||||
// TODO(e.burkov): Inspect if front-end doesn't requires this field as
|
||||
// openapi.yaml declares.
|
||||
IsDHCPAvailable bool `json:"dhcp_available"`
|
||||
IsRunning bool `json:"running"`
|
||||
Version string `json:"version"`
|
||||
Language string `json:"language"`
|
||||
IsDHCPAvailable bool `json:"dhcp_available"`
|
||||
IsRunning bool `json:"running"`
|
||||
}
|
||||
|
||||
func handleStatus(w http.ResponseWriter, r *http.Request) {
|
||||
|
|
@ -125,12 +126,12 @@ func handleStatus(w http.ResponseWriter, r *http.Request) {
|
|||
defer config.RUnlock()
|
||||
|
||||
resp = statusResponse{
|
||||
Version: version.Version(),
|
||||
DNSAddrs: dnsAddrs,
|
||||
DNSPort: config.DNS.Port,
|
||||
HTTPPort: config.BindPort,
|
||||
IsRunning: isRunning(),
|
||||
Version: version.Version(),
|
||||
Language: config.Language,
|
||||
IsRunning: isRunning(),
|
||||
}
|
||||
}()
|
||||
|
||||
|
|
@ -196,29 +197,26 @@ func httpRegister(method, url string, handler http.HandlerFunc) {
|
|||
Context.mux.Handle(url, postInstallHandler(optionalAuthHandler(gziphandler.GzipHandler(ensureHandler(method, handler)))))
|
||||
}
|
||||
|
||||
// ----------------------------------
|
||||
// helper functions for HTTP handlers
|
||||
// ----------------------------------
|
||||
func ensure(method string, handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
|
||||
// ensure returns a wrapped handler that makes sure that the request has the
|
||||
// correct method as well as additional method and header checks.
|
||||
func ensure(
|
||||
method string,
|
||||
handler func(http.ResponseWriter, *http.Request),
|
||||
) (wrapped func(http.ResponseWriter, *http.Request)) {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
log.Debug("%s %v", r.Method, r.URL)
|
||||
start := time.Now()
|
||||
m, u := r.Method, r.URL
|
||||
log.Debug("started %s %s %s", m, r.Host, u)
|
||||
defer func() { log.Debug("finished %s %s %s in %s", m, r.Host, u, time.Since(start)) }()
|
||||
|
||||
if r.Method != method {
|
||||
aghhttp.Error(r, w, http.StatusMethodNotAllowed, "only %s is allowed", method)
|
||||
if m != method {
|
||||
aghhttp.Error(r, w, http.StatusMethodNotAllowed, "only method %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,
|
||||
)
|
||||
|
||||
if modifiesData(m) {
|
||||
if !ensureContentType(w, r) {
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -230,6 +228,42 @@ func ensure(method string, handler func(http.ResponseWriter, *http.Request)) fun
|
|||
}
|
||||
}
|
||||
|
||||
// modifiesData returns true if m is an HTTP method that can modify data.
|
||||
func modifiesData(m string) (ok bool) {
|
||||
return m == http.MethodPost || m == http.MethodPut || m == http.MethodDelete
|
||||
}
|
||||
|
||||
// ensureContentType makes sure that the content type of a data-modifying
|
||||
// request is set correctly. If it is not, ensureContentType writes a response
|
||||
// to w, and ok is false.
|
||||
func ensureContentType(w http.ResponseWriter, r *http.Request) (ok bool) {
|
||||
const statusUnsup = http.StatusUnsupportedMediaType
|
||||
|
||||
cType := r.Header.Get(aghhttp.HdrNameContentType)
|
||||
if r.ContentLength == 0 {
|
||||
if cType == "" {
|
||||
return true
|
||||
}
|
||||
|
||||
// Assume that browsers always send a content type when submitting HTML
|
||||
// forms and require no content type for requests with no body to make
|
||||
// sure that the request comes from JavaScript.
|
||||
aghhttp.Error(r, w, statusUnsup, "empty body with content-type %q not allowed", cType)
|
||||
|
||||
return false
|
||||
|
||||
}
|
||||
|
||||
const wantCType = aghhttp.HdrValApplicationJSON
|
||||
if cType == wantCType {
|
||||
return true
|
||||
}
|
||||
|
||||
aghhttp.Error(r, w, statusUnsup, "only content-type %s is allowed", wantCType)
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func ensurePOST(handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
|
||||
return ensure(http.MethodPost, handler)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,28 @@
|
|||
|
||||
|
||||
|
||||
## v0.107.15: `POST` Requests Without Bodies
|
||||
|
||||
As an additional CSRF protection measure, AdGuard Home now ensures that requests
|
||||
that change its state but have no body do not have a `Content-Type` header set
|
||||
on them.
|
||||
|
||||
This concerns the following APIs:
|
||||
|
||||
* `POST /control/dhcp/reset_leases`;
|
||||
* `POST /control/dhcp/reset`;
|
||||
* `POST /control/parental/disable`;
|
||||
* `POST /control/parental/enable`;
|
||||
* `POST /control/querylog_clear`;
|
||||
* `POST /control/safebrowsing/disable`;
|
||||
* `POST /control/safebrowsing/enable`;
|
||||
* `POST /control/safesearch/disable`;
|
||||
* `POST /control/safesearch/enable`;
|
||||
* `POST /control/stats_reset`;
|
||||
* `POST /control/update`.
|
||||
|
||||
|
||||
|
||||
## v0.107.14: BREAKING API CHANGES
|
||||
|
||||
A Cross-Site Request Forgery (CSRF) vulnerability has been discovered. We have
|
||||
|
|
@ -13,6 +35,9 @@ 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 JSON APIs that expect a body now check if the request actually has
|
||||
`Content-Type` set to `application/json`.
|
||||
|
||||
All new formats for the request and response bodies are documented in
|
||||
`openapi.yaml`.
|
||||
|
||||
|
|
|
|||
|
|
@ -601,11 +601,10 @@
|
|||
'summary': 'Set user-defined filter rules'
|
||||
'requestBody':
|
||||
'content':
|
||||
'text/plain':
|
||||
'application/json':
|
||||
'schema':
|
||||
'type': 'string'
|
||||
'example': '@@||yandex.ru^|'
|
||||
'description': 'All filtering rules, one line per rule'
|
||||
'$ref': '#/components/schemas/SetRulesRequest'
|
||||
'description': 'Custom filtering rules.'
|
||||
'responses':
|
||||
'200':
|
||||
'description': 'OK.'
|
||||
|
|
@ -1538,6 +1537,19 @@
|
|||
'properties':
|
||||
'updated':
|
||||
'type': 'integer'
|
||||
'SetRulesRequest':
|
||||
'description': 'Custom filtering rules setting request.'
|
||||
'example':
|
||||
'rules':
|
||||
- '||example.com^'
|
||||
- '# comment'
|
||||
- '@@||www.example.com^'
|
||||
'properties':
|
||||
'rules':
|
||||
'items':
|
||||
'type': 'string'
|
||||
'type': 'array'
|
||||
'type': 'object'
|
||||
'GetVersionRequest':
|
||||
'type': 'object'
|
||||
'description': '/version.json request data'
|
||||
|
|
|
|||
Loading…
Reference in a new issue