Prior to this commit the `httpResp` result of `sendHTTPRequest` was
examined even in the case where `sendHTTPRequest` returns a non-nil
error. This can cause a nil panic since the `httpResp` may be `nil` when
the error is not. This commit returns an error from `Req()` immediately
when `sendHTTPRequest` returns one.
In `log/log.go` when we fail to write to syslog we log the message to STDERR but without a newline. This causes all of the STDERR logs (when we reconnect to syslog) to be delivered where they go to be on a single line which causes some debugging pain.
After looking at this and thinking about it a bit more it doesn't really make sense to remove either usage of shuffling for the challenges or combinations. If we shuffle one but not the other we don't really get the behavior we want for either the v1 or v2 API (if the v2 API actually ends up using this and not it's own implementation) especially since there are people still developing new clients against the v1 API that we'd prefer aren't broken in the case we have to introduce another challenge for security reasons.
Instead I've just gone with the easy fix of implementing a lock around the usage. Another option would be to just create a new source each time and seed it using `rand.Int63` but I doubt that would be much faster than the latency the lock contention will introduce.
Fixes#2890.
The test previously used an invalid encoding of the CT poison extension
(the value was empty, but a valid CT poison extension has a NULL value).
In preparation for testing specifically how the CT poison extension is
handled, change the test to use a different extension instead.
Split the profile issuance tests such that there is one call to IssueCertificate per test, like
the other certificate issuance tests. This will make it easier to later move the calls to
IssueCertificate() into TestIssueCertificate(), which will make it much easier to test the
precertificate-based flow in addition to the current issuance flow.
This PR renames wfe2/jose.go to wfe2/verify.go to better reflect
its purpose.
Additionally this PR moves signatureValidationError, extractJWSKey
and verifyPOST from wfe2/wfe.go to wfe2/verify.go. This is in
preparation of refactoring for the ACME v2 POST verification logic to
help keep diffs reviewable.
Take a step towards enabling the testing of precertificate issuance by
enabling the logic in these tests to be used for testing all forms
of Certificate and Precertificate issuance.
Switch certificates and certificateStatus to use autoincrement primary keys to avoid performance problems with clustered indexes (fixes#2754).
Remove empty externalCerts and identifierData tables (fixes#2881).
Make progress towards deleting unnecessary LockCol and subscriberApproved fields (#856, #873) by making them NULLable and not including them in INSERTs and UPDATEs.
The existing ReusePendingAuthz implementation had some bugs:
It would recycle deactivated authorizations, which then couldn't be fulfilled. (#2840)
Since it was implemented in the SA, it wouldn't get called until after the RA checks the Pending Authorizations rate limit. Which means it wouldn't fulfill its intended purpose of making accounts less likely to get stuck in a Pending Authorizations limited state. (#2831)
This factors out the reuse functionality, which used to be inside an "if" statement in the SA. Now the SA has an explicit GetPendingAuthorization RPC, which gets called from the RA before calling NewPendingAuthorization. This happens to obsolete #2807, by putting the recycling logic for both valid and pending authorizations in the RA.
This commit replaces the Boulder dependency on
gopkg.in/square/go-jose.v1 with gopkg.in/square/go-jose.v2. This is
necessary both to stay in front of bitrot and because the ACME v2 work
will require a feature from go-jose.v2 for JWS validation.
The largest part of this diff is cosmetic changes:
Changing import paths
jose.JsonWebKey -> jose.JSONWebKey
jose.JsonWebSignature -> jose.JSONWebSignature
jose.JoseHeader -> jose.Header
Some more significant changes were caused by updates in the API for
for creating new jose.Signer instances. Previously we constructed
these with jose.NewSigner(algorithm, key). Now these are created with
jose.NewSigner(jose.SigningKey{},jose.SignerOptions{}). At present all
signers specify EmbedJWK: true but this will likely change with
follow-up ACME V2 work.
Another change was the removal of the jose.LoadPrivateKey function
that the wfe tests relied on. The jose v2 API removed these functions,
moving them to a cmd's main package where we can't easily import them.
This function was reimplemented in the WFE's test code & updated to fail
fast rather than return errors.
Per CONTRIBUTING.md I have verified the go-jose.v2 tests at the imported
commit pass:
ok gopkg.in/square/go-jose.v2 14.771s
ok gopkg.in/square/go-jose.v2/cipher 0.025s
? gopkg.in/square/go-jose.v2/jose-util [no test files]
ok gopkg.in/square/go-jose.v2/json 1.230s
ok gopkg.in/square/go-jose.v2/jwt 0.073s
Resolves#2880
An early design mistake meant that some fields of our services were exported
unnecessarily. In particular, fields storing handles of other services (e.g.
"SA" or "PA") were exported. This introduces the possibility of race conditions,
though in practice these fields are set at startup and never modified
concurrently.
We'd like to go through our codebase and change these all to unexported fields,
set at construction time. This is one step in that process.
This is a step towards the long-term goal of eliminating wrappers and a step
towards the short-term goal of making it easier to refactor ca/ca_test.go to
add testing of precertificate-based issuance.
Initial implementation constructed the hash input incorrectly. New test uses a key modulus that is actually on the openssl weak key list instead of a random placeholder.
Fixes#639.
This resolves something that has bugged me for two+ years, our DNSResolverImpl is not a DNS resolver, it is a DNS client. This change just makes that obvious.
This uses the mysql driver library's capability to use `SET` to set the system
variables that prefixdb previously was.
Unfortunately, the library doesn't sort the params when making the string, so we
have to do a little munging to TestNewDbMap.
Ran it in a checkout of the repo since godeps now doesn't include the test files (which is great!).
```
MYSQL_TEST_ADDR=127.0.0.1:3306 go test .
ok github.com/go-sql-driver/mysql 46.099s
```
This will make it easier to add new tests of this form and will also
make it easier to adapt the tests to also test the precertificate +
certificate issuance flow.
Remove the use of mocks for stats in ca_test.go in order to make refactoring
those tests easier. To do so, switch to the same pattern used by the
signature metrics.
This commit updates the
`github.com/weppos/publicsuffix-go/publicsuffix` dependency to commit
e91dbc7, the tip of master at the time of writing.
Unit tests are confirmed to pass:
```
:~/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.006s
ok github.com/weppos/publicsuffix-go/publicsuffix 0.025s
```
This used to be used for AMQP queue names. Now that AMQP is gone, these consts
were only used when printing a version string at startup. This changes
VersionString to just use the name of the current program, and removes
`const clientName = ` from many of our main.go's.
If we hit a `syscall.ECONNRESET` error return a more useful error than `Error getting validation data`, updates the `TestDetailedError` test to cover this case.
Fixes#2851.
The implementation of the dialer used by the HTTP01 challenge, constructed with `resolveAndConstructDialer`, used the same wrapped `net.Dialer` for both the initial IPv6 connection, and any subsequent IPv4 fallback connections. This caused the IPv4 fallback to never succeed for cases where the initial IPv6 connection expended the `validationTimeout`.
This commit updates the http01Dialer (newly renamed from `dialer` since it is in fact specific to HTTP01 challenges) to use a fresh dialer for each connection. To facilitate testing the http01Dialer maintains
a count of how many dialer instances it has constructed. We use this in a unit test to ensure the correct behaviour without a great deal of new mocking/interfaces.
Resolves#2770
From... ahem... some frustrating debugging I determined that the Boulder
docker environment fails in strange & mysterious ways if you do not have
sufficient RAM. This commit adds this fact to the README to save future
souls my torment.
This commit also adds a cd to the intial git clone instructions to
ensure the user is in the correct directory to run docker-compose up
from.
This PR is the initial duplication of the WFE to create a WFE2
package. The rationale is briefly explained in `wfe2/README.md`.
Per #2822 this PR only lays the groundwork for further customization
and deduplication. Presently both the WFE and WFE2 are identical except
for the following configuration differences:
* The WFE offers HTTP and HTTPS on 4000 and 4430 respectively, the WFE2
offers HTTP on 4001 and 4431.
* The WFE has a debug port on 8000, the WFE2 uses the next free "8000
range port" and puts its debug service on 8013
Resolves https://github.com/letsencrypt/boulder/issues/2822
This commit improves the rather vague error message that was previously returned if an IPv6 challenge validation failed when IPv6First was enabled and there were no IPv4 addresses left to try as a fallback.
Resolves#2821
The IETF working group has published a [draft-06](https://tools.ietf.org/html/draft-ietf-acme-acme-07) and [draft-07 revision of ACME](https://tools.ietf.org/html/draft-ietf-acme-acme-07). This PR updates the Boulder `docs/acme-divergences.md` documentation for both drafts. Primarily this meant updating section numbers and links.
Notable updates:
* Added "index" directory Link divergence
* Removed divergence for "existing" field of authorizations - this was removed from the spec so it isn't a divergence anymore \o/
* Added divergence for the Boulder certificates endpoint not respecting client `Accept` headers and using the `application/pkix-cert` content type in responses vs `application/pem-certificate-chain`
* Added divergence for `unsupportedContact` and `accountDoesNotExist` errors.
* Added divergence for the `only-return-existing` field.
* Added divergence for retrying challenges
* Removed "meta" directory divergence since Boulder supports this now
Resolves#2825
Adds basic multi-path validation functionality. A new method `performRemoteValidation` is added to `boulder-va` which is called if it is configured with a list of remote VA gRPC addresses. In this initial implementation the remote VAs are only used to check the validation result of the main VA, if all of the remote validations succeed but the local validation failed, the overall validation will still fail. Remote VAs use the exact same code as the local VA to perform validation. If the local validation succeeds then a configured quorum of the remote VA successes must be met in order to fully complete the validation.
This implementation assumes that metrics are collected from the remote VAs in order to have visibility into their individual validation latencies etc.
Fixes#2621.
Update github.com/google/safebrowsing and block on database health before starting VA
before starting `boulder-va`.
```
$ go test .
ok github.com/google/safebrowsing 4.510s
$ go test .
ok github.com/golang/protobuf/ptypes 0.002s
```
Fixes#2742.
As described in Boulder issue #2800 the implementation of the SA's
`countCertificates` function meant that the renewal exemption for the
Certificates Per Domain rate limit was difficult to work with. To
maximize allotted certificates clients were required to perform all new
issuances first, followed by the "free" renewals. This arrangement was
difficult to coordinate.
In this PR `countCertificates` is updated such that renewals are
excluded from the count reliably. To do so the SA takes the serials it
finds for a given domain from the issuedNames table and cross references
them with the FQDN sets it can find for the associated serials. With the
FQDN sets a second query is done to find all the non-renewal FQDN sets
for the serials, giving a count of the total non-renewal issuances to
use for rate limiting.
Resolves#2800
In #2752, I accidentally introduced a change that would use a NewRegistry for
each NewPromScope, ignoring the Registry that was passed as an argument. Because
this registry was not attached to any HTTP server, the results would not get
exported. This fixes that, so the Registry passed into NewPromScope is
respected.
In the process, I noticed that stats were getting prefixed by a spurious "_". I
fixed that by turning prefix into a slice of strings, and combining them with "_"
only if it the slice is non-empty.
Fixes#2824.
After talking to @jsha, this updates Boulder's docker-compose.yml to version 2. I'm currently working on moving some Certbot tests from EC2 to Docker and this allows me to take advantage of networking features like embedded DNS which is used by default in newer versions of Docker Compose.
This shouldn't change any behavior of the file. One notable thing is I had to add network_mode: bridge to the bhsm service. I don't believe this is a change in behavior though since bhsm was included in the links section for boulder
Daniel McCarney