From ea46601ea38c310f2101c9bad65c17afd41f382c Mon Sep 17 00:00:00 2001 From: reinkrul Date: Mon, 11 Dec 2023 11:46:10 +0100 Subject: [PATCH 1/4] Auth: create session and validate signatures perform the same checks (#2664) --- auth/api/auth/v1/api.go | 1 + auth/services/selfsigned/signer.go | 19 +++++----- auth/services/selfsigned/signer_test.go | 46 +++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/auth/api/auth/v1/api.go b/auth/api/auth/v1/api.go index 1d7acb9105..bf5e27611c 100644 --- a/auth/api/auth/v1/api.go +++ b/auth/api/auth/v1/api.go @@ -124,6 +124,7 @@ func (w Wrapper) VerifySignature(_ context.Context, request VerifySignatureReque vpType := validationResult.VPType() response.VpType = &vpType } else { + log.Logger().Warnf("Signature verification failed, reason: %s", validationResult.Reason()) response.Validity = false } return VerifySignature200JSONResponse(response), nil diff --git a/auth/services/selfsigned/signer.go b/auth/services/selfsigned/signer.go index 8d80ab1e73..550f163fff 100644 --- a/auth/services/selfsigned/signer.go +++ b/auth/services/selfsigned/signer.go @@ -226,20 +226,19 @@ func checkSessionParams(params map[string]interface{}) error { if !ok { return fmt.Errorf("employee should be an object") } - _, ok = employeeMap["identifier"] - if !ok { - return fmt.Errorf("missing employee identifier") + identifier, _ := employeeMap["identifier"].(string) + if len(identifier) == 0 { + return fmt.Errorf("missing/invalid employee identifier") } - _, ok = employeeMap["initials"] - if !ok { - return fmt.Errorf("missing employee initials") + initials, _ := employeeMap["initials"].(string) + if len(initials) == 0 { + return fmt.Errorf("missing/invalid employee initials") } - _, ok = employeeMap["familyName"] - if !ok { - return fmt.Errorf("missing employee familyName") + familyName, _ := employeeMap["familyName"].(string) + if len(familyName) == 0 { + return fmt.Errorf("missing/invalid employee familyName") } return nil - } func (v *signer) Routes(router core.EchoRouter) { diff --git a/auth/services/selfsigned/signer_test.go b/auth/services/selfsigned/signer_test.go index 1b205e15cc..919b93971e 100644 --- a/auth/services/selfsigned/signer_test.go +++ b/auth/services/selfsigned/signer_test.go @@ -109,6 +109,52 @@ func TestSessionStore_StartSigningSession(t *testing.T) { require.Error(t, err) }) + + t.Run("empty employee familyName", func(t *testing.T) { + params := map[string]interface{}{ + "employer": employer.String(), + "employee": map[string]interface{}{ + "identifier": identifier, + "roleName": roleName, + "initials": initials, + "familyName": "", + }, + } + + ss := NewSigner(nil, "").(*signer) + _, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params) + require.ErrorContains(t, err, "missing/invalid employee familyName") + }) + t.Run("empty employee initials", func(t *testing.T) { + params := map[string]interface{}{ + "employer": employer.String(), + "employee": map[string]interface{}{ + "identifier": identifier, + "roleName": roleName, + "initials": "", + "familyName": familyName, + }, + } + + ss := NewSigner(nil, "").(*signer) + _, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params) + require.ErrorContains(t, err, "missing/invalid employee initials") + }) + t.Run("empty employee identifier", func(t *testing.T) { + params := map[string]interface{}{ + "employer": employer.String(), + "employee": map[string]interface{}{ + "identifier": "", + "roleName": roleName, + "initials": initials, + "familyName": familyName, + }, + } + + ss := NewSigner(nil, "").(*signer) + _, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params) + require.ErrorContains(t, err, "missing/invalid employee identifier") + }) } func TestSessionStore_SigningSessionStatus(t *testing.T) { From a1a264aaccc71a5453811b10b0db28612cbc6c53 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Mon, 11 Dec 2023 11:59:12 +0100 Subject: [PATCH 2/4] release notes v5.4.5 --- docs/pages/release_notes.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/pages/release_notes.rst b/docs/pages/release_notes.rst index 6cb9e71b99..4cd4628504 100644 --- a/docs/pages/release_notes.rst +++ b/docs/pages/release_notes.rst @@ -3,6 +3,16 @@ Release notes ############# +************************ +Hazelnut update (v5.4.5) +************************ + +Release date: 2023-12-11 + +- Auth: make sure create session and validate signatures perform the same checks (#2664) + +**Full Changelog**: https://github.com/nuts-foundation/nuts-node/compare/v5.4.4...v5.4.5 + ************************ Hazelnut update (v5.4.4) ************************ From 7209f96ee3ed624e297b150007da1e24757095c6 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Mon, 11 Dec 2023 13:39:17 +0100 Subject: [PATCH 3/4] upgrade to golang 1.21, fix tests --- Dockerfile | 2 +- auth/services/selfsigned/signer_test.go | 14 +++++++------- http/engine_test.go | 2 +- network/network_test.go | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Dockerfile b/Dockerfile index 34d49092f0..c3994b7794 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # golang alpine -FROM golang:1.20.6-alpine as builder +FROM golang:1.21.5-alpine as builder ARG TARGETARCH ARG TARGETOS diff --git a/auth/services/selfsigned/signer_test.go b/auth/services/selfsigned/signer_test.go index 919b93971e..339601b0dd 100644 --- a/auth/services/selfsigned/signer_test.go +++ b/auth/services/selfsigned/signer_test.go @@ -178,8 +178,8 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) { t.Run("status completed returns VP on SigningSessionResult", func(t *testing.T) { mockContext := newMockContext(t) ss := NewSigner(mockContext.vcr, "").(*signer) - mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).Return(&testVC, nil) - mockContext.holder.EXPECT().BuildVP(context.TODO(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil) + mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).Return(&testVC, nil) + mockContext.holder.EXPECT().BuildVP(context.Background(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil) sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params) require.NoError(t, err) @@ -237,7 +237,7 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) { t.Run("correct VC options are passed to issuer", func(t *testing.T) { mockContext := newMockContext(t) ss := NewSigner(mockContext.vcr, "").(*signer) - mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).DoAndReturn( + mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).DoAndReturn( func(arg0 interface{}, unsignedCredential interface{}, public interface{}, publish interface{}) (*vc.VerifiableCredential, error) { isPublic, ok := public.(bool) isPublished, ok2 := publish.(bool) @@ -265,7 +265,7 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) { return &testVC, nil }) - mockContext.holder.EXPECT().BuildVP(context.TODO(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil) + mockContext.holder.EXPECT().BuildVP(context.Background(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil) sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params) require.NoError(t, err) @@ -287,7 +287,7 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) { t.Run("error on VC issuance", func(t *testing.T) { mockContext := newMockContext(t) ss := NewSigner(mockContext.vcr, "").(*signer) - mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).Return(nil, errors.New("error")) + mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).Return(nil, errors.New("error")) sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params) require.NoError(t, err) @@ -302,8 +302,8 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) { t.Run("error on building VP", func(t *testing.T) { mockContext := newMockContext(t) ss := NewSigner(mockContext.vcr, "").(*signer) - mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).Return(&testVC, nil) - mockContext.holder.EXPECT().BuildVP(context.TODO(), gomock.Len(1), gomock.Any(), &employer, true).Return(nil, errors.New("error")) + mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).Return(&testVC, nil) + mockContext.holder.EXPECT().BuildVP(context.Background(), gomock.Len(1), gomock.Any(), &employer, true).Return(nil, errors.New("error")) sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params) require.NoError(t, err) diff --git a/http/engine_test.go b/http/engine_test.go index 1e7f244e01..efddf19309 100644 --- a/http/engine_test.go +++ b/http/engine_test.go @@ -163,7 +163,7 @@ func TestEngine_Configure(t *testing.T) { thisTLSConfig := tlsConfig.Clone() thisTLSConfig.Certificates = nil _, err = doHTTPSRequest(thisTLSConfig, engine.config.InterfaceConfig.Address) - assert.ErrorContains(t, err, "tls: bad certificate") + assert.ErrorContains(t, err, "tls: certificate required") err = engine.Shutdown() assert.NoError(t, err) diff --git a/network/network_test.go b/network/network_test.go index 3d4a7162af..d2d4375cdb 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -1246,7 +1246,7 @@ func TestNetwork_checkHealth(t *testing.T) { result := n.CheckHealth() assert.Equal(t, core.HealthStatusDown, result[healthTLS].Status) - assert.Equal(t, "x509: certificate signed by unknown authority", result[healthTLS].Details) + assert.Contains(t, result[healthTLS].Details, "x509: certificate has expired or is not yet valid") }) }) From a2aaf322e5e4ad7c0d9ee81eee80ec3669775284 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Mon, 11 Dec 2023 13:44:45 +0100 Subject: [PATCH 4/4] circle-go-version --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9d0a079688..6f1382be84 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -12,7 +12,7 @@ jobs: build: parallelism: 8 docker: - - image: cimg/go:1.20 + - image: cimg/go:1.21 steps: - checkout @@ -36,7 +36,7 @@ jobs: report: docker: - - image: cimg/go:1.20 + - image: cimg/go:1.21 steps: - checkout - attach_workspace: