Repository: scionproto/scion
Pull request #4690 (2025-01-30 11:31:11)

drkey: simplify Epoch type
During verification of the drkey package (as part of the `VerifiedSCION` project) we ran into an issue with our verifier. It complains about the embedded struct in `Epoch`, which in this specific case, does not seem to be supported.

(Just not to confuse anyone: The issue is with the verifier (gobra), not with Go or the `drkey` package itself.)

However, to me there does not seem to be a reason why `Epoch` embedds `cppki.Validity`. It could as well just be a type definition. This is what I propose with this PR. It changes the definition of `Epoch` and adapts (and simplifies) the clients of the `drkey` package.

jiceatscion: 

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/4690)
<!-- Reviewable:end -->

romshark: Please run `make gazelle` and commit the changes to fix the CI issues
JordiSubira: > Could you explain why the types Epoch and Validity continue to exist separately? If they were meant to be semantically interchangeable, and are now completely identical, why keep two different definitions? If we want to prevent invisible conversions we could create two different types that both refer to the common one: "type Validity NotBeforeNotAfter" and "type Epoch NotBeforeNotAfter" (note the absence of "=".

Initially Epoch existed independently from Validity was introduced (it existed in old versions of DRKey that didn't even belong to scionproto). I would say that currently they are semantically interchangeable and I do not think this will ever change in the future (or I cannot think about a good reason right now). However, keeping the alias may help with code readability and clarity. In the documentation we always refer to DRKey epochs, while probably TRCs talk about Validity. In that sense, I guess your proposal about creating ``NotBeforeNotAfter`` would also work.


Patch:
diff --git a/control/drkey/grpc/BUILD.bazel b/control/drkey/grpc/BUILD.bazel
index 9eb9cdef7..48d1b534f 100644
--- a/control/drkey/grpc/BUILD.bazel
+++ b/control/drkey/grpc/BUILD.bazel
@@ -19,7 +19,6 @@ go_library(
         "//pkg/private/serrors:go_default_library",
         "//pkg/proto/control_plane:go_default_library",
         "//pkg/proto/drkey:go_default_library",
-        "//pkg/scrypto/cppki:go_default_library",
         "//pkg/snet:go_default_library",
         "@org_go4_netipx//:go_default_library",
         "@org_golang_google_grpc//credentials:go_default_library",
diff --git a/control/drkey/grpc/protobuf.go b/control/drkey/grpc/protobuf.go
index 848d72ed4..cc9cfa557 100644
--- a/control/drkey/grpc/protobuf.go
+++ b/control/drkey/grpc/protobuf.go
@@ -22,7 +22,6 @@ import (
 	"github.com/scionproto/scion/pkg/private/serrors"
 	cppb "github.com/scionproto/scion/pkg/proto/control_plane"
 	dkpb "github.com/scionproto/scion/pkg/proto/drkey"
-	"github.com/scionproto/scion/pkg/scrypto/cppki"
 )
 
 func secretRequestToMeta(req *cppb.DRKeySecretValueRequest) (drkey.SecretValueMeta, error) {
@@ -65,10 +64,8 @@ func getLevel1KeyFromReply(
 		return drkey.Level1Key{}, serrors.Wrap("invalid EpochEnd from response", err)
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 	returningKey := drkey.Level1Key{
 		SrcIA:   meta.SrcIA,
diff --git a/daemon/drkey/grpc/BUILD.bazel b/daemon/drkey/grpc/BUILD.bazel
index c6df3c3bd..b0f787b23 100644
--- a/daemon/drkey/grpc/BUILD.bazel
+++ b/daemon/drkey/grpc/BUILD.bazel
@@ -15,7 +15,6 @@ go_library(
         "//pkg/private/serrors:go_default_library",
         "//pkg/proto/control_plane:go_default_library",
         "//pkg/proto/drkey:go_default_library",
-        "//pkg/scrypto/cppki:go_default_library",
         "//pkg/snet:go_default_library",
         "@org_golang_google_protobuf//types/known/timestamppb:go_default_library",
     ],
diff --git a/daemon/drkey/grpc/protobuf.go b/daemon/drkey/grpc/protobuf.go
index cb32f7ba4..3ca874578 100644
--- a/daemon/drkey/grpc/protobuf.go
+++ b/daemon/drkey/grpc/protobuf.go
@@ -21,7 +21,6 @@ import (
 	"github.com/scionproto/scion/pkg/private/serrors"
 	cppb "github.com/scionproto/scion/pkg/proto/control_plane"
 	drkeypb "github.com/scionproto/scion/pkg/proto/drkey"
-	"github.com/scionproto/scion/pkg/scrypto/cppki"
 )
 
 func asHostMetaToProtoRequest(meta drkey.ASHostMeta) *cppb.DRKeyASHostRequest {
@@ -48,10 +47,8 @@ func getASHostKeyFromReply(
 		return drkey.ASHostKey{}, serrors.Wrap("invalid EpochEnd from response", err)
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 
 	returningKey := drkey.ASHostKey{
@@ -94,10 +91,8 @@ func getHostASKeyFromReply(
 		return drkey.HostASKey{}, serrors.Wrap("invalid EpochEnd from response", err)
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 
 	returningKey := drkey.HostASKey{
@@ -140,10 +135,8 @@ func getHostHostKeyFromReply(
 		return drkey.HostHostKey{}, serrors.Wrap("invalid EpochEnd from response", err)
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 
 	returningKey := drkey.HostHostKey{
diff --git a/demo/drkey/BUILD.bazel b/demo/drkey/BUILD.bazel
index bcadc2e91..ad508fb3a 100644
--- a/demo/drkey/BUILD.bazel
+++ b/demo/drkey/BUILD.bazel
@@ -27,7 +27,6 @@ go_library(
         "//pkg/private/serrors:go_default_library",
         "//pkg/proto/control_plane:go_default_library",
         "//pkg/proto/drkey:go_default_library",
-        "//pkg/scrypto/cppki:go_default_library",
         "//pkg/snet:go_default_library",
         "//private/app/flag:go_default_library",
         "@com_github_spf13_pflag//:go_default_library",
diff --git a/demo/drkey/main.go b/demo/drkey/main.go
index 13e32df53..cd42b0dd8 100644
--- a/demo/drkey/main.go
+++ b/demo/drkey/main.go
@@ -34,7 +34,6 @@ import (
 	"github.com/scionproto/scion/pkg/private/serrors"
 	cppb "github.com/scionproto/scion/pkg/proto/control_plane"
 	dkpb "github.com/scionproto/scion/pkg/proto/drkey"
-	"github.com/scionproto/scion/pkg/scrypto/cppki"
 	"github.com/scionproto/scion/pkg/snet"
 	env "github.com/scionproto/scion/private/app/flag"
 )
@@ -308,10 +307,8 @@ func getSecretFromReply(
 		return drkey.SecretValue{}, err
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 	returningKey := drkey.SecretValue{
 		ProtoId: proto,
diff --git a/pkg/daemon/BUILD.bazel b/pkg/daemon/BUILD.bazel
index 63b2e6655..6d2c3e3ed 100644
--- a/pkg/daemon/BUILD.bazel
+++ b/pkg/daemon/BUILD.bazel
@@ -23,7 +23,6 @@ go_library(
         "//pkg/private/serrors:go_default_library",
         "//pkg/proto/daemon:go_default_library",
         "//pkg/proto/drkey:go_default_library",
-        "//pkg/scrypto/cppki:go_default_library",
         "//pkg/segment/iface:go_default_library",
         "//pkg/snet:go_default_library",
         "//pkg/snet/path:go_default_library",
diff --git a/pkg/daemon/grpc.go b/pkg/daemon/grpc.go
index 16bf2e4df..9ecfe9f7b 100644
--- a/pkg/daemon/grpc.go
+++ b/pkg/daemon/grpc.go
@@ -32,7 +32,6 @@ import (
 	"github.com/scionproto/scion/pkg/private/serrors"
 	sdpb "github.com/scionproto/scion/pkg/proto/daemon"
 	dkpb "github.com/scionproto/scion/pkg/proto/drkey"
-	"github.com/scionproto/scion/pkg/scrypto/cppki"
 	"github.com/scionproto/scion/pkg/segment/iface"
 	"github.com/scionproto/scion/pkg/snet"
 	"github.com/scionproto/scion/pkg/snet/path"
@@ -355,10 +354,8 @@ func getASHostKeyFromReply(rep *sdpb.DRKeyASHostResponse,
 		return drkey.ASHostKey{}, serrors.Wrap("invalid EpochEnd from response", err)
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 
 	returningKey := drkey.ASHostKey{
@@ -399,10 +396,8 @@ func getHostASKeyFromReply(rep *sdpb.DRKeyHostASResponse,
 		return drkey.HostASKey{}, serrors.Wrap("invalid EpochEnd from response", err)
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 
 	returningKey := drkey.HostASKey{
@@ -443,10 +438,8 @@ func getHostHostKeyFromReply(rep *sdpb.DRKeyHostHostResponse,
 		return drkey.HostHostKey{}, serrors.Wrap("invalid EpochEnd from response", err)
 	}
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: rep.EpochBegin.AsTime(),
-			NotAfter:  rep.EpochEnd.AsTime(),
-		},
+		NotBefore: rep.EpochBegin.AsTime(),
+		NotAfter:  rep.EpochEnd.AsTime(),
 	}
 
 	returningKey := drkey.HostHostKey{
diff --git a/pkg/drkey/drkey.go b/pkg/drkey/drkey.go
index 23811a88f..73621c344 100644
--- a/pkg/drkey/drkey.go
+++ b/pkg/drkey/drkey.go
@@ -36,17 +36,13 @@ const (
 )
 
 // Epoch represents a validity period.
-type Epoch struct {
-	cppki.Validity
-}
+type Epoch = cppki.Validity
 
 // NewEpoch constructs an Epoch from its uint32 encoded begin and end parts.
 func NewEpoch(begin, end uint32) Epoch {
 	return Epoch{
-		cppki.Validity{
-			NotBefore: util.SecsToTime(begin),
-			NotAfter:  util.SecsToTime(end),
-		},
+		NotBefore: util.SecsToTime(begin),
+		NotAfter:  util.SecsToTime(end),
 	}
 }
 


Test patch:
diff --git a/private/storage/drkey/level1/dbtest/BUILD.bazel b/private/storage/drkey/level1/dbtest/BUILD.bazel
index 9c0e89236..23d39e00d 100644
--- a/private/storage/drkey/level1/dbtest/BUILD.bazel
+++ b/private/storage/drkey/level1/dbtest/BUILD.bazel
@@ -8,7 +8,6 @@ go_library(
     deps = [
         "//pkg/addr:go_default_library",
         "//pkg/drkey:go_default_library",
-        "//pkg/scrypto/cppki:go_default_library",
         "//private/drkey/drkeytest:go_default_library",
         "@com_github_stretchr_testify//assert:go_default_library",
     ],
diff --git a/private/storage/drkey/level1/dbtest/dbtest.go b/private/storage/drkey/level1/dbtest/dbtest.go
index 8ecd3efe4..afa914adf 100644
--- a/private/storage/drkey/level1/dbtest/dbtest.go
+++ b/private/storage/drkey/level1/dbtest/dbtest.go
@@ -23,7 +23,6 @@ import (
 
 	"github.com/scionproto/scion/pkg/addr"
 	"github.com/scionproto/scion/pkg/drkey"
-	"github.com/scionproto/scion/pkg/scrypto/cppki"
 	"github.com/scionproto/scion/private/drkey/drkeytest"
 )
 
@@ -58,10 +57,8 @@ func testDRKeyLevel1(t *testing.T, db drkey.Level1DB) {
 	defer cancelF()
 
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: time.Now(),
-			NotAfter:  time.Now().Add(timeOffset),
-		},
+		NotBefore: time.Now(),
+		NotAfter:  time.Now().Add(timeOffset),
 	}
 	protoId := drkey.Protocol(0)
 	drkeyLevel1 := drkeytest.GetLevel1(t, protoId, epoch, srcIA, dstIA)
diff --git a/private/storage/drkey/level2/dbtest/BUILD.bazel b/private/storage/drkey/level2/dbtest/BUILD.bazel
index e98fb9459..514c2d0b5 100644
--- a/private/storage/drkey/level2/dbtest/BUILD.bazel
+++ b/private/storage/drkey/level2/dbtest/BUILD.bazel
@@ -8,7 +8,6 @@ go_library(
     deps = [
         "//pkg/addr:go_default_library",
         "//pkg/drkey:go_default_library",
-        "//pkg/scrypto/cppki:go_default_library",
         "//private/drkey/drkeytest:go_default_library",
         "@com_github_stretchr_testify//assert:go_default_library",
         "@com_github_stretchr_testify//require:go_default_library",
diff --git a/private/storage/drkey/level2/dbtest/dbtest.go b/private/storage/drkey/level2/dbtest/dbtest.go
index cfaac344c..e895c9c44 100644
--- a/private/storage/drkey/level2/dbtest/dbtest.go
+++ b/private/storage/drkey/level2/dbtest/dbtest.go
@@ -24,7 +24,6 @@ import (
 
 	"github.com/scionproto/scion/pkg/addr"
 	"github.com/scionproto/scion/pkg/drkey"
-	"github.com/scionproto/scion/pkg/scrypto/cppki"
 	"github.com/scionproto/scion/private/drkey/drkeytest"
 )
 
@@ -60,10 +59,8 @@ func testDRKeyLevel2(t *testing.T, db drkey.Level2DB) {
 	ctx, cancelF := context.WithTimeout(context.Background(), time.Second)
 	defer cancelF()
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: time.Now(),
-			NotAfter:  time.Now().Add(timeOffset),
-		},
+		NotBefore: time.Now(),
+		NotAfter:  time.Now().Add(timeOffset),
 	}
 	protoId := drkey.Protocol(0)
 	drkeyLevel1 := drkeytest.GetLevel1(t, protoId, epoch, srcIA, dstIA)
diff --git a/private/storage/drkey/secret/dbtest/BUILD.bazel b/private/storage/drkey/secret/dbtest/BUILD.bazel
index 60b43d8d8..37a568f18 100644
--- a/private/storage/drkey/secret/dbtest/BUILD.bazel
+++ b/private/storage/drkey/secret/dbtest/BUILD.bazel
@@ -8,7 +8,6 @@ go_library(
     deps = [
         "//pkg/addr:go_default_library",
         "//pkg/drkey:go_default_library",
-        "//pkg/scrypto/cppki:go_default_library",
         "@com_github_stretchr_testify//assert:go_default_library",
         "@com_github_stretchr_testify//require:go_default_library",
     ],
diff --git a/private/storage/drkey/secret/dbtest/dbtest.go b/private/storage/drkey/secret/dbtest/dbtest.go
index 33ba0d7e0..8f1959a45 100644
--- a/private/storage/drkey/secret/dbtest/dbtest.go
+++ b/private/storage/drkey/secret/dbtest/dbtest.go
@@ -24,7 +24,6 @@ import (
 
 	"github.com/scionproto/scion/pkg/addr"
 	"github.com/scionproto/scion/pkg/drkey"
-	"github.com/scionproto/scion/pkg/scrypto/cppki"
 )
 
 const (
@@ -57,10 +56,8 @@ func testDB(t *testing.T, db drkey.SecretValueDB) {
 	defer cancelF()
 
 	epoch := drkey.Epoch{
-		Validity: cppki.Validity{
-			NotBefore: time.Now(),
-			NotAfter:  time.Now().Add(timeOffset),
-		},
+		NotBefore: time.Now(),
+		NotAfter:  time.Now().Add(timeOffset),
 	}
 	asSecret := []byte{0, 1, 2, 3, 4, 5, 6, 7}
 	asSecret = append(asSecret, byte(srcIA))