From d8c67285cfa0a117dbfda6541a611e460457615a Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 31 Oct 2015 16:55:59 +0900 Subject: [PATCH 01/26] Enable configuration of supported challenges --- ca/certificate-authority_test.go | 2 +- cmd/boulder-ca/main.go | 2 +- cmd/boulder-ra/main.go | 2 +- cmd/cert-checker/main.go | 6 ++--- cmd/shell.go | 4 ++- policy/policy-authority.go | 45 ++++++++++++++++++++++++------- policy/policy-authority_test.go | 4 +-- ra/registration-authority_test.go | 17 +++--------- test/create_db.sh | 3 ++- 9 files changed, 53 insertions(+), 32 deletions(-) diff --git a/ca/certificate-authority_test.go b/ca/certificate-authority_test.go index d592669f0..4c3b3d23a 100644 --- a/ca/certificate-authority_test.go +++ b/ca/certificate-authority_test.go @@ -132,7 +132,7 @@ func setup(t *testing.T) *testCtx { paDbMap, err := sa.NewDbMap(vars.DBConnPolicy) test.AssertNotError(t, err, "Could not construct dbMap") - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, false) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, false, nil) test.AssertNotError(t, err, "Couldn't create PADB") paDBCleanUp := test.ResetPolicyTestDatabase(t) diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index c3bf8548b..1b83bab78 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -36,7 +36,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Couldn't connect to policy database") - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.ChallengeTypes) cmd.FailOnError(err, "Couldn't create PA") cai, err := ca.NewCertificateAuthorityImpl(c.CA, clock.Default(), stats, c.Common.IssuerCert) diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index f670fde1c..1b6740973 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -40,7 +40,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Couldn't connect to policy database") - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.ChallengeTypes) cmd.FailOnError(err, "Couldn't create PA") rateLimitPolicies, err := cmd.LoadRateLimitPolicies(c.RA.RateLimitPoliciesFilename) diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index 030fc0989..ee55b63dd 100644 --- a/cmd/cert-checker/main.go +++ b/cmd/cert-checker/main.go @@ -76,8 +76,8 @@ type certChecker struct { issuedReport report } -func newChecker(saDbMap *gorp.DbMap, paDbMap *gorp.DbMap, clk clock.Clock, enforceWhitelist bool) certChecker { - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, enforceWhitelist) +func newChecker(saDbMap *gorp.DbMap, paDbMap *gorp.DbMap, clk clock.Clock, enforceWhitelist bool, challengeTypes []string) certChecker { + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, enforceWhitelist, challengeTypes) cmd.FailOnError(err, "Failed to create PA") c := certChecker{ pa: pa, @@ -250,7 +250,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Could not connect to policy database") - checker := newChecker(saDbMap, paDbMap, clock.Default(), c.PA.EnforcePolicyWhitelist) + checker := newChecker(saDbMap, paDbMap, clock.Default(), c.PA.EnforcePolicyWhitelist, c.PA.ChallengeTypes) auditlogger.Info("# Getting certificates issued in the last 90 days") // Since we grab certificates in batches we don't want this to block, when it diff --git a/cmd/shell.go b/cmd/shell.go index 250016e39..57f2ccf4e 100644 --- a/cmd/shell.go +++ b/cmd/shell.go @@ -255,10 +255,12 @@ type CAConfig struct { } // PAConfig specifies how a policy authority should connect to its -// database, and what policies it should enforce. +// database, what policies it should enforce, and what challenges +// it should offer. type PAConfig struct { DBConnect string EnforcePolicyWhitelist bool + ChallengeTypes []string } // KeyConfig should contain either a File path to a PEM-format private key, diff --git a/policy/policy-authority.go b/policy/policy-authority.go index 9c70b69e2..74b1857b7 100644 --- a/policy/policy-authority.go +++ b/policy/policy-authority.go @@ -23,11 +23,12 @@ type PolicyAuthorityImpl struct { log *blog.AuditLogger DB *PolicyAuthorityDatabaseImpl - EnforceWhitelist bool + EnforceWhitelist bool + supportedChallenges map[string]bool } // NewPolicyAuthorityImpl constructs a Policy Authority. -func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool) (*PolicyAuthorityImpl, error) { +func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool, challengeTypes []string) (*PolicyAuthorityImpl, error) { logger := blog.GetAuditLogger() logger.Notice("Policy Authority Starting") @@ -42,6 +43,11 @@ func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool) (*PolicyAu EnforceWhitelist: enforceWhitelist, } + // Take note of which challenges to offer + for _, challengeType := range challengeTypes { + pa.supportedChallenges[challengeType] = true + } + return &pa, nil } @@ -204,13 +210,34 @@ func (pa PolicyAuthorityImpl) WillingToIssue(id core.AcmeIdentifier, regID int64 // // Note: Current implementation is static, but future versions may not be. func (pa PolicyAuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, accountKey *jose.JsonWebKey) (challenges []core.Challenge, combinations [][]int, err error) { - // TODO(https://github.com/letsencrypt/boulder/issues/894): Update these lines - challenges = []core.Challenge{ - core.SimpleHTTPChallenge(accountKey), - core.DvsniChallenge(accountKey), - core.HTTPChallenge01(accountKey), - core.TLSSNIChallenge01(accountKey), + challenges = []core.Challenge{} + combinations = [][]int{} + + // TODO(https://github.com/letsencrypt/boulder/issues/894): Remove this block + if pa.supportedChallenges[core.ChallengeTypeSimpleHTTP] { + challenges = append(challenges, core.SimpleHTTPChallenge(accountKey)) + } + + // TODO(https://github.com/letsencrypt/boulder/issues/894): Remove this block + if pa.supportedChallenges[core.ChallengeTypeDVSNI] { + challenges = append(challenges, core.SimpleHTTPChallenge(accountKey)) + } + + if pa.supportedChallenges[core.ChallengeTypeHTTP01] { + challenges = append(challenges, core.DvsniChallenge(accountKey)) + } + + if pa.supportedChallenges[core.ChallengeTypeTLSSNI01] { + challenges = append(challenges, core.HTTPChallenge01(accountKey)) + } + + if pa.supportedChallenges[core.ChallengeTypeDNS01] { + challenges = append(challenges, core.TLSSNIChallenge01(accountKey)) + } + + combinations = make([][]int, len(challenges)) + for i := range combinations { + combinations[i] = []int{i} } - combinations = [][]int{[]int{0}, []int{1}, []int{2}, []int{3}} return } diff --git a/policy/policy-authority_test.go b/policy/policy-authority_test.go index df6471842..61a17cdd9 100644 --- a/policy/policy-authority_test.go +++ b/policy/policy-authority_test.go @@ -23,7 +23,7 @@ var log = mocks.UseMockLog() func paImpl(t *testing.T) (*PolicyAuthorityImpl, func()) { dbMap, cleanUp := paDBMap(t) - pa, err := NewPolicyAuthorityImpl(dbMap, false) + pa, err := NewPolicyAuthorityImpl(dbMap, false, []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01}) if err != nil { cleanUp() t.Fatalf("Couldn't create policy implementation: %s", err) @@ -225,7 +225,7 @@ func TestChallengesFor(t *testing.T) { func TestWillingToIssueWithWhitelist(t *testing.T) { dbMap, cleanUp := paDBMap(t) defer cleanUp() - pa, err := NewPolicyAuthorityImpl(dbMap, true) + pa, err := NewPolicyAuthorityImpl(dbMap, true, nil) test.AssertNotError(t, err, "Couldn't create policy implementation") googID := core.AcmeIdentifier{ Type: core.IdentifierDNS, diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index f543d6ba8..dce43a3bf 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -189,7 +189,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut t.Fatalf("Failed to create dbMap: %s", err) } policyDBCleanUp := test.ResetPolicyTestDatabase(t) - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, false) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, false, []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01}) test.AssertNotError(t, err, "Couldn't create PA") ca := ca.CertificateAuthorityImpl{ Signer: signer, @@ -391,21 +391,12 @@ func TestNewAuthorization(t *testing.T) { // TODO Verify that challenges are correct // TODO(https://github.com/letsencrypt/boulder/issues/894): Update these lines - test.Assert(t, len(authz.Challenges) == 4, "Incorrect number of challenges returned") - test.Assert(t, authz.Challenges[0].Type == core.ChallengeTypeSimpleHTTP, "Challenge 0 not SimpleHTTP") - test.Assert(t, authz.Challenges[1].Type == core.ChallengeTypeDVSNI, "Challenge 1 not DVSNI") - - // TODO(https://github.com/letsencrypt/boulder/issues/894): Delete these lines - test.Assert(t, authz.Challenges[2].Type == core.ChallengeTypeHTTP01, "Challenge 2 not http-00") - test.Assert(t, authz.Challenges[3].Type == core.ChallengeTypeTLSSNI01, "Challenge 3 not tlssni-00") - + test.Assert(t, len(authz.Challenges) == 2, "Incorrect number of challenges returned") + test.Assert(t, authz.Challenges[0].Type == core.ChallengeTypeHTTP01, "Challenge 0 not SimpleHTTP") + test.Assert(t, authz.Challenges[1].Type == core.ChallengeTypeTLSSNI01, "Challenge 1 not DVSNI") test.Assert(t, authz.Challenges[0].IsSane(false), "Challenge 0 is not sane") test.Assert(t, authz.Challenges[1].IsSane(false), "Challenge 1 is not sane") - // TODO(https://github.com/letsencrypt/boulder/issues/894): Delete these lines - test.Assert(t, authz.Challenges[2].IsSane(false), "Challenge 2 is not sane") - test.Assert(t, authz.Challenges[3].IsSane(false), "Challenge 3 is not sane") - t.Log("DONE TestNewAuthorization") } diff --git a/test/create_db.sh b/test/create_db.sh index fca5355d5..cfebdd380 100755 --- a/test/create_db.sh +++ b/test/create_db.sh @@ -10,13 +10,14 @@ if [[ ! -z "MYSQL_CONTAINER" ]]; then fi # Drop all users to get a fresh start -mysql $dbconn < test/drop_users.sql +#mysql $dbconn < test/drop_users.sql for svc in $SERVICES; do for dbenv in $DBENVS; do ( db="boulder_${svc}_${dbenv}" create_script="drop database if exists \`${db}\`; create database if not exists \`${db}\`;" + echo $create_script mysql $dbconn -e "$create_script" || die "unable to create ${db}" From f8b6cd69c984acdb9ea21b5be1db679dfd866171 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 31 Oct 2015 16:57:19 +0900 Subject: [PATCH 02/26] Add supported challenges to test configuration --- test/boulder-config.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/boulder-config.json b/test/boulder-config.json index f52a34713..52d5b2808 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -116,7 +116,8 @@ }, "pa": { - "dbConnect": "mysql+tcp://policy@localhost:3306/boulder_policy_integration" + "dbConnect": "mysql+tcp://policy@localhost:3306/boulder_policy_integration", + "challengeTypes": ["simpleHttp", "http-01", "tls-sni-01"] }, "ra": { From f5b25ba6fbdee05d7bdcd75ed8247774302f4454 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 31 Oct 2015 16:58:27 +0900 Subject: [PATCH 03/26] Revert unneeded test script changes --- test/create_db.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/create_db.sh b/test/create_db.sh index cfebdd380..fca5355d5 100755 --- a/test/create_db.sh +++ b/test/create_db.sh @@ -10,14 +10,13 @@ if [[ ! -z "MYSQL_CONTAINER" ]]; then fi # Drop all users to get a fresh start -#mysql $dbconn < test/drop_users.sql +mysql $dbconn < test/drop_users.sql for svc in $SERVICES; do for dbenv in $DBENVS; do ( db="boulder_${svc}_${dbenv}" create_script="drop database if exists \`${db}\`; create database if not exists \`${db}\`;" - echo $create_script mysql $dbconn -e "$create_script" || die "unable to create ${db}" From 72ed655c0fe9189cab519b29677fd65f4382385d Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 31 Oct 2015 17:03:09 +0900 Subject: [PATCH 04/26] Fix cert-checker tests --- cmd/cert-checker/main_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go index e636b140f..a2dad1fb3 100644 --- a/cmd/cert-checker/main_test.go +++ b/cmd/cert-checker/main_test.go @@ -47,7 +47,7 @@ func BenchmarkCheckCert(b *testing.B) { fmt.Printf("Failed to truncate tables: %s\n", err) }() - checker := newChecker(saDbMap, paDbMap, clock.Default(), false) + checker := newChecker(saDbMap, paDbMap, clock.Default(), false, nil) testKey, _ := rsa.GenerateKey(rand.Reader, 1024) expiry := time.Now().AddDate(0, 0, 1) serial := big.NewInt(1337) @@ -89,7 +89,7 @@ func TestCheckCert(t *testing.T) { fc := clock.NewFake() fc.Add(time.Hour * 24 * 90) - checker := newChecker(saDbMap, paDbMap, fc, false) + checker := newChecker(saDbMap, paDbMap, fc, false, nil) issued := checker.clock.Now().Add(-time.Hour * 24 * 45) goodExpiry := issued.Add(checkPeriod) @@ -181,7 +181,7 @@ func TestGetAndProcessCerts(t *testing.T) { test.AssertNotError(t, err, "Couldn't connect to policy database") fc := clock.NewFake() - checker := newChecker(saDbMap, paDbMap, fc, false) + checker := newChecker(saDbMap, paDbMap, fc, false, nil) sa, err := sa.NewSQLStorageAuthority(saDbMap, fc) test.AssertNotError(t, err, "Couldn't create SA to insert certificates") saCleanUp := test.ResetSATestDatabase(t) From 587bd8c89a2bb7c6626517339ad6d569287196ec Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 31 Oct 2015 17:08:25 +0900 Subject: [PATCH 05/26] Initialize the challenge type map before using it --- policy/policy-authority.go | 1 + 1 file changed, 1 insertion(+) diff --git a/policy/policy-authority.go b/policy/policy-authority.go index 74b1857b7..d2d7a6660 100644 --- a/policy/policy-authority.go +++ b/policy/policy-authority.go @@ -44,6 +44,7 @@ func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool, challengeT } // Take note of which challenges to offer + pa.supportedChallenges = map[string]bool{} for _, challengeType := range challengeTypes { pa.supportedChallenges[challengeType] = true } From 39dcb9f285581a70149cb3a0e62d9ee4c08a21ba Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 31 Oct 2015 18:11:08 +0900 Subject: [PATCH 06/26] Further unit test fixes --- policy/policy-authority_test.go | 21 ++++++++------------- ra/registration-authority_test.go | 11 ++++++----- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/policy/policy-authority_test.go b/policy/policy-authority_test.go index 61a17cdd9..85a629807 100644 --- a/policy/policy-authority_test.go +++ b/policy/policy-authority_test.go @@ -21,9 +21,11 @@ import ( var log = mocks.UseMockLog() +var supportedChallenges = []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01} + func paImpl(t *testing.T) (*PolicyAuthorityImpl, func()) { dbMap, cleanUp := paDBMap(t) - pa, err := NewPolicyAuthorityImpl(dbMap, false, []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01}) + pa, err := NewPolicyAuthorityImpl(dbMap, false, supportedChallenges) if err != nil { cleanUp() t.Fatalf("Couldn't create policy implementation: %s", err) @@ -207,18 +209,11 @@ func TestChallengesFor(t *testing.T) { t.Errorf("Error generating challenges: %v", err) } - // TODO(https://github.com/letsencrypt/boulder/issues/894): Update these tests - if len(challenges) != 4 || - challenges[0].Type != core.ChallengeTypeSimpleHTTP || - challenges[1].Type != core.ChallengeTypeDVSNI || - challenges[2].Type != core.ChallengeTypeHTTP01 || - challenges[3].Type != core.ChallengeTypeTLSSNI01 { - t.Error("Incorrect challenges returned") - } - if len(combinations) != 4 || - combinations[0][0] != 0 || combinations[1][0] != 1 || - combinations[2][0] != 2 || combinations[3][0] != 3 { - t.Error("Incorrect combinations returned") + test.Assert(t, len(challenges) == len(supportedChallenges), "Wrong number of challenges returned") + test.Assert(t, len(combinations) == len(supportedChallenges), "Wrong number of combinations returned") + for i, challenge := range challenges { + test.Assert(t, challenges[i].Type == supportedChallenges[i], fmt.Sprintf("Challenge %d has incorrect type", i)) + test.Assert(t, len(combinations[i]) == 1 && combinations[i][0] == i, fmt.Sprintf("Combination %d is incorrect", i)) } } diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index dce43a3bf..35d1cbcc4 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -51,6 +51,8 @@ func (dva *DummyValidationAuthority) CheckCAARecords(identifier core.AcmeIdentif } var ( + SupportedChallenges = []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01} + // These values we simulate from the client AccountKeyJSONA = []byte(`{ "kty":"RSA", @@ -189,7 +191,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut t.Fatalf("Failed to create dbMap: %s", err) } policyDBCleanUp := test.ResetPolicyTestDatabase(t) - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, false, []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01}) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, false, SupportedChallenges) test.AssertNotError(t, err, "Couldn't create PA") ca := ca.CertificateAuthorityImpl{ Signer: signer, @@ -390,10 +392,9 @@ func TestNewAuthorization(t *testing.T) { test.Assert(t, authz.Status == core.StatusPending, "Initial authz not pending") // TODO Verify that challenges are correct - // TODO(https://github.com/letsencrypt/boulder/issues/894): Update these lines - test.Assert(t, len(authz.Challenges) == 2, "Incorrect number of challenges returned") - test.Assert(t, authz.Challenges[0].Type == core.ChallengeTypeHTTP01, "Challenge 0 not SimpleHTTP") - test.Assert(t, authz.Challenges[1].Type == core.ChallengeTypeTLSSNI01, "Challenge 1 not DVSNI") + test.Assert(t, len(authz.Challenges) == len(SupportedChallenges), "Incorrect number of challenges returned") + test.Assert(t, authz.Challenges[0].Type == SupportedChallenges[0], "Challenge 0 not http-01") + test.Assert(t, authz.Challenges[1].Type == SupportedChallenges[1], "Challenge 1 not tls-sni-01") test.Assert(t, authz.Challenges[0].IsSane(false), "Challenge 0 is not sane") test.Assert(t, authz.Challenges[1].IsSane(false), "Challenge 1 is not sane") From f67405bfbc12cb9b1e67a515f709d75c82ea9c2c Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Nov 2015 08:28:15 -0500 Subject: [PATCH 07/26] Fix unit test failures and a copy/paste error --- policy/policy-authority.go | 4 ++-- policy/policy-authority_test.go | 5 +++-- ra/registration-authority_test.go | 12 ++++++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/policy/policy-authority.go b/policy/policy-authority.go index d2d7a6660..1a9691333 100644 --- a/policy/policy-authority.go +++ b/policy/policy-authority.go @@ -225,11 +225,11 @@ func (pa PolicyAuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, acco } if pa.supportedChallenges[core.ChallengeTypeHTTP01] { - challenges = append(challenges, core.DvsniChallenge(accountKey)) + challenges = append(challenges, core.HTTPChallenge01(accountKey)) } if pa.supportedChallenges[core.ChallengeTypeTLSSNI01] { - challenges = append(challenges, core.HTTPChallenge01(accountKey)) + challenges = append(challenges, core.TLSSNIChallenge01(accountKey)) } if pa.supportedChallenges[core.ChallengeTypeDNS01] { diff --git a/policy/policy-authority_test.go b/policy/policy-authority_test.go index 85a629807..e6393d425 100644 --- a/policy/policy-authority_test.go +++ b/policy/policy-authority_test.go @@ -212,8 +212,9 @@ func TestChallengesFor(t *testing.T) { test.Assert(t, len(challenges) == len(supportedChallenges), "Wrong number of challenges returned") test.Assert(t, len(combinations) == len(supportedChallenges), "Wrong number of combinations returned") for i, challenge := range challenges { - test.Assert(t, challenges[i].Type == supportedChallenges[i], fmt.Sprintf("Challenge %d has incorrect type", i)) - test.Assert(t, len(combinations[i]) == 1 && combinations[i][0] == i, fmt.Sprintf("Combination %d is incorrect", i)) + test.AssertEquals(t, challenge.Type, supportedChallenges[i]) + test.AssertEquals(t, len(combinations[i]), 1) + test.AssertEquals(t, combinations[i][0], i) } } diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index 35d1cbcc4..4f9af7c14 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -148,10 +148,6 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut err = json.Unmarshal(ShortKeyJSON, &ShortKey) test.AssertNotError(t, err, "Failed to unmarshal JWK") - simpleHTTP := core.SimpleHTTPChallenge(&AccountKeyA) - dvsni := core.DvsniChallenge(&AccountKeyA) - AuthzInitial.Challenges = []core.Challenge{simpleHTTP, dvsni} - fc := clock.NewFake() dbMap, err := sa.NewDbMap(vars.DBConnSA) @@ -231,6 +227,10 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut AuthzInitial.RegistrationID = Registration.ID + challenges, combinations, err := pa.ChallengesFor(AuthzInitial.Identifier, &Registration.Key) + AuthzInitial.Challenges = challenges + AuthzInitial.Combinations = combinations + AuthzFinal = AuthzInitial AuthzFinal.Status = "valid" exp := time.Now().Add(365 * 24 * time.Hour) @@ -393,8 +393,8 @@ func TestNewAuthorization(t *testing.T) { // TODO Verify that challenges are correct test.Assert(t, len(authz.Challenges) == len(SupportedChallenges), "Incorrect number of challenges returned") - test.Assert(t, authz.Challenges[0].Type == SupportedChallenges[0], "Challenge 0 not http-01") - test.Assert(t, authz.Challenges[1].Type == SupportedChallenges[1], "Challenge 1 not tls-sni-01") + test.AssertEquals(t, authz.Challenges[0].Type, SupportedChallenges[0]) + test.AssertEquals(t, authz.Challenges[1].Type, SupportedChallenges[1]) test.Assert(t, authz.Challenges[0].IsSane(false), "Challenge 0 is not sane") test.Assert(t, authz.Challenges[1].IsSane(false), "Challenge 1 is not sane") From 36225bdf4f297c0bd311e3cef0f2144ddd2f251a Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Nov 2015 22:35:29 +0900 Subject: [PATCH 08/26] Fix go fmt errors --- policy/policy-authority_test.go | 2 +- ra/registration-authority_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/policy/policy-authority_test.go b/policy/policy-authority_test.go index e6393d425..cbbf7ac5f 100644 --- a/policy/policy-authority_test.go +++ b/policy/policy-authority_test.go @@ -214,7 +214,7 @@ func TestChallengesFor(t *testing.T) { for i, challenge := range challenges { test.AssertEquals(t, challenge.Type, supportedChallenges[i]) test.AssertEquals(t, len(combinations[i]), 1) - test.AssertEquals(t, combinations[i][0], i) + test.AssertEquals(t, combinations[i][0], i) } } diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index 4f9af7c14..b2d1df8fb 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -227,9 +227,9 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut AuthzInitial.RegistrationID = Registration.ID - challenges, combinations, err := pa.ChallengesFor(AuthzInitial.Identifier, &Registration.Key) + challenges, combinations, err := pa.ChallengesFor(AuthzInitial.Identifier, &Registration.Key) AuthzInitial.Challenges = challenges - AuthzInitial.Combinations = combinations + AuthzInitial.Combinations = combinations AuthzFinal = AuthzInitial AuthzFinal.Status = "valid" From 0f852a89ab14f30cbe7313eea922cda7527b0783 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Nov 2015 23:06:04 +0900 Subject: [PATCH 09/26] simpleHttp->dvsni to try to make client happy --- test/boulder-config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/boulder-config.json b/test/boulder-config.json index 52d5b2808..ca47db8d2 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -117,7 +117,7 @@ "pa": { "dbConnect": "mysql+tcp://policy@localhost:3306/boulder_policy_integration", - "challengeTypes": ["simpleHttp", "http-01", "tls-sni-01"] + "challengeTypes": ["dvsni", "http-01", "tls-sni-01"] }, "ra": { From a16e98a4a473786f7441c0c3f26bdae1d9bce3f7 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Nov 2015 23:20:28 +0900 Subject: [PATCH 10/26] Further copy/paste errors --- policy/policy-authority.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/policy/policy-authority.go b/policy/policy-authority.go index 1a9691333..c9f6bf16b 100644 --- a/policy/policy-authority.go +++ b/policy/policy-authority.go @@ -221,7 +221,7 @@ func (pa PolicyAuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, acco // TODO(https://github.com/letsencrypt/boulder/issues/894): Remove this block if pa.supportedChallenges[core.ChallengeTypeDVSNI] { - challenges = append(challenges, core.SimpleHTTPChallenge(accountKey)) + challenges = append(challenges, core.DvsniChallenge(accountKey)) } if pa.supportedChallenges[core.ChallengeTypeHTTP01] { @@ -233,7 +233,7 @@ func (pa PolicyAuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, acco } if pa.supportedChallenges[core.ChallengeTypeDNS01] { - challenges = append(challenges, core.TLSSNIChallenge01(accountKey)) + challenges = append(challenges, core.DNSChallenge01(accountKey)) } combinations = make([][]int, len(challenges)) From e4f0422b646778fc4ababd799fdd054457c169fd Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 2 Nov 2015 09:03:55 +0900 Subject: [PATCH 11/26] dvsni->simpleHttp to try to make the client happy --- test/boulder-config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/boulder-config.json b/test/boulder-config.json index ca47db8d2..52d5b2808 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -117,7 +117,7 @@ "pa": { "dbConnect": "mysql+tcp://policy@localhost:3306/boulder_policy_integration", - "challengeTypes": ["dvsni", "http-01", "tls-sni-01"] + "challengeTypes": ["simpleHttp", "http-01", "tls-sni-01"] }, "ra": { From a75da0eefe93196698aaeb375df54c9188349a4a Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Tue, 3 Nov 2015 08:22:10 +0900 Subject: [PATCH 12/26] Send simpleHttp and dvsni to try to make the client happy --- test/boulder-config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/boulder-config.json b/test/boulder-config.json index 52d5b2808..5b65459b4 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -117,7 +117,7 @@ "pa": { "dbConnect": "mysql+tcp://policy@localhost:3306/boulder_policy_integration", - "challengeTypes": ["simpleHttp", "http-01", "tls-sni-01"] + "challengeTypes": ["simpleHttp", "dvsni", "http-01", "tls-sni-01"] }, "ra": { From fe047a1da86745900f6a66583ff30278e6ed5716 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Tue, 3 Nov 2015 23:17:26 +0900 Subject: [PATCH 13/26] Change config to flags from strings --- cmd/boulder-ca/main.go | 2 +- cmd/boulder-ra/main.go | 2 +- cmd/cert-checker/main.go | 4 ++-- cmd/shell.go | 31 ++++++++++++++++++++++++++++++- policy/policy-authority.go | 15 +++++---------- policy/policy-authority_test.go | 10 ++++++++-- ra/registration-authority_test.go | 9 ++++++--- 7 files changed, 53 insertions(+), 20 deletions(-) diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index 1b83bab78..c96e2f7f3 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -36,7 +36,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Couldn't connect to policy database") - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.ChallengeTypes) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.SupportedChallenges()) cmd.FailOnError(err, "Couldn't create PA") cai, err := ca.NewCertificateAuthorityImpl(c.CA, clock.Default(), stats, c.Common.IssuerCert) diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index 1b6740973..27a736ef8 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -40,7 +40,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Couldn't connect to policy database") - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.ChallengeTypes) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.SupportedChallenges()) cmd.FailOnError(err, "Couldn't create PA") rateLimitPolicies, err := cmd.LoadRateLimitPolicies(c.RA.RateLimitPoliciesFilename) diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index ee55b63dd..0291c253c 100644 --- a/cmd/cert-checker/main.go +++ b/cmd/cert-checker/main.go @@ -76,7 +76,7 @@ type certChecker struct { issuedReport report } -func newChecker(saDbMap *gorp.DbMap, paDbMap *gorp.DbMap, clk clock.Clock, enforceWhitelist bool, challengeTypes []string) certChecker { +func newChecker(saDbMap *gorp.DbMap, paDbMap *gorp.DbMap, clk clock.Clock, enforceWhitelist bool, challengeTypes map[string]bool) certChecker { pa, err := policy.NewPolicyAuthorityImpl(paDbMap, enforceWhitelist, challengeTypes) cmd.FailOnError(err, "Failed to create PA") c := certChecker{ @@ -250,7 +250,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Could not connect to policy database") - checker := newChecker(saDbMap, paDbMap, clock.Default(), c.PA.EnforcePolicyWhitelist, c.PA.ChallengeTypes) + checker := newChecker(saDbMap, paDbMap, clock.Default(), c.PA.EnforcePolicyWhitelist, c.PA.SupportedChallenges()) auditlogger.Info("# Getting certificates issued in the last 90 days") // Since we grab certificates in batches we don't want this to block, when it diff --git a/cmd/shell.go b/cmd/shell.go index 57f2ccf4e..9dda9f6b6 100644 --- a/cmd/shell.go +++ b/cmd/shell.go @@ -260,7 +260,36 @@ type CAConfig struct { type PAConfig struct { DBConnect string EnforcePolicyWhitelist bool - ChallengeTypes []string + + EnableSimpleHTTP bool // TODO(#894) Remove this line + EnableDVSNI bool // TODO(#894) Remove this line + EnableHTTP01 bool + EnableTLSSNI01 bool + EnableDNS01 bool +} + +// SupportedChallenges returns the set of challenges supported by the +// configuration, as a map[string]bool. +func (pa PAConfig) SupportedChallenges() map[string]bool { + challenges := map[string]bool{} + + if pa.EnableSimpleHTTP { + challenges[core.ChallengeTypeSimpleHTTP] = true + } + if pa.EnableDVSNI { + challenges[core.ChallengeTypeDVSNI] = true + } + if pa.EnableHTTP01 { + challenges[core.ChallengeTypeHTTP01] = true + } + if pa.EnableTLSSNI01 { + challenges[core.ChallengeTypeTLSSNI01] = true + } + if pa.EnableDNS01 { + challenges[core.ChallengeTypeDNS01] = true + } + + return challenges } // KeyConfig should contain either a File path to a PEM-format private key, diff --git a/policy/policy-authority.go b/policy/policy-authority.go index c9f6bf16b..032ceb17a 100644 --- a/policy/policy-authority.go +++ b/policy/policy-authority.go @@ -28,7 +28,7 @@ type PolicyAuthorityImpl struct { } // NewPolicyAuthorityImpl constructs a Policy Authority. -func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool, challengeTypes []string) (*PolicyAuthorityImpl, error) { +func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool, challengeTypes map[string]bool) (*PolicyAuthorityImpl, error) { logger := blog.GetAuditLogger() logger.Notice("Policy Authority Starting") @@ -38,15 +38,10 @@ func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool, challengeT return nil, err } pa := PolicyAuthorityImpl{ - log: logger, - DB: padb, - EnforceWhitelist: enforceWhitelist, - } - - // Take note of which challenges to offer - pa.supportedChallenges = map[string]bool{} - for _, challengeType := range challengeTypes { - pa.supportedChallenges[challengeType] = true + log: logger, + DB: padb, + EnforceWhitelist: enforceWhitelist, + supportedChallenges: challengeTypes, } return &pa, nil diff --git a/policy/policy-authority_test.go b/policy/policy-authority_test.go index cbbf7ac5f..34ec774ac 100644 --- a/policy/policy-authority_test.go +++ b/policy/policy-authority_test.go @@ -21,7 +21,13 @@ import ( var log = mocks.UseMockLog() -var supportedChallenges = []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01} +var supportedChallenges = map[string]bool{ + core.ChallengeTypeSimpleHTTP: true, + core.ChallengeTypeDVSNI: true, + core.ChallengeTypeHTTP01: true, + core.ChallengeTypeTLSSNI01: true, + core.ChallengeTypeDNS01: true, +} func paImpl(t *testing.T) (*PolicyAuthorityImpl, func()) { dbMap, cleanUp := paDBMap(t) @@ -212,7 +218,7 @@ func TestChallengesFor(t *testing.T) { test.Assert(t, len(challenges) == len(supportedChallenges), "Wrong number of challenges returned") test.Assert(t, len(combinations) == len(supportedChallenges), "Wrong number of combinations returned") for i, challenge := range challenges { - test.AssertEquals(t, challenge.Type, supportedChallenges[i]) + test.Assert(t, supportedChallenges[challenge.Type], "Unsupported challenge returned") test.AssertEquals(t, len(combinations[i]), 1) test.AssertEquals(t, combinations[i][0], i) } diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index b2d1df8fb..1dce41d72 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -51,7 +51,10 @@ func (dva *DummyValidationAuthority) CheckCAARecords(identifier core.AcmeIdentif } var ( - SupportedChallenges = []string{core.ChallengeTypeHTTP01, core.ChallengeTypeTLSSNI01} + SupportedChallenges = map[string]bool{ + core.ChallengeTypeHTTP01: true, + core.ChallengeTypeTLSSNI01: true, + } // These values we simulate from the client AccountKeyJSONA = []byte(`{ @@ -393,8 +396,8 @@ func TestNewAuthorization(t *testing.T) { // TODO Verify that challenges are correct test.Assert(t, len(authz.Challenges) == len(SupportedChallenges), "Incorrect number of challenges returned") - test.AssertEquals(t, authz.Challenges[0].Type, SupportedChallenges[0]) - test.AssertEquals(t, authz.Challenges[1].Type, SupportedChallenges[1]) + test.Assert(t, SupportedChallenges[authz.Challenges[0].Type], fmt.Sprintf("Unsupported challenge: %s", authz.Challenges[0].Type)) + test.Assert(t, SupportedChallenges[authz.Challenges[1].Type], fmt.Sprintf("Unsupported challenge: %s", authz.Challenges[1].Type)) test.Assert(t, authz.Challenges[0].IsSane(false), "Challenge 0 is not sane") test.Assert(t, authz.Challenges[1].IsSane(false), "Challenge 1 is not sane") From 637f1eba61c63bc9826d23c39fcddfe6252755c6 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Tue, 3 Nov 2015 23:28:18 +0900 Subject: [PATCH 14/26] Update boulder test config to enable all challenges --- test/boulder-config.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/boulder-config.json b/test/boulder-config.json index 5b65459b4..1071f22d3 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -117,7 +117,11 @@ "pa": { "dbConnect": "mysql+tcp://policy@localhost:3306/boulder_policy_integration", - "challengeTypes": ["simpleHttp", "dvsni", "http-01", "tls-sni-01"] + "enableSimpleHTTP": true, + "enableDVSNI": true, + "enableHTTP01": true, + "enableTLSSNI01": true, + "enableDNS01": true }, "ra": { From 3df0592b150bd2dc7a4b4f1e44ce02d05809ef94 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Fri, 6 Nov 2015 17:59:05 -0800 Subject: [PATCH 15/26] move sourcing of activate into test.sh This gets us closer to allowing the client repo to use integration-test.py. They have a different path without "venv" in it for their virtualenv set up. Updates #1101 --- test.sh | 2 ++ test/integration-test.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test.sh b/test.sh index 79c94df7b..5193faed0 100755 --- a/test.sh +++ b/test.sh @@ -127,6 +127,8 @@ function build_letsencrypt() { run ./venv/bin/pip install -U pip run ./venv/bin/pip install -e acme -e . -e letsencrypt-apache -e letsencrypt-nginx + source ./venv/bin/activate + cd - } diff --git a/test/integration-test.py b/test/integration-test.py index e24b3f5e0..ac810bd55 100644 --- a/test/integration-test.py +++ b/test/integration-test.py @@ -209,7 +209,7 @@ def run_client_tests(): "Please set LETSENCRYPT_PATH env variable to point at " "initialized (virtualenv) client repo root") test_script_path = os.path.join(root, 'tests', 'boulder-integration.sh') - cmd = "source %s/venv/bin/activate && SIMPLE_HTTP_PORT=5002 %s" % (root, test_script_path) + cmd = "SIMPLE_HTTP_PORT=5002 %s" % (test_script_path) if subprocess.Popen(cmd, shell=True, cwd=root, executable='/bin/bash').wait() != 0: die(ExitStatus.PythonFailure) From f61183e1445ddd0d7df15f9ae2df901908ba1524 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 7 Nov 2015 12:39:57 -0500 Subject: [PATCH 16/26] Use a map and set defaults --- cmd/boulder-ca/main.go | 2 +- cmd/boulder-ra/main.go | 2 +- cmd/cert-checker/main.go | 2 +- cmd/shell.go | 58 ++++++++++++++++++--------------- core/core_test.go | 10 ++++++ core/objects.go | 21 ++++++++++++ policy/policy-authority.go | 23 ++++++------- policy/policy-authority_test.go | 10 +++--- test/boulder-config.json | 12 ++++--- 9 files changed, 90 insertions(+), 50 deletions(-) diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index c96e2f7f3..6dba26348 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -36,7 +36,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Couldn't connect to policy database") - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.SupportedChallenges()) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.Challenges) cmd.FailOnError(err, "Couldn't create PA") cai, err := ca.NewCertificateAuthorityImpl(c.CA, clock.Default(), stats, c.Common.IssuerCert) diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index 44e214daf..ad4fb6f7d 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -40,7 +40,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Couldn't connect to policy database") - pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.SupportedChallenges()) + pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist, c.PA.Challenges) cmd.FailOnError(err, "Couldn't create PA") rateLimitPolicies, err := cmd.LoadRateLimitPolicies(c.RA.RateLimitPoliciesFilename) diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index 0291c253c..e7812047f 100644 --- a/cmd/cert-checker/main.go +++ b/cmd/cert-checker/main.go @@ -250,7 +250,7 @@ func main() { paDbMap, err := sa.NewDbMap(c.PA.DBConnect) cmd.FailOnError(err, "Could not connect to policy database") - checker := newChecker(saDbMap, paDbMap, clock.Default(), c.PA.EnforcePolicyWhitelist, c.PA.SupportedChallenges()) + checker := newChecker(saDbMap, paDbMap, clock.Default(), c.PA.EnforcePolicyWhitelist, c.PA.Challenges) auditlogger.Info("# Getting certificates issued in the last 90 days") // Since we grab certificates in batches we don't want this to block, when it diff --git a/cmd/shell.go b/cmd/shell.go index 99e0fe21d..f60d3a69a 100644 --- a/cmd/shell.go +++ b/cmd/shell.go @@ -265,36 +265,42 @@ type CAConfig struct { type PAConfig struct { DBConnect string EnforcePolicyWhitelist bool - - EnableSimpleHTTP bool // TODO(#894) Remove this line - EnableDVSNI bool // TODO(#894) Remove this line - EnableHTTP01 bool - EnableTLSSNI01 bool - EnableDNS01 bool + Challenges map[string]bool } -// SupportedChallenges returns the set of challenges supported by the -// configuration, as a map[string]bool. -func (pa PAConfig) SupportedChallenges() map[string]bool { - challenges := map[string]bool{} - - if pa.EnableSimpleHTTP { - challenges[core.ChallengeTypeSimpleHTTP] = true - } - if pa.EnableDVSNI { - challenges[core.ChallengeTypeDVSNI] = true - } - if pa.EnableHTTP01 { - challenges[core.ChallengeTypeHTTP01] = true - } - if pa.EnableTLSSNI01 { - challenges[core.ChallengeTypeTLSSNI01] = true - } - if pa.EnableDNS01 { - challenges[core.ChallengeTypeDNS01] = true +// UnmarshalJSON is really actually vanilla, but with some validity checks and +// default setting added +func (pc *PAConfig) UnmarshalJSON(b []byte) error { + raw := struct { + DBConnect string + EnforcePolicyWhitelist bool + Challenges map[string]bool + }{} + err := json.Unmarshal(b, &raw) + if err != nil { + return err } - return challenges + // Set a default list of challenges if non are provided + if len(raw.Challenges) == 0 { + raw.Challenges = map[string]bool{} + raw.Challenges[core.ChallengeTypeSimpleHTTP] = true + raw.Challenges[core.ChallengeTypeDVSNI] = true + raw.Challenges[core.ChallengeTypeHTTP01] = true + raw.Challenges[core.ChallengeTypeTLSSNI01] = true + } + + // Check that the entries in the challenges map are valid + for name := range raw.Challenges { + if !core.ValidChallenge(name) { + return fmt.Errorf("Invalid challenge in PA config: %s", name) + } + } + + pc.DBConnect = raw.DBConnect + pc.EnforcePolicyWhitelist = raw.EnforcePolicyWhitelist + pc.Challenges = raw.Challenges + return nil } // KeyConfig should contain either a File path to a PEM-format private key, diff --git a/core/core_test.go b/core/core_test.go index f1a6bb2da..dc738983a 100644 --- a/core/core_test.go +++ b/core/core_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" + "github.com/letsencrypt/boulder/test" ) // challenges.go @@ -56,6 +57,15 @@ func TestChallenges(t *testing.T) { if !dns01.IsSane(false) { t.Errorf("New dns-01 challenge is not sane: %v", dns01) } + + // TODO(#894): Remove these lines + test.Assert(t, ValidChallenge(ChallengeTypeSimpleHTTP), "Refused valid challenge") + test.Assert(t, ValidChallenge(ChallengeTypeDVSNI), "Refused valid challenge") + + test.Assert(t, ValidChallenge(ChallengeTypeHTTP01), "Refused valid challenge") + test.Assert(t, ValidChallenge(ChallengeTypeTLSSNI01), "Refused valid challenge") + test.Assert(t, ValidChallenge(ChallengeTypeDNS01), "Refused valid challenge") + test.Assert(t, !ValidChallenge("nonsense-71"), "Accepted invalid challenge") } // objects.go diff --git a/core/objects.go b/core/objects.go index 6081e738e..459df2de2 100644 --- a/core/objects.go +++ b/core/objects.go @@ -97,6 +97,27 @@ const ( ChallengeTypeDNS01 = "dns-01" ) +// ValidChallenge tests whether the provided string names a known challenge +func ValidChallenge(name string) bool { + switch name { + // TODO(#894): Delete these lines + case ChallengeTypeSimpleHTTP: + fallthrough + case ChallengeTypeDVSNI: + fallthrough + + case ChallengeTypeHTTP01: + fallthrough + case ChallengeTypeTLSSNI01: + fallthrough + case ChallengeTypeDNS01: + return true + + default: + return false + } +} + // TLSSNISuffix is appended to pseudo-domain names in DVSNI challenges const TLSSNISuffix = "acme.invalid" diff --git a/policy/policy-authority.go b/policy/policy-authority.go index 032ceb17a..74e396747 100644 --- a/policy/policy-authority.go +++ b/policy/policy-authority.go @@ -23,8 +23,8 @@ type PolicyAuthorityImpl struct { log *blog.AuditLogger DB *PolicyAuthorityDatabaseImpl - EnforceWhitelist bool - supportedChallenges map[string]bool + EnforceWhitelist bool + enabledChallenges map[string]bool } // NewPolicyAuthorityImpl constructs a Policy Authority. @@ -37,11 +37,12 @@ func NewPolicyAuthorityImpl(dbMap *gorp.DbMap, enforceWhitelist bool, challengeT if err != nil { return nil, err } + pa := PolicyAuthorityImpl{ - log: logger, - DB: padb, - EnforceWhitelist: enforceWhitelist, - supportedChallenges: challengeTypes, + log: logger, + DB: padb, + EnforceWhitelist: enforceWhitelist, + enabledChallenges: challengeTypes, } return &pa, nil @@ -210,24 +211,24 @@ func (pa PolicyAuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, acco combinations = [][]int{} // TODO(https://github.com/letsencrypt/boulder/issues/894): Remove this block - if pa.supportedChallenges[core.ChallengeTypeSimpleHTTP] { + if pa.enabledChallenges[core.ChallengeTypeSimpleHTTP] { challenges = append(challenges, core.SimpleHTTPChallenge(accountKey)) } // TODO(https://github.com/letsencrypt/boulder/issues/894): Remove this block - if pa.supportedChallenges[core.ChallengeTypeDVSNI] { + if pa.enabledChallenges[core.ChallengeTypeDVSNI] { challenges = append(challenges, core.DvsniChallenge(accountKey)) } - if pa.supportedChallenges[core.ChallengeTypeHTTP01] { + if pa.enabledChallenges[core.ChallengeTypeHTTP01] { challenges = append(challenges, core.HTTPChallenge01(accountKey)) } - if pa.supportedChallenges[core.ChallengeTypeTLSSNI01] { + if pa.enabledChallenges[core.ChallengeTypeTLSSNI01] { challenges = append(challenges, core.TLSSNIChallenge01(accountKey)) } - if pa.supportedChallenges[core.ChallengeTypeDNS01] { + if pa.enabledChallenges[core.ChallengeTypeDNS01] { challenges = append(challenges, core.DNSChallenge01(accountKey)) } diff --git a/policy/policy-authority_test.go b/policy/policy-authority_test.go index 34ec774ac..9a20d0c06 100644 --- a/policy/policy-authority_test.go +++ b/policy/policy-authority_test.go @@ -21,7 +21,7 @@ import ( var log = mocks.UseMockLog() -var supportedChallenges = map[string]bool{ +var enabledChallenges = map[string]bool{ core.ChallengeTypeSimpleHTTP: true, core.ChallengeTypeDVSNI: true, core.ChallengeTypeHTTP01: true, @@ -31,7 +31,7 @@ var supportedChallenges = map[string]bool{ func paImpl(t *testing.T) (*PolicyAuthorityImpl, func()) { dbMap, cleanUp := paDBMap(t) - pa, err := NewPolicyAuthorityImpl(dbMap, false, supportedChallenges) + pa, err := NewPolicyAuthorityImpl(dbMap, false, enabledChallenges) if err != nil { cleanUp() t.Fatalf("Couldn't create policy implementation: %s", err) @@ -215,10 +215,10 @@ func TestChallengesFor(t *testing.T) { t.Errorf("Error generating challenges: %v", err) } - test.Assert(t, len(challenges) == len(supportedChallenges), "Wrong number of challenges returned") - test.Assert(t, len(combinations) == len(supportedChallenges), "Wrong number of combinations returned") + test.Assert(t, len(challenges) == len(enabledChallenges), "Wrong number of challenges returned") + test.Assert(t, len(combinations) == len(enabledChallenges), "Wrong number of combinations returned") for i, challenge := range challenges { - test.Assert(t, supportedChallenges[challenge.Type], "Unsupported challenge returned") + test.Assert(t, enabledChallenges[challenge.Type], "Unsupported challenge returned") test.AssertEquals(t, len(combinations[i]), 1) test.AssertEquals(t, combinations[i][0], i) } diff --git a/test/boulder-config.json b/test/boulder-config.json index 1071f22d3..3e6996fcc 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -117,11 +117,13 @@ "pa": { "dbConnect": "mysql+tcp://policy@localhost:3306/boulder_policy_integration", - "enableSimpleHTTP": true, - "enableDVSNI": true, - "enableHTTP01": true, - "enableTLSSNI01": true, - "enableDNS01": true + "challenges": { + "simpleHttp": true, + "dvsni": true, + "http-01": true, + "tls-sni-01": true, + "dns-01": true + } }, "ra": { From 38f553c48bf7620092c5feac49e4592948a0c257 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sat, 7 Nov 2015 16:15:42 -0500 Subject: [PATCH 17/26] Add tests for ValidChallenge --- cmd/shell_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 cmd/shell_test.go diff --git a/cmd/shell_test.go b/cmd/shell_test.go new file mode 100644 index 000000000..2a73d2bb9 --- /dev/null +++ b/cmd/shell_test.go @@ -0,0 +1,39 @@ +package cmd + +import ( + "encoding/json" + "testing" + + "github.com/letsencrypt/boulder/test" +) + +var ( + validPAConfig = []byte(`{ + "dbConnect": "dummyDBConnect", + "enforcePolicyWhitelist": false, + "challenges": { "simpleHttp": true } +}`) + invalidPAConfig = []byte(`{ + "dbConnect": "dummyDBConnect", + "enforcePolicyWhitelist": false, + "challenges": { "nonsense": true } +}`) + noChallengesPAConfig = []byte(`{ + "dbConnect": "dummyDBConnect", + "enforcePolicyWhitelist": false +}`) +) + +func TestPAConfigUnmarshal(t *testing.T) { + var pc PAConfig + + err := json.Unmarshal(validPAConfig, &pc) + test.AssertNotError(t, err, "Failed to unmarshal valid PAConfig") + + err = json.Unmarshal(invalidPAConfig, &pc) + test.AssertError(t, err, "Failed to reject invalid PAConfig") + + err = json.Unmarshal(noChallengesPAConfig, &pc) + test.AssertNotError(t, err, "Failed to unmarshal valid PAConfig") + test.Assert(t, len(pc.Challenges) == 4, "Incorrect number of challenges in default set") +} From ddb0b236035220e93a540de9e37decac45058315 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Sun, 8 Nov 2015 17:46:01 -0800 Subject: [PATCH 18/26] Add StatsD metric for lag between message publishing and server processing --- rpc/amqp-rpc.go | 11 +++++++++++ rpc/connection.go | 1 + 2 files changed, 12 insertions(+) diff --git a/rpc/amqp-rpc.go b/rpc/amqp-rpc.go index 4fdccd97c..4c5430ea8 100644 --- a/rpc/amqp-rpc.go +++ b/rpc/amqp-rpc.go @@ -22,6 +22,7 @@ import ( "time" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/streadway/amqp" "github.com/letsencrypt/boulder/cmd" @@ -170,6 +171,8 @@ type AmqpRPCServer struct { currentGoroutines int64 maxConcurrentRPCServerRequests int64 tooManyRequestsResponse []byte + stats statsd.Statter + clk clock.Clock } // NewAmqpRPCServer creates a new RPC server for the given queue and will begin @@ -186,12 +189,19 @@ func NewAmqpRPCServer(serverQueue string, maxConcurrentRPCServerRequests int64, reconnectMax = time.Minute } + stats, err := statsd.NewClient(c.Statsd.Server, c.Statsd.Prefix) + if err != nil { + return nil, err + } + return &AmqpRPCServer{ serverQueue: serverQueue, connection: newAMQPConnector(serverQueue, reconnectBase, reconnectMax), log: log, dispatchTable: make(map[string]func([]byte) ([]byte, error)), maxConcurrentRPCServerRequests: maxConcurrentRPCServerRequests, + clk: clock.Default(), + stats: stats, }, nil } @@ -426,6 +436,7 @@ func (rpc *AmqpRPCServer) Start(c cmd.Config) error { select { case msg, ok := <-rpc.connection.messages(): if ok { + rpc.stats.TimingDuration(fmt.Sprintf("RPC.MessageLag.%s", msg.Type), rpc.clk.Now().Sub(msg.Timestamp), 1.0) if rpc.maxConcurrentRPCServerRequests > 0 && atomic.LoadInt64(&rpc.currentGoroutines) >= rpc.maxConcurrentRPCServerRequests { rpc.replyTooManyRequests(msg) break // this breaks the select, not the for diff --git a/rpc/connection.go b/rpc/connection.go index df9f9aec0..585595add 100644 --- a/rpc/connection.go +++ b/rpc/connection.go @@ -129,6 +129,7 @@ func (ac *amqpConnector) publish(queueName, corrId, expiration, replyTo, msgType Expiration: expiration, ReplyTo: replyTo, Type: msgType, + Timestamp: ac.clk.Now(), }) } From 7ed558c32a2d7a6c26a8dae68ea02282224af6f1 Mon Sep 17 00:00:00 2001 From: EKR Date: Sun, 8 Nov 2015 18:17:24 -0800 Subject: [PATCH 19/26] Update README --- README.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 628316e06..5d9e11263 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ recent version. Also, Boulder requires Go 1.5. As of September 2015 this version is not yet available in OS repositories, so you will have to install from https://golang.org/dl/. +Add ```${GOPATH}/bin``` to your path. Ubuntu: @@ -47,20 +48,24 @@ or sudo port install libtool mariadb-server rabbitmq-server -(On OS X, using port, you will have to add `CGO_CFLAGS="-I/opt/local/include" CGO_LDFLAGS="-L/opt/local/lib"` to your environment or `go` invocations.) +Note: +(On OS X, using port, you will have to add `CGO_CFLAGS="-I/opt/local/include" CGO_LDFLAGS="-L/opt/local/lib"` to your environment or `go` invocations.) + > go get bitbucket.org/liamstask/goose/cmd/goose + > go get github.com/jsha/listenbuddy > go get github.com/letsencrypt/boulder/ # Ignore errors about no buildable files > cd $GOPATH/src/github.com/letsencrypt/boulder > ./test/create_db.sh # This starts each Boulder component with test configs. Ctrl-C kills all. > ./start.py # Run tests + > go get -u github.com/golang/lint/golint > ./test.sh -Note: `create_db.sh` it uses the root MariaDB user, so if you -have disabled that account you may have to adjust the file or -recreate the commands. +Note: `create_db.sh` it uses the root MariaDB user with the default +password, so if you have disabled that account or changed the password +you may have to adjust the file or recreate the commands. You can also check out the official client from https://github.com/letsencrypt/letsencrypt/ and follow the setup From 2e72c9d33d3f6b0bffa62431522503ed62e88e55 Mon Sep 17 00:00:00 2001 From: EKR Date: Sun, 8 Nov 2015 18:18:57 -0800 Subject: [PATCH 20/26] Editorial --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index 5d9e11263..37c60cfe1 100644 --- a/README.md +++ b/README.md @@ -48,8 +48,6 @@ or sudo port install libtool mariadb-server rabbitmq-server -Note: - (On OS X, using port, you will have to add `CGO_CFLAGS="-I/opt/local/include" CGO_LDFLAGS="-L/opt/local/lib"` to your environment or `go` invocations.) > go get bitbucket.org/liamstask/goose/cmd/goose From b2e7a3354d0fd529da349278e345fbb4024736ae Mon Sep 17 00:00:00 2001 From: EKR Date: Sun, 8 Nov 2015 18:19:47 -0800 Subject: [PATCH 21/26] Fix golint output --- cmd/boulder-ra/main.go | 2 +- ra/registration-authority_test.go | 2 +- ra/safe.go | 2 +- va/gsb.go | 6 +++--- va/gsb_test.go | 10 +++++----- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index 59fb55f64..ff875fc5c 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -68,7 +68,7 @@ func main() { var dc *ra.DomainCheck if c.RA.UseIsSafeDomain { - dc = &ra.DomainCheck{&vac} + dc = &ra.DomainCheck{VA: &vac} } rai := ra.NewRegistrationAuthorityImpl(clock.Default(), auditlogger, stats, diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index 51fe99d9d..da7bdff62 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -56,7 +56,7 @@ func (dva *DummyValidationAuthority) IsSafeDomain(req *core.IsSafeDomainRequest) if dva.IsSafeDomainErr != nil { return nil, dva.IsSafeDomainErr } - return &core.IsSafeDomainResponse{!dva.IsNotSafe}, nil + return &core.IsSafeDomainResponse{IsSafe: !dva.IsNotSafe}, nil } var ( diff --git a/ra/safe.go b/ra/safe.go index 733a345c6..64dbfd264 100644 --- a/ra/safe.go +++ b/ra/safe.go @@ -25,7 +25,7 @@ func (d *DomainCheck) IsSafe(domain string) (bool, error) { return true, nil } - resp, err := d.VA.IsSafeDomain(&core.IsSafeDomainRequest{domain}) + resp, err := d.VA.IsSafeDomain(&core.IsSafeDomainRequest{Domain: domain}) if err != nil { return false, err } diff --git a/va/gsb.go b/va/gsb.go index a78b40e94..b5905eac2 100644 --- a/va/gsb.go +++ b/va/gsb.go @@ -29,7 +29,7 @@ func (va *ValidationAuthorityImpl) IsSafeDomain(req *core.IsSafeDomainRequest) ( va.stats.Inc("VA.IsSafeDomain.Requests", 1, 1.0) if va.SafeBrowsing == nil { va.stats.Inc("VA.IsSafeDomain.Skips", 1, 1.0) - return &core.IsSafeDomainResponse{true}, nil + return &core.IsSafeDomainResponse{IsSafe: true}, nil } list, err := va.SafeBrowsing.IsListed(req.Domain) @@ -37,7 +37,7 @@ func (va *ValidationAuthorityImpl) IsSafeDomain(req *core.IsSafeDomainRequest) ( va.stats.Inc("VA.IsSafeDomain.Errors", 1, 1.0) if err == safebrowsing.ErrOutOfDateHashes { va.stats.Inc("VA.IsSafeDomain.OutOfDateHashErrors", 1, 1.0) - return &core.IsSafeDomainResponse{true}, nil + return &core.IsSafeDomainResponse{IsSafe: true}, nil } return nil, err } @@ -48,5 +48,5 @@ func (va *ValidationAuthorityImpl) IsSafeDomain(req *core.IsSafeDomainRequest) ( } else { va.stats.Inc("VA.IsSafeDomain.Status.Bad", 1, 1.0) } - return &core.IsSafeDomainResponse{status}, nil + return &core.IsSafeDomainResponse{IsSafe: status}, nil } diff --git a/va/gsb_test.go b/va/gsb_test.go index 6b951dde3..97a2b53a4 100644 --- a/va/gsb_test.go +++ b/va/gsb_test.go @@ -34,25 +34,25 @@ func TestIsSafeDomain(t *testing.T) { sbc.EXPECT().IsListed("outofdate.com").Return("", safebrowsing.ErrOutOfDateHashes) va := NewValidationAuthorityImpl(&PortConfig{}, sbc, stats, clock.NewFake()) - resp, err := va.IsSafeDomain(&core.IsSafeDomainRequest{"good.com"}) + resp, err := va.IsSafeDomain(&core.IsSafeDomainRequest{Domain: "good.com"}) if err != nil { t.Errorf("good.com: want no error, got '%s'", err) } if !resp.IsSafe { t.Errorf("good.com: want true, got %t", resp.IsSafe) } - resp, err = va.IsSafeDomain(&core.IsSafeDomainRequest{"bad.com"}) + resp, err = va.IsSafeDomain(&core.IsSafeDomainRequest{Domain: "bad.com"}) if err != nil { t.Errorf("bad.com: want no error, got '%s'", err) } if resp.IsSafe { t.Errorf("bad.com: want false, got %t", resp.IsSafe) } - _, err = va.IsSafeDomain(&core.IsSafeDomainRequest{"errorful.com"}) + _, err = va.IsSafeDomain(&core.IsSafeDomainRequest{Domain: "errorful.com"}) if err == nil { t.Errorf("errorful.com: want error, got none") } - resp, err = va.IsSafeDomain(&core.IsSafeDomainRequest{"outofdate.com"}) + resp, err = va.IsSafeDomain(&core.IsSafeDomainRequest{Domain: "outofdate.com"}) if err != nil { t.Errorf("outofdate.com: want no error, got '%s'", err) } @@ -67,7 +67,7 @@ func TestAllowNilInIsSafeDomain(t *testing.T) { // Be cool with a nil SafeBrowsing. This will happen in prod when we have // flag mismatch between the VA and RA. - resp, err := va.IsSafeDomain(&core.IsSafeDomainRequest{"example.com"}) + resp, err := va.IsSafeDomain(&core.IsSafeDomainRequest{Domain: "example.com"}) if err != nil { t.Errorf("nil SafeBrowsing, unexpected error: %s", err) } else if !resp.IsSafe { From eb44c76c438347c653527a7b85e05160b45601ce Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Sun, 8 Nov 2015 18:37:57 -0800 Subject: [PATCH 22/26] Switch from message type to server queue name --- rpc/amqp-rpc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/amqp-rpc.go b/rpc/amqp-rpc.go index 4c5430ea8..61f342ac4 100644 --- a/rpc/amqp-rpc.go +++ b/rpc/amqp-rpc.go @@ -436,7 +436,7 @@ func (rpc *AmqpRPCServer) Start(c cmd.Config) error { select { case msg, ok := <-rpc.connection.messages(): if ok { - rpc.stats.TimingDuration(fmt.Sprintf("RPC.MessageLag.%s", msg.Type), rpc.clk.Now().Sub(msg.Timestamp), 1.0) + rpc.stats.TimingDuration(fmt.Sprintf("RPC.MessageLag.%s", rpc.serverQueue), rpc.clk.Now().Sub(msg.Timestamp), 1.0) if rpc.maxConcurrentRPCServerRequests > 0 && atomic.LoadInt64(&rpc.currentGoroutines) >= rpc.maxConcurrentRPCServerRequests { rpc.replyTooManyRequests(msg) break // this breaks the select, not the for From 50a770e45094d6edf4b8f191eb47bedaaac2dc8a Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Sun, 8 Nov 2015 18:55:51 -0800 Subject: [PATCH 23/26] Fix connection test --- rpc/connection_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rpc/connection_test.go b/rpc/connection_test.go index 89c5926d1..f86b19558 100644 --- a/rpc/connection_test.go +++ b/rpc/connection_test.go @@ -31,6 +31,7 @@ func setup(t *testing.T) (*amqpConnector, *MockamqpChannel, func()) { }, queueName: "fooqueue", retryTimeoutBase: time.Second, + clk: clock.NewFake(), } return &ac, mockChannel, func() { mockCtrl.Finish() } } @@ -125,6 +126,7 @@ func TestPublish(t *testing.T) { Expiration: "3000", ReplyTo: "replyTo", Type: "testMsg", + Timestamp: ac.clk.Now(), }) ac.publish("fooqueue", "03c52e", "3000", "replyTo", "testMsg", []byte("body")) } From 174011f6d8e90196424c6b9ec935211d32c1f920 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 9 Nov 2015 15:30:13 -0500 Subject: [PATCH 24/26] Move validation and defaults out of UnmarshalJSON --- cmd/boulder-ca/main.go | 4 ++++ cmd/boulder-ra/main.go | 4 ++++ cmd/cert-checker/main.go | 4 ++++ cmd/shell.go | 46 ++++++++++++++++------------------------ cmd/shell_test.go | 23 ++++++++++++-------- 5 files changed, 44 insertions(+), 37 deletions(-) diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index 6dba26348..1a5d7b5df 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -27,6 +27,10 @@ func main() { cmd.FailOnError(err, "Could not connect to Syslog") auditlogger.Info(app.VersionString()) + // Validate PA config and set defaults if needed + cmd.FailOnError(c.PA.CheckChallenges(), "Invalid PA configuration") + c.PA.SetDefaultChallengesIfEmpty() + // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 defer auditlogger.AuditPanic() diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index 79014b1d6..f6f287a06 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -31,6 +31,10 @@ func main() { cmd.FailOnError(err, "Could not connect to Syslog") auditlogger.Info(app.VersionString()) + // Validate PA config and set defaults if needed + cmd.FailOnError(c.PA.CheckChallenges(), "Invalid PA configuration") + c.PA.SetDefaultChallengesIfEmpty() + // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 defer auditlogger.AuditPanic() diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index e7812047f..0ba52cc08 100644 --- a/cmd/cert-checker/main.go +++ b/cmd/cert-checker/main.go @@ -235,6 +235,10 @@ func main() { } app.Action = func(c cmd.Config) { + // Validate PA config and set defaults if needed + cmd.FailOnError(c.PA.CheckChallenges(), "Invalid PA configuration") + c.PA.SetDefaultChallengesIfEmpty() + stats, err := statsd.NewClient(c.Statsd.Server, c.Statsd.Prefix) cmd.FailOnError(err, "Couldn't connect to statsd") diff --git a/cmd/shell.go b/cmd/shell.go index f60d3a69a..f69bfed55 100644 --- a/cmd/shell.go +++ b/cmd/shell.go @@ -268,41 +268,31 @@ type PAConfig struct { Challenges map[string]bool } -// UnmarshalJSON is really actually vanilla, but with some validity checks and -// default setting added -func (pc *PAConfig) UnmarshalJSON(b []byte) error { - raw := struct { - DBConnect string - EnforcePolicyWhitelist bool - Challenges map[string]bool - }{} - err := json.Unmarshal(b, &raw) - if err != nil { - return err - } - - // Set a default list of challenges if non are provided - if len(raw.Challenges) == 0 { - raw.Challenges = map[string]bool{} - raw.Challenges[core.ChallengeTypeSimpleHTTP] = true - raw.Challenges[core.ChallengeTypeDVSNI] = true - raw.Challenges[core.ChallengeTypeHTTP01] = true - raw.Challenges[core.ChallengeTypeTLSSNI01] = true - } - - // Check that the entries in the challenges map are valid - for name := range raw.Challenges { +// CheckChallenges checks whether the list of challenges in the PA config +// actually contains valid challenge names +func (pc PAConfig) CheckChallenges() error { + for name := range pc.Challenges { if !core.ValidChallenge(name) { return fmt.Errorf("Invalid challenge in PA config: %s", name) } } - - pc.DBConnect = raw.DBConnect - pc.EnforcePolicyWhitelist = raw.EnforcePolicyWhitelist - pc.Challenges = raw.Challenges return nil } +// SetDefaultChallengesIfEmpty sets a default list of challenges if no +// challenges are enabled in the PA config. The set of challenges specified +// corresponds to the set that was hard-coded before these configuration +// options were added. +func (pc *PAConfig) SetDefaultChallengesIfEmpty() { + if len(pc.Challenges) == 0 { + pc.Challenges = map[string]bool{} + pc.Challenges[core.ChallengeTypeSimpleHTTP] = true + pc.Challenges[core.ChallengeTypeDVSNI] = true + pc.Challenges[core.ChallengeTypeHTTP01] = true + pc.Challenges[core.ChallengeTypeTLSSNI01] = true + } +} + // KeyConfig should contain either a File path to a PEM-format private key, // or a PKCS11Config defining how to load a module for an HSM. type KeyConfig struct { diff --git a/cmd/shell_test.go b/cmd/shell_test.go index 2a73d2bb9..6cbaa5d49 100644 --- a/cmd/shell_test.go +++ b/cmd/shell_test.go @@ -25,15 +25,20 @@ var ( ) func TestPAConfigUnmarshal(t *testing.T) { - var pc PAConfig + var pc1 PAConfig + err := json.Unmarshal(validPAConfig, &pc1) + test.AssertNotError(t, err, "Failed to unmarshal PAConfig") + test.AssertNotError(t, pc1.CheckChallenges(), "Flagged valid challenges as bad") - err := json.Unmarshal(validPAConfig, &pc) - test.AssertNotError(t, err, "Failed to unmarshal valid PAConfig") + var pc2 PAConfig + err = json.Unmarshal(invalidPAConfig, &pc2) + test.AssertNotError(t, err, "Failed to unmarshal PAConfig") + test.AssertError(t, pc2.CheckChallenges(), "Considered invalid challenges as good") - err = json.Unmarshal(invalidPAConfig, &pc) - test.AssertError(t, err, "Failed to reject invalid PAConfig") - - err = json.Unmarshal(noChallengesPAConfig, &pc) - test.AssertNotError(t, err, "Failed to unmarshal valid PAConfig") - test.Assert(t, len(pc.Challenges) == 4, "Incorrect number of challenges in default set") + var pc3 PAConfig + err = json.Unmarshal(noChallengesPAConfig, &pc3) + test.AssertNotError(t, err, "Failed to unmarshal PAConfig") + test.AssertNotError(t, pc3.CheckChallenges(), "Somehow found a bad challenge among none") + pc3.SetDefaultChallengesIfEmpty() + test.Assert(t, len(pc3.Challenges) == 4, "Incorrect number of challenges in default set") } From 88b8eb348033ad0eea390ed1bd0da5d9449509d0 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 10 Nov 2015 12:02:17 -0800 Subject: [PATCH 25/26] Add RPC server processing metrics and rename client RPC metrics --- rpc/amqp-rpc.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/rpc/amqp-rpc.go b/rpc/amqp-rpc.go index 61f342ac4..3c3ff6d96 100644 --- a/rpc/amqp-rpc.go +++ b/rpc/amqp-rpc.go @@ -439,12 +439,16 @@ func (rpc *AmqpRPCServer) Start(c cmd.Config) error { rpc.stats.TimingDuration(fmt.Sprintf("RPC.MessageLag.%s", rpc.serverQueue), rpc.clk.Now().Sub(msg.Timestamp), 1.0) if rpc.maxConcurrentRPCServerRequests > 0 && atomic.LoadInt64(&rpc.currentGoroutines) >= rpc.maxConcurrentRPCServerRequests { rpc.replyTooManyRequests(msg) + rpc.stats.Inc(fmt.Sprintf("RPC.CallsDropped.%s", rpc.serverQueue), 1, 1.0) break // this breaks the select, not the for } + rpc.stats.Inc(fmt.Sprintf("RPC.Traffic.Rx.%s", rpc.serverQueue), int64(len(msg.Body)), 1.0) go func() { atomic.AddInt64(&rpc.currentGoroutines, 1) defer atomic.AddInt64(&rpc.currentGoroutines, -1) + startedProcessing := rpc.clk.Now() rpc.processMessage(msg) + rpc.stats.TimingDuration(fmt.Sprintf("RPC.ServerProcessingLatency.%s", msg.Type), time.Since(startedProcessing), 1.0) }() } else { rpc.mu.RLock() @@ -634,10 +638,7 @@ func (rpc *AmqpRPCCLient) dispatch(method string, body []byte) (string, chan []b // DispatchSync sends a body to the destination, and blocks waiting on a response. func (rpc *AmqpRPCCLient) DispatchSync(method string, body []byte) (response []byte, err error) { - rpc.stats.Inc(fmt.Sprintf("RPC.Rate.%s", method), 1, 1.0) - rpc.stats.Inc("RPC.Traffic", int64(len(body)), 1.0) - rpc.stats.GaugeDelta("RPC.CallsWaiting", 1, 1.0) - defer rpc.stats.GaugeDelta("RPC.CallsWaiting", -1, 1.0) + rpc.stats.Inc(fmt.Sprintf("RPC.Traffic.Tx.%s", rpc.serverQueue), int64(len(body)), 1.0) callStarted := time.Now() corrID, responseChan := rpc.dispatch(method, body) select { @@ -649,15 +650,15 @@ func (rpc *AmqpRPCCLient) DispatchSync(method string, body []byte) (response []b } err = unwrapError(rpcResponse.Error) if err != nil { - rpc.stats.Inc(fmt.Sprintf("RPC.Latency.%s.Error", method), 1, 1.0) + rpc.stats.Inc(fmt.Sprintf("RPC.ClientCallLatency.%s.Error", method), 1, 1.0) return } rpc.stats.Inc("RPC.Rate.Success", 1, 1.0) - rpc.stats.TimingDuration(fmt.Sprintf("RPC.Latency.%s.Success", method), time.Since(callStarted), 1.0) + rpc.stats.TimingDuration(fmt.Sprintf("RPC.ClientCallLatency.%s.Success", method), time.Since(callStarted), 1.0) response = rpcResponse.ReturnVal return case <-time.After(rpc.timeout): - rpc.stats.TimingDuration(fmt.Sprintf("RPC.Latency.%s.Timeout", method), time.Since(callStarted), 1.0) + rpc.stats.TimingDuration(fmt.Sprintf("RPC.ClientCallLatency.%s.Timeout", method), time.Since(callStarted), 1.0) rpc.stats.Inc("RPC.Rate.Timeouts", 1, 1.0) rpc.log.Warning(fmt.Sprintf(" [c!][%s] AMQP-RPC timeout [%s]", rpc.clientQueue, method)) rpc.mu.Lock() From 77109fd60d0813a0c79d5ee8f8eb0a826d823c94 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 10 Nov 2015 12:50:04 -0800 Subject: [PATCH 26/26] Review fixes --- rpc/amqp-rpc.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/rpc/amqp-rpc.go b/rpc/amqp-rpc.go index 3c3ff6d96..4fcab4218 100644 --- a/rpc/amqp-rpc.go +++ b/rpc/amqp-rpc.go @@ -653,13 +653,11 @@ func (rpc *AmqpRPCCLient) DispatchSync(method string, body []byte) (response []b rpc.stats.Inc(fmt.Sprintf("RPC.ClientCallLatency.%s.Error", method), 1, 1.0) return } - rpc.stats.Inc("RPC.Rate.Success", 1, 1.0) rpc.stats.TimingDuration(fmt.Sprintf("RPC.ClientCallLatency.%s.Success", method), time.Since(callStarted), 1.0) response = rpcResponse.ReturnVal return case <-time.After(rpc.timeout): rpc.stats.TimingDuration(fmt.Sprintf("RPC.ClientCallLatency.%s.Timeout", method), time.Since(callStarted), 1.0) - rpc.stats.Inc("RPC.Rate.Timeouts", 1, 1.0) rpc.log.Warning(fmt.Sprintf(" [c!][%s] AMQP-RPC timeout [%s]", rpc.clientQueue, method)) rpc.mu.Lock() delete(rpc.pending, corrID)