From 7eb82cc65d64b61ee72dfba274398d203813312c Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Fri, 15 Apr 2022 20:10:21 +0200 Subject: [PATCH 1/3] Work around local times in CAs --- lib/services/authority.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/services/authority.go b/lib/services/authority.go index b29da19bc4e4d..088b39c69f3b2 100644 --- a/lib/services/authority.go +++ b/lib/services/authority.go @@ -397,6 +397,18 @@ func UnmarshalCertAuthority(bytes []byte, opts ...MarshalOption) (types.CertAuth if cfg.ID != 0 { ca.SetResourceID(cfg.ID) } + // Correct problems with existing CAs that contain non-UTC times, which + // causes panics when doing a gogoproto Clone; should only ever be + // possible with LastRotated, but we enforce it on all the times anyway. + // See https://github.com/gogo/protobuf/issues/519 . + if ca.Spec.Rotation != nil { + apiutils.UTC(&ca.Spec.Rotation.Started) + apiutils.UTC(&ca.Spec.Rotation.LastRotated) + apiutils.UTC(&ca.Spec.Rotation.Schedule.UpdateClients) + apiutils.UTC(&ca.Spec.Rotation.Schedule.UpdateServers) + apiutils.UTC(&ca.Spec.Rotation.Schedule.Standby) + } + return &ca, nil } From d355b1c13efc376625fa5f9f6fdb3d9c46dc4f65 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Tue, 19 Apr 2022 10:45:21 +0200 Subject: [PATCH 2/3] Test coverage --- lib/services/authority_test.go | 49 +++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/lib/services/authority_test.go b/lib/services/authority_test.go index 14f185b3b59ff..a1300320b8357 100644 --- a/lib/services/authority_test.go +++ b/lib/services/authority_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package services +package services_test import ( "crypto/x509/pkix" @@ -24,7 +24,10 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth/testauthority" + . "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/tlsca" + "github.com/gravitational/teleport/lib/utils" ) func TestCertPoolFromCertAuthorities(t *testing.T) { @@ -161,3 +164,47 @@ func TestCertAuthorityEquivalence(t *testing.T) { ca1modID.SetResourceID(ca1.GetResourceID() + 1) require.True(t, CertAuthoritiesEquivalent(ca1, ca1modID)) } + +func TestCertAuthorityUTCUnmarshal(t *testing.T) { + t.Parallel() + ta := testauthority.New() + t.Cleanup(ta.Close) + + _, pub, err := ta.GenerateKeyPair("") + require.NoError(t, err) + _, cert, err := tlsca.GenerateSelfSignedCA(pkix.Name{CommonName: "clustername"}, nil, time.Hour) + require.NoError(t, err) + + caLocal, err := types.NewCertAuthority(types.CertAuthoritySpecV2{ + Type: types.HostCA, + ClusterName: "clustername", + ActiveKeys: types.CAKeySet{ + SSH: []*types.SSHKeyPair{{PublicKey: pub}}, + TLS: []*types.TLSKeyPair{{Cert: cert}}, + }, + Rotation: &types.Rotation{ + LastRotated: time.Now().In(time.FixedZone("not UTC", 2*60*60)), + }, + }) + require.NoError(t, err) + // needed for CertAuthoritiesEquivalent, as this will get called by + // UnmarshalCertAuthority + require.NoError(t, SyncCertAuthorityKeys(caLocal)) + + _, offset := caLocal.GetRotation().LastRotated.Zone() + require.NotZero(t, offset) + + item, err := utils.FastMarshal(caLocal) + require.NoError(t, err) + require.Contains(t, string(item), "+02:00\"") + caUTC, err := UnmarshalCertAuthority(item) + require.NoError(t, err) + + _, offset = caUTC.GetRotation().LastRotated.Zone() + require.Zero(t, offset) + + // successfully Clone without panic, see https://github.com/gogo/protobuf/issues/519 + _ = caUTC.Clone() + + require.True(t, CertAuthoritiesEquivalent(caLocal, caUTC)) +} From 1979daa5df8091149f0ad9340adf68f28375aca6 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Tue, 19 Apr 2022 21:43:46 +0200 Subject: [PATCH 3/3] Use require.NotPanics --- lib/services/authority_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/services/authority_test.go b/lib/services/authority_test.go index a1300320b8357..9d456e9cab07a 100644 --- a/lib/services/authority_test.go +++ b/lib/services/authority_test.go @@ -203,8 +203,8 @@ func TestCertAuthorityUTCUnmarshal(t *testing.T) { _, offset = caUTC.GetRotation().LastRotated.Zone() require.Zero(t, offset) - // successfully Clone without panic, see https://github.com/gogo/protobuf/issues/519 - _ = caUTC.Clone() + // see https://github.com/gogo/protobuf/issues/519 + require.NotPanics(t, func() { caUTC.Clone() }) require.True(t, CertAuthoritiesEquivalent(caLocal, caUTC)) }