diff --git a/lib/services/authority.go b/lib/services/authority.go index 684895da2a770..6a06ac98a6e6d 100644 --- a/lib/services/authority.go +++ b/lib/services/authority.go @@ -352,6 +352,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 } diff --git a/lib/services/authority_test.go b/lib/services/authority_test.go index 35395d1c8cf1f..ce20e3de0eaff 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) { @@ -157,3 +160,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) + + // see https://github.com/gogo/protobuf/issues/519 + require.NotPanics(t, func() { caUTC.Clone() }) + + require.True(t, CertAuthoritiesEquivalent(caLocal, caUTC)) +} diff --git a/lib/services/local/trust.go b/lib/services/local/trust.go index ab87827b3bcfd..9da8ced18103f 100644 --- a/lib/services/local/trust.go +++ b/lib/services/local/trust.go @@ -92,33 +92,39 @@ func (s *CA) UpsertCertAuthority(ca types.CertAuthority) error { } // CompareAndSwapCertAuthority updates the cert authority value -// if the existing value matches existing parameter, returns nil if succeeds, +// if the existing value matches expected parameter, returns nil if succeeds, // trace.CompareFailed otherwise. -func (s *CA) CompareAndSwapCertAuthority(new, existing types.CertAuthority) error { +func (s *CA) CompareAndSwapCertAuthority(new, expected types.CertAuthority) error { if err := services.ValidateCertAuthority(new); err != nil { return trace.Wrap(err) } - newValue, err := services.MarshalCertAuthority(new) + + key := backend.Key(authoritiesPrefix, string(new.GetType()), new.GetName()) + + actualItem, err := s.Get(context.TODO(), key) if err != nil { return trace.Wrap(err) } - newItem := backend.Item{ - Key: backend.Key(authoritiesPrefix, string(new.GetType()), new.GetName()), - Value: newValue, - Expires: new.Expiry(), + actual, err := services.UnmarshalCertAuthority(actualItem.Value) + if err != nil { + return trace.Wrap(err) + } + + if !services.CertAuthoritiesEquivalent(actual, expected) { + return trace.CompareFailed("cluster %v settings have been updated, try again", new.GetName()) } - existingValue, err := services.MarshalCertAuthority(existing) + newValue, err := services.MarshalCertAuthority(new) if err != nil { return trace.Wrap(err) } - existingItem := backend.Item{ - Key: backend.Key(authoritiesPrefix, string(existing.GetType()), existing.GetName()), - Value: existingValue, - Expires: existing.Expiry(), + newItem := backend.Item{ + Key: key, + Value: newValue, + Expires: new.Expiry(), } - _, err = s.CompareAndSwap(context.TODO(), existingItem, newItem) + _, err = s.CompareAndSwap(context.TODO(), *actualItem, newItem) if err != nil { if trace.IsCompareFailed(err) { return trace.CompareFailed("cluster %v settings have been updated, try again", new.GetName())