swedenconnect / signservice Goto Github PK
View Code? Open in Web Editor NEWA signature service according to the Swedish eID Framework specifications
License: Apache License 2.0
A signature service according to the Swedish eID Framework specifications
License: Apache License 2.0
We get a NPE if we get a SAML response not including our requested authn context URI
Currently, there is no support for configuring an HTTP proxy for the HTTP metadata provider used in the SAML authentication provider. Add this!
When simple CA is run locally, the CRL is not available and signed documents can't be validated. To allow tests where the document is validated, the CRL can be posted to a public location.
To support this, it must be possible to specify an external CRLDP location, and not just a path under localhost
Currently, the engine implementation catches all "expected" exceptions and translates them into an error response message (when possible). However, unexpected exceptions (such as NPE) are not handled until we reach the catch-all where we display an error page.
This is on purpose, but after testing and before the first release we should catch all exceptions and try to send back an "internal error" response message (and audit log).
When using CA CMC it should be possible to configure a HTTP proxy.
MDQ (https://www.ietf.org/id/draft-young-md-query-17.html) enables us to only download the metadata that is needed and not the entire federation. We should use opensaml-addons - 1.2.5 to implement this.
The configuration for the SAML handler has a sign-authn-requests
setting and the metadata config has authn-requests-signed
. Remove the setting from metadata config and use the sign-authn-requests
setting.
Velocity has a lot of vulnerabilities and we don't want to have it in the classpath.
Fix!
When signing time is provided in the input To Be Signed data, different rules apply depending on the signature format.
If the signing time is provided in an XML Object as a signed signature property, then the signature service is allowed to update this time to current time and to process the signature.
However, if the document is a PDF, the signature process MUST be aborted. The reason for this is that PDF has multiple places to store signing time. Both as signed attribute, but also as a parameter in the signature dictionary. Since the sign service has no access to the signature dictionary, it must assume that there is a high risk that this dictionary contains false time information if the signed attributes holds a bad time.
Current implementation of sign service accepts sign request for PDF documents even if the signing time has substantial deviation from current time. This is in error.
For XML the sign service has a choice to either reject the request or to update the time. Sign service updates the time and process the request. We may want to consider rejecting such request in order to align XML and PDF document processing.
After all, a sign request with a signing time that is out of date is an indication that something is not quite right.
Offending test: StackedInMemoryRSAKeyProviderTest.getKeyPair:56 expected: <5> but was: <2>
Sometimes it succeeds, other times it fails.
The SADValidator at se.swedenconnect.opensaml.sweid.saml2.signservice.SADParser.SADValidator has a problem in the check of issue time at row 200
The current check: (long)sad.getIssuedAt() > now
This check does not allow any time skew. If the sign server clock is behind the IdP clock by just a few seconds, this check will fail. In test environments this will fail if time skew is a fraction of a second.
This check should have a configurable time skew margin.
The engine implementation does not have acceptable test coverage.
The latest updates of credential support is to be used as a generic component to support signing key generation both in HSM and in software based on the new PkiCredentialContainer
We should add audit logging events to the engine implementation.
Also, we should make it configurable what is actually included in an audit event. Some users of the service may want to limit what is logged. For example don't include any personal data.
This will change later when we implement the SignService according to the draft version of the SAP-protocol.
If a Sign Request is received with a wrong SignService identifier, the sign service accepts the request and process the signature.
As it is now, each SAML authentication handler defines its own metadata provider (downloads metadata), and this leads to that the same metadata is handled by several identical provider beans.
Signature activation data check should be added to the SAML Sweden Connect authentication handler
The class MetadataConfiguration.UIInfoConfig.UIInfoLogo
should be extended with a lang
-tag so that we can publish logotypes for different languages (required by Swamid).
Currently the service name set in certificates is a static value. Going through the old code, this should be more dynamic and the option to set the SP requester ID need to be added.
Current CMC key and certificate handler can only support one certificate type.
This is too restrictive and makes it very hard to setup the signservice in various test configurations. Configuration should decide if the CA supports one or more certificate types, not just a single type.
Currently we have a Spring Boot starter module that handles configuration of an application. For those deployments that do not use Spring this is of little help. We should make a framework-independent configuration module (and let the Spring Boot starter use that).
This activity also helps in documentation of the configuration settings.
When a certificate is generated, it is added to the signer's PkiCredential object.
Adding this certificate is not supported on the generic interface level, but only on the abstract class implementing the interface.
In theory there could be an implementation outside of this abstract class. The implementation of this is not straightforward using any function on the main interface but requires a test if the credential object is a subclass of the abstract implementation before calling the method in the abstract class. Once this has been added to the main interface, the implementation can be simplified.
A fundamental requirement for sign services has always been the principle that the Sign Service has no UI for the user unless the request can't be returned to the requesting service for one of the following reasons:
In all other instances, the user MUST be returned to the requesting service with an appropriate status indication in order to allow that application to get the user back and to handle the user in an appropriate manner.
Sign service currently does not meet this requirement. One case is there a request is made for an unsupported certificate type (QC, PKC, QC/SSSCD). If the requested certificate type is not supported, the user is presented by an error page claiming "internal error". In this case the user should be returned to the requesting service with an appropriate error response.
Currently the ResponseValidationSettings from the opensaml-addons repository is used. We should introduce our own configuration class for this.
When configuring clients (engines) we need to point out an external bean even if we don't want to make any changes to the definition. Example:
protocol:
external:
bean-name: signservice.DssProtocolHandler
We should make a change so that if there is only one bean of a particular type defined, we should default to that bean if nothing is set in the engine configuration. This makes configuration easier.
When configuring the metadata provider bean it is possible to configure the backup-file
. If this path points to a directory that does not exist, no cache will be written.
Solution: Make sure that the configuration process ensures that the directory exists, and if not, tries to create all intermediate directories.
We may limit some applications regarding audit logging. The introduction of an AuditLoggerListener would be an option.
The internals use HttpServletRequest.getRequestURI
that returns everything following the protocol. This makes it tricky to configure since the logic shouldn't be aware of the context path.
Since the getFactoryClass
of AbstractHandlerConfiguration will return the default factory class if the value hasn't been set, the merging of this property will never succeed. We need to handle this special case.
The inclusion of a SAD request is currently not configurable, but rather controlled by the status of the IdP and the sign request.
The following parts of the code is involved:
DefaultSignServiceEngine
has a //TODO at row 513 stating:
// Note: The authentication requirements may also be controlled by a policy ...
// TODO: We need to extend the input to authenticate with a listing of all attributes
// required. We get those from the signing certificate requirements ...
final AuthnRequirements reqs = signRequest.getAuthnRequirements();
This is a place where it could be possible to influence the authentication requirements. AuthenticationRequirements
has a parameter of type SignatureActivationRequestData
. If set to null, it will not send a SAD request.
The SignatureActivationRequestData
is set by parsing the SignRequest
in row 536 of DssSignRequestMessage
authnRequirements.setSignatureActivationRequestData(
new DefaultSignatureActivationRequestData(
this.signRequest.getRequestID(), docCount, certType == CertificateType.QC_SSCD));
Using this code, it will never be null.
Finally the decision to include SAD request is mad by SwedenConnectSamlAuthenticationHandler
at row 126
// Should we send a SAD request?
//
final SADRequest sadRequest = authnRequirements.getSignatureActivationRequestData() != null
&& signMessage != null
&& this.isSignatureActivationProtocolSupported(idpMetadata)
? (SADRequest) XMLObjectSupport.buildXMLObject(SADRequest.DEFAULT_ELEMENT_NAME)
: null;
if (sadRequest != null) {
I think this is the appropriate place to have a configurable option to not send a SAD request.
Given that the SignatureActivationRequestData
is never null. The current logic is:
If signMessage is present and IdP supports SAD, then send SAD request.
This should be replaced by the following logic:
boolean SADREQ_ALWAYS_IF_SUPPORTED = true;
boolean sendSad = false;
As it is today we expect a path and add the base URL, but there may be cases when the logotype is read from another location.
The current feature to allow default value for signature algorithm should be reverted and always require signature algorithm to be specified in the request.
The main reason for this is that the tbs-data that is provided in the sign request must be prepared for a specific signature algorithm and in some cases that algorithm is not specified (like in PDF message digest attribute).
If signature algorithm is not specified in the sign request, this will create ambiguity in how to understand the tbs data.
As it is now the implementation uses getSamlType
in too many places and we are ending up having to extend too many methods if we introduce a new SAML type.
Solution: Introduce an assertSamlType() method first in the flow (that can be extended). Then always have a default choice and don't fail on "unrecognized saml type".
Attribute mapping data is placed in AuthnContextExtension even when the attribute and its data is provided by the SP as a default value. This is an error since there is no source attribute and the result is a XML Schema violation.
We have some vulnerabilities concerning webjars reported by Snyk. Let's fix those ...
Setup of the certModelBuilder is incomplete. It should be initialised with:
certModelBuilder
.subject(subject)
.includeAki(true)
.includeSki(true);
If a Sign Request has a validity period that is too long (i.e. 1 hour), the sign request is processed as long as current time is within the validity period.
The risk here is that the validity period exceeds the replay checker retention period and opens for successful replay attacks.
The AuthContextExtension specifies the SAML source for every attribute in the certificate subject name.
However if that attribute is set by a default value, then no SAML name is specified.
This is a violation of the XML Schema of the SAMLAuthContext xml content.
In the reference signature service, the proper SAML attribute name is set. If none is set in the sign request, we should have a name for "default". It could be argued that the value "default" is more accurate as it does not imply that the IdP was the source of the value in the cert.
Current implementation only utilise the CRMF request format which provides less substantial Proof Of Possession (POP) to the issuing CA that the requestor has access to the corresponding private key.
PKCS10 request format requires access to the private key as the PKCS10 request is signed with the private key of the public key in the requested certificate.
Since signservice has access to the signer private key, there should be an option to use PKCS10 request format, which also allows greater flexibility of choice of compatible CA:s
We need to configure an application concerning clock skews and more. This config should be accessible by all handlers -> Singleton instance.
Fix any dependencies needed.
SwedenConnectSamlAuthenticationHandler has a setter for SADValidator that allows setting a custom SADValidator.
If no one is set, the SADValidator of se.swedenconnect.opensaml.sweid.saml2.signservice.SADParser.SADValidator is set.
The problem here is that SADValidator is not an interface, but rather a static sub-class of SADParser, making it very hard to provide a custom implementation of it as intended by SwedenConnectSamlAuthenticationHandler
This seems to be a logical mismatch.
We need to assert that the assertion issued is "fresh" and not created based on a previous authentication.
Signature validation is performed before simpler checks such as checks if the request is too old to be processed.
It is a good principle to do easier and less costly checks first, such as simple date checks before doing costly checks such as signature validation in order to reduce the attack surface for DOS attacks.
Currently the configuration only allows configuration of display names, description and logo.
Go through all documentation och fix anything that is missing/incorrect.
Upgrade to use v 2.0.0 of ca-engine.
Requests with a request time set in the past or in the future results in a sign response to the requesting Service Provider.
No Sign Response should be returned unless the date and replay checks pass.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.