This will allow implementing sub-problems without creating a cyclic
dependency between `core` and `problems`.
The `identifier` package is somewhat small/single-purpose and in the
future we may want to move more "ACME" bits beyond the `identifier`
types into a dedicated package outside of `core`.
Our existing hostname policy configs use JSON. We would like to switch to YAML
to match the rate limit policy configs and to support commenting/tagging
entries in the hostname policy.
The PA is updated to support both JSON and YAML while we migrate the existing
policy data to the new format. To verify there are no changes in functionality
the existing unit tests were updated to test the same policy expressed in YAML
and JSON form.
In integration tests the `config` tree continues to use a JSON hostname policy
file to test the legacy support. In `config-next` a YAML hostname policy file
is used instead.
The new YAML format allows separating out `HighRiskBlockedNames` (primarily
used to meet the requirement of managing issuance for "high risk" domains per
CABF BRs, mostly static) and `AdminBlockedNames` (used after administrative
action is taken to block a domain. Additions are made with some frequency).
Since the rate at which we change entries in these lists differs it is helpful
to separate them in the policy content.
Early ACME drafts supported a notion of "combinations" of challenges
that had to be completed together. This was removed from subsequent
drafts. Boulder has only ever supported "combinations" that exactly map
to the list of challenges, 1 for 1.
This removes all the plumbing for combinations, and adds a list of
combinations to the authz JSON right before marshaling it in WFE1.
* Remove the challenge whitelist
* Reduce the signature for ChallengesFor and ChallengeTypeEnabled
* Some unit tests in the VA were changed from testing TLS-SNI to testing the same behavior
in TLS-ALPN, when that behavior wasn't already tested. For instance timeouts during connect
are now tested.
Fixes#4109
Adds a feature which gates creation of authorizations following the style required for the new schema (and which can be used for gating the reset of our new schema code later down the road).
There was an internal discussion about an issue this creates regarding a predictable ordering of challenges within a challenge due to sequential challenge IDs which will always be static for each challenge type. It was suggested we could add some kind of obfuscation to the challenge ID when presented to the user to prevent this. This hasn't been done in this PR as it would only be focused in the WFE and would be better suited as its own changeset.
Fixes#3981.
The previous update was just [9 days ago](https://github.com/letsencrypt/boulder/pull/3808). However, since we merged some changes into the PSL that are related to IANA TLDs I though about providing an immediate patch.
Tests are passing:
```
➜ ~ cd ~/go/src/github.com/weppos/publicsuffix-go
➜ publicsuffix-go git:(master) GOCACHE=off go test ./...
? github.com/weppos/publicsuffix-go/cmd/load [no test files]
ok github.com/weppos/publicsuffix-go/net/publicsuffix 0.021s
ok github.com/weppos/publicsuffix-go/publicsuffix 0.034s
```
A very large number of the logger calls are of the form log.Function(fmt.Sprintf(...)).
Rather than sprinkling fmt.Sprintf at every logger call site, provide formatting versions
of the logger functions and call these directly with the format and arguments.
While here remove some unnecessary trailing newlines and calls to String/Error.
This code was never enabled in production. Our original intent was to
ship this as part of the ACMEv2 API. Before that could happen flaws were
identified in TLS-SNI-01|02 that resulted in TLS-SNI-02 being removed
from the ACME protocol. We won't ever be enabling this code and so we
might as well remove it.
Prior to this commit the comment on `pa.WillingToIssue` incorrectly
asserted that the input domain must not contain IDN labels. That's not
true anymore! 🎉
This change adds a feature flag, TLSSNIRevalidation. When it is enabled, Boulder
will create new authorization objects with TLS-SNI challenges if the requesting
account has issued a certificate with the relevant domain name, and was the most
recent account to do so*. This setting overrides the configured list of
challenges in the PolicyAuthority, so even if TLS-SNI is disabled in general, it
will be enabled for revalidation.
Note that this interacts with EnforceChallengeDisable. Because
EnforceChallengeDisable causes additional checked at validation time and at
issuance time, we need to update those two places as well. We'll send a
follow-up PR with that.
*We chose to make this work only for the most recent account to issue, even if
there were overlapping certificates, because it significantly simplifies the
database access patterns and should work for 95+% of cases.
Note that this change will let an account revalidate and reissue for a domain
even if the previous issuance on that account used http-01 or dns-01. This also
simplifies implementation, and fits within the intent of the mitigation plan: If
someone previously issued for a domain using http-01, we have high confidence
that they are actually the owner, and they are not going to "steal" the domain
from themselves using tls-sni-01.
Also note: This change also doesn't work properly with ReusePendingAuthz: true.
Specifically, if you attempted issuance in the last couple days and failed
because there was no tls-sni challenge, you'll still have an http-01 challenge
lying around, and we'll reuse that; then your client will fail due to lack of
tls-sni challenge again.
This change was joint work between @rolandshoemaker and @jsha.
This updates the PA component to allow authorization challenge types that are globally disabled if the account ID owning the authorization is on a configured whitelist for that challenge type.
This commit adds a new wildcardExactBlacklist map to the PA's
AuthorityImpl that is used by WillingToIssueWildcard to decide if
a wildcard issuance would cover a high value domain.
This prevents getting a wildcard for "*.example.com" if
"highvalue.example.com" is on the exact blacklist since it would
circumvent the intention of the exact blacklist by minting a certificate
that could be used for "highvalue.example.com".
Resolves#3239
This PR implements issuance for wildcard names in the V2 order flow. By policy, pending authorizations for wildcard names only receive a DNS-01 challenge for the base domain. We do not re-use authorizations for the base domain that do not come from a previous wildcard issuance (e.g. a normal authorization for example.com turned valid by way of a DNS-01 challenge will not be reused for a *.example.com order).
The wildcard prefix is stripped off of the authorization identifier value in two places:
When presenting the authorization to the user - ACME forbids having a wildcard character in an authorization identifier.
When performing validation - We validate the base domain name without the *. prefix.
This PR is largely a rewrite/extension of #3231. Instead of using a pseudo-challenge-type (DNS-01-Wildcard) to indicate an authorization & identifier correspond to the base name of a wildcard order name we instead allow the identifier to take the wildcard order name with the *. prefix.
This is required by RFC 5890, which is not explicitly required by the BRs _at the moment_ but prepares us for a world where RFC 5280 or the BRs are updated to refer to the most recent IDNA RFC (and is general best practice).
Fixes#2885.
RFC 5280 incorporates RFC 3490 by reference. RFC 3490 requires using the KC normalization form, which therefore is required by the BRs. There was some confusion in #2964 as RFC 3490 was obsoleted by RFC 5890 but since the BRs simply reference RFC 5280 this doesn't matter. This PR fixes the confusion.
Godep apparently breaks when trying to parse code that specifies build tags for versions of golang above that with which it was built (which it shouldn't be parsing in the first place). This breaks the travis tests since `golang.org/x/net/context` now contains golang 1.9 specific code. In order to get around this we temporarily disable the error check for `godep save ./...` in test.sh. Opened #2965 to revert this once Godep is fixed or we move to golang 1.9.
Requires an update to `golang.org/x/net` and adding `golang.org/x/text`.
```
[roland@niya:~/gopath/src/golang.org/x/net]$ go test ./...
ok golang.org/x/net/bpf 0.472s
ok golang.org/x/net/context 0.090s
ok golang.org/x/net/context/ctxhttp 0.161s
? golang.org/x/net/dict [no test files]
ok golang.org/x/net/dns/dnsmessage 0.044s
ok golang.org/x/net/html 0.094s
ok golang.org/x/net/html/atom 0.003s
ok golang.org/x/net/html/charset 0.027s
ok golang.org/x/net/http2 80.253s
? golang.org/x/net/http2/h2i [no test files]
ok golang.org/x/net/http2/hpack 0.064s
ok golang.org/x/net/icmp 0.026s
ok golang.org/x/net/idna 0.035s
? golang.org/x/net/internal/iana [no test files]
? golang.org/x/net/internal/nettest [no test files]
ok golang.org/x/net/internal/socket 0.005s
ok golang.org/x/net/internal/timeseries 0.024s
ok golang.org/x/net/ipv4 0.013s
ok golang.org/x/net/ipv6 0.036s
ok golang.org/x/net/lex/httplex 0.004s
ok golang.org/x/net/nettest 1.164s
ok golang.org/x/net/netutil 0.898s
ok golang.org/x/net/proxy 0.004s
ok golang.org/x/net/publicsuffix 0.202s
ok golang.org/x/net/trace 0.018s
ok golang.org/x/net/webdav 0.061s
ok golang.org/x/net/webdav/internal/xml 0.014s
ok golang.org/x/net/websocket 0.022s
ok golang.org/x/net/xsrftoken 0.025s
[roland@niya:~/gopath/src/golang.org/x/text]$ go test ./...
? golang.org/x/text [no test files]
ok golang.org/x/text/cases 0.439s
? golang.org/x/text/cmd/gotext [no test files]
ok golang.org/x/text/collate 0.038s
ok golang.org/x/text/collate/build 0.024s
? golang.org/x/text/collate/tools/colcmp [no test files]
ok golang.org/x/text/currency 2.961s
ok golang.org/x/text/encoding 0.005s
ok golang.org/x/text/encoding/charmap 0.060s
ok golang.org/x/text/encoding/htmlindex 0.005s
ok golang.org/x/text/encoding/ianaindex 0.030s
? golang.org/x/text/encoding/internal [no test files]
? golang.org/x/text/encoding/internal/enctest [no test files]
? golang.org/x/text/encoding/internal/identifier [no test files]
ok golang.org/x/text/encoding/japanese 0.098s
ok golang.org/x/text/encoding/korean 0.032s
ok golang.org/x/text/encoding/simplifiedchinese 0.100s
ok golang.org/x/text/encoding/traditionalchinese 0.012s
ok golang.org/x/text/encoding/unicode 0.013s
ok golang.org/x/text/encoding/unicode/utf32 0.071s
ok golang.org/x/text/feature/plural 0.352s
ok golang.org/x/text/internal 0.009s
ok golang.org/x/text/internal/catmsg 0.034s
ok golang.org/x/text/internal/colltab 1.817s
ok golang.org/x/text/internal/export/idna 0.040s
? golang.org/x/text/internal/format [no test files]
? golang.org/x/text/internal/gen [no test files]
ok golang.org/x/text/internal/number 0.028s
ok golang.org/x/text/internal/stringset 0.021s
ok golang.org/x/text/internal/tag 0.044s
? golang.org/x/text/internal/testtext [no test files]
ok golang.org/x/text/internal/triegen 0.357s
ok golang.org/x/text/internal/ucd 0.023s
? golang.org/x/text/internal/utf8internal [no test files]
ok golang.org/x/text/language 0.033s
ok golang.org/x/text/language/display 3.917s
ok golang.org/x/text/message 0.033s
ok golang.org/x/text/message/catalog 0.069s
ok golang.org/x/text/runes 0.039s
ok golang.org/x/text/search 0.019s
? golang.org/x/text/secure [no test files]
ok golang.org/x/text/secure/bidirule 0.032s
ok golang.org/x/text/secure/precis 0.066s
ok golang.org/x/text/transform 0.106s
? golang.org/x/text/unicode [no test files]
ok golang.org/x/text/unicode/bidi 0.026s
ok golang.org/x/text/unicode/cldr 0.114s
ok golang.org/x/text/unicode/norm 4.009s
ok golang.org/x/text/unicode/rangetable 1.516s
ok golang.org/x/text/unicode/runenames 0.011s
ok golang.org/x/text/width 0.310s
```
Fixes#2963.
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.
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
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 pull request updates the publicsuffix-go dependency to version 0.3.0, most notably including weppos/publicsuffix-go#40 and support for IDN TLDs.
The PA's TestWillingToIssue unit test is updated to confirm that Boulder is WillingToIssue a well formed IDN domain with an IDN TLD. Prior to c5cc328 this causes the PA unit tests to fail as expected with urn:acme:error:malformed :: Name does not end in a public suffix. After
c5cc328 everything is 💯
Per CONTRIBUTING.md the unit tests are confirmed to pass:
daniel@XXXXXX:~/go/src/github.com/weppos/publicsuffix-go$ git show -s
commit 49fe4b0e8276b314e6703300ac26940d9c090a06
Author: Simone Carletti <weppos@weppos.net>
Date: Mon Nov 21 19:26:37 2016 +0100
Release 0.3.0
daniel@XXXXXX:~/go/src/github.com/weppos/publicsuffix-go$ go test ./...
? github.com/weppos/publicsuffix-go/cmd/gen [no test files]
? 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.042s
❤️🍺 and 🎉's to @weppos for the upstream work required for this fix. We truly appreciate your volunteer work on the PSL and the publicsuffix-go library. You're the best!
This resolves#2277.
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.
This PR replaces the `x/net/publicsuffix` package with `weppos/publicsuffix-go`.
The conversations that leaded to this decision are #1479 and #1374. To summarize the discussion, the main issue with `x/net/publicsuffix` is that the package compiles the list into the Go source code and doesn't provide a way to easily pull updates (e.g. by re-parsing the original PSL) unless the entire package is recompiled.
The PSL update frequency is almost daily, which makes very hard to recompile the official Golang package to stay up-to-date with all the changes. Moreover, Golang maintainers expressed some concerns about rebuilding and committing changes with a frequency that would keep the package in sync with the original PSL. See https://github.com/letsencrypt/boulder/issues/1374#issuecomment-182429297
`weppos/publicsuffix-go` contains a compiled version of the list that is updated weekly (or more frequently). Moreover, the package can read and parse a PSL from a String or a File which will effectively decouple the Boulder source code with the list itself. The main benefit is that it will be possible to update the definition by simply downloading the latest list and restarting the application (assuming the list is persisted in memory).
Boulder uses MalformedRequestError as a universal error. This pull request adds the RejectedIdentifierError and use it on a blacklist error.
For a client implementation, it is easier and cleaner to use an exception than parse the error message.
1336c42813/policy/pa.go (L131)Fixes#1938
PR in acme : ietf-wg-acme/acme#142
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.
* Delete Policy DB.
This is no longer needed now that we have a JSON policy file.
* Fix tests.
* Revert Dockerfile.
* Fix create_db
* Simplify user addition.
* Fix tests.
* Fix tests
* Review fixes.
https://github.com/letsencrypt/boulder/pull/1773
* Fix all errcheck errors
* Add errcheck to test.sh
* Add a new sa.Rollback method to make handling errors in rollbacks easier.
This also causes a behavior change in the VA. If a HTTP connection is
abruptly closed after serving the headers for a non-200 response, the
reported error will be the read failure instead of the non-200.
Roland Bracewell Shoemaker
Roland Bracewell Shoemaker
Daniel McCarney
Jacob Hoffman-Andrews
Simone Carletti
Maciej Dębski
Joel Sing
Daniel McCarney
Kleber Correia
David Calavera
Maxime Boisvert
Ben Irving
Kane York