In #4331 I introduced this new more efficient query for
GetOrderForNames, and commented about why we needed an ORDER BY... ASC
to efficiently use the index. However, the actually query did not match
the comment, and it used DESC. This fixes the query.
To demonstrate that the index is actually used with the ASC version,
here's the EXPLAIN output after filling up the table with a bunch of
failed orders:
MariaDB [boulder_sa_integration]> explain select orderID, registrationID FROM orderFqdnSets
-> WHERE setHash = UNHEX('B60FE34E4A6735D5A575D81C97F4DFED2102DC179B34252E4AA18F6E2A375C98')
-> AND expires > NOW() ORDER BY EXPIRES ASC LIMIT 1 \G
*************************** 1. row ***************************
id: 1
select_type: SIMPLE
table: orderFqdnSets
type: range
possible_keys: setHash_expires_idx
key: setHash_expires_idx
key_len: 37
ref: NULL
rows: 1500
Extra: Using index condition
1 row in set (0.000 sec)
MariaDB [boulder_sa_integration]> explain select orderID, registrationID FROM orderFqdnSets
-> WHERE setHash = UNHEX('B60FE34E4A6735D5A575D81C97F4DFED2102DC179B34252E4AA18F6E2A375C98')
-> AND expires > NOW() ORDER BY EXPIRES DESC LIMIT 1 \G
*************************** 1. row ***************************
id: 1
select_type: SIMPLE
table: orderFqdnSets
type: range
possible_keys: setHash_expires_idx
key: setHash_expires_idx
key_len: 37
ref: NULL
rows: 1500
Extra: Using where
1 row in set (0.000 sec)
The `SCTReceipts` database table and associated model linkages are
legacy cruft from before Boulder implemented SCT embedding. We can
safely remove all of this stuff.
Similar to #4334, this fixes a bug where authzs with a randomly generated id
starting with "v2" would incorrectly get treated as v2 authzs.
It accomplishes this change by splitting out v2 authzs into their own path and
using our regular HTTP mux to split them out. It uses a "-v3" name in the
public-facing URLs to avoid confusion.
Previously we made v2 authz handling part of regular authz handling, and
just tried to strip the v2/ prefix. This led to a bug: #4327, where
authzv1s that had an id randomly start with "v2" would 404 because they
were treated as authzv2's.
This change moves authz2's and related challenges onto their own prefix,
and uses HandleFunc to dispatch them appropriately. To reduce code
duplication, I factored out the code that was common to v1 and v2 into
new Common functions.
I also changed the path names to contain "v3" instead of "v2" to avoid
potential confusion.
I noticed this stale bit of text in `test/sa_db_users.sql` while working on something unrelated.
The CA component does not have its own DB, it relies on the SA and its DB the same as
other components. Consulting a historian tells me that this was once true in a lost age :-)
The explicit license header comment isn't required. We don't follow this practice in any of
the other files and assume the base repo LICENSE file is sufficient.
Similarly we don't need to drop each user and recreate them, we use
`CREATE USER IF NOT EXISTS` from MariaDB 10.1+ We also don't need the related
`drop_users.sql` script.
In the case where the DB `Select()` returns a non-nil `err` result the
SA's `getAllOrderAuthorizationStatuses` function needs to ensure it
rolls back the transaction it opened or it will be leaked.
I introduced test_fail_thrice as a specific regression test for #4329,
but I realized that a more general test of the failed validation limit
would have better coverage and also serve as a regression test at the
same time.
Fixes#4332.
This rolls forward #4326 after it was reverted in #4328.
Resolves https://github.com/letsencrypt/boulder/issues/4329
The older query didn't have a `LIMIT 1` so it was returning multiple results,
but gorp's `SelectOne` was okay with multiple results when the selection was
going into an `int64`. When I changed this to a `struct` in #4326, gorp started
producing errors.
For this bug to manifest, an account needs to create an order, then fail
validation, twice in a row for a given domain name, then create an order once
more for the same domain name - that third request will fail because there are
multiple orders in the orderFqdnSets table for that domain.
Note that the bug condition doesn't happen when an account does three successful
issuances in a row, because finalizing an order (that is, issuing a certificate
for it) deletes the row in orderFqdnSets. Failing an authorization does not
delete the row in orderFqdnSets. I believe this was an intentional design
decision because an authorization can participate in many orders, and those
orders can have many other authorizations, so computing the updated state of
all those orders would be expensive (remember, order state is not persisted in
the DB but is calculated dynamically based on the authorizations it contains).
This wasn't detected in integration tests because we don't have any tests that
fail validation for the same domain multiple times. I filed an issue for an
integration test that would have incidentally caught this:
https://github.com/letsencrypt/boulder/issues/4332. There's also a more specific
test case in #4331.
Brings it to be more in line with the responses from the other two challenges and
will hopefully make the challenge a lot easier to debug (like in the recent community
thread).
```json
"error": {
"type": "urn:ietf:params:acme:error:unauthorized",
"detail": "Incorrect validation certificate for tls-alpn-01 challenge. Expected acmeValidationV1 extension value 836bf5358f8a32826c61faeff2e0225b00756f935b00ed3002cabb9d536b9f53 for this challenge but got 8539b12e31c306b81a0aedab4128722c6ad71f71f46316a3c71612f47df0e532",
"status": 403
},
```
This reverts commit 9fa360769e.
This commit can cause "gorp: multiple rows returned for: ..." under certain situations.
See #4329 for details of followup.
When there are a lot of potential orders to reuse, the query could scan
unnecessary rows, sometimes leading to timeouts. The new query used
when the FasterGetOrderForNames feature flag is enabled uses the
available index more effectively and adds a LIMIT clause.
Right now we sometimes get errors like:
rpc error: code = Unknown desc = rpc error:
code = DeadlineExceeded desc = context deadline exceeded
For instance, when an SA call times out, and the RA returns that
timed-out error to the WFE. These are kind of confusing because they
have two layers of nested gRPC error, and they don't provide additional
information about which SA call timed out.
This change replaces DeadlineExceeded errors with our own error type
that includes the service and the method that were called, as well as
the amount of time it took (which helps understand if timeouts are
happening because earlier calls ate up time towards the deadline).
When the RA->SA NewOrder call times out, and the RA returns that error to WFE:
"InternalErrors":["rpc error: code = Unknown desc =
sa.StorageAuthority.NewOrder timed out after 14954 ms"]
When the WFE->RA NewOrder call times out:
"InternalErrors":["ra.RegistrationAuthority.NewOrder timed out after 15000 ms"]
Note that this change only handles timeouts at one level deep, which I
think is sufficient for our needs.
`cert-checker` assumes an undefined behavior of MySQL which is only sometimes true, which means sometimes we select fewer certificates than we actually expect to. Instead of adding an explicit ORDER BY we simply switch to cursoring using the primary key, which gets us overall much more efficient usage of indexes.
Fixes#4315.
This reverts commit 796a7aa2f4.
People's tests have been breaking on `docker-compose up` with the following output:
```
ImportError: No module named requests
```
Fixes#4322
* integration: move to Python3
- Add parentheses to all print and raise calls.
- Python3 distinguishes bytes from strings. Add encode() and
decode() calls as needed to provide the correct type.
- Use requests library consistently (urllib3 is not in Python3).
- Remove shebang from Python files without a main, and update
shebang for integration-test.py.
Basically a complete re-write/re-design of the forwarding concept introduced in
#4297 (sorry for the rapid churn here). Instead of nonce-services blindly
forwarding nonces around to each other in an attempt to find out who issued the
nonce we add an identifying prefix to each nonce generated by a service. The
WFEs then use this prefix to decide which nonce-service to ask to validate the
nonce.
This requires a slightly more complicated configuration at the WFE/2 end, but
overall I think ends up being a way cleaner, more understandable, easy to
reason about implementation. When configuring the WFE you need to provide two
forms of gRPC config:
* one gRPC config for retrieving nonces, this should be a DNS name that
resolves to all available nonce-services (or at least the ones you want to
retrieve nonces from locally, in a two DC setup you might only configure the
nonce-services that are in the same DC as the WFE instance). This allows
getting a nonce from any of the configured services and is load-balanced
transparently at the gRPC layer.
* a map of nonce prefixes to gRPC configs, this maps each individual
nonce-service to it's prefix and allows the WFE instances to figure out which
nonce-service to ask to validate a nonce it has received (in a two DC setup
you'd want to configure this with all the nonce-services across both DCs so
that you can validate a nonce that was generated by a nonce-service in another
DC).
This balancing is implemented in the integration tests.
Given the current remote nonce code hasn't been deployed anywhere yet this
makes a number of hard breaking changes to both the existing nonce-service
code, and the forwarding code.
Fixes#4303.
In #4179 we added a different method of counting the certificatesPerName
rate limit that can provide the correct behavior for exact public suffix
matches without the need for a separate RPC call. This cleans up the
separate code paths in the SA and RA that are no longer necesary.
Also excises the existing bad padding metrics code, adds a special error for when we encounter badly padded keys, and adds a test for the new special error.
Fixes#4070 and fixes#3964.
Before #4275 if a CSR only contained SANs longer than the max CN limit
it would set the CN to one anyway and would cause the 'CN too long'
check to get triggered. After #4275 if all of the SANs were too long
the CN wouldn't get set and we didn't have a check for `forceCNFromSAN
&& cn == ""` which would allow empty CNs despite `forceCNFromSAN`
being set. This adds that check and a test for the corner case.
This is a holdover from one of the tables (I think orderToAuthz2?) used a string
authorization ID and we needed to convert in order to get what we wanted but
apparently I never cleaned it up when we switched to integers for both tables.
What is really confusing here is why we ever needed a CONVERT in the first place
if Maria is happy to arbitrarily compare strings to integers itself... what fun.
Add flag to explicitly disable orders containing authz2 authorizations. After looking at a handful of much more complex solutions this feels like the best option. With NewAuthorizationSchema disabled and DisableAuthz2Orders enabled any requests for orders that include authz2 authorizations will return a 404 (where previously they would return a 500).
Fixes#4263.
The three new cases separately test:
- Rechecking CAA during authz reuse.
- Successful issuance for a positive CAA record
- Rejected issuance for a negative CAA record
- The various CAA extensions from https://tools.ietf.org/html/draft-ietf-acme-caa-06
Importantly, this also switches `recheck.good-caa-reserved.com` to use a
dynamically generated random name. This should fix the problem where
running integration tests locally several times resulted in hitting an
exact match rate limit error, requiring a clear of the fqdnSets table.
This also moves the creation of the client for test_recheck_caa into its
own early-setup function, so there is less test-case-specific setup in
integration-test.py.
When NewAuthorizationSchema is enabled, we still want v1 authzs to be reusable in
new orders. This tests that that code is implemented correctly.
Updates #4241
These two setup phases were only used by `test_expired_authz_404`,
which is adequately covered by unittests. Since each setup and teardown
is rather time consuming, this speeds up and simplifies integration
tests.
Before: 5m10
After: 4m46
Move from using `requests` to `urllib2` in `helpers.py`. Verified
this works with `docker-compose up`. In the future we really should
be installing our own python dependencies in the boulder-tools image
rather than relying on getting them by using the certbot virtualenv.
* Switch to instant OCSP verification in integration tests
* Move waitport to helpers and use it to determine if ocsp-responder is
alive in test_single_ocsp
If a berror with suberrors is being wrapped then we must marshal the
suberrors as JSON and include this data in the RPC metadata trailer that
also carries the berror type. When unwrapping metadata with JSON
suberrors they should be unmarshalled into the returned berror's
suberrors.
Jacob Hoffman-Andrews