Also had to change all references from `path.` to `filepath.` when
dealing with files, so that it works properly on Windows.
Fixed also lots of tests to defer the shutdown of the server
after the removal of the storage, and fixed some config files
directories to use the single quote `'` to surround the file path,
again to work on Windows.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
The established ordering is client -> Account, so fixed few places
where we had Account -> client.
Added a new file, locksordering.txt with the list of known ordering
for some of the objects.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
When a gateway connection was created (either accepted or initiated)
the timer to fire the first PING was started at that time, which
means that for an outbound connection, if the INFO coming from
the other side was delayed, it was possible for the outbound to
send the PING protocol before the CONNECT, which would cause
the accepting side to close the connection due to a "parse" error
(since the CONNECT for an inbound is supposed to be the very
first protocol).
Also noticed that we were not setting the auth timer like we do
for the other type of connections. If authorization{timeout:<n>}
is not set, the default is 2 seconds.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
For TLS configuration with `verify_and_map` set to true, if a
connection connects and has a certificate with ID that matches
a user, but that user's `allowed_connection_types` is specified
and does not have the connection type in its list, then the
server will panic.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Currently, OCSP responses that aren't signed by the root CA will fail
with a verification error. This change allows intermediates, or
designated responders, to sign responses on behalf of the CA.
- Add Go 1.17
- Fix go fmt from Go 1.17 (build directives)
- Download version of misspell and staticcheck instead of doing
"go get" since current staticcheck would be broken without go.mod
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
The bug occures when latency tracking is on, a requestor and responder are not connected to the same server, and the responder sends two responses for a single request.
Signed-off-by: Derek Collison <derek@nats.io>
* Redact URLs before logging or returning in error
This does not affect strings which failed to parse, and in such a scenario
there's a mix of "which evil" to accept; we can't sanely find what should be
redacted in those cases, so we leave them alone for debugging.
The JWT library returns some errors for Operator URLs, but it rejects URLs
which contain userinfo, so there can't be passwords in those and they're safe.
Fixes#2597
* Test the URL redaction auxiliary functions
* End-to-end tests for secrets in debug/trace
Create internal/testhelper and move DummyLogger there, so it can be used from
the test/ sub-dir too.
Let DummyLogger optionally accumulate all log messages, not just retain the
last-seen message.
Confirm no passwords logged by TestLeafNodeBasicAuthFailover.
Change TestNoPasswordsFromConnectTrace to check all trace messages, not just the
most recent.
Validate existing trace redaction in TestRouteToSelf.
* Test for password in solicited route reconnect debug
Updated some tests based on this change but also missing defer
connection close or server shutdown.
Fixed how the OCSP run go routine would shutdown, which would
never complete because grWG was not decremented by this go routine
prior to invoking s.Shutdown()
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Note since the hub will disconnect currently on a subscription from a soliciting leaf, we still do strict checks there.
We always properly check if data can flow, so we could remove the sub checks all together.
I did look into ways of returning a scoped subject for explicit allow subscriptions when presented with a wildcard, however this would have meant resolving multiple items.
E.g. allow ['foo', 'bar', 'foo.bar']
With a sub of '*' that would have to expand to ['foo', 'bar']
With a sub of '>' that would have to expand to ['foo', 'bar', "foo.bar']
With a sub of 'foo.*' that would have to expand to ['foo.bar']
I may sleep on this and revisit if I think I can get it to work properly.
Signed-off-by: Derek Collison <derek@nats.io>
This commit simply includes a change to the test that was added
for PR #2455 that fixed a similar issue (in deliverMsg). This
issue has to do with initial send of subscription interest.
Resolves#2469
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
When trying to update NATS Streaming server dependencies with
latest NATS Server, I noticed that a TLS test was failing and
this was because the TLS configuration was manually set like this:
```
o := DefaultTestOptions
o.HTTPHost = "127.0.0.1"
o.HTTPSPort = -1
o.TLSConfig = &tls.Config{ServerName: "localhost"}
cert, err := tls.LoadX509KeyPair("configs/certs/server-cert.pem", "configs/certs/server-key.pem")
if err != nil {
t.Fatalf("Got error reading certificates: %s", err)
}
o.TLSConfig.Certificates = []tls.Certificate{cert}
```
Notice how the `cert.Leaf` is not parsed. This cause the NATS Server
OCSP code to fail when hasOCSPStatusRequest() is invoked with
a `nil` pointer.
My first approach was to add a `nil` check in hasOCSPStatusRequest()
and return `false` in that case.
But then I thought that maybe the correct approach is to parse the
leaf it it is not done in the provided TLS config?
It could be simply a case of fixing the test that I have in
NATS Streaming server repo, but a quick check in this repo's own
dependencies show that not setting the Leaf is something that may
happen in some cases. For instance here is how the Postgres library
build the certs: caa87158f5/ssl.go (L133)
As you can see, the leaf is not parsed here, so I am not sure if
having Leaf nil is valid or not.
The go doc regarding Leaf says:
```
// Leaf is the parsed form of the leaf certificate, which may be initialized
// using x509.ParseCertificate to reduce per-handshake processing. If nil,
// the leaf certificate will be parsed as needed.
Leaf *x509.Certificate
```
This is the last statement that made me chose the current approach of
parsing it if detected as `nil` instead of just ignoring a nil cert.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
* [changed] pinned certs to check the server connected to as well
on reload clients with removed pinned certs will be disconnected.
The check happens only on tls handshake now.
Signed-off-by: Matthias Hanel <mh@synadia.com>
* [added] pinned_cert option to tls block hex(sha256(spki))
When read form config, the values are automatically lower cased.
The check when seeing the values programmatically requires
lower case to avoid having to alter the map at this point.
Signed-off-by: Matthias Hanel <mh@synadia.com>
This allows a domain to be set in the JetStream server block that sets a domain name.
Once set this signals that any leafnode connections should operate as separate JetStream domains.
Each domain <NAME> is accessible via "$JS.<NAME>.API.>", even when connected to the same domain.
Also for mixed mode you can set a jetstream block now that defines a domain but specifies "enabled: false".
Signed-off-by: Derek Collison <derek@nats.io>
On connect all subscription where sent by the soliciting leaf node.
If creds contains sub deny permissions, the leaf node would be
disconnected.
This waits for the permissions to be exchanged and checks permissions
before sending subscriptions.
Signed-off-by: Matthias Hanel <mh@synadia.com>
Currently in tests, we have calls to os.Remove and os.RemoveAll where we
don't check the returned error. This hides useful error messages when
tests fail to run, such as "too many open files".
This change checks for more filesystem related errors and calls t.Fatal
if there is an error.
Currently, temporary test files and directories are written in lots of
different paths within the OS's temp dir. This makes it hard to know
which files are from nats-server and which are unrelated. This in turn
makes it hard to clean up nats-server test files.
