Merge pull request #346 from privacybydesign/keyshare-attribute-expir…

…es-fix

Feat: allow expired credentials to be disclosed and add renew service for keyshare attribute

.github/actions/build/action.yml

@@ -19,7 +19,7 @@ runs:
     - name: Set up Go
       uses: actions/setup-go@v4
       with:
-        go-version: ^1.18
+        go-version: ^1.21
 
     - name: Determine artifact output filename
       id: artifact-name-generator

.github/workflows/status-checks.yml

@@ -61,7 +61,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@v4
       with:
-        go-version: ^1.18
+        go-version-file: go.mod
 
     - name: Run gofmt
       # gofmt does not return non-zero exit codes on failure, so we have to check that there are no issues using grep.
@@ -145,6 +145,11 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@v3
 
+      - name: Set up Go
+        uses: actions/setup-go@v4
+        with:
+          go-version-file: go.mod
+
       - name: Initialize CodeQL
         uses: github/codeql-action/init@v2
         with:

CHANGELOG.md

@@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+### Added
+- Option `skipExpiryCheck` in disclosure requests to allow disclosure of expired credentials (e.g. `"skipExpiryCheck": ["irma-demo.sidn-pbdf.email"]`)
+- Renewal endpoint for keyshare attribute in the keyshare server (`/users/renewKeyshareAttribute`)
+
+### Changed
+- `KeyshareVerifyPin` function in irmaclient ensures the keyshare attribute is valid
 
 ### Changed
 - Sending the account expiry email is done when user has only valid e-mail addresses

go.mod

@@ -1,6 +1,8 @@
 module github.com/privacybydesign/irmago
 
-go 1.18
+go 1.21
+
+toolchain go1.21.1
 
 require (
 	github.com/alexandrevicenzi/go-sse v1.6.0

go.sum

@@ -98,6 +98,7 @@ github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7
 github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
 github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
 github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE=
+github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
 github.com/fsnotify/fsnotify v1.5.4 h1:jRbGcIw6P2Meqdwuo0H1p6JVLbL5DHKAKlYndzMwVZI=
@@ -172,6 +173,7 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
+github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
 github.com/google/martian/v3 v3.1.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
@@ -283,10 +285,12 @@ github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+W
 github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
 github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
 github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE=
+github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU=
 github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
 github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
 github.com/onsi/gomega v1.16.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY=
 github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE=
+github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs=
 github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1/go.mod h1:eD5JxqMiuNYyFNmyY9rkJ/slN8y59oEu4Ei7F8OoKWQ=
 github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8=
 github.com/pelletier/go-toml v1.9.5/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c=

internal/sessiontest/keyshare_test.go

@@ -2,11 +2,13 @@ package sessiontest
 
 import (
 	"testing"
+	"time"
 
 	irma "github.com/privacybydesign/irmago"
 	"github.com/privacybydesign/irmago/internal/test"
 	"github.com/privacybydesign/irmago/internal/testkeyshare"
 	"github.com/privacybydesign/irmago/irmaclient"
+	"github.com/privacybydesign/irmago/server/irmaserver"
 	"github.com/stretchr/testify/require"
 )
 
@@ -50,6 +52,45 @@ func TestKeyshareRegister(t *testing.T) {
 	keyshareSessions(t, client, irmaServer)
 }
 
+func TestKeyshareAttributeRenewal(t *testing.T) {
+	keyshareServer := testkeyshare.StartKeyshareServer(t, logger, irma.NewSchemeManagerIdentifier("test"))
+	defer keyshareServer.Stop()
+
+	client, handler := parseStorage(t)
+	defer test.ClearTestStorage(t, client, handler.storage)
+
+	irmaServer := StartIrmaServer(t, nil)
+	defer irmaServer.Stop()
+
+	irmaserver.AllowIssuingExpiredCredentials = true
+	defer func() {
+		irmaserver.AllowIssuingExpiredCredentials = false
+	}()
+
+	// Make keyshare attribute invalid.
+	invalidValidity := irma.Timestamp(time.Now())
+	issuanceRequest := irma.NewIssuanceRequest([]*irma.CredentialRequest{
+		{
+			Validity:         &invalidValidity,
+			CredentialTypeID: irma.NewCredentialTypeIdentifier("test.test.mijnirma"),
+			Attributes:       map[string]string{"email": "testusername"},
+		},
+	})
+	doSession(t, issuanceRequest, client, irmaServer, nil, nil, nil)
+
+	// Validate that keyshare attribute is invalid.
+	disclosureRequest := getDisclosureRequest(irma.NewAttributeTypeIdentifier("test.test.mijnirma.email"))
+	doSession(t, disclosureRequest, client, irmaServer, nil, nil, nil, optionUnsatisfiableRequest)
+
+	// Do a PIN verification. This should detect the invalid keyshare attribute and renew it.
+	valid, _, _, err := client.KeyshareVerifyPin("12345", irma.NewSchemeManagerIdentifier("test"))
+	require.NoError(t, err)
+	require.True(t, valid)
+
+	// Keyshare attribute should be valid again.
+	doSession(t, disclosureRequest, client, irmaServer, nil, nil, nil)
+}
+
 // Use the existing keyshare enrollment and credentials
 // in a keyshare session of each session type.
 func TestKeyshareSessions(t *testing.T) {

internal/sessiontest/session_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/privacybydesign/irmago/internal/test"
 	"github.com/privacybydesign/irmago/irmaclient"
 	"github.com/privacybydesign/irmago/server"
+	"github.com/privacybydesign/irmago/server/irmaserver"
 	sseclient "github.com/sietseringers/go-sse"
 	"github.com/stretchr/testify/require"
 )
@@ -1263,3 +1264,42 @@ func TestIssueExpiredKey(t *testing.T) {
 	_, _, _, err := irmaServer.irma.StartSession(getIssuanceRequest(true), nil)
 	require.Error(t, err)
 }
+
+func TestExpiredCredential(t *testing.T) {
+	irmaserver.AllowIssuingExpiredCredentials = true
+	defer func() {
+		irmaserver.AllowIssuingExpiredCredentials = false
+	}()
+
+	client, handler := parseStorage(t)
+	defer test.ClearTestStorage(t, client, handler.storage)
+
+	irmaServer := StartIrmaServer(t, nil)
+	defer irmaServer.Stop()
+
+	// Issue an expired credential
+	invalidValidity := irma.Timestamp(time.Now())
+	value := "13371337"
+	issuanceRequest := irma.NewIssuanceRequest([]*irma.CredentialRequest{
+		{
+			Validity:         &invalidValidity,
+			CredentialTypeID: irma.NewCredentialTypeIdentifier("irma-demo.RU.studentCard"),
+			Attributes: map[string]string{
+				"university":        "Radboud",
+				"studentCardNumber": value,
+				"studentID":         "s1234567",
+				"level":             "42",
+			},
+		},
+	})
+	doSession(t, issuanceRequest, client, irmaServer, nil, nil, nil)
+
+	// Try to disclose it and check that it fails.
+	disclosureRequest := irma.NewDisclosureRequest()
+	disclosureRequest.AddSingle(irma.NewAttributeTypeIdentifier("irma-demo.RU.studentCard.studentCardNumber"), &value, nil)
+	doSession(t, disclosureRequest, client, irmaServer, nil, nil, nil, optionUnsatisfiableRequest)
+
+	// Try to disclose it when allowing expired credentials and check that it succeeds.
+	disclosureRequest.SkipExpiryCheck = []irma.CredentialTypeIdentifier{issuanceRequest.Credentials[0].CredentialTypeID}
+	doSession(t, disclosureRequest, client, irmaServer, nil, nil, nil)
+}

irmaclient/client.go

@@ -3,6 +3,7 @@ package irmaclient
 import (
 	"encoding/json"
 	"path/filepath"
+	"slices"
 	"sync"
 	"time"
 
@@ -591,7 +592,7 @@ func (client *Client) credCandidates(request irma.SessionRequest, con irma.Attri
 		var c []*credCandidate
 		haveUsableCred := false
 		for _, attrlist := range attrlistlist {
-			satisfies, usable := client.satisfiesCon(request.Base(), attrlist, con)
+			satisfies, usable := client.satisfiesCon(request, attrlist, con)
 			if satisfies { // add it to the list, even if they are unusable
 				c = append(c, &credCandidate{Type: credTypeID, Hash: attrlist.Hash()})
 				if usable { // having one usable credential will do
@@ -667,7 +668,7 @@ func (client *Client) addCredSuggestion(
 //   - if the attrs can satisfy the conjunction (as long as it is usable),
 //   - if the attrs are usable (they are not expired, or revoked, or not revocation-aware while
 //     a nonrevocation proof is required).
-func (client *Client) satisfiesCon(base *irma.BaseRequest, attrs *irma.AttributeList, con irma.AttributeCon) (bool, bool) {
+func (client *Client) satisfiesCon(request irma.SessionRequest, attrs *irma.AttributeList, con irma.AttributeCon) (bool, bool) {
 	var credfound bool
 	credtype := attrs.CredentialType().Identifier()
 	for _, attr := range con {
@@ -687,7 +688,13 @@ func (client *Client) satisfiesCon(base *irma.BaseRequest, attrs *irma.Attribute
 		return false, false
 	}
 	cred, _, _ := client.credentialByHash(attrs.Hash())
-	usable := !attrs.Revoked && attrs.IsValid() && (!base.RequestsRevocation(credtype) || cred.NonRevocationWitness != nil)
+	usable := !attrs.Revoked && (!request.Base().RequestsRevocation(credtype) || cred.NonRevocationWitness != nil)
+
+	skipExpiryCheck := slices.Contains(request.Disclosure().SkipExpiryCheck, attrs.CredentialType().Identifier())
+	if !skipExpiryCheck {
+		usable = usable && attrs.IsValid()
+	}
+
 	return true, usable
 }
 
@@ -1155,11 +1162,32 @@ func (client *Client) keyshareEnrollWorker(managerID irma.SchemeManagerIdentifie
 	// If the session succeeds or fails, the keyshare server is stored to disk or
 	// removed from the client by the keyshareEnrollmentHandler.
 	client.keyshareServers[managerID] = kss
-	client.newQrSession(qr, &keyshareEnrollmentHandler{
-		client: client,
-		pin:    pin,
-		kss:    kss,
-	})
+	handler := &backgroundIssuanceHandler{
+		pin: pin,
+		credentialsToBeIssuedCallback: func(creds []*irma.CredentialRequest) {
+			// We need to store the keyshare username before the issuance permission is granted.
+			// Otherwise, authentication to the keyshare server fails during issuance of the keyshare attribute.
+			for _, attr := range creds[0].Attributes {
+				kss.Username = attr
+				break
+			}
+		},
+		resultErr: make(chan error),
+	}
+	client.newQrSession(qr, handler)
+	go func() {
+		err := <-handler.resultErr
+		if err != nil {
+			client.handler.EnrollmentFailure(managerID, irma.WrapErrorPrefix(err, "keyshare attribute issuance"))
+			return
+		}
+		err = client.storage.StoreKeyshareServers(client.keyshareServers)
+		if err != nil {
+			client.handler.EnrollmentFailure(managerID, err)
+			return
+		}
+		client.handler.EnrollmentSuccess(kss.SchemeManagerIdentifier)
+	}()
 
 	return nil
 }
@@ -1182,9 +1210,12 @@ func (client *Client) KeyshareVerifyPin(pin string, schemeid irma.SchemeManagerI
 		}
 	}
 	kss := client.keyshareServers[schemeid]
-	return client.verifyPinWorker(pin, kss,
-		irma.NewHTTPTransport(scheme.KeyshareServer, !client.Preferences.DeveloperMode),
-	)
+	transport := irma.NewHTTPTransport(scheme.KeyshareServer, !client.Preferences.DeveloperMode)
+	success, tries, blocked, err := client.verifyPinWorker(pin, kss, transport)
+	if err == nil && success {
+		client.ensureKeyshareAttributeValid(pin, kss, transport)
+	}
+	return success, tries, blocked, err
 }
 
 func (client *Client) KeyshareChangePin(oldPin string, newPin string) {
@@ -1393,6 +1424,33 @@ func (client *Client) keyshareRemoveMultiple(schemeIDs []irma.SchemeManagerIdent
 	})
 }
 
+func (client *Client) ensureKeyshareAttributeValid(pin string, kss *keyshareServer, transport *irma.HTTPTransport) {
+	// The user has no way to deal with the errors that may occur here, so we just report them and return.
+	manager := client.Configuration.SchemeManagers[kss.SchemeManagerIdentifier]
+	attrs := client.Attributes(irma.NewAttributeTypeIdentifier(manager.KeyshareAttribute).CredentialTypeIdentifier(), 0)
+	if attrs == nil {
+		client.reportError(errors.New("keyshare attribute not present"))
+		return
+	}
+	// Renew the keyshare attribute if it expires within a month.
+	if attrs.MetadataAttribute.Expiry().Before(time.Now().AddDate(0, 1, 0)) {
+		qr := &irma.Qr{}
+		if err := transport.Get("users/renewKeyshareAttribute", &qr); err != nil {
+			client.reportError(err)
+			return
+		}
+		handler := &backgroundIssuanceHandler{
+			pin:       pin,
+			resultErr: make(chan error),
+		}
+		client.newQrSession(qr, handler)
+		if err := <-handler.resultErr; err != nil {
+			client.reportError(err)
+			return
+		}
+	}
+}
+
 // Add, load and store log entries
 
 // LoadNewestLogs returns the log entries of latest past events
@@ -1492,9 +1550,6 @@ func (dcs DisclosureCandidates) Choose() ([]*irma.AttributeIdentifier, error) {
 		if !attr.Present() {
 			return nil, errors.New("credential not present")
 		}
-		if attr.Expired {
-			return nil, errors.New("cannot choose expired credential")
-		}
 		if attr.Revoked {
 			return nil, errors.New("cannot choose revoked credential")
 		}