Security features

[gnieser]

Hello,

I'm currently evaluating spam protection solutions and I've just found mosparo.
First of all, congratulations for all the hard work to create such a nice open source product, that makes the internet a safer place.

I have some security requirements that are not met out of the box:

• TLS encrypted database connection
• Deployment on OpenShift (in other words, Docker/Kubernetes with lesser permissions, arbitrary uid, etc.)

I think can somehow help mosparo meet them.
I have a workaround for the TLS database connection by override SetupController.php and doctrine.yaml
I have a very complicated setup for OpenShift to ditch the nginx privileged process and avoid writing inside the image

I also need to integrate mosparo with a java backend, so I will probably also write a java client library.

Please let me know whether you are interested, and if so I shall proceed.
Best regards

[gnieser]

Here is my workaround to enable TLS for MariaDB by directly editing the mosparo sources.

SetupController.php

                $tmpConnection = DriverManager::getConnection([
[...]
                    'driverOptions' => [
                        \PDO::MYSQL_ATTR_SSL_KEY => '/certs/client-key.pem',
                        \PDO::MYSQL_ATTR_SSL_CERT => '/certs/client-cert.pem',
                        \PDO::MYSQL_ATTR_SSL_CA => '/certs/ca.pem',
                        \PDO::MYSQL_ATTR_SSL_VERIFY_SERVER_CERT => true
                    ]
                ]);

In doctrine.yaml

doctrine:
    dbal:
[...]
        options:
            !php/const PDO::MYSQL_ATTR_SSL_KEY: '%env(DATABASE_PRIV_KEY)%'
            !php/const PDO::MYSQL_ATTR_SSL_CERT: '%env(DATABASE_PUB_KEY)%'
            !php/const PDO::MYSQL_ATTR_SSL_CA: '%env(DATABASE_CA_CERT)%'
            !php/const PDO::MYSQL_ATTR_SSL_VERIFY_SERVER_CERT: true

[zepich]

Hi @gnieser

First of all, congratulations for all the hard work to create such a nice open source product, that makes the internet a safer place.

Thank you very much for your kind words! It's incredible to read such feedback!

I have some security requirements that are not met out of the box:

TLS encrypted database connection
Deployment on OpenShift (in other words, Docker/Kubernetes with lesser permissions, arbitrary uid, etc.)

Thank you very much for bringing these things to our attention and for the workaround.

TLS Encrypted Database Connection

The easiest thing for us is to add some additional environment variables and a hint in the setup to let users know how to configure the encrypted connection.

Adding the required configuration fields is also possible, but it makes the setup more complicated.

What do you think? Would the environment variables be enough?

Deployment on OpenShift

What exactly do you need here? Do you need a different Docker image? Or does such a setup need changes in the code?

I think can somehow help mosparo meet them.
I have a workaround for the TLS database connection by override SetupController.php and doctrine.yaml
I have a very complicated setup for OpenShift to ditch the nginx privileged process and avoid writing inside the image

Our main goal is to provide software that can be used in every situation and on every host (on a simple shared web host or a cluster of servers). So we're interested in learning more about other setups that we don't know or for which we don't have knowledge.

Please let me know what problems we have to solve. We're very interested in all suggestions and ideas to make mosparo better.

Java Client Library

That would be awesome! I worked with Java before (Android and desktop apps), but I'm not that good in Java to write a library. I helped another user integrate mosparo into Keycloak. Here is the code: https://git.schuerz.at/jakob/keycloak-mosparo (Just in case you need an example of how to communicate with the mosparo API in Java).

Thank you very much for your help in making mosparo better! Please let me know if you need anything or have any questions!

Kind regards,

zepich

[zepich]

Hi @gnieser

Thank you very much for your response.

Deployment on Open Shift

Here is my additional configuration to nginx-unprivileged image to make it work as a reverse proxy for mosparo.
I'm not sure it is correct, and it surely not reviewed for production use.
In my setup, I use fastcgi over the 127.0.0.1 address within the pod to send request to mosparo container.

I'm sorry for this stupid question, but how does the nginx container know that 127.0.0.1:9000 is in the mosparo container? As far as I understand, you would need some tunnel or connection between the nginx pod and the mosparo pod.

I haven't looked at this part... I will need to test it. I have never run cron jobs inside containers. My first thought is it may deviate from the "one process per container" usual recommendation. The preferred way in Kubernetes could be to use the CronJob resource. https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/

Thank you very much for mentioning this. I think it should be no problem to execute the mosparo cron jobs via the Kubernetes cron jobs. It's a good topic to add to our documentation.

If it can help, there's OpenShift Local you may want to use: https://developers.redhat.com/products/openshift-local/overview

Thank you very much! That's awesome! I was already thinking about how I could test it.

Java API client

I've created the repo here: https://github.com/mosparo/java-api-client

Please let me know if you need anything from our side!

Thank you for all your work!

Kind regards,

zepich

[gnieser]

Hi @zepich

Thanks for the details

Deployment on Open Shift

Here is my additional configuration to nginx-unprivileged image to make it work as a reverse proxy for mosparo.
I'm not sure it is correct, and it surely not reviewed for production use.
In my setup, I use fastcgi over the 127.0.0.1 address within the pod to send request to mosparo container.

I'm sorry for this stupid question, but how does the nginx container know that 127.0.0.1:9000 is in the mosparo container? As far as I understand, you would need some tunnel or connection between the nginx pod and the mosparo pod.

That's actually a very good question and is a concept at the heart of Kubernetes. https://kubernetes.io/docs/concepts/workloads/pods/#pod-networking
A pod consists of one or more containers that share resources such as network, volumes, and so on.
While I'm not 100% sure of the low level details, my understanding is that containers of a same pod share a virtual ethernet device, so they are can see each other on localhost (and cannot open the same port).
From outside the pod, they are addressable on the same IP and there comes in play the Service resource which removes the need to know the pods IP (pods are ephemeral and their IP can change when recreated).

Java API client

I've created the repo here: https://github.com/mosparo/java-api-client

Please let me know if you need anything from our side!

I'll start working on it. Any insights and feedback are welcome!

Best regards

[zepich]

Hi @gnieser

Thank you very much for the details about pod networking!

I guess I have to learn a lot about Kubernetes...

From that perspective, we could add an ENV variable with which you can decide what program you want to start in a container (nginx, PHP, or both). For example, you could use the same (unprivileged) image to start only nginx in one container and php-fpm in the other. With that, we would have one process per container.

I understand the problem and the required changes now, and I'll start working on them.

Have a great day!

Kind regards,

zepich

[gnieser]

Hi @zepich

While working on the Java client, I noticed that in a test the PHP client treats null as empty string when it comes to hashing, the javascript and python clients don't have this null field in their equivalent tests.
https://github.com/mosparo/php-api-client/blob/master/tests/RequestHelperTest.php#L48

May I asked whether it is the intended behavior?

Thanks!

[zepich]

Hi @gnieser

Thank you very much for your question.

That's a very good question, and it took me a moment to answer it.

In the mosparo Verification API, we automatically convert null to an empty string, since we cannot create hashes for it (https://github.com/mosparo/mosparo/blob/master/src/Helper/VerificationHelper.php#L35).

In the PHP API client, we have the same problem: str_replace and hash accept only strings, and null is not a string, so we replace null with the empty string (mosparo/php-api-client@075d019).

I think we forgot to add that to the JavaScript and Python API clients. From my perspective, no language should be able to create a hash for null, so the language will automatically fail with that task or convert it to some other value (for example, integer 0), which will result in a different hash.

From this perspective, we should always use an empty string for any null/None/nil value.

Thank you for pointing this difference out!

Kind regards,

zepich

[zepich]

Hi @gnieser

I have an update for you. :)

I've worked on the unprivileged image, and I think I did it... :)

https://hub.docker.com/r/mosparo/mosparo-unprivileged

There is no latest tag right now on purpose because I do not want to signal that this image should be used for any kind of production environment right now. Also, v1.3.1-alpha.4 is not a stable version (but there have not been many changes since v1.3.0, so it is not dangerous to use it).

Unprivileged image

As discussed, I've adjusted the Dockerfile, the run.sh, and the nginx configuration and added some functions to mosparo to make it happen (primarily to define a config file stored in a different location). I used the nginx-unprivileged Dockerfile as an example and adjusted the required things to make it work. The image contains nginx and PHP, but you can choose what to turn on with environment variables. You can see the example configuration in this additional Docker Compose file: https://github.com/mosparo/mosparo/blob/master/docker/unprivileged/docker-compose.yaml

The unprivileged Docker image does not contain the cron daemon. I did not find an easy solution to run it unprivileged. But, as you already mentioned, the official way for Kubernetes is to use the Kubernetes cronjobs anyway. Additionally, since mosparo v1.2, you can use the web cronjob, so technically, you could also use an additional cronjob image and call the cronjob via web request. Of course, this part needs some documentation, but from the technical aspect, I think we don't need the cron daemon inside the image.

I tested the image with OpenShift Local but only with SQLite as a database and not with an additional database container (I was too busy (or lazy?) to learn how to add another container).

Please have a look at the new image and let me know what you think.

MySQL TLS

I've also added the environment variables to enable TLS for MySQL. You can configure the following environment variables:

DATABASE_MYSQL_SSL=0      # 0 = disabled, 1 = enabled
DATABASE_MYSQL_SSL_KEY=   # Path to the SSL key file
DATABASE_MYSQL_SSL_CERT=  # Path to the SSL certificate file
DATABASE_MYSQL_SSL_CA=    # Path to the SSL CA file
DATABASE_MYSQL_SSL_VERIFY_SERVER_CERT=0   # 0 = do not verify, 1 = verify

There is no visual hint regarding these variables in the setup right now.

I would also like to hear your feedback regarding this part.

Thank you very much for all your help, and I look forward to hearing from you!

Kind regards,

zepich

[gnieser]

Hi @zepich

Thank you so much!
I'll test these new features as soon as I can and give you feedback.

Best regards!

[gnieser]

Hi @zepich

The unprivileged image works for me without any further customization and with the new environment variables for MySQL TLS.
It greatly simplifies the setup. There's only one container now and nothing to override.

Here is the deployment resource:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: deployment
spec:
  replicas: 1
  selector:
    matchLabels: { }
  template:
    spec:
      containers:
      - name: mosparo
        image: mosparo/mosparo-unprivileged:v1.3.1-alpha.4
        imagePullPolicy: Always
        envFrom:
          - configMapRef:
              name: configmap
        ports:
        - containerPort: 8080
          name: http-8080
          protocol: TCP
        volumeMounts:
        - mountPath: /mosparo-config
          name: mosparo-config
        - mountPath: /mosparo/public/resources
          name: mosparo-public-resources
        - mountPath: /mosparo/var
          name: mosparo-var
        - mountPath: /certs
          name: certs
      volumes:
      - name: certs
        secret:
          defaultMode: 420
          secretName: certs
      - name: mosparo-config
        persistentVolumeClaim:
          claimName: config-pvc
      - name: mosparo-public-resources
        persistentVolumeClaim:
          claimName: public-resources-pvc
      - name: mosparo-var
        persistentVolumeClaim:
          claimName: var-pvc

Thanks a lot!

I'm also making progress on the Java client library.

Kind regards

[zepich]

Hi @gnieser

Thank you very much for your feedback!

I think this can be addressed by providing two variants of the verifySubmission method. This first one would not check the fields, while the second one will.

VerificationResult verifySubmission(Map<String, Object> formData)

VerificationResult verifySubmission(Map<String, Object> formData, Set<String> requiredFields)

In this situation, I would still throw an exception from the second method if the requiredFields parameter is null or empty. In my opinion, the use of this method declares the intention of field verification, so the absence of required fields is a mistake / issue. What's your take on this, are there other scenario for empty requiredFields parameter?

That would be great! Edit: You've already adjusted the code. The two methods are perfect! With that, you can decide whether to check for the required fields yourself or if the API client should do that for you.

Ah! Sure, it will help... I wasn't sure of the meaning of the absence of a required field in the result :)

That means that someone has manipulated the form after clicking the mosparo checkbox. So the form could now contain spam... Technically, this is not only the 'required form input fields' but all verifiable fields (text, textarea, email, url, and so on) (simple rule: every field where the user can write text with letters).

I will add a dedicated test for this class.
The HealthCheck feature has a unit test and an integration test.

Awesome, thank you very much!

So the whole quality of the documentation depends on the quality of the javadoc in the code.
I hope it's sufficient and correct. If not, please let me know where it's lacking.

Perfect. Thank you very much for the information regarding Javadoc. I saw the doc blocks in the code, and from my side, you documented everything. No adjustments are needed.

This, you will have to handle by yourself. I'm only able to publish under my github namespace.
You will need to configure the secrets for the GitHub actions in mosparo repository.
You will also need a GPG key to sign the artifacts to publish.

Yes, of course. :) I read about that process yesterday, so I think it should be no problem.

I'm not an expert with GitHub actions. I've configured the publication to be triggered upon a release. There are many other options out there :)
Also I've not enable the auto-publish in Maven Central, so the publication first goes through a staging place where one can validate the publication in the Sonatype account. A true publication, I believe, is irreversible.
The auto-publish can be enabled in the pom.xml under the configuration of the corresponding Sonatype plugin.

Perfect, thank you very much for the information! I'm not a big fan of (fully) automatic stuff (like publishing), so that's okay.

---

So, how do we continue? Are you still working on it, or should we start publishing it?

Thank you very much for all your work! Truly amazing!

Kind regards,

zepich

[gnieser]

Hi @zepich

That means that someone has manipulated the form after clicking the mosparo checkbox. So the form could now contain spam... Technically, this is not only the 'required form input fields' but all verifiable fields (text, textarea, email, url, and so on) (simple rule: every field where the user can write text with letters).

Shall I rename the parameter to verifiableFields for clarity?

So, how do we continue? Are you still working on it, or should we start publishing it?

I'm happy to hand it over the mosparo repository :)

I can submit the PR and you can start publishing it! 🚀

Thank you very much for all your work! Truly amazing!

Thanks a lot for the kind words and for the support during the process.

Best regards

[zepich]

Hi @gnieser

Shall I rename the parameter to verifiableFields for clarity?

I thought about that yesterday when I reviewed the code. From my perspective, both names are good. From the usage perspective of the API client, these are the required fields: the fields that must be verified to continue. The important thing is just the difference between the "required form fields" and the "fields that are required to be verified". I checked the documentation in the code yesterday, and the comment you wrote there is clear that it's not the required fields but the fields that need to be verified.

So, from my perspective, we're good.

I'm happy to hand it over the mosparo repository :)

I can submit the PR and you can start publishing it! 🚀

Cool! Let's do that! 😊

Thanks a lot for the kind words and for the support during the process.

You're welcome, but you did the hard work!

Kind regards,

zepich

[gnieser]

Hi @zepich

I've created the PR: mosparo/java-api-client#1

I felt like updating the doc slightly for clarity. Here is the corresponding commit (squashed for the PR) gnieser/mosparo-java-api-client@f89c7af

Kind regards

[zepich]

Hi @gnieser

Thank you very much for the PR and for adjusting the doc!

Can you use it from here (just as a test): https://central.sonatype.com/service/rest/repository/browse/maven-snapshots/io/mosparo/java-api-client/

Kind regards,

zepich

[gnieser]

Hi @zepich

It works fine, sources and javadoc as well.

That's indeed very handy to test snapshots versions!

Here is a pom.xml pointing to Sonatype central portal snapshots.

I added the needed dependencies because I've defined their scope to provided in the library. I thought it would be easier for users who embeds the library in existing tools (e.g. Keycloak) so they don't have to tweak transitive dependencies. Would I have put them in the default scope, they won't be needed in the below pom.xml. I'm not sure what's best...

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>io.mosparo</groupId>
    <artifactId>my-mosparo-client</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

        <mosparo.version>1.0.0-SNAPSHOT</mosparo.version>
        <httpclient.version>4.5.14</httpclient.version>
        <jackson.version>2.18.2</jackson.version>
        <commons-codec.version>1.13</commons-codec.version>
    </properties>

    <repositories>
        <repository>
            <name>Central Portal Snapshots</name>
            <id>central-portal-snapshots</id>
            <url>https://central.sonatype.com/service/rest/repository/browse/maven-snapshots</url>
            <releases>
                <enabled>false</enabled>
            </releases>
            <snapshots>
                <enabled>true</enabled>
            </snapshots>
        </repository>
    </repositories>

    <dependencies>
        <dependency>
            <groupId>io.mosparo</groupId>
            <artifactId>java-api-client</artifactId>
            <version>${mosparo.version}</version>
        </dependency>
        <dependency>
            <groupId>org.apache.httpcomponents</groupId>
            <artifactId>httpclient</artifactId>
            <version>${httpclient.version}</version>
        </dependency>
        <dependency>
            <groupId>com.fasterxml.jackson.core</groupId>
            <artifactId>jackson-databind</artifactId>
            <version>${jackson.version}</version>
        </dependency>
        <dependency>
            <groupId>com.fasterxml.jackson.datatype</groupId>
            <artifactId>jackson-datatype-jsr310</artifactId>
            <version>${jackson.version}</version>
        </dependency>
        <dependency>
            <groupId>commons-codec</groupId>
            <artifactId>commons-codec</artifactId>
            <version>${commons-codec.version}</version>
        </dependency>
    </dependencies>
</project>

Best regards

[zepich]

Hi @gnieser

Ah, okay, now I know why I had the error about the missing HttpClient when I wanted to test it yesterday... ;)

So, from my perspective, I expect the library to install all required dependencies when I install it. I'm not sure how it is with different versions; let's say Keycloak uses version X.0 of the org.apache.httpcomponents.httpclient, but we use Y.0. Is that compatible if we require version Y.0 in our pom.xml (the pom.xml of the library, not the one where we use the library)?

To be honest, you know way more about developing with Java than I do, so I'm open to your suggestions about what we should do.

At least it should be adjusted in the README that you have to include these dependencies, too.

Kind regards,

zepich

[gnieser]

Hi @zepich

This behavior can be achieved by removing the <scope>provided</scope> for the three previous dependencies (lombok is another topic and should stay in provided, it's code generation)

Out of my mind (I haven't checked the doc in a long time), Maven will resolve the dependencies transitively and will solve conflicts, keeping the oldest one. If there's byte-code compatibility between the two versions, it will work just fine. If not, one can guide the conflict resolution (by explicitly excluding the problematic versions). And if no versions are compatible, well it is an issue :)

Best regards!

[gnieser]

I haven't really documented the build, and forgot to mention that the revision property, is a special Maven property.
The value set in the pom.xml is only use locally when developing the library, you don't need to update it prior any release.
https://github.com/mosparo/java-api-client/blob/96c49b12841f2709fa5d05f7f1b4f02004175909/pom.xml#L47

The GitHub release action overrides it from the release tag (-Drevision, removing the first letter, assuming the tag are vX.Y.Z).

mvn ${MAVEN_ARGS} clean deploy -Drevision="${BUILD:1}"

[zepich]

Hi @gnieser

Thank you very much for the update.

I was first confused about the 999-SNAPSHOT version but then figured that out (with the -D argument).

I adjusted the version in the pom.xml because I like to specify the version somewhere in the code so it's documented which version this branch/commit/tag is.

Is it negative or a problem if we specify the version number in the pom.xml?

Kind regards,

zepich

[gnieser]

Hi @zepich

Apart from being more work when doing a release, I don't think it matters.
Anyone downloading the code to do some development, may need to ensure the revision does not conflict with anything already published. But that's as easy at setting the revision number.

If you want it to always reflect the version, you will need to create a commit with the release revision, then a new commit with the next snapshot. This kind of workflow was implemented in the maven release plugin (which asked for write permission to the repository), but it felt out of favor some time ago. It's still actively maintained: https://maven.apache.org/maven-release/maven-release-plugin/

Kind regards

[zepich]

Hi @gnieser

Perfect. Thank you very much for your feedback.

I'll do this manually since it's no big work, and I have to do it for mosparo itself and all the other integrations, too, so it's not something new.

Kind regards,

zepich