Bundle SK CA certificates, fix transport error handling, tighten ADVANCED certificate validation - #1
Merged
Conversation
Signer certificates chain to SK's own eID CAs, which are not in any operating system CA bundle -- that store holds TLS roots. Chain validation therefore failed for every real Smart-ID certificate, in both environments. The authentication path never exposed this because AuthenticationCertificateValidator does no chain building at all. Ships the same certificates smart-id-java-client does, for production and demo, kept strictly separate. TrustedCaCertStore gains production/demo/ default entry points plus from_directory, from_pkcs12 and from_certificates for supplying your own, and CertificateValidator now defaults to the bundled store for the configured environment instead of the system CA store alone. The two production roots are not in smart-id-java-client; they were fetched from the caIssuers AIA URLs of the intermediates that they sign. A spec verifies every bundled intermediate still chains to a bundled anchor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GP9ZnyKyjjFfBmv3evRGV3
handle_server_error and handle_client_error read error.response[:status] unconditionally, but Faraday::TimeoutError is a Faraday::ServerError with a nil response, so a session status long poll that timed out raised "NoMethodError: undefined method '[]' for nil" instead of something a caller could rescue. Seen against the live demo service. Timeouts now raise Errors::NetworkTimeoutError, distinct from SessionTimeoutError which means the user did not respond. Faraday errors that are neither ClientError nor ServerError -- a dropped connection, a TLS failure -- were not caught at all and leaked to callers; they now raise Errors::ResponseError. The duplicated rescue chains in get and post are shared, since TimeoutError has to be matched before ServerError. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GP9ZnyKyjjFfBmv3evRGV3
Certificate purpose validation returned early for ADVANCED, so any certificate with Non-Repudiation key usage from any CA in the trust store was accepted as a signer certificate. Requires the non-qualified Smart-ID policy OIDs (1.3.6.1.4.1.10015.17.1, 0.4.0.2042.1.1), matching NonQualifiedSmartIdCertificateValidator in smart-id-java-client. The existing happy-path spec used an ADVANCED certificate with no policy OIDs at all, so it was asserting the lax behaviour; it now uses a certificate that is a real Smart-ID one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GP9ZnyKyjjFfBmv3evRGV3
…own errors The only piece of real cryptography in the gem had no specs. Covers a valid RSASSA-PSS signature, signatures over different data or from a different key, a PKCS#1 v1.5 signature presented as PSS, and each PSS parameter mismatched individually. Writing them surfaced a bug: a blanket "rescue StandardError" swallowed the RequestSetupErrors raised by validate_inputs and reported all four of them as "Signature value validation failed", so the documented parameter errors were unreachable. Gem errors are now re-raised as they are. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GP9ZnyKyjjFfBmv3evRGV3
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GP9ZnyKyjjFfBmv3evRGV3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
▎ Bundled CA certificates. Signer certificates chain to SK's own eID CAs, which are not in any OS CA bundle — that store holds TLS roots — so chain validation failed for every real Smart-ID certificate in both environments. The authentication path never exposed this because AuthenticationCertificateValidator does no chain building at all. Ships the same certificates smart-id-java-client does, production and demo kept strictly separate, with TrustedCaCertStore.production / .demo / .default and from_directory / from_pkcs12 / from_certificates for your own. The two production roots aren't in the Java client; they came from the caIssuers AIA URLs of the intermediates they sign, and a spec re-verifies every chain.
▎ Transport errors no longer crash the client. Faraday::TimeoutError is a Faraday::ServerError with a nil response, so a long poll that timed out raised NoMethodError: undefined method '[]' for nil. Now Errors::NetworkTimeoutError. Dropped connections and TLS failures weren't caught at all and leaked raw Faraday exceptions; now Errors::ResponseError.
▎ ADVANCED certificates are checked. Purpose validation returned early for ADVANCED, accepting any Non-Repudiation certificate from any CA in the trust store. Now requires the non-qualified Smart-ID policy OIDs, matching the Java client.
▎ SignatureValueValidator got its first specs, which found a bug: a blanket rescue StandardError swallowed its own RequestSetupErrors and reported them all as "Signature value validation failed".