Feature cleanup (#7320)
Remove three deprecated feature flags which have been removed from all production configs: - StoreLintingCertificateInsteadOfPrecertificate - LeaseCRLShards - AllowUnrecognizedFeatures Deprecate three flags which are set to true in all production configs: - CAAAfterValidation - AllowNoCommonName - SHA256SubjectKeyIdentifier IN-9879 tracked the removal of these flags.
This commit is contained in:
Aaron Gable
GitHub
parent
ad699af3d4
commit
78e4e82ffa
20
ca/ca.go
20
ca/ca.go
|
|
@ -4,7 +4,6 @@ import (
|
|||
"context"
|
||||
"crypto"
|
||||
"crypto/rand"
|
||||
"crypto/sha1"
|
||||
"crypto/sha256"
|
||||
"crypto/x509"
|
||||
"crypto/x509/pkix"
|
||||
|
|
@ -356,11 +355,17 @@ func (ca *certificateAuthorityImpl) generateSerialNumberAndValidity() (*big.Int,
|
|||
return serialBigInt, validity, nil
|
||||
}
|
||||
|
||||
// generateSKID computes the Subject Key Identifier using one of the methods in
|
||||
// RFC 7093 Section 2 Additional Methods for Generating Key Identifiers:
|
||||
// The keyIdentifier [may be] composed of the leftmost 160-bits of the
|
||||
// SHA-256 hash of the value of the BIT STRING subjectPublicKey
|
||||
// (excluding the tag, length, and number of unused bits).
|
||||
func generateSKID(pk crypto.PublicKey) ([]byte, error) {
|
||||
pkBytes, err := x509.MarshalPKIXPublicKey(pk)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var pkixPublicKey struct {
|
||||
Algo pkix.AlgorithmIdentifier
|
||||
BitString asn1.BitString
|
||||
|
|
@ -369,17 +374,8 @@ func generateSKID(pk crypto.PublicKey) ([]byte, error) {
|
|||
return nil, err
|
||||
}
|
||||
|
||||
if features.Get().SHA256SubjectKeyIdentifier {
|
||||
// RFC 7093 Section 2 Additional Methods for Generating Key Identifiers:
|
||||
// The keyIdentifier [may be] composed of the leftmost 160-bits of the
|
||||
// SHA-256 hash of the value of the BIT STRING subjectPublicKey
|
||||
// (excluding the tag, length, and number of unused bits).
|
||||
skid := sha256.Sum256(pkixPublicKey.BitString.Bytes)
|
||||
return skid[0:20:20], nil
|
||||
} else {
|
||||
skid := sha1.Sum(pkixPublicKey.BitString.Bytes)
|
||||
return skid[:], nil
|
||||
}
|
||||
skid := sha256.Sum256(pkixPublicKey.BitString.Bytes)
|
||||
return skid[0:20:20], nil
|
||||
}
|
||||
|
||||
func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context, issueReq *capb.IssueCertificateRequest, serialBigInt *big.Int, validity validity) ([]byte, *issuance.Issuer, error) {
|
||||
|
|
|
|||
|
|
@ -915,20 +915,9 @@ func TestGenerateSKID(t *testing.T) {
|
|||
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
|
||||
test.AssertNotError(t, err, "Error generating key")
|
||||
|
||||
features.Set(features.Config{SHA256SubjectKeyIdentifier: true})
|
||||
defer features.Reset()
|
||||
// RFC 7093 section 2 method 1 allows us to use 160 of the leftmost bits for
|
||||
// the Subject Key Identifier. This is the same amount of bits as the
|
||||
// related SHA1 hash.
|
||||
sha256skid, err := generateSKID(key.Public())
|
||||
test.AssertNotError(t, err, "Error generating SKID")
|
||||
test.AssertEquals(t, len(sha256skid), 20)
|
||||
test.AssertEquals(t, cap(sha256skid), 20)
|
||||
features.Reset()
|
||||
|
||||
features.Set(features.Config{SHA256SubjectKeyIdentifier: false})
|
||||
sha1skid, err := generateSKID(key.Public())
|
||||
test.AssertNotError(t, err, "Error generating SKID")
|
||||
test.AssertEquals(t, len(sha1skid), 20)
|
||||
test.AssertEquals(t, cap(sha1skid), 20)
|
||||
}
|
||||
|
|
|
|||
17
csr/csr.go
17
csr/csr.go
|
|
@ -9,7 +9,6 @@ import (
|
|||
|
||||
"github.com/letsencrypt/boulder/core"
|
||||
berrors "github.com/letsencrypt/boulder/errors"
|
||||
"github.com/letsencrypt/boulder/features"
|
||||
"github.com/letsencrypt/boulder/goodkey"
|
||||
)
|
||||
|
||||
|
|
@ -30,13 +29,12 @@ var goodSignatureAlgorithms = map[x509.SignatureAlgorithm]bool{
|
|||
}
|
||||
|
||||
var (
|
||||
invalidPubKey = berrors.BadCSRError("invalid public key in CSR")
|
||||
unsupportedSigAlg = berrors.BadCSRError("signature algorithm not supported")
|
||||
invalidSig = berrors.BadCSRError("invalid signature on CSR")
|
||||
invalidEmailPresent = berrors.BadCSRError("CSR contains one or more email address fields")
|
||||
invalidIPPresent = berrors.BadCSRError("CSR contains one or more IP address fields")
|
||||
invalidNoDNS = berrors.BadCSRError("at least one DNS name is required")
|
||||
invalidAllSANTooLong = berrors.BadCSRError("CSR doesn't contain a SAN short enough to fit in CN")
|
||||
invalidPubKey = berrors.BadCSRError("invalid public key in CSR")
|
||||
unsupportedSigAlg = berrors.BadCSRError("signature algorithm not supported")
|
||||
invalidSig = berrors.BadCSRError("invalid signature on CSR")
|
||||
invalidEmailPresent = berrors.BadCSRError("CSR contains one or more email address fields")
|
||||
invalidIPPresent = berrors.BadCSRError("CSR contains one or more IP address fields")
|
||||
invalidNoDNS = berrors.BadCSRError("at least one DNS name is required")
|
||||
)
|
||||
|
||||
// VerifyCSR checks the validity of a x509.CertificateRequest. Before doing checks it normalizes
|
||||
|
|
@ -74,9 +72,6 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
|
|||
if len(names.SANs) == 0 && names.CN == "" {
|
||||
return invalidNoDNS
|
||||
}
|
||||
if names.CN == "" && !features.Get().AllowNoCommonName {
|
||||
return invalidAllSANTooLong
|
||||
}
|
||||
if len(names.CN) > maxCNLength {
|
||||
return berrors.BadCSRError("CN was longer than %d bytes", maxCNLength)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -153,7 +153,7 @@ func TestVerifyCSR(t *testing.T) {
|
|||
100,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
invalidAllSANTooLong,
|
||||
nil,
|
||||
},
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,9 +17,9 @@ import (
|
|||
type Config struct {
|
||||
// Deprecated features. Safe for removal once all references to them have
|
||||
// been removed from deployed configuration.
|
||||
StoreLintingCertificateInsteadOfPrecertificate bool
|
||||
LeaseCRLShards bool
|
||||
AllowUnrecognizedFeatures bool
|
||||
CAAAfterValidation bool
|
||||
AllowNoCommonName bool
|
||||
SHA256SubjectKeyIdentifier bool
|
||||
|
||||
// EnforceMultiVA causes the VA to block on remote VA PerformValidation
|
||||
// requests in order to make a valid/invalid decision with the results.
|
||||
|
|
@ -65,26 +65,6 @@ type Config struct {
|
|||
// for the cert URL to appear.
|
||||
AsyncFinalize bool
|
||||
|
||||
// AllowNoCommonName defaults to false, and causes the CA to fail to issue a
|
||||
// certificate if we can't put a CommonName in it. When true, the
|
||||
// CA will be willing to issue certificates with no CN if and only if there
|
||||
// is no SAN short enough to be hoisted into the CN.
|
||||
//
|
||||
// According to the BRs Section 7.1.4.2.2(a), the commonName field is
|
||||
// Deprecated, and its inclusion is discouraged but not (yet) prohibited.
|
||||
AllowNoCommonName bool
|
||||
|
||||
// CAAAfterValidation causes the VA to only kick off CAA checks after the base
|
||||
// domain control validation has completed and succeeded. This makes
|
||||
// successful validations slower by serializing the DCV and CAA work, but
|
||||
// makes unsuccessful validations easier by not doing CAA work at all.
|
||||
CAAAfterValidation bool
|
||||
|
||||
// SHA256SubjectKeyIdentifier enables the generation and use of an RFC 7093
|
||||
// compliant truncated SHA256 Subject Key Identifier in end-entity
|
||||
// certificates.
|
||||
SHA256SubjectKeyIdentifier bool
|
||||
|
||||
// DOH enables DNS-over-HTTPS queries for validation
|
||||
DOH bool
|
||||
|
||||
|
|
|
|||
|
|
@ -117,9 +117,7 @@
|
|||
"ocspLogPeriod": "500ms",
|
||||
"ctLogListFile": "test/ct-test-srv/log_list.json",
|
||||
"features": {
|
||||
"ECDSAForAll": true,
|
||||
"AllowNoCommonName": true,
|
||||
"SHA256SubjectKeyIdentifier": true
|
||||
"ECDSAForAll": true
|
||||
}
|
||||
},
|
||||
"pa": {
|
||||
|
|
|
|||
|
|
@ -102,8 +102,7 @@
|
|||
}
|
||||
},
|
||||
"features": {
|
||||
"AsyncFinalize": true,
|
||||
"AllowNoCommonName": true
|
||||
"AsyncFinalize": true
|
||||
},
|
||||
"ctLogs": {
|
||||
"stagger": "500ms",
|
||||
|
|
|
|||
|
|
@ -33,7 +33,6 @@
|
|||
}
|
||||
},
|
||||
"features": {
|
||||
"CAAAfterValidation": true,
|
||||
"DOH": true
|
||||
},
|
||||
"accountURIPrefixes": [
|
||||
|
|
|
|||
|
|
@ -33,7 +33,6 @@
|
|||
}
|
||||
},
|
||||
"features": {
|
||||
"CAAAfterValidation": true,
|
||||
"DOH": true
|
||||
},
|
||||
"accountURIPrefixes": [
|
||||
|
|
|
|||
|
|
@ -40,7 +40,6 @@
|
|||
"features": {
|
||||
"EnforceMultiVA": true,
|
||||
"MultiVAFullResults": true,
|
||||
"CAAAfterValidation": true,
|
||||
"EnforceMultiCAA": true,
|
||||
"MultiCAAFullResults": true,
|
||||
"DOH": true
|
||||
|
|
|
|||
|
|
@ -127,8 +127,7 @@
|
|||
"Overrides": "test/config-next/wfe2-ratelimit-overrides.yml"
|
||||
},
|
||||
"features": {
|
||||
"ServeRenewalInfo": true,
|
||||
"AllowNoCommonName": true
|
||||
"ServeRenewalInfo": true
|
||||
}
|
||||
},
|
||||
"syslog": {
|
||||
|
|
|
|||
|
|
@ -7,8 +7,6 @@ import (
|
|||
"crypto/elliptic"
|
||||
"crypto/rand"
|
||||
"fmt"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/letsencrypt/boulder/test"
|
||||
|
|
@ -96,15 +94,6 @@ func TestCommonNameSANsTooLong(t *testing.T) {
|
|||
|
||||
// Issue a cert using a CSR with no CN set.
|
||||
ir, err := authAndIssue(client, key, []string{san1, san2}, false)
|
||||
|
||||
// By default, the AllowNoCommonName flag is false, so issuance should have failed.
|
||||
if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
|
||||
test.AssertError(t, err, "issuing cert with no CN")
|
||||
return
|
||||
}
|
||||
|
||||
// But in config-next, the AllowNoCommonName flag is true, so issuance should
|
||||
// have succeeded.
|
||||
test.AssertNotError(t, err, "failed to issue test cert")
|
||||
cert := ir.certs[0]
|
||||
|
||||
|
|
|
|||
|
|
@ -1188,19 +1188,6 @@ def test_new_order_policy_errs():
|
|||
if not ok:
|
||||
raise(Exception("Expected problem, got no error"))
|
||||
|
||||
def test_long_san_no_cn():
|
||||
if CONFIG_NEXT:
|
||||
return
|
||||
try:
|
||||
chisel2.auth_and_issue(["".join(random.choice(string.ascii_uppercase) for x in range(61)) + ".com"])
|
||||
# if we get to this raise the auth_and_issue call didn't fail, so fail the test
|
||||
raise(Exception("Issuance didn't fail when the only SAN in a certificate was longer than the max CN length"))
|
||||
except messages.Error as e:
|
||||
if e.typ != "urn:ietf:params:acme:error:rejectedIdentifier":
|
||||
raise(Exception("Expected malformed type problem, got {0}".format(e.typ)))
|
||||
if e.detail != "NewOrder request did not include a SAN short enough to fit in CN":
|
||||
raise(Exception("Problem detail did not match expected"))
|
||||
|
||||
def test_delete_unused_challenges():
|
||||
order = chisel2.auth_and_issue([random_domain()], chall_type="dns-01")
|
||||
a = order.authorizations[0]
|
||||
|
|
|
|||
41
va/va.go
41
va/va.go
|
|
@ -17,6 +17,8 @@ import (
|
|||
"time"
|
||||
|
||||
"github.com/jmhodges/clock"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
|
||||
"github.com/letsencrypt/boulder/bdns"
|
||||
"github.com/letsencrypt/boulder/canceled"
|
||||
"github.com/letsencrypt/boulder/core"
|
||||
|
|
@ -28,7 +30,6 @@ import (
|
|||
"github.com/letsencrypt/boulder/metrics"
|
||||
"github.com/letsencrypt/boulder/probs"
|
||||
vapb "github.com/letsencrypt/boulder/va/proto"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
)
|
||||
|
||||
var (
|
||||
|
|
@ -436,43 +437,17 @@ func (va *ValidationAuthorityImpl) validate(
|
|||
baseIdentifier.Value = strings.TrimPrefix(identifier.Value, "*.")
|
||||
}
|
||||
|
||||
// Create this channel outside of the feature-conditional block so that it is
|
||||
// also in scope for the matching block below.
|
||||
ch := make(chan error, 1)
|
||||
if !features.Get().CAAAfterValidation {
|
||||
// va.checkCAA accepts wildcard identifiers and handles them appropriately so
|
||||
// we can dispatch `checkCAA` with the provided `identifier` instead of
|
||||
// `baseIdentifier`
|
||||
go func() {
|
||||
params := &caaParams{
|
||||
accountURIID: regid,
|
||||
validationMethod: challenge.Type,
|
||||
}
|
||||
ch <- va.checkCAA(ctx, identifier, params)
|
||||
}()
|
||||
}
|
||||
|
||||
// TODO(#1292): send into another goroutine
|
||||
validationRecords, err := va.validateChallenge(ctx, baseIdentifier, challenge)
|
||||
if err != nil {
|
||||
return validationRecords, err
|
||||
}
|
||||
|
||||
if !features.Get().CAAAfterValidation {
|
||||
for i := 0; i < cap(ch); i++ {
|
||||
if extraErr := <-ch; extraErr != nil {
|
||||
return validationRecords, extraErr
|
||||
}
|
||||
}
|
||||
} else {
|
||||
params := &caaParams{
|
||||
accountURIID: regid,
|
||||
validationMethod: challenge.Type,
|
||||
}
|
||||
err := va.checkCAA(ctx, identifier, params)
|
||||
if err != nil {
|
||||
return validationRecords, err
|
||||
}
|
||||
err = va.checkCAA(ctx, identifier, &caaParams{
|
||||
accountURIID: regid,
|
||||
validationMethod: challenge.Type,
|
||||
})
|
||||
if err != nil {
|
||||
return validationRecords, err
|
||||
}
|
||||
|
||||
return validationRecords, nil
|
||||
|
|
|
|||
|
|
@ -18,6 +18,10 @@ import (
|
|||
"time"
|
||||
|
||||
"github.com/jmhodges/clock"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"google.golang.org/grpc"
|
||||
"gopkg.in/go-jose/go-jose.v2"
|
||||
|
||||
"github.com/letsencrypt/boulder/bdns"
|
||||
"github.com/letsencrypt/boulder/core"
|
||||
corepb "github.com/letsencrypt/boulder/core/proto"
|
||||
|
|
@ -28,9 +32,6 @@ import (
|
|||
"github.com/letsencrypt/boulder/probs"
|
||||
"github.com/letsencrypt/boulder/test"
|
||||
vapb "github.com/letsencrypt/boulder/va/proto"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"google.golang.org/grpc"
|
||||
"gopkg.in/go-jose/go-jose.v2"
|
||||
)
|
||||
|
||||
func TestImplementation(t *testing.T) {
|
||||
|
|
@ -334,8 +335,8 @@ func TestPerformValidationWildcard(t *testing.T) {
|
|||
func TestDCVAndCAASequencing(t *testing.T) {
|
||||
va, mockLog := setup(nil, 0, "", nil, nil)
|
||||
|
||||
// When performing validation without the CAAAfterValidation flag, CAA should
|
||||
// be checked.
|
||||
// When validation succeeds, CAA should be checked.
|
||||
mockLog.Clear()
|
||||
req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01)
|
||||
res, err := va.PerformValidation(context.Background(), req)
|
||||
test.AssertNotError(t, err, "performing validation")
|
||||
|
|
@ -343,21 +344,7 @@ func TestDCVAndCAASequencing(t *testing.T) {
|
|||
caaLog := mockLog.GetAllMatching(`Checked CAA records for`)
|
||||
test.AssertEquals(t, len(caaLog), 1)
|
||||
|
||||
features.Set(features.Config{CAAAfterValidation: true})
|
||||
defer features.Reset()
|
||||
|
||||
// When performing successful validation with the CAAAfterValidation flag,
|
||||
// CAA should be checked.
|
||||
mockLog.Clear()
|
||||
req = createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01)
|
||||
res, err = va.PerformValidation(context.Background(), req)
|
||||
test.AssertNotError(t, err, "performing validation")
|
||||
test.Assert(t, res.Problems == nil, fmt.Sprintf("validation failed: %#v", res.Problems))
|
||||
caaLog = mockLog.GetAllMatching(`Checked CAA records for`)
|
||||
test.AssertEquals(t, len(caaLog), 1)
|
||||
|
||||
// When performing failed validation with the CAAAfterValidation flag,
|
||||
// CAA should be skipped
|
||||
// When validation fails, CAA should be skipped.
|
||||
mockLog.Clear()
|
||||
req = createValidationRequest("bad-dns01.com", core.ChallengeTypeDNS01)
|
||||
res, err = va.PerformValidation(context.Background(), req)
|
||||
|
|
|
|||
13
wfe2/wfe.go
13
wfe2/wfe.go
|
|
@ -2192,7 +2192,6 @@ func (wfe *WebFrontEndImpl) NewOrder(
|
|||
return
|
||||
}
|
||||
|
||||
var hasValidCNLen bool
|
||||
// Collect up all of the DNS identifier values into a []string for
|
||||
// subsequent layers to process. We reject anything with a non-DNS
|
||||
// type identifier here. Check to make sure one of the strings is
|
||||
|
|
@ -2211,18 +2210,6 @@ func (wfe *WebFrontEndImpl) NewOrder(
|
|||
return
|
||||
}
|
||||
names[i] = ident.Value
|
||||
// The max length of a CommonName is 64 bytes. Check to make sure
|
||||
// at least one DNS name meets this requirement to be promoted to
|
||||
// the CN.
|
||||
if len(names[i]) <= 64 {
|
||||
hasValidCNLen = true
|
||||
}
|
||||
}
|
||||
if !hasValidCNLen && !features.Get().AllowNoCommonName {
|
||||
wfe.sendError(response, logEvent,
|
||||
probs.RejectedIdentifier("NewOrder request did not include a SAN short enough to fit in CN"),
|
||||
nil)
|
||||
return
|
||||
}
|
||||
|
||||
logEvent.DNSNames = names
|
||||
|
|
|
|||
|
|
@ -2575,9 +2575,20 @@ func TestNewOrder(t *testing.T) {
|
|||
ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"NotBefore and NotAfter are not supported","status":400}`,
|
||||
},
|
||||
{
|
||||
Name: "POST, no potential CNs 64 bytes or smaller",
|
||||
Request: signAndPost(signer, targetPath, signedURL, tooLongCNBody),
|
||||
ExpectedBody: `{"type":"` + probs.ErrorNS + `rejectedIdentifier","detail":"NewOrder request did not include a SAN short enough to fit in CN","status":400}`,
|
||||
Name: "POST, good payload, all names too long to fit in CN",
|
||||
Request: signAndPost(signer, targetPath, signedURL, tooLongCNBody),
|
||||
ExpectedBody: `
|
||||
{
|
||||
"status": "pending",
|
||||
"expires": "2021-02-01T01:01:01Z",
|
||||
"identifiers": [
|
||||
{ "type": "dns", "value": "thisreallylongexampledomainisabytelongerthanthemaxcnbytelimit.com"}
|
||||
],
|
||||
"authorizations": [
|
||||
"http://localhost/acme/authz-v3/1"
|
||||
],
|
||||
"finalize": "http://localhost/acme/finalize/1/1"
|
||||
}`,
|
||||
},
|
||||
{
|
||||
Name: "POST, good payload, one potential CNs less than 64 bytes and one longer",
|
||||
|
|
|
|||
Loading…
Reference in New Issue