Moves the wfe to it's own config file.
Each config will now belong in `test/config` and `test/config-next` analogous to `boulder-config` and `boulder-config-next`.
Whenever I run into a test failure using `AssertEquals` I end up changing the error format string to include the full type information. I think given the context of when the message will be printed it makes sense to include type information all the time.
This commit changes the format string from `%v` to `%#v`. This avoids otherwise unhelpful test failures like:
`ra_test.go:789: [0] != [0]`
where the underlying types differ while their String() representation matches.
This PR adds a expvar.Int published under the key "lastIssuance" that contains the timestamp of the last successful certificate issuance. This allows easy creation of a script that monitors the RA debug server (port 8002) to ensure that there has been a successful issuance within a set period (e.g. last five minutes).
The underlying expvar.Int code uses the atomic package to ensure safe updates/reads across multiple goroutines.
This resolves#1945 and was selected in place of the more complex circular bucket design. While the timestamp approach doesn't provide the issuance volume as readily it is less complex and meets the immediate need of a reliable external monitoring process hook.
https://github.com/letsencrypt/boulder/pull/1982
It looks like the filesystem on Mac OSX El Captain
(and potentially previous versions) require at least
one second before you get a different timestamp.
Any value lower than one second caused:
- a deadlock error in a test in the reloader module
- a rate limit error in the ra module
See GH-1955.
While testing with real proxies I noticed the original CDR implementation was actually pretty broken, this refactors a bit and fixes a number of bugs. With this patch fallback to GPDNS over three distributed test proxies worked perfectly.
(Side note: `nginx` is not a viable forward proxy for this use as it doesn't support SSL, and a bunch of other _real_ forward proxy features, I ended up just using `squid3`.)
The main error in the previous implementation was the fallback was implemented in `getCAASet` which is only called in the old code path (the local CAA impl instead of the remote service) which mean't it wasn't actually being tested in the integration test. This also refactors a few repeated blocks into their own functions. Also there was a unicode encoding problem somewhere with the query string but for the life of me I can't figure out why it was broken now.
Moving to Docker meant that we weren't passing through the BOULDER_CONFIG
variable properly, which meant we weren't testing the boulder-config-next.json
configuration. That allowed https://github.com/letsencrypt/boulder/issues/1948
to pass unnoticed.
Credit to @benileo for asking the question of why that issue wasn't caught in
testing, which led to this fix. Thanks for the attention to detail!
Moves the `subscriberAgreementURL` from a top-level config variable to the WFE section. The top level configuration is still respected until we can change the prod configs.
The `regID` parameter in the PA's `WillingToIssue` function was originally used for whitelisting purposes, but is not used any longer. This PR removes it.
This PR, adds a check in registration authority for non-ASCII encoded characters in an email address. This is due to a 'funky email implementation'.
Fixes#1350
In https://github.com/letsencrypt/boulder/pull/774 we introduced and account key stored with the challenge. This was a stopgap fix to the now-defunct SimpleHTTP and DNS challenges in the face of https://mailarchive.ietf.org/arch/msg/acme/F71iz6qq1o_QPVhJCV4dqWf-4Yc. However, we no longer offer or implement those challenges, so the extra field is unnecessary. It also take up a huge amount of space in the challenges table, which is our biggest table. SimpleHTTP and DNS challenges were removed in https://github.com/letsencrypt/boulder/pull/1247.
We can provide a follow-up migration to delete the column later, once we have a plan for large migrations without downtime.
Fixes#1909
Adds a server side unary RPC interceptor which includes basic stats. We could also use this to add a server request ID to the context.Context to identify the call through the system, but really I'd rather do that on the client side before the RPC is sent which requires the client interceptor implementation upstream. Also updates google.golang.org/grpc.
Updates #1880.
For #1928 a notify-mailer command was added to facilitate sending notification emails to a list of email addresses.
The other half of this use-case is generating the list of emails given to notify-mailer as input. This pull request adds a contact-exporter command that allows the operator to dump all of the contact email addresses from boulder into a text file or stdout.
Only contacts with at least one non-expired certificate will be exported. Similarly, only contacts that are a mailto: email address (e.g. not contacts with just a phone number) will be exported.
A new read-only database user (contact_exporter) is created with select permissions on the two required tables (certificates and registrations) for use with the exporter tool.
Future work:
Support exporting registration IDs instead of email addresses
https://github.com/letsencrypt/boulder/pull/1939
Under the new defaults, if the syslog section is missing, we'll use the default
config that we use in prod: no logs to stdout, INFO and below to syslog.
This allows us to remove the syslog section from prod configs, and potentially
move it to individual service configs in the future.
* Improve syslog defaults.
* Add stdout logging for purger test.
* Use plain int for sysloglevel.
* Fix JSON syntax
* Fix syslog config for expired-authz-purger.
https://github.com/letsencrypt/boulder/pull/1932
This commit adds a new notify-mailer command. Outside of the new command, this PR also:
Adds a new SMTPConfig to cmd/config.go that is shared between the expiration mailer and the notify mailer.
Modifies mail/mailer.go to add an smtpClient interface.
Adds a dryRunClient to mail/mailer.go that implements the smtpClient interface.
Modifies the mail/mailer.go MailerImpl and constructor to use the SMTPConfig and a dialer. The missing functions from the smtpClient interface are added.
The notify-mailer command supports checkpointing through --start and --end parameters. It supports dry runs by using the new dryRunClient from the mail package when given the --dryRun flag. The speed at which emails are sent can be tweaked using the --sleep flag.
Unit tests for notify-mailer's checkpointing behaviour, the checkpoint interval/sleep parameter sanity, the sleep behaviour, and the message content construction are included in main_test.go.
Future work:
A separate command to generate the list of destination emails provided to notify-mailer
Support for using registration IDs as input and resolving the email address at runtime.
Resolves#1928. Credit to @jsha for the initial work - I'm just completing the branch he started.
* Adds `notify-mailer` command.
* Adds a new SMTPConfig to `cmd/config.go` that is shared between the
expiration mailer and the notify mailer.
* Modifies `mail/mailer.go` to add an `smtpClient` interface.
* Adds a `dryRunClient` to `mail/mailer.go` that implements the
`smtpClient` interface.
* Modifies the `mail/mailer.go` `MailerImpl` and constructor to use the
SMTPConfig and a dialer. The missing functions from the `smtpClient`
interface are added.
* Fix errcheck warnings
* Review feedback
* Review feedback pt2
* Fixes#1446 - invalid message-id generation.
* Change -configFile to -config
* Test message ID with friendly email
https://github.com/letsencrypt/boulder/pull/1936
As reported in #1925 the Certificates per Domain rate limit was being
incorrectly enforced on certificate renewals for FQDN sets that have
been previously issued. This is counter to the described rate
limit policies[0] that detail a separate rate limit for certificates
issued for the "exact same set of Fully Qualified Domain Names".
The bug was caused by the result of `domainsForRateLimiting` overwriting
the original `names []string` provided to the RA's
`checkCertificatesPerNameLimit` function. This meant instead of looking
for an existing FQDN set for the full set of domain names being
requested we checked for an FQDN set for just the eTLD+1's of the
domains. (e.g "www.example.com, foo.example.com, bar.example.com" vs
"example.com").
This commit preserves the original `names` values for doing an FQDN set
lookup and uses the `tldNames` from `domainsForRateLimiting` elsewhere.
This fixes#1925.
A test is added to ensure that `checkCertificatesPerNameLimit` does the
correct thing both with and without an existing FQDN set.
[0] https://community.letsencrypt.org/t/rate-limits-for-lets-encrypt/6769
Clarifies the `UnknownHost` problem details error message created in the VA's `getAddr` when there is no valid IP address for the domain.
Previously this was reported as "No IPv4 addresses found for x" leading to user confusion (ref #1790) when a domain resolved to a private IP.
The RA UpdateRegistration function merges a base registration object with an update by calling Registration.MergeUpdate. Prior to this commit MergeUpdate only allowed the updated registration object to overwrite the Contact field of the existing registration if the updated reg. defined at least one AcmeURL. This prevented clients from being able to outright remove the contact associated with an existing registration.
This commit removes the len() check on the input.Contact in MergeUpdate to allow the r.Contact field to be overwritten by a []*core.AcmeURL(nil) Contact field. Subsequently clients can now send an empty contacts list in the update registration POST in order to remove their reg contact.
Fixes#1846
* Allow removing registration contact.
* Adds a test for `MergeUpdate` contact removal.
* Change `Registration.Contact` type to `*[]*core.AcmeURL`.
* End validateContacts early for empty contacts
* Test removing reg. contact more thoroughly.
Presently clients may request a new AuthZ be created for a domain that they have already proved authorization over. This results in unnecessary bloat in the authorizations table and duplicated effort.
This commit alters the `NewAuthorization` function of the RA such that before going through the work of creating a new AuthZ it checks whether there already exists a valid AuthZ for the domain/regID that expires in more than 24 hours from the current date. If there is, then we short circuit creation and return the existing AuthZ. When this case occurs the `RA.ReusedValidAuthz` counter is incremented to provide visibility.
Since clients requesting a new AuthZ and getting an AuthZ back expect to turn around and post updates to the corresponding challenges we also return early in `UpdateAuthorization` when asked to update an AuthZ that is already valid. When this case occurs the `RA.ReusedValidAuthzChallenge` counter is incremented.
All of the above behaviour is gated by a new RA config flag `reuseValidAuthz`. In the default case (false) the RA does **not** reuse any AuthZ's and instead maintains the historic behaviour; always creating a new AuthZ when requested, irregardless of whether there are already valid AuthZ's that could be reused. In the true case (enabled only in `boulder-config-next.json`) the AuthZ reuse described above is enabled.
Resolves#1854
This PR, makes testing the bdns package more reliable. A race condition in TestMain was resulting in the test running before the test dns server had started. This is fixed by actively polling for the DNS server to be ready before starting the test suite.
Furthermore, a 1 millisecond server read/write timeout was proving to time out on occasion. This is fixed increasing to a 1 second read/write timeout to increase test reliability.
FYI: ran package bdns tests 1000 times with 22 failures previously, after this PR ran 1000 times with 0 failures.
fixes#1317
In https://github.com/letsencrypt/boulder/pull/1885 I tried to simplify setting the GORACE environment variable, but that actually had the effect of inhibiting pass-through of all environment variables. This reverts that part of the change.
Resolves#1810 by automatically updating the RA ratelimit.RateLimitConfig whenever the backing config file is changed. Much like the Policy Authority uses a reloader instance to support updating the Hostname policy on the fly, this PR changes the Registration Authority to use a reloader for the rate limit policy file.
Access to the ra.rlPolicies member is protected with a RWMutex now that there is a potential for the values to be reloaded while a reader is active.
A test is introduced to ensure that writing a new policy YAML to the policy config file results in new values being set in the RA's rlPolicies instance.
https://github.com/letsencrypt/boulder/pull/1894
* WFE returns headers Boulder-Request-ID and Boulder-Requester
* improve test
* add the requestHeader after calls to verifyPOST
* move call of addRequesterHeader in NewRegistration
* move setting of request header to context, improve test
* remove pointless comment
Both `test-cert.pem` and `test-cert-b.pem` have expiry dates of June 5th, 2016. Since I'm writing this commit on June 6th, 2016 you might be able to guess that this caused a problem :-)
Both `TestGenerateOCSPResponse` and `TestFindStaleOCSPResponses` were failing with a differing actual vs expected value in testing. The SQL query for `findStaleOCSPResponses` specifically excludes certificates that have a `cert.expires` that is `< now()`. Since both test certs were newly expired, the unit tests were getting different results than expected.
This commit replaces both test certs with regenerated copies that expire in 2036. Note: since these are regenerated the public keys and a few other fields not-relevant to testing may have changed.
As documented in Issue #1872 the `getSerialsIssuedSince` `for { }` loop will never terminate under sufficient issuance load. The behaviour prior to this commit would only terminate the loop when `sql.ErrNoRows` or exactly 0 rows were returned by the query. If we sustain sufficient issuance requests that there are always a few certificates issued since the last query the loop will never terminate.
This commit changes the loop behaviour such that rather than terminating when exactly *0* rows are returned, we terminate when a number of rows less than the configured `batchSize` is returned.
* Add cmd/expired-authz-purger with integration test
* Return count
* gofmt >.>
* add to boulder-config-next.json
* Commit missing file
* Exec on the dbMap
* fprintf the error message
* Review fixes + test
* Review fixes pt. 1
* Review fixes pt. 2 (actually add test file this time :|)
* Fix prompt
* Switch to using flag lib
* Use COUNT(1)
* Revert config -> flag stuff
* Review fixes
* Revert-revert COUNT(1) change
* Review fixes pt. 1
* Nest config struct
* Test review fixes
* Factor out getting future output with FAKECLOCK
* Review fixes pt. 2
* Review fixes pt. 3
Fixes#799, ensuring that these files at least count towards our coverage numbers and show up on coveralls' "least covered" list. Improving their coverage is part of the overall long-term project of coverage improvement.
This PR implements `http.StripPrefix` as a wrapper to the existing handler in `wfe.go`. `StripPrefix` is used to remove the path that it expects for each handler. Such as /acme/reg/. The remaining path is used as a slug, since multiple slugs are outside the scope of the specification.
Several tests bypassed the `mux.Handle()` and called the wrapped handler directly. Ex `Registration`. As a result of this, many tests had to be modified to no longer pass in the full path. `request.URL.Path` should now only ever contain the slug (if there is one).
Fixes#437
As detailed in issue #1872 the getSerialsIssuedSince function of the ocsp-updater cmd performs with poor runtime, likely due to a filesort and lack of index on the issued field.
This commit adds a migration to create a new index on the issued field.
* Add index to certificates table issued field.
* Rename index to use _idx suffix
In this PR, logger is passed to the following callers:
NewWebFrontEndImpl
NewCertificateAuthorityImpl
NewValidationAuthorityImpl
NewAmqpRPCServer
newUpdater
NewRegistrationAuthorityServer
This reduces the usage of a global singleton logger and allows tests to consistently use a mock logger.
Fixes#1642
* remove blog.Get() in wfe
* remove blog.Get() from va
* remove Blog.Get() from ca
* remove blog.Get() from oscp updater, ampq rpc server, registration authority server
* removed some pointless logging code
* remove one added newline
* fix format issue
* fix setup function to return *blog.Mock instead of being passed in
* remove useless blog.NewMock() call
Jacob Hoffman-Andrews
Ben Irving
Daniel McCarney
Simone Carletti
Roland Bracewell Shoemaker
Kane York