From 1cbf0f2a709e771343a80c159dbd5c54923ff29d Mon Sep 17 00:00:00 2001 From: stevenvegt Date: Fri, 13 Dec 2024 09:16:26 +0100 Subject: [PATCH 1/4] Fix did issuer encoding Replaced the url.PathEncode with custom percent encoder --- did_x509/did_x509.go | 31 ++++++++++--- did_x509/did_x509_test.go | 75 +++++++++++++++++++++++--------- uzi_vc_issuer/ura_issuer.go | 11 +++-- uzi_vc_issuer/ura_issuer_test.go | 18 ++++++++ 4 files changed, 106 insertions(+), 29 deletions(-) diff --git a/did_x509/did_x509.go b/did_x509/did_x509.go index 5133a82..d09a8f7 100644 --- a/did_x509/did_x509.go +++ b/did_x509/did_x509.go @@ -5,11 +5,13 @@ import ( "encoding/base64" "errors" "fmt" - "github.com/nuts-foundation/go-did/did" - "github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert" "net/url" "regexp" "strings" + "unicode" + + "github.com/nuts-foundation/go-did/did" + "github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert" ) type X509Did struct { @@ -49,9 +51,28 @@ func CreateDid(signingCert, caCert *x509.Certificate, subjectAttributes []x509_c formattedDid, err := FormatDid(caCert, policies...) return formattedDid, err } + +// PercentEncode encodes a string using percent encoding. +// we can not use url.PathEscape because it does not escape : $ & + = : @ characters. +// See https://github.com/golang/go/issues/27559#issuecomment-449652574 +func PercentEncode(input string) string { + var encoded strings.Builder + for _, r := range input { + if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' || r == '_' || r == '.' || r == '~' { + encoded.WriteRune(r) + } else { + encoded.WriteString(fmt.Sprintf("%%%02X", r)) + } + } + return encoded.String() +} + func ParseDid(didString string) (*X509Did, error) { x509Did := X509Did{} - didObj := did.MustParseDID(didString) + didObj, err := did.ParseDID(didString) + if err != nil { + return nil, err + } if didObj.Method != "x509" { return nil, errors.New("invalid didString method") } @@ -96,7 +117,7 @@ func ParseDid(didString string) (*X509Did, error) { func CreateOtherNamePolicies(otherNames []*x509_cert.OtherNameValue) []string { var policies []string for _, otherName := range otherNames { - value := url.PathEscape(otherName.Value) + value := PercentEncode(otherName.Value) fragments := []string{string(otherName.PolicyType), string(otherName.Type), value} policy := strings.Join(fragments, ":") policies = append(policies, policy) @@ -107,7 +128,7 @@ func CreateOtherNamePolicies(otherNames []*x509_cert.OtherNameValue) []string { func CreateSubjectPolicies(subjectValues []*x509_cert.SubjectValue) []string { var policies []string for _, subjectValue := range subjectValues { - value := url.PathEscape(subjectValue.Value) + value := PercentEncode(subjectValue.Value) fragments := []string{string(subjectValue.PolicyType), string(subjectValue.Type), value} policy := strings.Join(fragments, ":") policies = append(policies, policy) diff --git a/did_x509/did_x509_test.go b/did_x509/did_x509_test.go index 0ce0b3e..2b0035d 100644 --- a/did_x509/did_x509_test.go +++ b/did_x509/did_x509_test.go @@ -3,15 +3,37 @@ package did_x509 import ( "crypto/x509" "encoding/base64" - "github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert" - "reflect" "strings" "testing" + + "github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert" + "github.com/stretchr/testify/assert" ) -// TestDefaultDidCreator_CreateDid tests the CreateDid function of DefaultDidProcessor by providing different certificate chains. +func TestPercentEncode(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"hello world", "hello%20world"}, + {"foo@bar.com", "foo%40bar.com"}, + {"100%", "100%25"}, + {"a+b=c", "a%2Bb%3Dc"}, + {"~!@#$%^&*()_+", "~%21%40%23%24%25%5E%26%2A%28%29_%2B"}, + {"FauxCare & Co", "FauxCare%20%26%20Co"}, + } + + for _, test := range tests { + result := PercentEncode(test.input) + if result != test.expected { + t.Errorf("PercentEncode(%q) = %q; want %q", test.input, result, test.expected) + } + } +} + +// TestCreateDid tests the CreateDid function of DefaultDidProcessor by providing different certificate chains. // It checks for correct DID generation and appropriate error messages. -func TestDefaultDidCreator_CreateDidSingle(t *testing.T) { +func TestCreateDidSingle(t *testing.T) { type fields struct { } type args struct { @@ -99,7 +121,7 @@ func TestDefaultDidCreator_CreateDidSingle(t *testing.T) { }) } } -func TestDefaultDidCreator_CreateDidDouble(t *testing.T) { +func TestCreateDidDouble(t *testing.T) { type fields struct { } type args struct { @@ -183,9 +205,9 @@ func TestDefaultDidCreator_CreateDidDouble(t *testing.T) { } } -// TestDefaultDidCreator_ParseDid tests the ParseDid function of DefaultDidProcessor by providing different DID strings. +// TestParseDid tests the ParseDid function of DefaultDidProcessor by providing different DID strings. // It checks for correct X509Did parsing and appropriate error messages. -func TestDefaultDidCreator_ParseDid(t *testing.T) { +func TestParseDid(t *testing.T) { policies := []*x509_cert.GenericNameValue{ { PolicyType: "san", @@ -206,24 +228,30 @@ func TestDefaultDidCreator_ParseDid(t *testing.T) { errMsg string }{ { - name: "Invalid DID method", + name: "ok - happy path", + fields: fields{}, + args: args{didString: "did:x509:0:sha512:hash::san:otherName:A_BIG_STRING"}, + want: &X509Did{Version: "0", RootCertificateHashAlg: "sha512", RootCertificateHash: "hash", Policies: policies}, + errMsg: "", + }, + { + name: "nok - invalid DID method", fields: fields{}, args: args{didString: "did:abc:0:sha512:hash::san:otherName:A_BIG_STRING"}, want: nil, errMsg: "invalid didString method", }, { - name: "Invalid DID format", + name: "nok - invalid DID format", fields: fields{}, args: args{didString: "did:x509:0:sha512::san:otherName:A_BIG_STRING"}, want: nil, errMsg: "invalid didString format, expected didString:x509:0:alg:hash::san:type:ura", }, - { - name: "Happy path", + {name: "ok - correct unescaping", fields: fields{}, - args: args{didString: "did:x509:0:sha512:hash::san:otherName:A_BIG_STRING"}, - want: &X509Did{Version: "0", RootCertificateHashAlg: "sha512", RootCertificateHash: "hash", Policies: policies}, + args: args{didString: "did:x509:0:sha512:hash::san:otherName:hello%20world%20from%20FauxCare%20%26%20Co"}, + want: &X509Did{Version: "0", RootCertificateHashAlg: "sha512", RootCertificateHash: "hash", Policies: []*x509_cert.GenericNameValue{{PolicyType: "san", Type: "otherName", Value: "hello world from FauxCare & Co"}}}, errMsg: "", }, } @@ -232,21 +260,26 @@ func TestDefaultDidCreator_ParseDid(t *testing.T) { got, err := ParseDid(tt.args.didString) wantErr := tt.errMsg != "" if (err != nil) != wantErr { - t.Errorf("DefaultDidProcessor.ParseDid() error = %v, expected error = %v", err, tt.errMsg) + t.Errorf("ParseDid() error = %v, expected error = %v", err, tt.errMsg) return } else if wantErr { if err.Error() != tt.errMsg { - t.Errorf("DefaultDidProcessor.ParseDid() expected = \"%v\", got = \"%v\"", tt.errMsg, err.Error()) + t.Errorf("ParseDid() expected = \"%v\", got = \"%v\"", tt.errMsg, err.Error()) } } - if tt.want != nil && got != nil && - (tt.want.Version != got.Version || - tt.want.RootCertificateHashAlg != got.RootCertificateHashAlg || - tt.want.RootCertificateHash != got.RootCertificateHash || - !reflect.DeepEqual(tt.want.Policies, got.Policies)) { - t.Errorf("DefaultDidProcessor.ParseDid() = %v, want = %v", got, tt.want) + if tt.want != nil && got != nil { + + assert.Equal(t, tt.want.Policies, got.Policies) } + + // if tt.want != nil && got != nil && + // (tt.want.Version != got.Version || + // tt.want.RootCertificateHashAlg != got.RootCertificateHashAlg || + // tt.want.RootCertificateHash != got.RootCertificateHash || + // !reflect.DeepEqual(tt.want.Policies, got.Policies)) { + // t.Errorf("ParseDid() expected = %v, got = %v", tt.want, got) + // } }) } } diff --git a/uzi_vc_issuer/ura_issuer.go b/uzi_vc_issuer/ura_issuer.go index ad02e61..fee95ab 100644 --- a/uzi_vc_issuer/ura_issuer.go +++ b/uzi_vc_issuer/ura_issuer.go @@ -6,6 +6,7 @@ import ( "crypto/sha1" "crypto/x509" "encoding/base64" + "encoding/pem" "errors" "fmt" "os" @@ -392,8 +393,7 @@ func convertHeaders(headers map[string]interface{}) (jws.Headers, error) { return hdr, nil } -// uraCredential generates a VerifiableCredential for a given URA and UZI number, including the subject's DID. -// It sets a 1-year expiration period from the current issuance date. +// uraCredential builds a VerifiableCredential for a given URA and UZI number, including the subject's DID. func uraCredential(issuer string, expirationDate time.Time, otherNameValues []*x509_cert.OtherNameValue, subjectTypes []*x509_cert.SubjectValue, subjectDID subjectDID) (*vc.VerifiableCredential, error) { iat := time.Now() subject := map[string]interface{}{ @@ -407,8 +407,13 @@ func uraCredential(issuer string, expirationDate time.Time, otherNameValues []*x subject[string(subjectType.Type)] = subjectType.Value } + issuerDID, err := did.ParseDID(issuer) + if err != nil { + return nil, fmt.Errorf("failed to parse issuer DID '%s': %w", issuer, err) + } + id := did.DIDURL{ - DID: did.MustParseDID(issuer), + DID: *issuerDID, Fragment: uuid.NewString(), }.URI() return &vc.VerifiableCredential{ diff --git a/uzi_vc_issuer/ura_issuer_test.go b/uzi_vc_issuer/ura_issuer_test.go index 2aba303..d06bb94 100644 --- a/uzi_vc_issuer/ura_issuer_test.go +++ b/uzi_vc_issuer/ura_issuer_test.go @@ -200,6 +200,24 @@ func TestIssue(t *testing.T) { assert.Equal(t, validChain[0].NotAfter, *vc.ExpirationDate, "expiration date of VC must match signing certificate") }) + + t.Run("ok - correct escaping of special characters", func(t *testing.T) { + validChain, err := NewValidCertificateChain("testdata/valid_chain.pem") + require.NoError(t, err, "failed to read chain") + + validChain[0].Subject.Organization = []string{"FauxCare & Co"} + + vc, err := Issue(validChain, validKey, "did:example:123", SubjectAttributes(x509_cert.SubjectTypeCountry, x509_cert.SubjectTypeOrganization)) + + require.NoError(t, err, "failed to issue verifiable credential") + require.NotNil(t, vc, "verifiable credential is nil") + + assert.Equal(t, "https://www.w3.org/2018/credentials/v1", vc.Context[0].String()) + assert.Equal(t, "VerifiableCredential", vc.Type[0].String()) + assert.Equal(t, "UziServerCertificateCredential", vc.Type[1].String()) + assert.Equal(t, "did:x509:0:sha512:0OXDVLevEnf_sE-Ayopm0Yof_gmBwxwKZmzbDhKeAwj9vcsI_Q14TBArYsCftQTABLM-Vx9BB6zI05Me2aksaA::san:otherName:2.16.528.1.1007.99.2110-1-1111111-S-2222222-00.000-333333::subject:O:FauxCare%20%26%20Co", vc.Issuer.String()) + }) + } func TestParsePemBytes(t *testing.T) { From cdf4440da46c1c4ec2563426aca08177fc921e27 Mon Sep 17 00:00:00 2001 From: stevenvegt Date: Fri, 13 Dec 2024 09:23:52 +0100 Subject: [PATCH 2/4] Remove commented code --- did_x509/did_x509_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/did_x509/did_x509_test.go b/did_x509/did_x509_test.go index 2b0035d..a26f1ef 100644 --- a/did_x509/did_x509_test.go +++ b/did_x509/did_x509_test.go @@ -269,17 +269,8 @@ func TestParseDid(t *testing.T) { } if tt.want != nil && got != nil { - assert.Equal(t, tt.want.Policies, got.Policies) } - - // if tt.want != nil && got != nil && - // (tt.want.Version != got.Version || - // tt.want.RootCertificateHashAlg != got.RootCertificateHashAlg || - // tt.want.RootCertificateHash != got.RootCertificateHash || - // !reflect.DeepEqual(tt.want.Policies, got.Policies)) { - // t.Errorf("ParseDid() expected = %v, got = %v", tt.want, got) - // } }) } } From e3646d193a6bb6262acfe10e671e5c5fafe32f7b Mon Sep 17 00:00:00 2001 From: stevenvegt Date: Fri, 13 Dec 2024 09:25:50 +0100 Subject: [PATCH 3/4] Remove duplicate checks These are already checked in the happy path --- uzi_vc_issuer/ura_issuer_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/uzi_vc_issuer/ura_issuer_test.go b/uzi_vc_issuer/ura_issuer_test.go index d06bb94..ed46343 100644 --- a/uzi_vc_issuer/ura_issuer_test.go +++ b/uzi_vc_issuer/ura_issuer_test.go @@ -209,12 +209,6 @@ func TestIssue(t *testing.T) { vc, err := Issue(validChain, validKey, "did:example:123", SubjectAttributes(x509_cert.SubjectTypeCountry, x509_cert.SubjectTypeOrganization)) - require.NoError(t, err, "failed to issue verifiable credential") - require.NotNil(t, vc, "verifiable credential is nil") - - assert.Equal(t, "https://www.w3.org/2018/credentials/v1", vc.Context[0].String()) - assert.Equal(t, "VerifiableCredential", vc.Type[0].String()) - assert.Equal(t, "UziServerCertificateCredential", vc.Type[1].String()) assert.Equal(t, "did:x509:0:sha512:0OXDVLevEnf_sE-Ayopm0Yof_gmBwxwKZmzbDhKeAwj9vcsI_Q14TBArYsCftQTABLM-Vx9BB6zI05Me2aksaA::san:otherName:2.16.528.1.1007.99.2110-1-1111111-S-2222222-00.000-333333::subject:O:FauxCare%20%26%20Co", vc.Issuer.String()) }) From 166cd2ff0f18dbba5e0b18b25dd637f8ce6931fb Mon Sep 17 00:00:00 2001 From: stevenvegt Date: Fri, 13 Dec 2024 09:49:08 +0100 Subject: [PATCH 4/4] Include tilde in percent encoding --- did_x509/did_x509.go | 5 ++++- did_x509/did_x509_test.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/did_x509/did_x509.go b/did_x509/did_x509.go index d09a8f7..9e03065 100644 --- a/did_x509/did_x509.go +++ b/did_x509/did_x509.go @@ -45,6 +45,9 @@ func CreateDid(signingCert, caCert *x509.Certificate, subjectAttributes []x509_c policies := CreateOtherNamePolicies(otherNames) subjectTypes, err := x509_cert.SelectSubjectTypes(signingCert, subjectAttributes...) + if err != nil { + return "", err + } policies = append(policies, CreateSubjectPolicies(subjectTypes)...) @@ -58,7 +61,7 @@ func CreateDid(signingCert, caCert *x509.Certificate, subjectAttributes []x509_c func PercentEncode(input string) string { var encoded strings.Builder for _, r := range input { - if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' || r == '_' || r == '.' || r == '~' { + if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' || r == '_' || r == '.' { encoded.WriteRune(r) } else { encoded.WriteString(fmt.Sprintf("%%%02X", r)) diff --git a/did_x509/did_x509_test.go b/did_x509/did_x509_test.go index a26f1ef..b190945 100644 --- a/did_x509/did_x509_test.go +++ b/did_x509/did_x509_test.go @@ -19,7 +19,7 @@ func TestPercentEncode(t *testing.T) { {"foo@bar.com", "foo%40bar.com"}, {"100%", "100%25"}, {"a+b=c", "a%2Bb%3Dc"}, - {"~!@#$%^&*()_+", "~%21%40%23%24%25%5E%26%2A%28%29_%2B"}, + {"~!@#$%^&*()_+", "%7E%21%40%23%24%25%5E%26%2A%28%29_%2B"}, {"FauxCare & Co", "FauxCare%20%26%20Co"}, }