Skip to content

Commit

Permalink
Add nil-check for newWithOptions.
Browse files Browse the repository at this point in the history
This makes sure that ks in a Handle is never nil. A handle with a nil ks doens't work properly anyways, and will result in an error or even a panic later anyways.

This check already exists for NewHandleWithNoSecrets, we also add a test for that.

This changes the behavior of the deprecated insecurecleartextkeyset.KeysetHandle function: calling insecurecleartextkeyset.KeysetHandle(nil) will now return nil instead of a keyset.Handle with ks=nil, which is basically unusable.

PiperOrigin-RevId: 635760959
Change-Id: I2be29d9ee0310754815d14bac0c32a9a5d7b9664
  • Loading branch information
juergw authored and copybara-github committed May 21, 2024
1 parent 63c71c2 commit f3be051
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
7 changes: 7 additions & 0 deletions insecurecleartextkeyset/insecurecleartextkeyset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ func TestLegacyKeysetHandle(t *testing.T) {
}
}

func TestLegacyKeysetHandleWithNilKeysetReturnsNil(t *testing.T) {
handle := insecurecleartextkeyset.KeysetHandle(nil)
if handle != nil {
t.Error("insecurecleartextkeyset.KeysetHandle(nil) != nil, want nil")
}
}

func TestHandleFromReaderWithAnnotationsGetsMonitored(t *testing.T) {
defer internalregistry.ClearMonitoringClient()
client := &fakemonitoring.Client{}
Expand Down
5 changes: 4 additions & 1 deletion keyset/handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,14 @@ var errInvalidKeyset = fmt.Errorf("keyset.Handle: invalid keyset")
// Handle provides access to a Keyset protobuf, to limit the exposure of actual protocol
// buffers that hold sensitive key material.
type Handle struct {
ks *tinkpb.Keyset
ks *tinkpb.Keyset // must be non-nil
annotations map[string]string
}

func newWithOptions(ks *tinkpb.Keyset, opts ...Option) (*Handle, error) {
if ks == nil {
return nil, errors.New("keyset.Handle: nil keyset")
}
h := &Handle{ks: ks}
if err := applyOptions(h, opts...); err != nil {
return nil, err
Expand Down
22 changes: 22 additions & 0 deletions keyset/handle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ func TestWriteAndReadWithNoSecrets(t *testing.T) {
}
serialized := buff.Bytes()

// Using ReadWithNoSecrets.
handle2, err := keyset.ReadWithNoSecrets(keyset.NewBinaryReader(bytes.NewBuffer(serialized)))
if err != nil {
t.Fatalf("keyset.ReadWithNoSecrets() err = %v, want nil", err)
Expand All @@ -221,6 +222,27 @@ func TestWriteAndReadWithNoSecrets(t *testing.T) {
if !proto.Equal(testkeyset.KeysetMaterial(handle), testkeyset.KeysetMaterial(handle2)) {
t.Fatalf("keyset.ReadWithNoSecrets() = %v, want %v", handle2, handle)
}

// Using Read() and then NewHandleWithNoSecrets.
reader := keyset.NewBinaryReader(bytes.NewBuffer(serialized))
protoPublicKeyset, err := reader.Read()
if err != nil {
t.Fatalf("reader.Read() err = %v, want nil", err)
}
handle3, err := keyset.NewHandleWithNoSecrets(protoPublicKeyset)
if err != nil {
t.Fatalf("keyset.NewHandleWithNoSecrets() err = %v, want nil", err)
}

if !proto.Equal(testkeyset.KeysetMaterial(handle), testkeyset.KeysetMaterial(handle3)) {
t.Fatalf("keyset.NewHandleWithNoSecrets() = %v, want %v", handle3, handle)
}
}

func TestNewHandleWithNoSecretsReturnsErrorIfInputIsNil(t *testing.T) {
if _, err := keyset.NewHandleWithNoSecrets(nil); err == nil {
t.Fatal("keyset.NewHandleWithNoSecrets(nil) err = nil, want error")
}
}

func TestWriteWithNoSecretsFailsWithSymmetricSecretKey(t *testing.T) {
Expand Down

0 comments on commit f3be051

Please sign in to comment.