This commit allows the mailer to treat a SMTP 421 err as an event that
should produce a reconnect attempt. Issue #2249 describes a case where
we see this SMTP error code from the remote server when our connection
has been idle for too long. We now reconnect when this happens rather
than failing ungracefully.
The logic in the `mail-test-srv` used to force a number of initial
connections to be disconnected is changed such that half of these forced
disconnects are the normal clean connection close and half are a SMTP
421. This allows the existing integration test for server disconnects to
be reused to test the 421 reconnect logic.
Per #2250 when we connect to the remote SMTP server at start-up time by
calling `mailer.Connect()` but do not actually call `mailer.SendMail()`
until after we have done some potentially expensive/time-consuming work
we are liable to have our connection closed due to timeout.
This commit moves the `Connect()` call in `expiration-mailer` and
`notify-mailer` to be closer to where the actual messages are sent via
`SendMail()`.
This commit updates the `expiration-mailer` to import `boulder/mail` as
`bmail` to differentiate from upstream `mail` the same way that the
`notify-mailer` does.
In #2178 we moved to explicit `SELECT` statements using a set of `const`
fields for each type to support db migrations and forward compatibility.
This commit removes the temptation to interpolate queries by providing
convenience `SelectFoo` functions for each type allowing the caller to
provide the `WHERE` clause and arguments.
Resolves#2214.
To remove challenges with expired/pending authz's when they are deleted we want to introduce a foreign key relationship to the challenges table's authorizationID field with instruction to cascade on delete (#2155). As pointed out in a comment this is made difficult by the current usage of a separate pendingAuthorizations table for pending authorizations.
To be able to remove the pendingAuthorizations table entirely (#2163) we need to first stop using it. This PR introduces the code changes required to achieve this.
Notes:
The SA's NewPendingAuthorization function was updated to store all new pending auths in the authz table and to ensure the status is StatusPending.
The SA's GetAuthorization, UpdatePendingAuthorization, FinalizeAuthorization, and RevokeAuthorizationsByDomain functions were updated to properly handle the fact that a pending authz could be in either the pendingAuthorizations table, or the authz table, and to do the right thing accordingly.
Several places in the RA unit tests created a pending authorization with a status "Valid", then finalized it later. This broke when NewPendingAuthorization was changed to enforce Pending status before creating the authz row since the FinalizeAuthorization code expected to only finalize Valid rows. To fix this some of the RA unit tests were changed to explicitly set status to Valid before calling FinalizeAuthorization. This matches the true intention of the tests to quickly create a pending & then finalized authorization.
The expired-authz-purger utility was updated to purge from both the pendingAuthorizations and authz table as required.
The return values of RevokeAuthorizationsByDomain have changed slightly. Previously it returned a 2 element array where the first element was the number of pending authorizations revoked and the second element was the number of finalized authorizations revoked. This is changed so that now it is the number of rows from the pendingAuthorizations and authz tables respectively. E.g. the second count for the authz table may now include non-finalized authzs in its count of affected rows. The admin-revoker is the only place that used this SA method and it was updated appropriately to describe the "rows" change.
The "purger" database user needs to have a new GRANT SELECT, DELETE for the authz table in addition to its existing GRANT for the pendingAuthorizations table.
This resolves#2162
Based on experience with the new gRPC staging deployment. gRPC generates `FullMethod` names such as `-ServiceName-MethodName` which can be confusing. For client calls to a service we actually want something formatted like `ServiceName-MethodName` and for server requests we want just `MethodName`.
This PR adds a method to clean up the `FullMethod` names returned by gRPC and formats them the way we expect.
The "20160817143417_AddCertStatusNotAfter.sql" db migration adds a "notAfter" column to the certificateStatus database table. This field duplicates the contents of the certificates table "expires" column. This enables performance improvements (see #1864) for both the ocsp-updater and the expiration-mailer utilities.
Since existing rows will have a NULL value in the new field the notafter-backfill utility exists to perform a one-time update of the existing certificateStatus rows to set their notAfter column based on the data that exists in the certificates table.
This follows on https://github.com/letsencrypt/boulder/pull/2177 and requires that the migration be applied & the feature flag set accordingly before use.
Fixes#2237.
Add feature flagged support for issuing for IDNs, fixes#597.
This patch expects that clients have performed valid IDN2008 encoding on any label that includes unicode characters. Invalid encodings (including non-compatible IDN2003 encoding) will be rejected. No script-mixing or script exclusion checks are performed as we assume that if a name is resolvable that it conforms to the registrar's policies on these matters and if it uses non-standard scripts in sub-domains etc that browsers should be the ones choosing how to display those names.
Required a full update of the golang.org/x/net tree to pull in golang.org/x/net/idna, all test suites pass.
Move features sections to the correct JSON object and only test registration validity if regCheck is true
* Pull other flag up to correct level
* Only check status update when status is non-empty
This commit fixes#2190 by allowing the OS to choose what port the test
Listener's bind to. This allows the tests to be run concurrently
without panicing when they all try to bind the same TCP port.
Previously, if we received a CSR with IPAddress or EmailAddress SANs, we would
ignore those fields, issuing only for the DNSNames in the CSR. However, we would
later check in MatchesCSR that the CSR's IPAddresses and EmailAddresses matches
those in the issued certificate. This check would fail, serving a 500 to the end
user.
Instead, we now reject the CSR earlier in the process, and send a
meaningful error message.
Fixes#2203
This PR adds a migration to create two new fields on the `certificateStatus` table: `notAfter` and `isExpired`. The rationale for these fields is explained in #1864. Usage of these fields is gated behind `features.CertStatusOptimizationsMigrated` per [CONTRIBUTING.md](https://github.com/letsencrypt/boulder/blob/master/CONTRIBUTING.md#gating-migrations). This flag should be set to true **only** when the `20160817143417_CertStatusOptimizations.sql` migration has been applied.
Points of difference from #2132 (the initial preparatory "all-in-one go" PR):
**Note 1**: Updating the `isExpired` field in the OCSP updater can not be done yet, the `notAfter` field needs to be fully populated first - otherwise a separate query or a messy `JOIN` would have to be used to determine if a certStatus `isExpired` by using the `certificates` table's `expires` field.
**Note 2**: Similarly we can't remove the `JOIN` on `certificates` from the `findStaleOCSPResponse` query yet until all DB rows have `notAfter` populated. This will happen in a separate **Part Two** PR.
This PR adds a divergence to the acme-divergence doc for Section 6.6 "Certificate Revocation". Boulder does not currently support authorizing a revocation request using an arbitrary account key that is authorized for the same domains as in the certificate.
Fixes#2160.
When we use Gorp's built-in `Get` method, it generates `SELECT *` queries. If we do a migration without a simultaneous change of the data structure, Gorp will subsequently error out when it sees a column in the output of the `SELECT *` which doesn't have a corresponding field in the struct it is trying to marshal. In order to be forward compatible with schema changes, we need to always use `SELECT a, b, c`, where `a`, `b`, and `c` are columns / fields in the current struct.
Unlike the `$GOTESTFLAGS` var there was no way to pass in a default
value to `test.sh` via `docker-compose -e` to indicate you only want to
run specific unit tests.
This commit puts the default `$TESTPATHS` into `$DEFAULT_TESTPATHS` and
assigns it to `$TESTPATHS` only if there isn't already a `$TESTPATHS`
value provided.
Example usage, running just the SA unit tests, with the "next" config,
using `-race -v`:
```
docker-compose run -e BOULDER_CONFIG_DIR="test/config-next"
-e RUN="unit" -e GOTESTFLAGS="-v -race"
-e TESTPATHS="github.com/letsencrypt/boulder/sa" boulder ./test.sh
```
This PR makes two improvements to how we handle migrations locally:
1) Prior to this PR an optimization was present in `test/create_db.sh` that would `exit 0` if the `boulder_sa_integration` database existed. This early exit meant that after the first invocation of `create_db.sh` no further `goose` migrations would be applied unless the operator dropped their databases or edited the script.
This PR reworks the existing DB optimization so that it only skips the `CREATE DATABASE` statements and allows `goose` to try and apply migrations. This doesn't result in significantly longer start up times because Goose is smart enough to know when no migrations are required and outputs something similar to:
`goose: no migrations to run. current version: 20160602142227`
This should address #2174.
2) This PR also implements a separate `sa/_db-next/` directory for "pending" migrations. This is meant to follow the "test/config" vs "test/config-next" approach to managing changes that are developed but not yet activated in production.
Migrations that are to-be-performed by Ops should be created in the `sa/_db-next` directory first. Once they have been performed by ops in staging/prod and the config flag gate for the migration (see CONTRIBUTING.md) has been set to true, the migration can be moved from `_db-next` to `_db`.
By default all pending migrations from the `-next` directory are applied in the local dev env. If you **do not** wish these migrations to be applied then set the `APPLY_NEXT_MIGRATIONS` env var to false. E.g.:
`docker-compose run -eAPPLY_NEXT_MIGRATIONS=false boulder`
This should address #2195
The LookupIPv6 flag has been enabled in production and isn't required anymore. This PR removes the flag entirely.
The errA and errAAAA error handling in LookupHost is left as-is, meaning that a non-nil errAAAA will not be returned to the caller. This matches the existing behaviour, and the expectations of the TestDNSLookupHost unit tests.
This commit also removes the tests from TestDNSLookupHost that tested the LookupIPv6 == false behaviours since those are no longer implemented.
Resolves#2191
The embedded `personModelv1` in the `personModelv2` struct should not be
a pointer. The instantiation code later on already uses
`personModelv1{}` instead of `&personModelv1{}` and does not need
updating, just the initial struct definition.
Updates #1699.
Adds a new package, `features`, which exposes methods to set and check if various internal features are enabled. The implementation uses global state to store the features so that services embedded in another service do not each require their own features map in order to check if something is enabled.
Requires a `boulder-tools` image update to include `golang.org/x/tools/cmd/stringer`.
* Fixes `mockEmailResolver` to return `sql.ErrNoRows`.
This commit reproduces the error observed in #2183 where a registration
ID is provided that doesn't match a row with a valid contact.
First a bug is fixed in the ID range check done by the
`mockEmailResolver` - it was using a `||` where it should have been
using a `&&` and also had a `> 0` where it needed `>= 0`, oops! slipped
past review!
Second the `mockEmailResolver` is modified to return `sql.ErrNoRows`
when the index is out of bounds for the mock data.
Lastly a ID of `999` is added to the `TestResolveEmails` function to
elicit the "mailer.send returned error: sql: no rows in result set"
error.
* Handles `sql.ErrNoRows` in `emailsForReg`.
This commit fixes#2183 (and the failing unit test introduced in the
prior commit) by handling `sql.ErrNoRows` in `emailsForReg` gracefully.
* Clarfies mockEmailResolver comment
Use labels ending in _key for private key labels.
Create two separate slots in make-softhsm rather than overwriting a single slot.
Update make-softhsm instructions to point out both files to edit.
Improve error output from integation test.
Output base64-encoded DER, as expected by ocsp-responder.
Use flags instead of template for Status, ThisUpdate, NextUpdate.
Provide better help.
Remove old test (wasn't run automatically).
Add it to integration test, and use its output for integration test of issuer ocsp-responder.
Add another slot to boulder-tools HSM image, to store root key.
Fixes#1880.
Updates google.golang.org/grpc and github.com/jmhodges/clock, both test suites pass. A few of the gRPC interfaces changed so this also fixes those breakages.
Daniel
Daniel McCarney
Jacob Hoffman-Andrews
Roland Bracewell Shoemaker
Ben Irving
Blake Griffith