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.
This change changes the returning values from boolean to error.
It makes `checkConsistency` an internal function and removes the
optional argument in favor of making checks explicit where they are
used.
It also renames those functions to CheckConsistency* to not
give the impression of still returning boolean values.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Instead of executing the prefix for every statement only do it when creating the connection.
Leaves most of the existing naming conventions alone but updates the relevant comments
to reflect setting variables is now connection level instead of statement.
Fixes#2673.
First, commit c0ad8d9040 (PR #2658)
had a minor bug: It didn't update the "pem=" in the audit log
message to "cert=" to be consistent with the rest of the code.
But, more importantly, we don't need to include the cert DER in the
audit log at this point because we've already logged the DER and its
serial number prior to this. Thus, at this point logging the serial
number is good enough.
In 18f4c5c we introduced a workaround for the CT submission integration
test to allow exactly expected, or twice as many CT log submissions as
expected to account for the case where the ocsp-updater and the CA race.
This didn't completely patch over the issue because the number of
submissions can fall between `n` and `2n`.
This commit updates the hack to be even hackier (twice as hacky or your
money back). Now we consider any value *between* `n` and `2n` as a test
pass.
Previously RegistrationAuthorityImpl.NewCertificate would call
MatchesCSR() and then verify that the certificate can be successfully
parsed. However, MatchesCSR() itself parses the certificate, so the
latter check was pointless.
Instead, parse the certificate once, fail if it can't be parsed,
then pass the parsed certificate to MatchesCSR().
0e112ae updates ra/ra.go such that when onValidationUpdate returns a non-nil error the AuditErr
message includes the affected authorization ID in addition to the registration ID.
Resolves#2661.
These are monitoring tools, originally from
https://github.com/jsha/go/tree/master/ocsp. We'd like to formalize their role
in monitoring Boulder, so I'm adding them to the Boulder repo and getting them
reviewed.
Deletes github.com/streadway/amqp and the various RabbitMQ setup tools etc. Changes how listenbuddy is used to proxy all of the gRPC client -> server connections so we test reconnection logic.
+49 -8,221 😁Fixes#2640 and #2562.
Instead of running it at the current time to clean out left over cruft run it with a FAKECLOCK of +1 year so that we catch everything that could get in the way.
Generate first OCSP response in ca.IssueCertificate instead of ocsp-updater.newCertificateTick
if features.GenerateOCSPEarly is enabled. Adds a new field to the sa.AddCertiifcate RPC for
the OCSP response and only adds it to the certificate status + sets ocspLastUpdated if it is a
non-empty slice. ocsp-updater.newCertificateTick stays the same so we can catch certificates
that were successfully signed + stored but a OCSP response couldn't be generated (for whatever
reason).
Fixes#2477.
This was accidentally changed in
https://github.com/letsencrypt/boulder/pull/2634/, and broke Certbot's tests.
This also includes an update to chisel that fetches the certificate chain, which
would have caught this error.
Rename HTTPMonitor to MeasuredHandler.
Remove inflight stat (we didn't use it).
Add timing stat by method, endpoint, and status code.
The timing stat subsumes the "rate" stat, so remove that.
WFE now wraps in MeasuredHandler, instead of relying on its cmd/main.go.
Remove FBAdapter stats.
MeasuredHandler tracks stats by method, status code, and endpoint.
In VA, add a Prometheus histogram for validation timing.
This PR updates the `publicsuffix-go` dependency to `fb1fc94`, the
latest autopull and the HEAD of master at the time of writing.
Per CONTRIBUTING.md the tests were verified to pass:
```
? github.com/weppos/publicsuffix-go/cmd/load [no test files]
ok github.com/weppos/publicsuffix-go/net/publicsuffix 0.007s
ok github.com/weppos/publicsuffix-go/publicsuffix 0.027s
```
Updates the various gRPC/protobuf libs (google.golang.org/grpc/... and github.com/golang/protobuf/proto) and the boulder-tools image so that we can update to the newest github.com/grpc-ecosystem/go-grpc-prometheus. Also regenerates all of the protobuf definition files.
Tests run on updated packages all pass.
Unblocks #2633fixes#2636.
Adds a daemon mode to `expiration-mailer` that is triggered by using the flag `--daemon` in order to follow deployability guidelines. If the `--daemon` flag is used the `mailer.runPeriod` config field is checked for a tick duration, if the value is `0` it exits.
Super lightweight implementation, OCSP-Updater has some custom ticker code which we use to do fancy things when the method being invoked in the loop takes longer expected, but that isn't necessary here.
Fixes#2617.
This fixes an issue caused by #2583. Prior to that PR, we would serve the "invalidEmail" problem type when a DNS lookup for an email base domain failed. After that PR, we would map "berrors.InvalidEmail" to the "InternalServerError" problem type, which caused 500 errors to be returned to the user.
This PR restores the behavior of returning "type": "...invalidEmail" to the user.
Fixes#2148.
Instead of just doing a blanket `DELETE FROM ...` this changes the `expired-authz-purger` to select all of the expired IDs (for both pending and finalized authorizations) then loop over them deleting each and its associated challenges from their respective tables.
Local testing indicates the performance of this is not awful but we should do a test run on staging to verify. If it ends up taking way too long to run there the easiest optimization would be to turn the slice of IDs into a channel and run multiple workers looping over the channel deleting stuff instead of just a single one.
Makes a few small integration test changes in order to facilitate deleting both pending and finalized authorizations.
This patch removes all usages of the `core.XXXError` and almost all usages of `probs` outside of the WFE and VA and replaces them with a unified internal error type. Since the VA uses `probs.ProblemDetails` quite extensively in challenges, and currently stores them in the DB I've saved this change for another change (it'll also require a migration). Since `ProblemDetails` should only ever be exposed to end-users all of its related logic should be moved into the `WFE` but since it still needs to be exposed to the VA and SA I've left it in place for now.
The new internal `errors` package offers the same convenience functions as `probs` does as well as a new simpler type testing method. A few small changes have also been made to error messages, mainly adding the library and function name to internal server errors for easier debugging (i.e. where a number of functions return the exact same errors and there is no other way to distinguish which method threw the error).
Also adds proper encoding of internal errors transferred over gRPC (the current encoding scheme is kept for `core` and `probs` errors since it'll be ideally be removed after we deploy this and follow-up changes) using `grpc/metadata` instead of the gRPC status codes.
Fixes#2507. Updates #2254 and #2505.
I think these are all the necessary changes to implement TLS-SNI-02 validations, according to the section 7.3 of draft 05:
https://tools.ietf.org/html/draft-ietf-acme-acme-05#section-7.3
I don't have much experience with this code, I'll really appreciate your feedback.
Signed-off-by: David Calavera <david.calavera@gmail.com>
This commit resolves#2599 by adding support to the expiration-mailer to
treat the subject for email messages as a template. This allows for the
dynamic subject lines from #2435 to be used with a prefix for staging
emails.
Presently the CA and the ocsp-updater can race on the initial
submission of a certificate to the configured logs. This results
in double submitting certificates. In integration tests with the fake CT
server this manifests as an occasional failure of the
`test_ct_submission` test (Issue #2579).
The race we currently experience is expected to be fixed in
the future by a planned redesign so for now this commit works around the
failure by allowing either the expected number of submissions, or
exactly double the expected. This fixes#2579. The need to fix the
underlying race was captured in #2610.
The workaround was verified by submitting 10 builds to travis, all
succeeded.
The VA's `getAddr` function prior to this commit had an outdated comment
& a pointer to a TODO for Boulder Issue #593. That issue has been closed
and bdns' `LookupHost` supports AAAA records now. This commit updates
the comment to match the current behaviour and removes the TODO.
Switch from `gorp.v1` to `gorp.v2`. Removes `vendor/gopkg.in/gorp.v1` and vendors `vendor/gopkg/go-gorp/gorp.v2`, all tests pass.
Changes between `v1.7.1` and `v2.0.0`: c87af80f3c...4deece6103Fixes#2490.
Daniel McCarney