From 7de2c3f6915c4225303479fcc1220f1bb89ebeff Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 13 Sep 2023 00:24:24 +0200 Subject: [PATCH 01/12] Feat: allow expired credentials to be disclosed and add renew service for keyshare attribute --- internal/sessiontest/keyshare_test.go | 41 ++++++++ internal/sessiontest/session_test.go | 40 ++++++++ irmaclient/client.go | 73 +++++++++++--- irmaclient/handlers.go | 122 +++++++++++++++-------- irmaclient/keyshare.go | 1 + irmago_test.go | 4 +- legacy.go | 17 ++-- requests.go | 5 +- server/irmaserver/api.go | 3 + server/irmaserver/helpers.go | 2 +- server/keyshare/keyshareserver/server.go | 45 +++++++-- verify.go | 16 ++- 12 files changed, 291 insertions(+), 78 deletions(-) diff --git a/internal/sessiontest/keyshare_test.go b/internal/sessiontest/keyshare_test.go index bc4efae5a..686f679f7 100644 --- a/internal/sessiontest/keyshare_test.go +++ b/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 TestKeyshareRenewal(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) { diff --git a/internal/sessiontest/session_test.go b/internal/sessiontest/session_test.go index 800ecec73..831eb6339 100644 --- a/internal/sessiontest/session_test.go +++ b/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) +} diff --git a/irmaclient/client.go b/irmaclient/client.go index 75ae16994..015ee885a 100644 --- a/irmaclient/client.go +++ b/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,25 @@ 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 := newBackgroundIssuanceHandler(pin) + client.newQrSession(qr, handler) + go func() { + err := <-handler.resultErr + if err != nil { + client.handler.EnrollmentFailure(managerID, irma.WrapErrorPrefix(err, "keyshare attribute issuance")) + return + } + for _, attr := range handler.credentials[0].Attributes { + kss.Username = attr + break + } + err = client.storage.StoreKeyshareServers(client.keyshareServers) + if err != nil { + client.handler.EnrollmentFailure(managerID, err) + return + } + client.handler.EnrollmentSuccess(kss.SchemeManagerIdentifier) + }() return nil } @@ -1182,9 +1203,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 +1417,30 @@ 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 := newBackgroundIssuanceHandler(pin) + 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 +1540,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") } diff --git a/irmaclient/handlers.go b/irmaclient/handlers.go index d51efa6a8..48b3199b0 100644 --- a/irmaclient/handlers.go +++ b/irmaclient/handlers.go @@ -1,35 +1,77 @@ package irmaclient import ( + "fmt" + "github.com/go-errors/errors" irma "github.com/privacybydesign/irmago" ) -// keyshareEnrollmentHandler handles the keyshare attribute issuance session -// after registering to a new keyshare server. -type keyshareEnrollmentHandler struct { - pin string - client *Client - kss *keyshareServer +// backgroundIssuanceHandler handles an IRMA issuance session in the background. +type backgroundIssuanceHandler struct { + pin string + + credentials []*irma.CredentialRequest + resultErr chan error +} + +func newBackgroundIssuanceHandler(pin string) *backgroundIssuanceHandler { + return &backgroundIssuanceHandler{ + pin: pin, + resultErr: make(chan error, 1), + } } // Force keyshareEnrollmentHandler to implement the Handler interface -var _ Handler = (*keyshareEnrollmentHandler)(nil) +var _ Handler = (*backgroundIssuanceHandler)(nil) // Session handlers in the order they are called -func (h *keyshareEnrollmentHandler) RequestIssuancePermission(request *irma.IssuanceRequest, satisfiable bool, candidates [][]DisclosureCandidates, ServerName *irma.RequestorInfo, callback PermissionHandler) { - // Fetch the username from the credential request and save it along with the scheme manager - for _, attr := range request.Credentials[0].Attributes { - h.kss.Username = attr - break +func (h *backgroundIssuanceHandler) RequestIssuancePermission(request *irma.IssuanceRequest, satisfiable bool, candidates [][]DisclosureCandidates, ServerName *irma.RequestorInfo, callback PermissionHandler) { + // First, collect all attributes that are to be issued. + attrsToBeIssued := map[irma.AttributeTypeIdentifier]string{} + for _, credReq := range request.Credentials { + for id, value := range credReq.Attributes { + attrsToBeIssued[irma.NewAttributeTypeIdentifier(fmt.Sprintf("%s.%s", credReq.CredentialTypeID, id))] = value + } + } + + // We only allow disclosing the previous values if the new values are the same. + var choice irma.DisclosureChoice + for _, discon := range candidates { + for _, con := range discon { + valid := true + for _, attr := range con { + if attr.CredentialHash == "" { + valid = false + break + } + if newValue, ok := attrsToBeIssued[attr.Type]; !ok || newValue != attr.Value[""] { + valid = false + break + } + } + if valid { + attrs, err := con.Choose() + if err != nil { + callback(false, nil) + return + } + choice.Attributes = append(choice.Attributes, attrs) + break + } + } + } + // Check whether we chose an option from every candidate discon. + if len(choice.Attributes) != len(candidates) { + callback(false, nil) + return } - // Do the issuance - callback(true, nil) + callback(true, &choice) } -func (h *keyshareEnrollmentHandler) RequestPin(remainingAttempts int, callback PinHandler) { +func (h *backgroundIssuanceHandler) RequestPin(remainingAttempts int, callback PinHandler) { if remainingAttempts == -1 { // -1 signifies that this is the first attempt callback(true, h.pin) } else { @@ -37,52 +79,52 @@ func (h *keyshareEnrollmentHandler) RequestPin(remainingAttempts int, callback P } } -func (h *keyshareEnrollmentHandler) Success(result string) { - _ = h.client.storage.StoreKeyshareServers(h.client.keyshareServers) // TODO handle err? - h.client.handler.EnrollmentSuccess(h.kss.SchemeManagerIdentifier) +func (h *backgroundIssuanceHandler) Success(result string) { + h.resultErr <- nil + close(h.resultErr) } -func (h *keyshareEnrollmentHandler) Failure(err *irma.SessionError) { +func (h *backgroundIssuanceHandler) Failure(err *irma.SessionError) { h.fail(err) } // fail is a helper to ensure the kss is removed from the client in case of any problem -func (h *keyshareEnrollmentHandler) fail(err error) { - delete(h.client.keyshareServers, h.kss.SchemeManagerIdentifier) - h.client.handler.EnrollmentFailure(h.kss.SchemeManagerIdentifier, err) +func (h *backgroundIssuanceHandler) fail(err error) { + h.resultErr <- err + close(h.resultErr) } // Not interested, ingore -func (h *keyshareEnrollmentHandler) StatusUpdate(action irma.Action, status irma.ClientStatus) {} +func (h *backgroundIssuanceHandler) StatusUpdate(action irma.Action, status irma.ClientStatus) {} // The methods below should never be called, so we let each of them fail the session -func (h *keyshareEnrollmentHandler) RequestVerificationPermission(request *irma.DisclosureRequest, satisfiable bool, candidates [][]DisclosureCandidates, ServerName *irma.RequestorInfo, callback PermissionHandler) { +func (h *backgroundIssuanceHandler) RequestVerificationPermission(request *irma.DisclosureRequest, satisfiable bool, candidates [][]DisclosureCandidates, ServerName *irma.RequestorInfo, callback PermissionHandler) { callback(false, nil) } -func (h *keyshareEnrollmentHandler) RequestSignaturePermission(request *irma.SignatureRequest, satisfiable bool, candidates [][]DisclosureCandidates, ServerName *irma.RequestorInfo, callback PermissionHandler) { +func (h *backgroundIssuanceHandler) RequestSignaturePermission(request *irma.SignatureRequest, satisfiable bool, candidates [][]DisclosureCandidates, ServerName *irma.RequestorInfo, callback PermissionHandler) { callback(false, nil) } -func (h *keyshareEnrollmentHandler) RequestSchemeManagerPermission(manager *irma.SchemeManager, callback func(proceed bool)) { +func (h *backgroundIssuanceHandler) RequestSchemeManagerPermission(manager *irma.SchemeManager, callback func(proceed bool)) { callback(false) } -func (h *keyshareEnrollmentHandler) Cancelled() { - h.fail(errors.New("Keyshare enrollment session unexpectedly cancelled")) +func (h *backgroundIssuanceHandler) Cancelled() { + h.fail(errors.New("session unexpectedly cancelled")) } -func (h *keyshareEnrollmentHandler) KeyshareBlocked(manager irma.SchemeManagerIdentifier, duration int) { - h.fail(errors.New("Keyshare enrollment failed: blocked")) +func (h *backgroundIssuanceHandler) KeyshareBlocked(manager irma.SchemeManagerIdentifier, duration int) { + h.fail(errors.New("user is blocked")) } -func (h *keyshareEnrollmentHandler) KeyshareEnrollmentIncomplete(manager irma.SchemeManagerIdentifier) { - h.fail(errors.New("Keyshare enrollment failed: registration incomplete")) +func (h *backgroundIssuanceHandler) KeyshareEnrollmentIncomplete(manager irma.SchemeManagerIdentifier) { + h.fail(errors.New("keyshare registration incomplete")) } -func (h *keyshareEnrollmentHandler) KeyshareEnrollmentDeleted(manager irma.SchemeManagerIdentifier) { - h.fail(errors.New("Keyshare enrollment failed: not enrolled")) +func (h *backgroundIssuanceHandler) KeyshareEnrollmentDeleted(manager irma.SchemeManagerIdentifier) { + h.fail(errors.New("keyshare enrollment deleted")) } -func (h *keyshareEnrollmentHandler) KeyshareEnrollmentMissing(manager irma.SchemeManagerIdentifier) { - h.fail(errors.New("Keyshare enrollment failed: unenrolled")) +func (h *backgroundIssuanceHandler) KeyshareEnrollmentMissing(manager irma.SchemeManagerIdentifier) { + h.fail(errors.New("keyshare enrollment missing")) } -func (h *keyshareEnrollmentHandler) ClientReturnURLSet(clientReturnURL string) { - h.fail(errors.New("Keyshare enrollment session unexpectedly found an external return url")) +func (h *backgroundIssuanceHandler) ClientReturnURLSet(clientReturnURL string) { + h.fail(errors.New("unexpectedly found an external return url")) } -func (h *keyshareEnrollmentHandler) PairingRequired(pairingCode string) { - h.fail(errors.New("Keyshare enrollment session failed: device pairing required")) +func (h *backgroundIssuanceHandler) PairingRequired(pairingCode string) { + h.fail(errors.New("device pairing required")) } diff --git a/irmaclient/keyshare.go b/irmaclient/keyshare.go index 08097474d..1e9b597f6 100644 --- a/irmaclient/keyshare.go +++ b/irmaclient/keyshare.go @@ -280,6 +280,7 @@ func (client *Client) verifyPinWorker(pin string, kss *keyshareServer, transport case kssPinSuccess: success = true kss.token = pinresult.Message + transport.SetHeader(kssUsernameHeader, kss.Username) transport.SetHeader(kssAuthHeader, kss.token) return case kssPinFailure: diff --git a/irmago_test.go b/irmago_test.go index c8a120865..535edb84c 100644 --- a/irmago_test.go +++ b/irmago_test.go @@ -484,7 +484,7 @@ func TestSessionRequests(t *testing.T) { { expected: &SignatureRequest{ - DisclosureRequest{BaseRequest{LDContext: LDContextSignatureRequest}, base.Disclose, base.Labels}, + DisclosureRequest{BaseRequest{LDContext: LDContextSignatureRequest}, base.Disclose, base.Labels, base.SkipExpiryCheck}, sigMessage, }, old: &SignatureRequest{}, @@ -539,7 +539,7 @@ func TestSessionRequests(t *testing.T) { { expected: &IssuanceRequest{ - DisclosureRequest: DisclosureRequest{BaseRequest{LDContext: LDContextIssuanceRequest}, base.Disclose, base.Labels}, + DisclosureRequest: DisclosureRequest{BaseRequest{LDContext: LDContextIssuanceRequest}, base.Disclose, base.Labels, base.SkipExpiryCheck}, Credentials: []*CredentialRequest{ { CredentialTypeID: NewCredentialTypeIdentifier("irma-demo.MijnOverheid.root"), diff --git a/legacy.go b/legacy.go index b5f5432fe..1dd535dac 100644 --- a/legacy.go +++ b/legacy.go @@ -227,9 +227,10 @@ func (sr *SignatureRequest) UnmarshalJSON(bts []byte) (err error) { if ldContext != "" { var req struct { // Identical type with default JSON unmarshaler BaseRequest - Disclose AttributeConDisCon `json:"disclose"` - Labels map[int]TranslatedString `json:"labels"` - Message string `json:"message"` + Disclose AttributeConDisCon `json:"disclose"` + Labels map[int]TranslatedString `json:"labels"` + SkipExpiryCheck []CredentialTypeIdentifier `json:"skipExpiryCheck,omitempty"` + Message string `json:"message"` } if err = json.Unmarshal(bts, &req); err != nil { return err @@ -239,6 +240,7 @@ func (sr *SignatureRequest) UnmarshalJSON(bts []byte) (err error) { req.BaseRequest, req.Disclose, req.Labels, + req.SkipExpiryCheck, }, req.Message, } @@ -284,15 +286,16 @@ func (ir *IssuanceRequest) UnmarshalJSON(bts []byte) (err error) { if ldContext != "" { var req struct { // Identical type with default JSON unmarshaler BaseRequest - Disclose AttributeConDisCon `json:"disclose"` - Labels map[int]TranslatedString `json:"labels"` - Credentials []*CredentialRequest `json:"credentials"` + Disclose AttributeConDisCon `json:"disclose"` + Labels map[int]TranslatedString `json:"labels"` + SkipExpiryCheck []CredentialTypeIdentifier `json:"skipExpiryCheck,omitempty"` + Credentials []*CredentialRequest `json:"credentials"` } if err = json.Unmarshal(bts, &req); err != nil { return err } *ir = IssuanceRequest{ - DisclosureRequest: DisclosureRequest{req.BaseRequest, req.Disclose, req.Labels}, + DisclosureRequest: DisclosureRequest{req.BaseRequest, req.Disclose, req.Labels, req.SkipExpiryCheck}, Credentials: req.Credentials, } return nil diff --git a/requests.go b/requests.go index 2ccf58281..4d0fc48c0 100644 --- a/requests.go +++ b/requests.go @@ -69,6 +69,8 @@ type DisclosureRequest struct { Disclose AttributeConDisCon `json:"disclose,omitempty"` Labels map[int]TranslatedString `json:"labels,omitempty"` + + SkipExpiryCheck []CredentialTypeIdentifier `json:"skipExpiryCheck,omitempty"` } // A SignatureRequest is a a request to sign a message with certain attributes. Construct new @@ -792,9 +794,6 @@ func (ir *IssuanceRequest) Validate() error { if count != 2 { return errors.Errorf("Expected credential ID to consist of 3 parts, %d found", count+1) } - if cred.Validity != nil && cred.Validity.Floor().Before(Timestamp(time.Now())) { - return errors.New("Expired credential request") - } } var err error for _, discon := range ir.Disclose { diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 1b6b2a5a4..694b33031 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -25,6 +25,9 @@ import ( "github.com/sirupsen/logrus" ) +// AllowIssuingExpiredCredentials indicates whether or not expired credentials can be issued. +var AllowIssuingExpiredCredentials = false + type Server struct { conf *server.Configuration router *chi.Mux diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index 391c4b3f3..f805248a7 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -303,7 +303,7 @@ func (s *Server) validateIssuanceRequest(request *irma.IssuanceRequest) error { if cred.Validity == nil { cred.Validity = &defaultValidity } - if cred.Validity.Before(irma.Timestamp(now)) { + if !AllowIssuingExpiredCredentials && cred.Validity.Before(irma.Timestamp(now)) { return errors.New("cannot issue expired credentials") } } diff --git a/server/keyshare/keyshareserver/server.go b/server/keyshare/keyshareserver/server.go index 5450ea459..ce9874af5 100644 --- a/server/keyshare/keyshareserver/server.go +++ b/server/keyshare/keyshareserver/server.go @@ -150,8 +150,13 @@ func (s *Server) routeHandler(r chi.Router) http.Handler { r.Group(func(router chi.Router) { router.Use(s.userMiddleware) router.Use(s.authorizationMiddleware) + + // Keyshare protocol router.Post("/prove/getCommitments", s.handleCommitments) router.Post("/prove/getResponse", s.handleResponse) + + // User management + router.Get("/users/renewKeyshareAttribute", s.handleRenewKeyshareAttribute) }) return r @@ -505,6 +510,26 @@ func (s *Server) handleChangePin(w http.ResponseWriter, r *http.Request) { server.WriteJson(w, result) } +// /users/renewKeyshareAttribute +func (s *Server) handleRenewKeyshareAttribute(w http.ResponseWriter, r *http.Request) { + // Fetch from context + user := r.Context().Value("user").(*User) + + // Make an issuance request that first validates whether an old (expired) instance of the keyshare attribute is present + // and issues a new one if the username matches. + irmaReq := s.keyshareAttributeIssuanceRequest(user.Username) + irmaReq.AddSingle(s.conf.KeyshareAttribute, &user.Username, nil) + irmaReq.Disclosure().SkipExpiryCheck = []irma.CredentialTypeIdentifier{s.conf.KeyshareAttribute.CredentialTypeIdentifier()} + + sessionptr, _, _, err := s.irmaserv.StartSession(irmaReq, nil) + if err != nil { + server.WriteError(w, server.ErrorUnknown, err.Error()) + return + } + + server.WriteJson(w, sessionptr) +} + func (s *Server) updatePin(ctx context.Context, user *User, jwtt string) (irma.KeysharePinStatus, error) { // Check whether pin check is currently allowed ok, tries, wait, err := s.reservePinCheck(ctx, user) @@ -620,13 +645,7 @@ func (s *Server) register(ctx context.Context, msg irma.KeyshareEnrollment) (*ir } // Setup and return issuance session for keyshare credential. - request := irma.NewIssuanceRequest([]*irma.CredentialRequest{ - { - CredentialTypeID: s.conf.KeyshareAttribute.CredentialTypeIdentifier(), - Attributes: map[string]string{ - s.conf.KeyshareAttribute.Name(): username, - }, - }}) + request := s.keyshareAttributeIssuanceRequest(username) sessionptr, _, _, err := s.irmaserv.StartSession(request, nil) if err != nil { s.conf.Logger.WithField("error", err).Error("Could not start keyshare credential issuance sessions") @@ -717,3 +736,15 @@ func (s *Server) reservePinCheck(ctx context.Context, user *User) (bool, int, in } return true, tries, wait, nil } + +func (s *Server) keyshareAttributeIssuanceRequest(username string) *irma.IssuanceRequest { + validity := irma.Timestamp(time.Now().AddDate(1, 0, 0)) // 1 year from now + return irma.NewIssuanceRequest([]*irma.CredentialRequest{ + { + CredentialTypeID: s.conf.KeyshareAttribute.CredentialTypeIdentifier(), + Attributes: map[string]string{ + s.conf.KeyshareAttribute.Name(): username, + }, + Validity: &validity, + }}) +} diff --git a/verify.go b/verify.go index 142b44d6f..10cf0a536 100644 --- a/verify.go +++ b/verify.go @@ -2,6 +2,7 @@ package irma import ( "crypto/rsa" + "slices" "time" "github.com/go-errors/errors" @@ -73,7 +74,7 @@ func (pl ProofList) ExtractPublicKeys(configuration *Configuration) ([]*gabikeys // Expired returns true if any of the contained disclosure proofs is specified at the specified time, // or now, when the specified time is nil. -func (pl ProofList) Expired(configuration *Configuration, t *time.Time) (bool, error) { +func (pl ProofList) Expired(configuration *Configuration, t *time.Time, skipExpiryCheck []CredentialTypeIdentifier) (bool, error) { if t == nil { temp := time.Now() t = &temp @@ -83,10 +84,14 @@ func (pl ProofList) Expired(configuration *Configuration, t *time.Time) (bool, e if !ok { continue } + metadata := MetadataFromInt(proofd.ADisclosed[1], configuration) // index 1 is metadata attribute - if metadata.Expiry().Before(*t) { + + skipCheck := slices.Contains(skipExpiryCheck, metadata.CredentialType().Identifier()) + if !skipCheck && metadata.Expiry().Before(*t) { return true, nil } + pk, err := metadata.PublicKey() if err != nil { return false, err @@ -360,8 +365,11 @@ func (d *Disclosure) VerifyAgainstRequest( // Next extract the contained attributes from the proofs, and match them to the signature request if present var required AttributeConDisCon + var skipExpiryCheck []CredentialTypeIdentifier if request != nil { - required = request.Disclosure().Disclose + disclosureRequest := request.Disclosure() + required = disclosureRequest.Disclose + skipExpiryCheck = disclosureRequest.SkipExpiryCheck } allmatched, list, err := d.DisclosedAttributes(configuration, required, revtimes) if err != nil { @@ -374,7 +382,7 @@ func (d *Disclosure) VerifyAgainstRequest( } // Check that all credentials were unexpired - expired, err := ProofList(d.Proofs).Expired(configuration, validAt) + expired, err := ProofList(d.Proofs).Expired(configuration, validAt, skipExpiryCheck) if err != nil { return nil, ProofStatusInvalid, err } From ae1e38e3998963962f2cbff1ff09d48936459102 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 13 Sep 2023 12:00:18 +0200 Subject: [PATCH 02/12] Chore: bump go version to 1.21.1 --- go.mod | 4 +++- go.sum | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 968723be9..4ad65d337 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index c8c0552bf..413e4b6a7 100644 --- a/go.sum +++ b/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= From 9fdeddb4d1ecc909daf6501d875091dd657f3885 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 14 Sep 2023 09:38:03 +0200 Subject: [PATCH 03/12] Fix: keyshare username not known during keyshare attribute issuance --- internal/sessiontest/keyshare_test.go | 2 +- irmaclient/client.go | 22 ++++++++++++++++------ irmaclient/handlers.go | 25 ++++++++++++------------- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/internal/sessiontest/keyshare_test.go b/internal/sessiontest/keyshare_test.go index 686f679f7..d97909791 100644 --- a/internal/sessiontest/keyshare_test.go +++ b/internal/sessiontest/keyshare_test.go @@ -52,7 +52,7 @@ func TestKeyshareRegister(t *testing.T) { keyshareSessions(t, client, irmaServer) } -func TestKeyshareRenewal(t *testing.T) { +func TestKeyshareAttributeRenewal(t *testing.T) { keyshareServer := testkeyshare.StartKeyshareServer(t, logger, irma.NewSchemeManagerIdentifier("test")) defer keyshareServer.Stop() diff --git a/irmaclient/client.go b/irmaclient/client.go index 015ee885a..b4a227a3b 100644 --- a/irmaclient/client.go +++ b/irmaclient/client.go @@ -1162,7 +1162,18 @@ 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 - handler := newBackgroundIssuanceHandler(pin) + 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 @@ -1170,10 +1181,6 @@ func (client *Client) keyshareEnrollWorker(managerID irma.SchemeManagerIdentifie client.handler.EnrollmentFailure(managerID, irma.WrapErrorPrefix(err, "keyshare attribute issuance")) return } - for _, attr := range handler.credentials[0].Attributes { - kss.Username = attr - break - } err = client.storage.StoreKeyshareServers(client.keyshareServers) if err != nil { client.handler.EnrollmentFailure(managerID, err) @@ -1432,7 +1439,10 @@ func (client *Client) ensureKeyshareAttributeValid(pin string, kss *keyshareServ client.reportError(err) return } - handler := newBackgroundIssuanceHandler(pin) + handler := &backgroundIssuanceHandler{ + pin: pin, + resultErr: make(chan error), + } client.newQrSession(qr, handler) if err := <-handler.resultErr; err != nil { client.reportError(err) diff --git a/irmaclient/handlers.go b/irmaclient/handlers.go index 48b3199b0..a7e1ac385 100644 --- a/irmaclient/handlers.go +++ b/irmaclient/handlers.go @@ -11,15 +11,8 @@ import ( type backgroundIssuanceHandler struct { pin string - credentials []*irma.CredentialRequest - resultErr chan error -} - -func newBackgroundIssuanceHandler(pin string) *backgroundIssuanceHandler { - return &backgroundIssuanceHandler{ - pin: pin, - resultErr: make(chan error, 1), - } + credentialsToBeIssuedCallback func([]*irma.CredentialRequest) + resultErr chan error } // Force keyshareEnrollmentHandler to implement the Handler interface @@ -28,6 +21,10 @@ var _ Handler = (*backgroundIssuanceHandler)(nil) // Session handlers in the order they are called func (h *backgroundIssuanceHandler) RequestIssuancePermission(request *irma.IssuanceRequest, satisfiable bool, candidates [][]DisclosureCandidates, ServerName *irma.RequestorInfo, callback PermissionHandler) { + if h.credentialsToBeIssuedCallback != nil { + h.credentialsToBeIssuedCallback(request.Credentials) + } + // First, collect all attributes that are to be issued. attrsToBeIssued := map[irma.AttributeTypeIdentifier]string{} for _, credReq := range request.Credentials { @@ -80,8 +77,9 @@ func (h *backgroundIssuanceHandler) RequestPin(remainingAttempts int, callback P } func (h *backgroundIssuanceHandler) Success(result string) { - h.resultErr <- nil - close(h.resultErr) + if h.resultErr != nil { + h.resultErr <- nil + } } func (h *backgroundIssuanceHandler) Failure(err *irma.SessionError) { @@ -90,8 +88,9 @@ func (h *backgroundIssuanceHandler) Failure(err *irma.SessionError) { // fail is a helper to ensure the kss is removed from the client in case of any problem func (h *backgroundIssuanceHandler) fail(err error) { - h.resultErr <- err - close(h.resultErr) + if h.resultErr != nil { + h.resultErr <- err + } } // Not interested, ingore From e0f59dc623a50c309c7f7bfca4583f931a53bfec Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 14 Sep 2023 10:39:24 +0200 Subject: [PATCH 04/12] Chore: update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bd306b76..c25c4d562 100644 --- a/CHANGELOG.md +++ b/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 ### Fixed - User account expiry continues when one or more e-mail addresses are marked for revalidation From e84592f72d42e91b6d8b46fb2775b7c075c821a0 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 14 Sep 2023 10:52:47 +0200 Subject: [PATCH 05/12] Docs: improve grammar in comments irmaclient/handlers.go --- irmaclient/handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/irmaclient/handlers.go b/irmaclient/handlers.go index a7e1ac385..5a14c736e 100644 --- a/irmaclient/handlers.go +++ b/irmaclient/handlers.go @@ -25,7 +25,7 @@ func (h *backgroundIssuanceHandler) RequestIssuancePermission(request *irma.Issu h.credentialsToBeIssuedCallback(request.Credentials) } - // First, collect all attributes that are to be issued. + // First, collect all attributes that are going to be issued. attrsToBeIssued := map[irma.AttributeTypeIdentifier]string{} for _, credReq := range request.Credentials { for id, value := range credReq.Attributes { From 1a62b53f822ffbf3691f3a1b62395f77db0ea35b Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Fri, 15 Sep 2023 10:48:26 +0200 Subject: [PATCH 06/12] CI/CD: use go 1.21 --- .github/actions/build/action.yml | 2 +- .github/workflows/status-checks.yml | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml index 2fad73e8e..8e69c326f 100644 --- a/.github/actions/build/action.yml +++ b/.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 diff --git a/.github/workflows/status-checks.yml b/.github/workflows/status-checks.yml index 5a50447a7..275d1224f 100644 --- a/.github/workflows/status-checks.yml +++ b/.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: From 8dd2a4a529bac322071dd7b201934db92b9eb50b Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Mon, 25 Sep 2023 13:28:58 +0200 Subject: [PATCH 07/12] Feat: option to overrule host name in IRMA QR --- CHANGELOG.md | 35 ++++++++++ internal/sessiontest/helper_servers_test.go | 9 +++ internal/sessiontest/session_test.go | 50 +++++++++++++ requests.go | 2 + server/irmaserver/api.go | 13 +++- server/irmaserver/helpers.go | 6 ++ server/requestorserver/conf.go | 77 +++++++++++++++------ server/requestorserver/server.go | 24 ++----- 8 files changed, 173 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a85ea09f..099b8ab7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,41 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added - Option `skipExpiryCheck` in disclosure requests to allow disclosure of expired credentials (e.g. `"skipExpiryCheck": ["irma-demo.sidn-pbdf.email"]`) +- Option `host` in session request to overrule host name in IRMA QR if permission has been granted (see below) + ``` + { + "@context": "https://irma.app/ld/request/disclosure/v2", + "host": "irma.example.com", + "disclose": ... + } + ``` + This leads to the following session package: + ``` + { + "token":"KzxuWKwL5KGLKr4uerws", + "sessionPtr": {"u":"https://irma.example.com/irma/session/ysDohpoySavbHAUDjmpz","irmaqr":"disclosing"}, + "frontendRequest": { + "authorization":"qGrMmL8UZwZ88Sq8gobV", + "minProtocolVersion": "1.0", + "maxProtocolVersion": "1.1" + } + } + ``` +- Permission option `hosts` in the requestor configuration to specify which values a requestor may use for the `host` option in session requests + ``` + { + "requestors": { + "myapp": { + "disclose_perms": [ "irma-demo.MijnOverheid.ageLower.over18" ], + "sign_perms": [ "irma-demo.MijnOverheid.ageLower.*" ], + "issue_perms": [ "irma-demo.MijnOverheid.ageLower" ], + "hosts": ["*.example.com"] + "auth_method": "token", + "key": "eGE2PSomOT84amVVdTU" + } + } + } + ``` - Renewal endpoint for keyshare attribute in the keyshare server (`/users/renewKeyshareAttribute`) ### Changed diff --git a/internal/sessiontest/helper_servers_test.go b/internal/sessiontest/helper_servers_test.go index b7afe7194..195112bfc 100644 --- a/internal/sessiontest/helper_servers_test.go +++ b/internal/sessiontest/helper_servers_test.go @@ -281,14 +281,23 @@ func RequestorServerAuthConfiguration() *requestorserver.Configuration { "requestor1": { AuthenticationMethod: requestorserver.AuthenticationMethodPublicKey, AuthenticationKeyFile: filepath.Join(testdata, "jwtkeys", "requestor1.pem"), + Permissions: requestorserver.Permissions{ + Hosts: []string{"localhost:48682"}, + }, }, "requestor2": { AuthenticationMethod: requestorserver.AuthenticationMethodToken, AuthenticationKey: TokenAuthenticationKey, + Permissions: requestorserver.Permissions{ + Hosts: []string{"localhost:48682"}, + }, }, "requestor3": { AuthenticationMethod: requestorserver.AuthenticationMethodHmac, AuthenticationKey: HmacAuthenticationKey, + Permissions: requestorserver.Permissions{ + Hosts: []string{"localhost:48682"}, + }, }, } return conf diff --git a/internal/sessiontest/session_test.go b/internal/sessiontest/session_test.go index 831eb6339..ab747e1a1 100644 --- a/internal/sessiontest/session_test.go +++ b/internal/sessiontest/session_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + "github.com/golang-jwt/jwt/v4" "github.com/privacybydesign/gabi/big" irma "github.com/privacybydesign/irmago" "github.com/privacybydesign/irmago/internal/common" @@ -1303,3 +1304,52 @@ func TestExpiredCredential(t *testing.T) { disclosureRequest.SkipExpiryCheck = []irma.CredentialTypeIdentifier{issuanceRequest.Credentials[0].CredentialTypeID} doSession(t, disclosureRequest, client, irmaServer, nil, nil, nil) } + +func TestRequestorHostPermissions(t *testing.T) { + client, handler := parseStorage(t) + defer test.ClearTestStorage(t, client, handler.storage) + rs := StartRequestorServer(t, RequestorServerAuthConfiguration()) + defer rs.Stop() + + // Check that a requestor can use a host that is allowed. + id := irma.NewAttributeTypeIdentifier("irma-demo.RU.studentCard.studentID") + request := getDisclosureRequest(id) + sesPkg := &server.SessionPackage{} + + // Check that a requestor can't use a host that is not allowed. + request.Base().Host = "127.0.0.1:48682" + err := irma.NewHTTPTransport(requestorServerURL, false).Post("session", &server.SessionPackage{}, signSessionRequest(t, request)) + require.Error(t, err) + require.Contains(t, err.Error(), "requestor not allowed to use the requested host") + + // Start a new session using the allowed host. + request.Base().Host = "localhost:48682" + err = irma.NewHTTPTransport(requestorServerURL, false).Post("session", sesPkg, signSessionRequest(t, request)) + require.NoError(t, err) + realURL := sesPkg.SessionPtr.URL + + // Check that a client can't use another host than the requestor wanted. + sesPkg.SessionPtr.URL = strings.Replace(sesPkg.SessionPtr.URL, "localhost", "127.0.0.1", 1) + sessionHandler, resultChan := createSessionHandler(t, optionIgnoreError, client, sesPkg, nil, nil) + startSessionAtClient(t, sesPkg, client, sessionHandler) + result := <-resultChan + require.Error(t, result.Err) + require.Contains(t, result.Err.Error(), "Host mismatch") + + // Check that a client can use the host the requestor wanted. + sesPkg.SessionPtr.URL = realURL + sessionHandler, resultChan = createSessionHandler(t, 0, client, sesPkg, nil, nil) + startSessionAtClient(t, sesPkg, client, sessionHandler) + result = <-resultChan + require.Nil(t, result) +} + +func signSessionRequest(t *testing.T, req irma.SessionRequest) string { + skbts, err := os.ReadFile(filepath.Join(testdata, "jwtkeys", "requestor1-sk.pem")) + require.NoError(t, err) + sk, err := jwt.ParseRSAPrivateKeyFromPEM(skbts) + require.NoError(t, err) + j, err := irma.SignSessionRequest(req, jwt.SigningMethodRS256, sk, "requestor1") + require.NoError(t, err) + return j +} diff --git a/requests.go b/requests.go index 4d0fc48c0..cde5c948d 100644 --- a/requests.go +++ b/requests.go @@ -51,6 +51,8 @@ type BaseRequest struct { ClientReturnURL string `json:"clientReturnUrl,omitempty"` // URL to proceed to when IRMA session is completed AugmentReturnURL bool `json:"augmentReturnUrl,omitempty"` // Whether to augment the return url with the server session token + + Host string `json:"host,omitempty"` // Host to use in the IRMA session QR } // An AttributeCon is only satisfied if all of its containing attribute requests are satisfied. diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 694b33031..535bb7fe5 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -9,6 +9,7 @@ import ( "crypto/tls" "crypto/x509" "net/http" + "net/url" "time" "github.com/go-co-op/gocron" @@ -315,9 +316,19 @@ func (s *Server) startNextSession( Info("Session request (purged of attribute values): ", server.ToJson(purgeRequest(rrequest))) } session.handler = handler + + url, err := url.Parse(s.conf.URL) + if err != nil { + return nil, "", nil, err + } + url = url.JoinPath("session", string(session.ClientToken)) + if request.Base().Host != "" { + url.Host = request.Base().Host + } + return &irma.Qr{ Type: action, - URL: s.conf.URL + "session/" + string(session.ClientToken), + URL: url.String(), }, session.RequestorToken, &irma.FrontendSessionRequest{ diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index f805248a7..edf11ce86 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -604,6 +604,12 @@ func (s *Server) sessionMiddleware(next http.Handler) http.Handler { } }() + expectedHost := session.request.Base().Host + if expectedHost != "" && expectedHost != r.Host { + server.WriteError(w, server.ErrorUnauthorized, "Host mismatch") + return + } + next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), "session", session))) }) } diff --git a/server/requestorserver/conf.go b/server/requestorserver/conf.go index a453d73a1..a0440a0b7 100644 --- a/server/requestorserver/conf.go +++ b/server/requestorserver/conf.go @@ -3,6 +3,9 @@ package requestorserver import ( "crypto/tls" "fmt" + "net/url" + "path/filepath" + "slices" "strings" "github.com/go-errors/errors" @@ -64,6 +67,8 @@ type Permissions struct { Signing []string `json:"sign_perms" mapstructure:"sign_perms"` Issuing []string `json:"issue_perms" mapstructure:"issue_perms"` Revoking []string `json:"revoke_perms" mapstructure:"revoke_perms"` + + Hosts []string `json:"hosts" mapstructure:"hosts"` } // Requestor contains all configuration (disclosure or verification permissions and authentication) @@ -76,6 +81,42 @@ type Requestor struct { AuthenticationKeyFile string `json:"key_file" mapstructure:"key_file"` } +func (conf *Configuration) CanRequest(requestor string, request irma.SessionRequest) (bool, string) { + if request.Action() == irma.ActionIssuing { + if ok, reason := conf.CanIssue(requestor, request.(*irma.IssuanceRequest).Credentials); !ok { + return false, reason + } + } + + condiscon := request.Disclosure().Disclose + if len(condiscon) > 0 { + if ok, reason := conf.CanVerifyOrSign(requestor, request.Action(), condiscon); !ok { + return false, reason + } + } + + host := request.Base().Host + if host != "" { + if len(conf.Requestors[requestor].Hosts) == 0 { + defaultURL, err := url.Parse(conf.URL) + if err != nil { + return false, "default host is invalid" + } + if host == defaultURL.Host { + return true, "" + } + } + for _, hostPattern := range conf.Requestors[requestor].Hosts { + if match, _ := filepath.Match(hostPattern, host); match { + return true, "" + } + } + return false, "requestor not allowed to use the requested host" + } + + return true, "" +} + // CanIssue returns whether or not the specified requestor may issue the specified credentials. // (In case of combined issuance/disclosure sessions, this method does not check whether or not // the identity provider is allowed to verify the attributes being verified; use CanVerifyOrSign @@ -88,10 +129,10 @@ func (conf *Configuration) CanIssue(requestor string, creds []*irma.CredentialRe for _, cred := range creds { id := cred.CredentialTypeID - if contains(permissions, "*") || - contains(permissions, id.Root()+".*") || - contains(permissions, id.IssuerIdentifier().String()+".*") || - contains(permissions, id.String()) { + if slices.Contains(permissions, "*") || + slices.Contains(permissions, id.Root()+".*") || + slices.Contains(permissions, id.IssuerIdentifier().String()+".*") || + slices.Contains(permissions, id.String()) { continue } else { return false, id.String() @@ -118,11 +159,11 @@ func (conf *Configuration) CanVerifyOrSign(requestor string, action irma.Action, } err := disjunctions.Iterate(func(attr *irma.AttributeRequest) error { - if contains(permissions, "*") || - contains(permissions, attr.Type.Root()+".*") || - contains(permissions, attr.Type.CredentialTypeIdentifier().IssuerIdentifier().String()+".*") || - contains(permissions, attr.Type.CredentialTypeIdentifier().String()+".*") || - contains(permissions, attr.Type.String()) { + if slices.Contains(permissions, "*") || + slices.Contains(permissions, attr.Type.Root()+".*") || + slices.Contains(permissions, attr.Type.CredentialTypeIdentifier().IssuerIdentifier().String()+".*") || + slices.Contains(permissions, attr.Type.CredentialTypeIdentifier().String()+".*") || + slices.Contains(permissions, attr.Type.String()) { return nil } else { return errors.New(attr.Type.String()) @@ -143,10 +184,10 @@ func (conf *Configuration) CanRevoke(requestor string, cred irma.CredentialTypeI if err != nil { return false, err.Error() } - if contains(permissions, "*") || - contains(permissions, cred.Root()+".*") || - contains(permissions, cred.IssuerIdentifier().String()+".*") || - contains(permissions, cred.String()) { + if slices.Contains(permissions, "*") || + slices.Contains(permissions, cred.Root()+".*") || + slices.Contains(permissions, cred.IssuerIdentifier().String()+".*") || + slices.Contains(permissions, cred.String()) { return true, "" } return false, cred.String() @@ -383,13 +424,3 @@ func (conf *Configuration) tlsConfig() (*tls.Config, error) { func (conf *Configuration) separateClientServer() bool { return conf.ClientPort != 0 } - -// Return true iff query equals an element of strings. -func contains(strings []string, query string) bool { - for _, s := range strings { - if s == query { - return true - } - } - return false -} diff --git a/server/requestorserver/server.go b/server/requestorserver/server.go index 4a32e7921..363a1f514 100644 --- a/server/requestorserver/server.go +++ b/server/requestorserver/server.go @@ -492,25 +492,11 @@ func (s *Server) createSession(w http.ResponseWriter, requestor string, rrequest // Authorize request: check if the requestor is allowed to verify or issue // the requested attributes or credentials request := rrequest.SessionRequest() - if request.Action() == irma.ActionIssuing { - allowed, reason := s.conf.CanIssue(requestor, request.(*irma.IssuanceRequest).Credentials) - if !allowed { - s.conf.Logger.WithFields(logrus.Fields{"requestor": requestor, "id": reason}). - Warn("Requestor not authorized to issue credential; full request: ", server.ToJson(request)) - server.WriteError(w, server.ErrorUnauthorized, reason) - return - } - } - - condiscon := request.Disclosure().Disclose - if len(condiscon) > 0 { - allowed, reason := s.conf.CanVerifyOrSign(requestor, request.Action(), condiscon) - if !allowed { - s.conf.Logger.WithFields(logrus.Fields{"requestor": requestor, "id": reason}). - Warn("Requestor not authorized to verify attribute; full request: ", server.ToJson(request)) - server.WriteError(w, server.ErrorUnauthorized, reason) - return - } + if allowed, reason := s.conf.CanRequest(requestor, request); !allowed { + s.conf.Logger.WithFields(logrus.Fields{"requestor": requestor, "id": reason}). + Warn("Requestor not authorized to do session; full request: ", server.ToJson(request)) + server.WriteError(w, server.ErrorUnauthorized, reason) + return } if rrequest.Base().NextSession != nil && rrequest.Base().NextSession.URL == "" { From be40d6333cdbad79059d50ea8d3ada00a7d7ada6 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Tue, 26 Sep 2023 10:29:18 +0200 Subject: [PATCH 08/12] Improvement: also check permission when falling back to default host --- server/requestorserver/conf.go | 38 +++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/server/requestorserver/conf.go b/server/requestorserver/conf.go index a0440a0b7..ad558f1e6 100644 --- a/server/requestorserver/conf.go +++ b/server/requestorserver/conf.go @@ -95,26 +95,30 @@ func (conf *Configuration) CanRequest(requestor string, request irma.SessionRequ } } + defaultURL, err := url.Parse(conf.URL) + if err != nil { + return false, "default host is invalid" + } + + // If no host is specified in the request, then the default URL from the configuration will be used. + // We should take this into account when checking the permissions. host := request.Base().Host - if host != "" { - if len(conf.Requestors[requestor].Hosts) == 0 { - defaultURL, err := url.Parse(conf.URL) - if err != nil { - return false, "default host is invalid" - } - if host == defaultURL.Host { - return true, "" - } - } - for _, hostPattern := range conf.Requestors[requestor].Hosts { - if match, _ := filepath.Match(hostPattern, host); match { - return true, "" - } - } - return false, "requestor not allowed to use the requested host" + if host == "" { + host = defaultURL.Host } - return true, "" + // If no host are specified in the requestor configuration, then we only allow the default host. + if len(conf.Requestors[requestor].Hosts) == 0 && host == defaultURL.Host { + return true, "" + } + + // For all host patterns being set in the requestor configuration, check whether the requested host matches it. + for _, hostPattern := range conf.Requestors[requestor].Hosts { + if match, _ := filepath.Match(hostPattern, host); match { + return true, "" + } + } + return false, "requestor not allowed to use the requested host" } // CanIssue returns whether or not the specified requestor may issue the specified credentials. From 1f8bea45705c672da5c439f978d039a32e76add7 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Tue, 26 Sep 2023 10:32:16 +0200 Subject: [PATCH 09/12] Docs: fix spelling mistake in comments --- server/requestorserver/conf.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/requestorserver/conf.go b/server/requestorserver/conf.go index ad558f1e6..9c0913b6b 100644 --- a/server/requestorserver/conf.go +++ b/server/requestorserver/conf.go @@ -107,7 +107,7 @@ func (conf *Configuration) CanRequest(requestor string, request irma.SessionRequ host = defaultURL.Host } - // If no host are specified in the requestor configuration, then we only allow the default host. + // If no host is specified in the requestor configuration, then we only allow the default host. if len(conf.Requestors[requestor].Hosts) == 0 && host == defaultURL.Host { return true, "" } From 328d7b62a33b7ca4132bd108378404fa9357a0c4 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Tue, 26 Sep 2023 13:59:40 +0200 Subject: [PATCH 10/12] Test: fix port in url in TestRedisRedundancy --- internal/sessiontest/redis_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/sessiontest/redis_test.go b/internal/sessiontest/redis_test.go index bc09544be..b8de08bbd 100644 --- a/internal/sessiontest/redis_test.go +++ b/internal/sessiontest/redis_test.go @@ -246,7 +246,7 @@ func TestRedisRedundancy(t *testing.T) { for i, port := range ports { c := redisRequestorConfigDecorator(mr, cert, "", RequestorServerAuthConfiguration)() - c.Configuration.URL = fmt.Sprintf("http://localhost:%d/irma", port) + c.Configuration.URL = fmt.Sprintf("http://localhost:%d/irma", requestorServerPort) c.Port = port rs := StartRequestorServer(t, c) servers[i] = rs From 5b6c270b6fe4db7fb0db76f7b4df9f0c75d87d48 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Tue, 26 Sep 2023 14:00:22 +0200 Subject: [PATCH 11/12] Refactor: make json keys consistent in requestor permissions --- CHANGELOG.md | 4 ++-- server/requestorserver/conf.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 099b8ab7a..ad991e711 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 } } ``` -- Permission option `hosts` in the requestor configuration to specify which values a requestor may use for the `host` option in session requests +- Permission option `host_perms` in the requestor configuration to specify which values a requestor may use for the `host` option in session requests ``` { "requestors": { @@ -35,7 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 "disclose_perms": [ "irma-demo.MijnOverheid.ageLower.over18" ], "sign_perms": [ "irma-demo.MijnOverheid.ageLower.*" ], "issue_perms": [ "irma-demo.MijnOverheid.ageLower" ], - "hosts": ["*.example.com"] + "host_perms": ["*.example.com"] "auth_method": "token", "key": "eGE2PSomOT84amVVdTU" } diff --git a/server/requestorserver/conf.go b/server/requestorserver/conf.go index 9c0913b6b..63455dd88 100644 --- a/server/requestorserver/conf.go +++ b/server/requestorserver/conf.go @@ -68,7 +68,7 @@ type Permissions struct { Issuing []string `json:"issue_perms" mapstructure:"issue_perms"` Revoking []string `json:"revoke_perms" mapstructure:"revoke_perms"` - Hosts []string `json:"hosts" mapstructure:"hosts"` + Hosts []string `json:"host_perms" mapstructure:"host_perms"` } // Requestor contains all configuration (disclosure or verification permissions and authentication) From f46822804e08c7d20a649245b1e369552f998224 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Tue, 26 Sep 2023 14:09:23 +0200 Subject: [PATCH 12/12] Refactor: use path.Match instead of filepath.Match --- server/requestorserver/conf.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/requestorserver/conf.go b/server/requestorserver/conf.go index 63455dd88..aa889e93d 100644 --- a/server/requestorserver/conf.go +++ b/server/requestorserver/conf.go @@ -4,7 +4,7 @@ import ( "crypto/tls" "fmt" "net/url" - "path/filepath" + "path" "slices" "strings" @@ -114,7 +114,7 @@ func (conf *Configuration) CanRequest(requestor string, request irma.SessionRequ // For all host patterns being set in the requestor configuration, check whether the requested host matches it. for _, hostPattern := range conf.Requestors[requestor].Hosts { - if match, _ := filepath.Match(hostPattern, host); match { + if match, _ := path.Match(hostPattern, host); match { return true, "" } }