Upstream #383
Draft
Upstream #383
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Documentation builds (`make html`) were reporting various warnings. There were two sources of the errors: - Most pages from the autogenerated command line docs are not in any toctree, because they use their own internal linking. Fixed by adding :orphan: to these pages. - One image in the manual pages (gateway/prefix_pinning.rst) would always be reported as not found, but was working correctly. This was due to the structure of the manual pages, which are built from multiple partial rst files using the `.. include:` directive. The figures are found relative to the main rst file, not the partial. As the partials were _also_ being built individually, the error resulted. Fixed by excluding the partial rst files (manuals/*/*) from the sphinx build. Also includes the following improvements: - more specific short/long doc string for the `scion` tool - add requirements.in and pip-compile generated requirements.txt - doc/Makefile: add targets autobuild and command-gendocs for convenience, explicitly list the main sphinx-build targets to get make target auto-completion on shells - Disable the "Atuo generated by spf13/cobra ..." footer for the cobra gendocs, getting rid of post processing command in the bazel build. Use "proper" sphinx :ref: cross-references instead of html links. - Fix (or remove) some broken links reported by `make linkcheck`
In #4319, the python requirements file was generated for python3.8 but our build configuration file still referred to 3.7, resulting in broken builds. Also bump the ubuntu version to latest LTS, because, why not. Also move the .readthedocs.yaml file to the project root, where this should be located according to the documentation. The readthedocs build log (confusingly) mentioned that this file was not present. Either this was a spurious error message or the file was indeed ignored and it just happened to match the defaults.
This commit updates almost all of the direct dependencies to the latest version.
…ys (#4326) Fix simple logic error in DRKey service engine (assignment to variable was too late, after use).
Deprecate unused attributes `authoritative`, `voting` and `issuing` from the topology.json configuration. These are no longer considered by any service or tool and the functionality has been moved into the cs.toml (containing a `[ca]` section for "issuing" ASes) or into the TRC and the configuration of the corresponding tooling. Only the `core` attribute option remains. In the long run, this should either disappear entirely from the topology configuration or be changed to a boolean flag. In the internal `private/topology`, remove the unused `CA() bool` from the interface, and simplify the representation of the state from a list of attributes to a simpler `IsCore`. The goal here is mainly to avoid confusion caused by the unused attributes.
…#4332) Add demo/integration test for key derivation with different protocols identifiers, in particular for a "niche" protocol using the generic key derivation. Make fetching the SV optional for the server-side demo and use the non-privileged derivation mechanism otherwise.
Bumps [vm2](https://github.com/patriksimek/vm2) from 3.9.14 to 3.9.15. - [Release notes](https://github.com/patriksimek/vm2/releases) - [Changelog](https://github.com/patriksimek/vm2/blob/master/CHANGELOG.md) - [Commits](patriksimek/vm2@3.9.14...3.9.15) vm2 is in indirect dependency of spectral-cli, which is (only!) the linter used for the openapi specs. --- updated-dependencies: - dependency-name: vm2 dependency-type: indirect Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [vm2](https://github.com/patriksimek/vm2) from 3.9.15 to 3.9.16. - [Release notes](https://github.com/patriksimek/vm2/releases) - [Changelog](https://github.com/patriksimek/vm2/blob/master/CHANGELOG.md) - [Commits](patriksimek/vm2@3.9.15...3.9.16) vm2 is an indirect dependency of spectral-cli, which is (only!) the linter used for the openapi specs. --- updated-dependencies: - dependency-name: vm2 dependency-type: indirect Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
The scion.sh sciond-addr command was returning an invalid address for IPv6 setups, resulting in "too many colons address" error when used as the `--sciond` parameter of tool invocations. Fixed by wrapping the IP in brackets before appending the port.
Add openapi_build_doc rule to build the API documentation as zero-dependency HTML file. This replaces separate genrules in the build files for the individual tools. These had all been using the `redoc-cli` command which is deprecated (in favor of redocly/cli) and had been creating broken HTML files since #4323. Fix the link in the dummy index.html page from the private/mgmtapi package, used when not bundling the generated documentation page, to point to openapi.json (not spec.json). Drop the yarn dependency on `redoc-cli` and update all dependencies.
Replace deprecated inet.af/netaddr with its standard library replacement package net/netip. Use go4.org/netipx for some the pieces that were not added to the standard library. This change does not affect the `snet` API, or any other public API.
There is a small number of of packet processing test cases that cover cases resulting in SCMPs. The packet processor signals SCMPs by returning an `scmpError`. Previously, the test cases just asserted that any error is returned from processing and so they would pass even if an internal error occurred. In this instance there was an internal error while serializing the SCMP message, as the internal IP of the dataplane, the source address of the SCMP message, was not initialized in the test setup. Add a dummy internal IP address to the test dataplane setup, and change the type of the dataplane's internal IP so that a missing initialization of the IP leads to a panic instead of a runtime error. Assert that an `scmpError` is returned for the relevant test cases, and check the SCMP type/code. The full SCMP message is still not checked.
The documentation has been a somewhat unsorted bag of documents. It was mostly focused on contributors to this project, but some sections did target users of the software, without a clear distinction.
This PR attempts to bring some more structure into this. There are now three separate sections:
- Technology:
Explanations and Specifications. Explain SCION on a conceptual level.
Should introduce concepts that a user or dev needs to make sense of the manuals etc.
Target audience: anyone (users, developers, outsiders)
- Reference manuals:
Target audience: users of this SCION implementation (operator of SCION infrastructure or hosts), users of any of the SCION APIs.
- Developer section:
Target audience: contributors to this SCION implementation
In particular, the developer section now "hides" the design documents away a bit and the intended workflow for these documents is clarified.
There is still a lot to do, in particular for overview and introductory material and the manuals. I've started to work on the manual for the control service, but I've decided to defer this for a separate PR so that we can finalize the organizational changes.
Note that the content of some design documents, in particular the one on path policies is now relegated to the less findable design document section. The relevant information will be included in the manuals again.
* Setup link * Contribution link
The Traffic Class field used to be abbreviated by QoS, an unnecessary alias. Traffic Class is also the name used for the equivalent field in the IPv6 header. Also, fix the documented NextHdr value for the SCMP protocol.
Fix a couple grammar errors with past/present tense and capitalizing the nouns.
Both decoding and serializing of the address type fields (DstAddrType and SrcAddrType) was truncating the most significant bit. This has not caused any issues so far as no address type with type greater than 1 is defined and used. This bug was introduced in #4160, by consistently using the wrong (!) value 0x7, instead of 0xF, to mask the lowest four bits. Fixed the same mistake in copies of the serialization logic.
Fix a couple issues with tense, nouns, and punctuation.
Fix misspellings of the Daemon, was spelled as "Deamon" in some places.
Update generated documentation by running `make command-gendocs` in `doc/`.
Change link to point to the proper EPIC-HP design doc PR instead of a PR for COLIBRI design document.
… (#4346)
Replace the `addr.HostAddr` type hierarchy, consisting of the `HostAddr` interface and the `HostNone`, `HostIPv4`, `HostIPv6` and `HostSVC` implementations with a single type `addr.Host`, representing all the different host address types as a tagged union ("sum type"). This uses, and follows the spirit of, the `net/netip.Addr` types to represent IP addresses for both IPv4 and IPv6.
The `addr.Host` type is a simple value type, which can be created and copied without heap allocations and can be used in comparisons and as map keys.
In addition to replacing all uses of `addr.HostAddr`, the new `addr.Host` type is used in the `slayers.SCION.Get/SetSrc/DstAddr` interfaces, as well as in the DRKey infrastructure. Using a consistent representation of the addresses allows to get rid of duplicate functionality and a number of address conversions, e.g. in `snet`.
In contrast to the `addr.HostAddr` interface, the new `addr.Host` does not implement the `net.Addr` interface. In particular, service addresses were passed through layers requiring a `net.Addr`. Use `snet.SVCAddr` in these cases.
Additionally, add a new `addr.Addr` type representing a full SCION address (ISD, AS and host address), including parsing functionality. This definition is identical to the `snet.SCIONAddress` type, which is now only kept as a type alias for compatibility.
Future work:
- `pkg/addr` is a public API and should have a more SCION-specific and less clash-likely name, e.g. `saddr`
- replace `snet.SCIONAddress` by `addr.Addr`
- adopt using `addr.Addr` in the ping, traceroute etc tools, in place of the ill-fitting use of `snet.UDPAddr`.
- add `addr.MustParse{ISD,AS,IA}` for completeness sake (currently in xtest package)
- get rid of the IP-slice to `netip.Addr` conversions by gradually increasing the adoption of `netip.Addr` and related APIs
The fork of rules_antlr that we depend on has disappeared https://github.com/artisoft-io We switch to another fork that contains the same changes that artisoft-io had.
Use write_source_files to generate the docs. The write_source_files comes with automatic tests that the generated files are up to date. We create a wrapper in tools/lint which checks that the generated files are up to date if we run bazel test with --config=lint. `make lint` checks that the `write_all` target is up to date with all the additional targets to run.
Update the pyyaml dependency to 6.0.0. 6.0.0 includes [this commit](yaml/pyyaml@a6d384c) that removes the use of the [deprecated](https://setuptools.pypa.io/en/latest/userguide/declarative_config.html#metadata) `license_file` field in the setup.cfg. This addresses the following warning: ``` The license_file parameter is deprecated, use license_files instead. By 2023-Oct-30, you need to update your project and remove deprecated calls or your builds will no longer be supported. See https://setuptools.pypa.io/en/latest/userguide/declarative_config.html for details. ```
Restructure the border router's packet processing loop into receiver, processing and forwarders goroutines. This is implementation part one of three described in the design document (github.com/scionproto/scion/pull/4339, doc/dev/design/BorderRouter.rst).
Update pyyaml dependency from 6.0.0 -> 6.0.1, as amendment to #4367. The actual issue that had caused our builds to fail suddenly, was not related to the license_file warning, but an AttributeError related to a "cython_sources" option. It appears that pyyaml had an unconstrained version of for Cython as a build system dependency. When Cython 3 was released, this started to fail. Pyyaml 6.0.1 now has a constraint for Cython version < 3, which should keep things working. Somewhat worrying, and not addressed by this change, is the fact that the build system dependencies do not seem to be pinned by our requirements.txt file.
A partial implementation of colibri was added in a series of PRs (#3446, ..., #3905). This has not been sufficiently completed to become operational, i.e. the colibri implementation in this repository has effectively remained dead code. The colibri prototype was further developed in a fork, github.com/netsec-ethz/scion, and changes have no longer been upstreamed. Consequently the remaining colibri code here has become stale and keeping this is no longer of great use.
Reported by dependabot in https://github.com/scionproto/scion/security/dependabot/82
In every case where the router modified packets it would serialize updated headers to a temporary buffer and then copy that to the packet buffer. To avoid this extra copy, replaced gopacket.serializeBuffer with a custom implementation that writes to a given buffer. In this case, the packet's raw buffer. There is one remaining copy for some SCMP messages because we have to move the existing packet to the payload. This too could be avoided but that's for another PR; it would require to leave headroom in received packets. The performance impact is very small since, on the critical path, it just avoids copying a scion hdr per packet, but it is a simplification. It also pays back the copy added by a previous simplification of the BFD code. As such... Contributes to #4593
Local topologies (created using the scion.sh topology command) do not support ASN < 2^32. If such ASNs are written in decimal notation, validation of the ISD-ASN fails. Writing the ASN in hexadecimal also fails, because the crytographic material is generated in the wrong directory. All that seems to be missing to support BGP ASNs is allowing the decimal format and making sure that all ASNs < 2^32 are converted to decimal format as expected by the scion-pki tool.
Selecting a path when using the ```-i``` option doesn't work in Windows. Windows uses ```"\r\n"``` as delimiter, the current code only removes the last character from the user input to select a path (i.e. ```"\n"```). This PR instead right-trims all appearances of ```\n``` and ```\r``` from the input to make it work on both, Windows and Unix systems.
Currently there seems to be no way to cleanly terminate a SCION service on Windows. Typing Ctrl-C terminates the program without running cleanup code. This PR allows running SCION as services on Windows. Applications detect automatically whether they were started as a service or as console applications and will react to control request appropriately. If running as console application Ctrl-C is handled correctly.
Since making the packet size 1500, the expectations are too close to the best case. The test has become flaky.
…#4670) So far, the `snet/SCIONPacketConn` was bubbling up decoding/parsing errors to the calling applications. In principle, these errors are not recoverable by the application. The current solution simulates that the network stack simply discards incorrect/malformated packets. Otherwise,some libraries/applications would misbehave (similar to what would happen if were to receive SCMP errors). If we believe that SCION-aware applications using the low-level `SCIONPacketConn` should receive the error, we can move this up to `Conn`.
The condition was missing brackets around the IP. While at it this also simplifies the code a bit.
Dial and DialContext is deprecated in newer version of gRPC. (See https://pkg.go.dev/google.golang.org/grpc#Dial)
Instead of having the snet.Topology as an interface, change it to a struct. This makes the contract clearer, LocalIA and PortRange are only considered when creating a connection. The Interface information however can dynamically change. The daemon API now exposes an adapter to load the topology information once or to load the information periodically.
Update the TC members list. Add a link to the online godoc in the Contribute section. Try and improve package descriptions (in the router only for now) to make the code easier to navigate with godoc.
Fix typos and clarify comment on SCMP on legacy hosts. Co-authored-by: jiceatscion <[email protected]>
Some tests start sig in the same time as sciond and have become flaky (presumably due to ordering or timing changes in docker). This resolves it.
I went through the hoops of setting up the development environment on my M4 Macbook and found [Lima](https://github.com/lima-vm/lima) to be the most convenient way: - Licensed under [Apache-2.0 license](https://github.com/lima-vm/lima?tab=Apache-2.0-1-ov-file#readme) - Actively maintained and [stable](https://github.com/lima-vm/lima/releases)
Added a summary of the stack and a summary of the topology.
- replace deprecated `github.com/golang/protobuf` with `google.golang.org/protobuf` - replace deprecated `ptypes.TimestampProto` from `github.com/golang/protobuf/ptypes` with `google.golang.org/protobuf/types/known/timestamppb` - replace deprecated `grpc.WithInsecure()` with `google.golang.org/grpc/credentials/insecure` Deprecation notice: https://pkg.go.dev/github.com/golang/protobuf
This is the result of running:
```sh
find . -name "*.go" ! -exec grep -q "// Code generated" {} \; -exec gofmt -w -r 'interface{} -> any' {} +
```
It safely replaces all usage of `interface{}` with `any`, excluding
generated files and is hence a purely **cosmetic change**.
Also see golang/go#49884
Upgrade quic-go/quic-go from `v0.43.1` to `v0.49.0` to fix https://pkg.go.dev/vuln/GO-2024-3302
google/gopacket is no longer maintained by Google since August 2020. See issue: google/gopacket#1016. I propose we switch to the new actively maintained fork: https://github.com/gopacket/gopacket. The most prominent orgs on GitHub already [depending](https://pkg.go.dev/github.com/gopacket/gopacket?tab=importedby) on the new fork appear to be github.com/apache and github.com/usnistgov which gives me confidence.
The original helper function written 7 years ago is no longer useful since Go 1.15 introduced [`TempDir`](https://pkg.go.dev/testing#B.TempDir).
- **refactor:** Use `math/rand/v2` instead of `math/rand` (better performance, better API like automatic seeding, etc.). - **refactor:** Remove unused functions `scrypto.RandUint64` and `scrypto.RandInt64` and error sentinel values `ErrInvalidNonceSize` and `ErrUnableToGenerateNonce` from pkg/scrypto. - **refactor:** Replace deprecated `common.ErrMsg` sentinel errors with `errors.New` in `pkg/scrypto`. - **test:** Use `crypto/rand.Read` instead of deprecated `math/rand.Read` in dataplane test.
During verification of the drkey package (as part of the `VerifiedSCION` project) we ran into an issue with our verifier. It complains about the embedded struct in `Epoch`, which in this specific case, does not seem to be supported. (Just not to confuse anyone: The issue is with the verifier (gobra), not with Go or the `drkey` package itself.) However, to me there does not seem to be a reason why `Epoch` embedds `cppki.Validity`. It could as well just be a type definition. This is what I propose with this PR. It changes the definition of `Epoch` and adapts (and simplifies) the clients of the `drkey` package. --------- Co-authored-by: aaronbojarski <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is not supposed to be merged for now!
This PR is part of my (Aaron Bojarski) practical work project. Its goal is to evolve the
VerifiedSCIONrepository to contain and verify recent refactors of thescionproto/scionrepository. Furthermore we want to explore (automated) procedures to keep the repositories in sync.