diff --git a/changelog.d/67.feature b/changelog.d/67.feature new file mode 100644 index 0000000..f58daa3 --- /dev/null +++ b/changelog.d/67.feature @@ -0,0 +1 @@ +Add config option to block unknown appplication names. diff --git a/docs/blocked_rageshake.md b/docs/blocked_rageshake.md new file mode 100644 index 0000000..a39a876 --- /dev/null +++ b/docs/blocked_rageshake.md @@ -0,0 +1,35 @@ +# Rageshake server not accepting rageshakes + +This page contains information useful to someone who has had their rageshake rejected by a rageshake server. + +We include it within the error messages to provide a place with context for users reading the error message and wanting +to know more. + +## For matrix client users + +Thank you for attempting to report a bug with your matrix client; unfortunately your client application is likely incorrectly configured. + +The rageshake server you attempted to upload a report to is not accepting rageshakes from your client at this time. + +Generally, the developers who run a rageshake server will only be able to handle reports for applications they are developing, +and your application is not listed as one of those applications. + +Please contact the distributor of your application or the administrator of the web site you visit to report this as a problem. + +## For developers of matrix clients + +Your application is likely based on one of the matrix SDKs or element applications, if it is submitting rageshakes to a rageshake server. + +A change has been made to pre-filter reports that the developers using this rageshake server for applications they do not have control over. +Typically reports from unknown applications would have to be manually triaged and discarded; there is now automatic filtering in place, which reduces overall effort. + +There is generally a configuration file in your application that you can alter to change where these reports are sent, which may require rebuilding and releasing the client. + +The easiest solution to this error is to stop sending rageshakes entirely, which may require a code or configuration change in your client. + +However, if you wish to accept bug reports from your users applications, you will need to run your own copy of this rageshake server and update the URL appropriately. + +## Application specific config locations: + * element-web: `bug_report_endpoint_url` in the [sample configuration for element web](https://github.com/vector-im/element-web/blob/develop/config.sample.json). + * element-ios: `bugReportEndpointUrlString` in the [BuildSettings.swift](https://github.com/vector-im/element-ios/blob/develop/Config/BuildSettings.swift) + * element-android: `bug_report_url` in the [config.xml file for the build](https://github.com/vector-im/element-android/blob/develop/vector-config/src/main/res/values/config.xml) diff --git a/main.go b/main.go index 65a300d..287e5e6 100644 --- a/main.go +++ b/main.go @@ -48,6 +48,9 @@ type config struct { // External URI to /api APIPrefix string `yaml:"api_prefix"` + // Allowed rageshake app names + AllowedAppNames []string `yaml:"allowed_app_names"` + // A GitHub personal access token, to create a GitHub issue for each report. GithubToken string `yaml:"github_token"` @@ -149,10 +152,13 @@ func main() { // remove trailing / apiPrefix = strings.TrimRight(apiPrefix, "/") } + + appNameMap := configureAppNameMap(cfg) + log.Printf("Using %s/listing as public URI", apiPrefix) rand.Seed(time.Now().UnixNano()) - http.Handle("/api/submit", &submitServer{ghClient, glClient, apiPrefix, slack, genericWebhookClient, cfg}) + http.Handle("/api/submit", &submitServer{ghClient, glClient, apiPrefix, slack, genericWebhookClient, appNameMap, cfg}) // Make sure bugs directory exists _ = os.Mkdir("bugs", os.ModePerm) @@ -180,6 +186,17 @@ func main() { log.Fatal(http.ListenAndServe(*bindAddr, nil)) } +func configureAppNameMap(cfg *config) map[string]bool { + if len(cfg.AllowedAppNames) == 0 { + fmt.Println("Warning: allowed_app_names is empty. Accepting requests from all app names") + } + var allowedAppNameMap = make(map[string]bool) + for _, app := range cfg.AllowedAppNames { + allowedAppNameMap[app] = true + } + return allowedAppNameMap +} + func configureGenericWebhookClient(cfg *config) *http.Client { if len(cfg.GenericWebhookURLs) == 0 { fmt.Println("No generic_webhook_urls configured.") diff --git a/rageshake.sample.yaml b/rageshake.sample.yaml index dcc1493..283e812 100644 --- a/rageshake.sample.yaml +++ b/rageshake.sample.yaml @@ -8,6 +8,10 @@ listings_auth_pass: secret # report to the GitHub issue. If unspecified, based on the listen address. # api_prefix: https://riot.im/bugreports +# List of approved AppNames we accept. Names not in the list or missing an application name will be rejected. +# An empty or missing list will retain legacy behaviour and permit reports from any application name. +allowed_app_names: [] + # a GitHub personal access token (https://github.com/settings/tokens), which # will be used to create a GitHub issue for each report. It requires # `public_repo` scope. If omitted, no issues will be created. diff --git a/scripts/lint.sh b/scripts/lint.sh index 482a4af..bbaad61 100755 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -7,6 +7,9 @@ set -eu +echo "golint:" golint -set_exit_status +echo "go vet:" go vet -vettool=$(which shadow) +echo "gocyclo:" gocyclo -over 12 . diff --git a/submit.go b/submit.go index 9b875da..b733cdb 100644 --- a/submit.go +++ b/submit.go @@ -58,6 +58,7 @@ type submitServer struct { slack *slackClient genericWebhookClient *http.Client + allowedAppNameMap map[string]bool cfg *config } @@ -191,6 +192,17 @@ func (s *submitServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } + // Filter out unwanted rageshakes, if a list is defined + if len(s.allowedAppNameMap) != 0 && !s.allowedAppNameMap[p.AppName] { + log.Printf("Blocking rageshake because app name %s not in list", p.AppName) + if err := os.RemoveAll(reportDir); err != nil { + log.Printf("Unable to remove report dir %s after rejected upload: %v\n", + reportDir, err) + } + http.Error(w, "This server does not accept rageshakes from your application. See https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md", 400) + return + } + // We use this prefix (eg, 2022-05-01/125223-abcde) as a unique identifier for this rageshake. // This is going to be used to uniquely identify rageshakes, even if they are not submitted to // an issue tracker for instance with automatic rageshakes that can be plentiful @@ -243,6 +255,7 @@ func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *p http.Error(w, fmt.Sprintf("Could not decode payload: %s", err.Error()), 400) return nil } + return p } @@ -273,35 +286,13 @@ func parseJSONRequest(w http.ResponseWriter, req *http.Request, reportDir string } } - // backwards-compatibility hack: current versions of riot-android - // don't set 'app', so we don't correctly file github issues. - if p.AppName == "" && p.UserAgent == "Android" { - parsed.AppName = "riot-android" + parsed.AppName = p.AppName - // they also shove lots of stuff into 'Version' which we don't really - // want in the github report - for _, line := range strings.Split(p.Version, "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } - parts := strings.SplitN(line, ":", 2) - key := strings.TrimSpace(parts[0]) - val := "" - if len(parts) > 1 { - val = strings.TrimSpace(parts[1]) - } - parsed.Data[key] = val - } - } else { - parsed.AppName = p.AppName - - if p.UserAgent != "" { - parsed.Data["User-Agent"] = p.UserAgent - } - if p.Version != "" { - parsed.Data["Version"] = p.Version - } + if p.UserAgent != "" { + parsed.Data["User-Agent"] = p.UserAgent + } + if p.Version != "" { + parsed.Data["Version"] = p.Version } return &parsed, nil diff --git a/submit_test.go b/submit_test.go index e0f15b8..aa54a33 100644 --- a/submit_test.go +++ b/submit_test.go @@ -56,6 +56,55 @@ func testParsePayload(t *testing.T, body, contentType string, tempDir string) (* return p, rr.Result() } + +func submitSimpleRequestToServer(t *testing.T, allowedAppNameMap map[string]bool, body string) int { + // Submit a request without files to the server and return statusCode + // Could be extended with more complicated config; aimed here just to + // test options for allowedAppNameMap + + req, err := http.NewRequest("POST", "/api/submit", strings.NewReader(body)) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Length", strconv.Itoa(len(body))) + w := httptest.NewRecorder() + + var cfg config + s := &submitServer{nil, nil, "/", nil, nil, allowedAppNameMap, &cfg} + + s.ServeHTTP(w, req) + rsp := w.Result() + return rsp.StatusCode +} + +func TestAppNames(t *testing.T) { + body := `{ + "app": "alice", + "logs": [ ], + "text": "test message", + "user_agent": "Mozilla/0.9", + "version": "0.9.9" +}` + validAppNameMap := map[string]bool{ + "alice": true, + } + if submitSimpleRequestToServer(t, validAppNameMap, body) != 200 { + t.Fatal("matching app was not accepted") + } + + invalidAppNameMap := map[string]bool{ + "bob": true, + } + if submitSimpleRequestToServer(t, invalidAppNameMap, body) != 400 { + t.Fatal("nonmatching app was not rejected") + } + + emptyAppNameMap := make(map[string]bool) + if submitSimpleRequestToServer(t, emptyAppNameMap, body) != 200 { + t.Fatal("empty map did not allow all") + } +} + func TestEmptyJson(t *testing.T) { body := "{}" @@ -108,36 +157,6 @@ func TestJsonUpload(t *testing.T) { checkUploadedFile(t, reportDir, "logs-0000.log.gz", true, "line1\nline2") } -// check that we can unpick the json submitted by the android clients -func TestUnpickAndroidMangling(t *testing.T) { - body := `{"text": "test ylc 001", -"version": "User : @ylc8001:matrix.org\nPhone : Lenovo P2a42\nVector version: 0:6:9\n", -"user_agent": "Android" -}` - p, _ := testParsePayload(t, body, "", "") - if p == nil { - t.Fatal("parseRequest returned nil") - } - if p.UserText != "test ylc 001" { - t.Errorf("user text: got %s, want %s", p.UserText, "test ylc 001") - } - if p.AppName != "riot-android" { - t.Errorf("appname: got %s, want %s", p.AppName, "riot-android") - } - if p.Data["Version"] != "" { - t.Errorf("version: got %s, want ''", p.Data["Version"]) - } - if p.Data["User"] != "@ylc8001:matrix.org" { - t.Errorf("data.user: got %s, want %s", p.Data["User"], "@ylc8001:matrix.org") - } - if p.Data["Phone"] != "Lenovo P2a42" { - t.Errorf("data.phone: got %s, want %s", p.Data["Phone"], "Lenovo P2a42") - } - if p.Data["Vector version"] != "0:6:9" { - t.Errorf("data.version: got %s, want %s", p.Data["Version"], "0:6:9") - } -} - func TestMultipartUpload(t *testing.T) { reportDir := mkTempDir(t) defer os.RemoveAll(reportDir)