Sometimes sendError gets a nil argument for ierr (internal error). When this
happens we print a line like:
[AUDIT] Internal error - Failed to get registration by key - %!s(<nil>)
This is fine but distracting, since it looks like a logging bug.
Instead, print a shorter message with just the external-facing problem.
Following up on #2752, we don't need to use global vars for our Prometheus stats. We already have a custom registry plumbed through using Scope objects. In this PR, expose the MustRegister method of that registry through the Scope interface, and move existing global vars to be fields of objects. This should improve testability somewhat.
Note that this has a bit of an unfortunate side effect: two instances of the same stats-using class (e.g. VA) can't use the same Scope object, because their MustRegister calls will conflict. In practice this is fine since we never instantiate duplicates of the the classes that use stats, but it's something we should keep an eye on.
Updates #2733
When running `gomock` to generate mocks in the boulder-tools image there is a requirement on `github.com/golang/mock/mockgen/model` but only during runtime (it is not required to build `gomock`). So that we don't require users to `go get` this package so that it exists in their GOPATH we need to vendor it so that it is always in the GOPATH of the boulder-tools image. In order to vendor this package (since it isn't actually used anywhere) we need to add a special file that imports this package and uses it for a variable that isn't actually used anywhere so that we can satisfy `godep`, this is done in the `test` package.
Fixes#2751.
This allows us to look at logs in more detail.
Also, remove RequestNonce, ResponseNonce, and ClientAddr, which we don't use and
take up log space. And set "Errors" to "omitempty."
Fixes#2747.
Prior to this PR the SA's `CountRegistrationsByIP` treated IPv6
differently than IPv4 by counting registrations within a /48 for IPv6 as
opposed to exact matches for IPv4. This PR updates
`CountRegistrationsByIP` to treat IPv4 and IPv6 the
same, always matching exactly. The existing RegistrationsPerIP rate
limit policy will be applied against this exact matching count.
A new `CountRegistrationsByIPRange` function is added to the SA that
performs the historic matching process, e.g. for IPv4 it counts exactly
the same as `CountRegistrationsByIP`, but for IPv6 it counts within
a /48.
A new `RegistrationsPerIPRange` rate limit policy is added to allow
configuring the threshold/window for the fuzzy /48 matching registration
limit. Stats for the "Exceeded" and "Pass" events for this rate limit are
separated into a separate `RegistrationsByIPRange` stats scope under
the `RateLimit` scope to allow us to track it separate from the exact
registrations per IP rate limit.
Resolves https://github.com/letsencrypt/boulder/issues/2738
- Add OCSP graphs
- Graph overall request rate
- Separate out WFE vs OCSP graphs
- Fix challenge graph (add a / to endpoint)
- Some incidental changes to "step"
- Add a lint script to check for common dashboard mistakes
Adds a basic truncated modulus hash check for RSA keys that can be used to check keys against the Debian `{openssl,openssh,openvpn}-blacklist` lists of weak keys generated during the [Debian weak key incident](https://wiki.debian.org/SSLkeys).
Testing is gated on adding a new configuration key to the WFE, RA, and CA configs which contains the path to a directory which should contain the weak key lists.
Fixes#157.
The `submissions_b` count in the integration test `test_ct_submission` function was being populated initially by using `url_a` when it _should_ be initialized using `url_b` since it's the count of submissions to log b.
This resolves https://github.com/letsencrypt/boulder/issues/2723
I tested this fix with a branch that ran this test 12 times per build. Prior to this fix multiple builds out of 20 (~4-5) would fail. With this fix, all 20 passed.
Per review policy, running tests in updated dependencies yields:
```
$ go test ./vendor/github.com/cloudflare/cfssl/ocsp/
? github.com/letsencrypt/boulder/vendor/github.com/cloudflare/cfssl/ocsp [no test files]
```
The VA previously had a lot of repetitive code of the form:
```
hs := httpSrv(t, expectedToken)
defer hs.Close()
port, err := getPort(hs)
test.AssertNotError(t, err, "failed to get test server port")
va, _, log := setup()
va.httpPort = port
```
This PR moves the port management to the `setup()` func and updates the tests accordingly.
Resolves https://github.com/letsencrypt/boulder/issues/2740
Switches imports from `github.com/google/certificate-transparency` to `github.com/google/certificate-transparency-go` and vendors the new code. Also fixes a number of small breakages caused by API changes since the last time we vendored the code. Also updates `github.com/cloudflare/cfssl` since you can't vendor both `github.com/google/certificate-transparency` and `github.com/google/certificate-transparency-go`.
Side note: while doing this `godep` tried to pull in a number of imports under the `golang.org/x/text` repo that I couldn't find actually being used anywhere so I just dropped the changes to `Godeps/Godeps.json` and didn't add the vendored dir to the tree, let's see if this breaks any tests...
All tests pass
```
$ go test ./...
ok github.com/google/certificate-transparency-go 0.640s
ok github.com/google/certificate-transparency-go/asn1 0.005s
ok github.com/google/certificate-transparency-go/client 22.054s
? github.com/google/certificate-transparency-go/client/ctclient [no test files]
ok github.com/google/certificate-transparency-go/fixchain 0.133s
? github.com/google/certificate-transparency-go/fixchain/main [no test files]
ok github.com/google/certificate-transparency-go/fixchain/ratelimiter 27.752s
ok github.com/google/certificate-transparency-go/gossip 0.322s
? github.com/google/certificate-transparency-go/gossip/main [no test files]
ok github.com/google/certificate-transparency-go/jsonclient 25.701s
ok github.com/google/certificate-transparency-go/merkletree 0.006s
? github.com/google/certificate-transparency-go/preload [no test files]
? github.com/google/certificate-transparency-go/preload/dumpscts/main [no test files]
? github.com/google/certificate-transparency-go/preload/main [no test files]
ok github.com/google/certificate-transparency-go/scanner 0.013s
? github.com/google/certificate-transparency-go/scanner/main [no test files]
ok github.com/google/certificate-transparency-go/tls 0.033s
ok github.com/google/certificate-transparency-go/x509 1.071s
? github.com/google/certificate-transparency-go/x509/pkix [no test files]
? github.com/google/certificate-transparency-go/x509util [no test files]
```
```
$ ./test.sh
...
ok github.com/cloudflare/cfssl/api 1.089s coverage: 81.1% of statements
ok github.com/cloudflare/cfssl/api/bundle 1.548s coverage: 87.2% of statements
ok github.com/cloudflare/cfssl/api/certadd 13.681s coverage: 86.8% of statements
ok github.com/cloudflare/cfssl/api/client 1.314s coverage: 55.2% of statements
ok github.com/cloudflare/cfssl/api/crl 1.124s coverage: 75.0% of statements
ok github.com/cloudflare/cfssl/api/gencrl 1.067s coverage: 72.5% of statements
ok github.com/cloudflare/cfssl/api/generator 2.809s coverage: 33.3% of statements
ok github.com/cloudflare/cfssl/api/info 1.112s coverage: 84.1% of statements
ok github.com/cloudflare/cfssl/api/initca 1.059s coverage: 90.5% of statements
ok github.com/cloudflare/cfssl/api/ocsp 1.178s coverage: 93.8% of statements
ok github.com/cloudflare/cfssl/api/revoke 2.282s coverage: 75.0% of statements
ok github.com/cloudflare/cfssl/api/scan 2.729s coverage: 62.1% of statements
ok github.com/cloudflare/cfssl/api/sign 2.483s coverage: 83.3% of statements
ok github.com/cloudflare/cfssl/api/signhandler 1.137s coverage: 26.3% of statements
ok github.com/cloudflare/cfssl/auth 1.030s coverage: 68.2% of statements
ok github.com/cloudflare/cfssl/bundler 15.014s coverage: 85.1% of statements
ok github.com/cloudflare/cfssl/certdb/dbconf 1.042s coverage: 78.9% of statements
ok github.com/cloudflare/cfssl/certdb/ocspstapling 1.919s coverage: 69.2% of statements
ok github.com/cloudflare/cfssl/certdb/sql 1.265s coverage: 65.7% of statements
ok github.com/cloudflare/cfssl/cli 1.050s coverage: 61.9% of statements
ok github.com/cloudflare/cfssl/cli/bundle 1.023s coverage: 0.0% of statements
ok github.com/cloudflare/cfssl/cli/crl 1.669s coverage: 57.8% of statements
ok github.com/cloudflare/cfssl/cli/gencert 9.278s coverage: 83.6% of statements
ok github.com/cloudflare/cfssl/cli/gencrl 1.310s coverage: 73.3% of statements
ok github.com/cloudflare/cfssl/cli/genkey 3.028s coverage: 70.0% of statements
ok github.com/cloudflare/cfssl/cli/ocsprefresh 1.106s coverage: 64.3% of statements
ok github.com/cloudflare/cfssl/cli/revoke 1.081s coverage: 88.2% of statements
ok github.com/cloudflare/cfssl/cli/scan 1.217s coverage: 36.0% of statements
ok github.com/cloudflare/cfssl/cli/selfsign 2.201s coverage: 73.2% of statements
ok github.com/cloudflare/cfssl/cli/serve 1.133s coverage: 39.0% of statements
ok github.com/cloudflare/cfssl/cli/sign 1.210s coverage: 54.8% of statements
ok github.com/cloudflare/cfssl/cli/version 2.475s coverage: 100.0% of statements
ok github.com/cloudflare/cfssl/cmd/cfssl 1.082s coverage: 0.0% of statements
ok github.com/cloudflare/cfssl/cmd/cfssljson 1.016s coverage: 4.0% of statements
ok github.com/cloudflare/cfssl/cmd/mkbundle 1.024s coverage: 0.0% of statements
ok github.com/cloudflare/cfssl/config 2.754s coverage: 67.7% of statements
ok github.com/cloudflare/cfssl/crl 1.063s coverage: 68.3% of statements
ok github.com/cloudflare/cfssl/csr 27.016s coverage: 89.6% of statements
ok github.com/cloudflare/cfssl/errors 1.081s coverage: 81.2% of statements
ok github.com/cloudflare/cfssl/helpers 1.217s coverage: 80.4% of statements
ok github.com/cloudflare/cfssl/helpers/testsuite 7.658s coverage: 65.8% of statements
ok github.com/cloudflare/cfssl/initca 205.809s coverage: 74.2% of statements
ok github.com/cloudflare/cfssl/log 1.016s coverage: 59.3% of statements
ok github.com/cloudflare/cfssl/multiroot/config 1.107s coverage: 77.4% of statements
ok github.com/cloudflare/cfssl/ocsp 1.524s coverage: 77.7% of statements
ok github.com/cloudflare/cfssl/revoke 1.775s coverage: 79.6% of statements
ok github.com/cloudflare/cfssl/scan 1.022s coverage: 1.1% of statements
ok github.com/cloudflare/cfssl/selfsign 1.119s coverage: 70.0% of statements
ok github.com/cloudflare/cfssl/signer 1.019s coverage: 20.0% of statements
ok github.com/cloudflare/cfssl/signer/local 3.146s coverage: 81.2% of statements
ok github.com/cloudflare/cfssl/signer/remote 2.328s coverage: 71.8% of statements
ok github.com/cloudflare/cfssl/signer/universal 2.280s coverage: 67.7% of statements
ok github.com/cloudflare/cfssl/transport 1.028s
ok github.com/cloudflare/cfssl/transport/ca/localca 1.056s coverage: 94.9% of statements
ok github.com/cloudflare/cfssl/transport/core 1.538s coverage: 90.9% of statements
ok github.com/cloudflare/cfssl/transport/kp 1.054s coverage: 37.1% of statements
ok github.com/cloudflare/cfssl/ubiquity 1.042s coverage: 88.3% of statements
ok github.com/cloudflare/cfssl/whitelist 2.304s coverage: 100.0% of statements
```
Fixes#2746.
When the VA validates a domain, it runs both CAA check and validation check in
parallel. If the validation check returns a failed result before the CAA check is done,
the VA returns immediately. However, the CAA check increments a stat. So there's
a flaky condition where, depending on the timing, that stat increment may or may
not get called on the MockScope. Add `.AnyTimes()` to our mock statements to
indicate we don't care whether Inc gets called.
This removes the config and code to output to statsd.
- Change `cmd.StatsAndLogging` to output a `Scope`, not a `Statter`.
- Remove the prefixing of component name (e.g. "VA") in front of stats; this was stripped by `autoProm` but now no longer needs to be.
- Delete vendored statsd client.
- Delete `MockStatter` (generated by gomock) and `mocks.Statter` (hand generated) in favor of mocking `metrics.Scope`, which is the interface we now use everywhere.
- Remove a few unused methods on `metrics.Scope`, and update its generated mock.
- Refactor `autoProm` and add `autoRegisterer`, which can be included in a `metrics.Scope`, avoiding global state. `autoProm` now registers everything with the `prometheus.Registerer` it is given.
- Change va_test.go's `setup()` to not return a stats object; instead the individual tests that care about stats override `va.stats` directly.
Fixes#2639, #2733.
This commit adds the acme draft-02+ optional "meta" element for the
/directory response. Presently we only include the optional
"terms-of-service" URL. Whether the meta entry is included is controlled
by two factors:
1. The state of the "DirectoryMeta" feature flag, which defaults to
off
2. Whether the client advertises the UA we know to be intolerant of
new directory entries.
The TestDirectory unit test is updated to test both states of the flag
and the UA detection.
With the CountCertificatesExact feature flag enabled if the RA's
checkCertificatesPerNameLimit was called with names only containing
domains exactly matching a public suffix entry then the legacy
ra.enforceNameCounts function will be called with an empty tldNames
argument. This in turn will cause the RA->SA RPC to fail with an
"incomplete gRPC request message error".
This commit fixes this bug by only calling ra.enforceNameCounts when
len(tldNames) > 0.
Resolves#2758
Having "coverage" in the default RUN is leaving ".coverprofile" files
all over the src tree. This commit removes this task from the default
RUN list for local work. It is included in travis.yml and will still run
for CI or when specified explicitly with a command line env var
override.
Uses a special mux for the OCSP Responder so that we stop collapsing double slashes in GET requests which cause a small number of requests to be considered malformed.
This commit adds the acme draft-02+ optional "meta" element for the
/directory response. Presently we only include the optional
"terms-of-service" URL. Whether the meta entry is included is controlled
by two factors:
1. The state of the "DirectoryMeta" feature flag, which defaults to
off
2. Whether the client advertises the UA we know to be intolerant of
new directory entries.
The TestDirectory unit test is updated to test both states of the flag
and the UA detection.
This PR introduces a new feature flag "IPv6First".
When the "IPv6First" feature is enabled the VA's HTTP dialer and TLS SNI
(01 and 02) certificate fetch requests will attempt to automatically
retry when the initial connection was to IPv6 and there is an IPv4
address available to retry with.
This resolves https://github.com/letsencrypt/boulder/issues/2623
This commit updates the `publicsuffix-go` dependency to 908fd3b. Per
CONTRIBUTING.md the upstream unit tests were verified to pass:
```
daniel@XXXX:~/go/src/github.com/weppos/publicsuffix-go$ git log --oneline | head -n1
908fd3b autopull: 2017-04-25T06:00:35Z (#75)
daniel@XXXX:~/go/src/github.com/weppos/publicsuffix-go$ go test ./...
? github.com/weppos/publicsuffix-go/cmd/load [no test files]
ok github.com/weppos/publicsuffix-go/net/publicsuffix 0.014s
ok github.com/weppos/publicsuffix-go/publicsuffix 0.027s
```
Both `ra.domainsForRateLimiting` and `ra.suffixesForRateLimiting` were
doing their own "unique"ing with a `map[string]struct{}` when they could
have used `core.UniqueLowerNames`. This commit updates them both to do
so and adjusts the unit tests to expect the new sorted order return.
This PR removes two berrors that aren't used anywhere in the codebase:
TooManyRequests , a holdover from AMQP, and is no longer used.
UnsupportedIdentifier, used just for rejecting IDNs, which we no longer do.
In addition, the SignatureValidation error was only used by the WFE so it is moved there and unexported.
Note for reviewers: To remove berrors.UnsupportedIdentifierError I replaced the errIDNNotSupported error in policy/pa.go with a berrors.MalformedError with the same name. This allows removing UnsupportedIdentifierError ahead of #2712 which removes the IDNASupport feature flag. This seemed OK to me, but I can restore UnsupportedIdentifierError and clean it up after 2712 if that's preferred.
Resolves#2709
The unit test runs in CI have been taking ~20 minutes. The root cause is
using `-race` on every individual `go test` invocation. We can't switch
to one big `go test` with `-race` instead of individuals if we want test
coverage to be reported. The workaround is to do one big `go test` with
`-race` first, and then many individual `go test`'s to collect coverage
*without* `-race`. This is still faster overall than the current state
of affairs.
Resolves https://github.com/letsencrypt/boulder/issues/2695
Prior to this PR if a domain was an exact match to a public suffix
list entry the certificates per name rate limit was applied based on the
count of certificates issued for that exact name and all of its
subdomains.
This PR introduces an exception such that exact public suffix
matches correctly have the certificate per name rate limit applied based
on only exact name matches.
In order to accomplish this a new RPC is added to the SA
`CountCertificatesByExactNames`. This operates similar to the existing
`CountCertificatesByNames` but does *not* include subdomains in the
count, only exact matches to the names provided. The usage of this new
RPC is feature flag gated behind the "CountCertificatesExact" feature flag.
The RA unit tests are updated to test the new code paths both with and
without the feature flag enabled.
Resolves#2681
I'm interested in seeing both how often DNS responses we see are signed (mainly for CAA, but
also interested in other query types). This change adds a new counter, `Authenticated`, that can
be compared against the `Successes` counter to find the percentage of signed responses we see.
The counter is incremented if the `msg.AuthenticatedData` bit is set by the upstream resolver.
Remove `SetEdns0` call in `bdns.exchangeOne`. Since we talk over TCP to the production
resolver and we don't do any local validation of DNSSEC records adding the EDNS0 OPT
record is pointless and confusing. Testing against a local `unbound` instance shows you
don't need to set the DO bit for DNSSEC requests/validation to be done at the resolver
level.
In #2583 the internal error usage was reworked. Previously the rejected
identifier and invalid email problems were constructed directly with
a meaningful detail message and then piped straight through the
`core.ProblemDetailsForError` function unaltered allowing the detail to
make it all the way through to the error returned by the WFE to the
client.
Since the refactor Boulder has not been appending the detail message for
these two problem types in `problemDetailsForBoulderError`, making the
errors harder to diagnose client-side.
This commit restores the previous behaviour by updating
`problemDetailsForBoulderError`. The `TestProblemDetailsFromError` unit
test is also updated to check that the correct amount of detail is being
embedded in the problem detail based on the error type.
Removes the reliance on `$GOPATH` being set in order to use `docker-compose`. Also removes
a few unnecessary commands from the `Dockerfile` that were no longer doing anything. If you
get weird errors along the lines of `oci runtime error: cannot chdir to ...` you will need to
`docker-compose rm; docker-compose build; docker-compose up` to fix them.
Fixes#2660.
Jacob Hoffman-Andrews
Roland Bracewell Shoemaker
Daniel McCarney
Daniel
Roland Bracewell Shoemaker
Brian Smith