Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated assembly version to match nuget version #30

Merged
merged 4 commits into from
Jan 2, 2024

Conversation

simenstoa
Copy link
Contributor

@simenstoa simenstoa commented Dec 18, 2023

Updates assembly version to match nuget version, trying to solve problems described in #29.

Also removes expired certs from unit tests, which means that we now only test validation for SEID 2 certs, and only test chain validator for test chain.

@shamglam
Copy link
Contributor

shamglam commented Jan 2, 2024

@simenstoa after this is merged, when can we expect new nuget packages to be released for this (and the signature apis)?

@runeflobakk
Copy link
Member

@shamglam I am looking into releasing this ASAP, so if all nuts and bolts are properly aligned, I will hopefully have it released during the day.

@runeflobakk runeflobakk self-assigned this Jan 2, 2024
@runeflobakk runeflobakk merged commit 9ca1da0 into main Jan 2, 2024
1 check passed
@runeflobakk runeflobakk deleted the updated-assembly-version branch January 2, 2024 18:38
@runeflobakk
Copy link
Member

@shamglam
Copy link
Contributor

shamglam commented Jan 3, 2024

@runeflobakk well this is certainly getting interesting. In order to isolate the issue I was having I had created a test console application which refers to this nuget package directly - so I upgraded it to 7.1.4 and indeed looking at the file versions i could see that the dll was indeed updated.

Running the testprogram produced the same output as before - the program shows the certificate chain of our certificate and the chain of test certificates embedded in this library.

The interesting part is - the certificate in your library (which my PR would overwrite) has a thumbprint of f1e343f90f13c138e8b937fcb5702f48993ff4c4
whereas the thumbprint of the file I'm trying to replace it with has a thumbprint
abba804a59cf8f5da060e13ffbd263e3d02e486b

I tried download all test certificates from this page https://buypassdev.atlassian.net/wiki/spaces/BCA/pages/2195062891/Rotsertifikater+Root+certificates+test

to check if any of them had the "f1e3..." thumbprint, but I couldn't find one, but the one starting with "abba.." is there

So I'm not sure why this is the case, but it is still causing the issue at hand.

I've also sent an inquiry to Buypass about the "f1e3.." certificate to ask where it originates from.

@runeflobakk
Copy link
Member

runeflobakk commented Jan 3, 2024

@shamglam I did a rudimentary inspection of the certificate files currently provided by Buypass by looking at the output from openssl x509 -text -in <cer-file>, and I see that the root certificates give different output to what we currently have in this library, but the CA certificates (which issues the Enterprise certificates for end-users) are the same as we already have. The diff of the root certificates is checksum/signature/control values, serial number, the validity differ by a few days, but all textual attributes are the same (e.g. subject, issuer). This is peculiar to say the least.

Just to be sure. Do you still experience that your SEID2 test enterprise certificate is not accepted by our library with all the most recent version of api-client-shared-dotnet? Or does the validation pass, but we are solely looking at a case where the root certs have different thumbprints?

We speculate that maybe the root certs currently made available from Buypass are (re)made from the same private key material as the ones we already had acquired, and as such is possible to be replaced as a valid issuer of the CA certificates (which are not changed). Why would be another mystery, but it would be simpler than to issue a whole new hierarchy with root and CA, and maybe considered a better option since this is "only" a trust chain for test.

@shamglam
Copy link
Contributor

shamglam commented Jan 3, 2024

Hi, below is the response from Buypass support regarding this matter.

Heisann,

 
Har hørt litt internt her og, vi beklager at vi har skapt litt forvirring ved at vi har benyttet 2 ulike rot-sertifikater for denne rot CA-en i Test.

Rot-sertifikatet med thumbprint = f1e343f90f13c138e8b937fcb5702f48993ff4c4 ble utstedt den 3.november 2020 og har vært registrert på dokumentsenteret frem til desember 2023 . Vi utstedte et nytt rot-sertifikat med noen endringer i sertifikatet den 4.november 2020 – dette har thumbprint = abba804a59cf8f5da060e13ffbd263e3d02e486b. Dette er det offisielle rot-sertifikatet som vi ønsker at alle skal bruke og dette er nå lagt ut på dokumentsenteret.

Vi vil understreke at begge rot-sertifikater inneholder den samme rot CA nøkkelen, det er kun enkelte attributter i sertifikatene som er ulike.

Fint om dere kan be Posten om å oppdatere sitt rot-sertifikat med det som nå er tilgjengelig på dokumentsenteret.

runeflobakk added a commit to digipost/certificate-validator that referenced this pull request Jan 3, 2024
Long story. We originally acquired the certs they published
valid from nov. 3rd 2020, but they got replaced shortly after.
digipost/api-client-shared-dotnet#30 (comment)

This is possible due to the root certificate, which is self-signed, is created from the same keys,
and the already issued intermediate certs will still validate as issued
by the (new) root cert. This is OK, due to these certs are solely for
use for testing, and does not constitute any authority for actual
identification of parties.
@shamglam
Copy link
Contributor

shamglam commented Jan 3, 2024

@runeflobakk - I guess the best course of action is to keep both root certificates - the old one and the new one to avoid other users suddenly experiencing issues because they rely on the old certificate being present.

@runeflobakk
Copy link
Member

runeflobakk commented Jan 4, 2024

@shamglam I have replaced the test root certs from Buypass in the API client validation in an internal test env server, and verified that existing client integrations using Buypass G2 (SEID2) enterprise certs are unaffected, so I will roll out the new root certs to production as well as replace them in this library.

As for keeping around the old root certs, I see no need to, as no one is supposed to rely on them being present directly via this library. The replacement is a backwards compatible fix (as I have verified internally on our servers).

runeflobakk added a commit that referenced this pull request Jan 4, 2024
We originally acquired the certs they published
valid from Nov. 3rd 2020, but they got replaced shortly after.
See #30 (comment)

Replacing the certs is possible due to the root certificate, which is self-signed, is created from the same keys,
and the already issued intermediate certs will still validate as issued
by the (new) root cert. This is OK, due to these certs are solely for
use for testing, and does not constitute any authority for actual
identification of parties.

Supersedes and closes #29

Co-authored-by: shamglam <[email protected]>
runeflobakk added a commit that referenced this pull request Jan 4, 2024
We originally acquired the certs they published
valid from Nov. 3rd 2020, but they got replaced shortly after.
See #30 (comment)

Replacing the certs is possible due to the root certificate, which is self-signed, is created from the same keys,
and the already issued intermediate certs will still validate as issued
by the (new) root cert. This is OK, due to these certs are solely for
use for testing, and does not constitute any authority for actual
identification of parties.

Supersedes and closes #29

Co-authored-by: shamglam <[email protected]>
@runeflobakk
Copy link
Member

@shamglam Please give signature-api-client-dotnet 9.0.5 a go. It should correctly refer to api-client-shared-dotnet 7.1.5, which contains the updated test root certs.

@shamglam
Copy link
Contributor

shamglam commented Jan 8, 2024

seems to work 👍🏿

@runeflobakk
Copy link
Member

Perfect! Thanks for your assistance in solving this issue, @shamglam!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants