We had a few places where we had perhaps too much of an opinion on the
permissions on created files and directories, sometimes fuled by a
misconception about how permissions work in both Unix and Windows. Recap
on the ground rules:
- On all unixes, all file & directory creation (`Mkdir`, `MkdirAll`,
`Create`, `WriteFile`, `Open`) has the given permission bits filtered
via the user's umask. The proper permissions for us to use are in almost
all cases 0o666 for files and 0o777 for directories, strange as that may
look at the call site.
- On Windows, there is no umask but in turn all of the permission bits
except the user write bit are ignored. The absence of user write bit is
converted into the read only attribute. This means that what is proper
for Unix above is also proper for Windows.
- We make an exception when creating files for certificate keys and the
config / database directories, as those contain secrets we think should remain closed
even if the user generally collaborates with other users on the system.
(Also removal of a bugfixed copy of MkdirAll for Windows that hasn't
been necessary for a few years.)
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Fix a response body leak in `githubSourceCodeLoader.Load` where the body
was not closed when the HTTP status was non-200.
Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
lib/ur brings in a lot of dependencies we don't need in e.g.
stcrashreceiver, who only needs the small failure reporting structs.
Make those part of the lean `contract` package instead.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
The runtime prints a lot of context for crashes due to bad pointers etc,
which is required to understand the crash, but this context comes before
the `fatal error: ...` line. Currently those lines get filtered out and
not included in the crash report. This change modifies the criteria so
that we start collecting crash data also at a line that begins with
`runtime:`, and tweaks the parsing later to look for the specific
`panic:` or `fatal error:` which may come later as the subject.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
## Summary
- Replace vendored `jquery-2.2.2.js` with `jquery-3.7.1.js` in
`gui/default/vendor/jquery/`
- Update script reference in `gui/default/index.html` to point to the
new file
- Update CDN reference in `cmd/infra/strelaypoolsrv/gui/index.html` from
`jquery-2.1.4.min.js` to `jquery-3.7.1.min.js`
## Why
The previously used jQuery versions (2.2.2 and 2.1.4) are vulnerable to
three known CVEs:
| CVE | Description | Fixed in |
|-----|-------------|----------|
| CVE-2015-9251 | XSS via cross-domain Ajax requests with non-text
content types | jQuery 3.0.0 |
| CVE-2020-11022 | XSS when passing HTML containing `<option>` elements
to manipulation methods | jQuery 3.5.0 |
| CVE-2020-11023 | XSS via passing HTML from untrusted sources to
manipulation methods | jQuery 3.5.0 |
jQuery 3.7.1 is the latest stable release and resolves all three.
## Compatibility notes
The GUI code was audited for jQuery 2→3 breaking changes. No removed
APIs are used:
- `.success()` / `.error()` calls throughout the codebase are
**AngularJS `$http`** promise methods, not jQuery — unaffected
- `.bind('beforeunload', ...)` is deprecated in jQuery 3 but not removed
— still works
- No usage of `.size()`, `.load()` event shorthand, `jQuery.isFunction`,
or `$.type()`
---------
Signed-off-by: Umer Azaz <umer_azaz@yahoo.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This updates our logging framework from legacy freetext strings using
the `log` package to structured log entries using `log/slog`. I have
updated all INFO or higher level entries, but not yet DEBUG (😓)... So,
at a high level:
There is a slight change in log levels, effectively adding a new warning
level:
- DEBUG is still debug (ideally not for users but developers, though
this is something we need to work on)
- INFO is still info, though I've added more data here, effectively
making Syncthing more verbose by default (more on this below)
- WARNING is a new log level that is different from the _old_ WARNING
(more below)
- ERROR is what was WARNING before -- problems that must be dealt with,
and also bubbled as a popup in the GUI.
A new feature is that the logging level can be set per package to
something other than just debug or info, and hence I feel that we can
add a bit more things into INFO while moving some (in fact, most)
current INFO level warnings into WARNING. For example, I think it's
justified to get a log of synced files in INFO and sync failures in
WARNING. These are things that have historically been tricky to debug
properly, and having more information by default will be useful to many,
while still making it possible get close to told level of inscrutability
by setting the log level to WARNING. I'd like to get to a stage where
DEBUG is never necessary to just figure out what's going on, as opposed
to trying to narrow down a likely bug.
Code wise:
- Our logging object, generally known as `l` in each package, is now a
new adapter object that provides the old API on top of the newer one.
(This should go away once all old log entries are migrated.) This is
only for `l.Debugln` and `l.Debugf`.
- There is a new level tracker that keeps the log level for each
package.
- There is a nested setup of handlers, since the structure mandated by
`log/slog` is slightly convoluted (imho). We do this because we need to
do formatting at a "medium" level internally so we can buffer log lines
in text format but with separate timestamp and log level for the API/GUI
to consume.
- The `debug` API call becomes a `loglevels` API call, which can set the
log level to `DEBUG`, `INFO`, `WARNING` or `ERROR` per package. The GUI
is updated to handle this.
- Our custom `sync` package provided some debugging of mutexes quite
strongly integrated into the old logging framework, only turned on when
`STTRACE` was set to certain values at startup, etc. It's been a long
time since this has been useful; I removed it.
- The `STTRACE` env var remains and can be used the same way as before,
while additionally permitting specific log levels to be specified,
`STTRACE=model:WARN,scanner:DEBUG`.
- There is a new command line option `--log-level=INFO` to set the
default log level.
- The command line options `--log-flags` and `--verbose` go away, but
are currently retained as hidden & ignored options since we set them by
default in some of our startup examples and Syncthing would otherwise
fail to start.
Sample format messages:
```
2009-02-13 23:31:30 INF A basic info line (attr1="val with spaces" attr2=2 attr3="val\"quote" a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with grouped values (attr1=val1 foo.attr2=2 foo.bar.attr3=3 a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with grouped values via logger (foo.attr1=val1 foo.attr2=2 a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with nested grouped values via logger (bar.foo.attr1=val1 bar.foo.attr2=2 a=a log.pkg=slogutil)
2009-02-13 23:31:30 WRN A warning entry (a=a log.pkg=slogutil)
2009-02-13 23:31:30 ERR An error (a=a log.pkg=slogutil)
```
---------
Co-authored-by: Ross Smith II <ross@smithii.com>
This updates our key generation to use Ed25519 keys/certificates for
sync connections. Certificates for browser use remain ECDSA for wider
compatibility.
Ed25519 is more modern and has fewer concerns for the future than the
ECDSA curves we used previously. It is supported from Go 1.13 and
forwards, which is Syncthing 1.3.0 (October 2019).
The sort package is still used in places that were not trivial to
change. Since Go 1.21 slices package can be uswed for sort. See
https://go.dev/doc/go1.21#slices
### Purpose
Make some progress with the migration to a more up-to-date syntax.
At a high level, this is what I've done and why:
- I'm moving the protobuf generation for the `protocol`, `discovery` and
`db` packages to the modern alternatives, and using `buf` to generate
because it's nice and simple.
- After trying various approaches on how to integrate the new types with
the existing code, I opted for splitting off our own data model types
from the on-the-wire generated types. This means we can have a
`FileInfo` type with nicer ergonomics and lots of methods, while the
protobuf generated type stays clean and close to the wire protocol. It
does mean copying between the two when required, which certainly adds a
small amount of inefficiency. If we want to walk this back in the future
and use the raw generated type throughout, that's possible, this however
makes the refactor smaller (!) as it doesn't change everything about the
type for everyone at the same time.
- I have simply removed in cold blood a significant number of old
database migrations. These depended on previous generations of generated
messages of various kinds and were annoying to support in the new
fashion. The oldest supported database version now is the one from
Syncthing 1.9.0 from Sep 7, 2020.
- I changed config structs to be regular manually defined structs.
For the sake of discussion, some things I tried that turned out not to
work...
### Embedding / wrapping
Embedding the protobuf generated structs in our existing types as a data
container and keeping our methods and stuff:
```
package protocol
type FileInfo struct {
*generated.FileInfo
}
```
This generates a lot of problems because the internal shape of the
generated struct is quite different (different names, different types,
more pointers), because initializing it doesn't work like you'd expect
(i.e., you end up with an embedded nil pointer and a panic), and because
the types of child types don't get wrapped. That is, even if we also
have a similar wrapper around a `Vector`, that's not the type you get
when accessing `someFileInfo.Version`, you get the `*generated.Vector`
that doesn't have methods, etc.
### Aliasing
```
package protocol
type FileInfo = generated.FileInfo
```
Doesn't help because you can't attach methods to it, plus all the above.
### Generating the types into the target package like we do now and
attaching methods
This fails because of the different shape of the generated type (as in
the embedding case above) plus the generated struct already has a bunch
of methods that we can't necessarily override properly (like `String()`
and a bunch of getters).
### Methods to functions
I considered just moving all the methods we attach to functions in a
specific package, so that for example
```
package protocol
func (f FileInfo) Equal(other FileInfo) bool
```
would become
```
package fileinfos
func Equal(a, b *generated.FileInfo) bool
```
and this would mostly work, but becomes quite verbose and cumbersome,
and somewhat limits discoverability (you can't see what methods are
available on the type in auto completions, etc). In the end I did this
in some cases, like in the database layer where a lot of things like
`func (fv *FileVersion) IsEmpty() bool` becomes `func fvIsEmpty(fv
*generated.FileVersion)` because they were anyway just internal methods.
Fixes#8247
Move infrastructure related commands to under `cmd/infra` and
development stuff to `cmd/dev`. The default build command builds the
regular user facing binaries: syncthing, stdiscosrv, and strelaysrv.
