WIP: LDAP over TLS, DKIM and robots.conf.j2 by ulvida · Pull Request #1 · cielito/ansible-role-sympa

ulvida · 2025-08-08T00:00:44Z

In this PR we fix:

LDAP authentication with encryption over TLS,
DKIM signature message handle,
we extend the robots.conf.j2 template, to handle mail parameters

We are also working in Spamassassin and Clamav integration.

Cuando tengamos algo listo sacamos el "WIP;" y proponemos la PR al upstream

ulvida

A first review of this "work in progress", before proposing to the upstream.

ulvida · 2025-08-08T00:21:39Z

+- name: Add ldap-secure to features if LDAP auth is enabled
+  set_fact:
+    sympa_cpanm_features: "{{ sympa_cpanm_features + ['ldap-secure'] }}"
+  when: sympa_ldap_auth_enabled | bool


Shouldn't we allow a non encrypted ldap authentication, with a second boolean flag or several values for this variable?

By the way, there is another PR for LDAP auth. It takes another approach, defining a structure sympa_config_ldap, and changing auth.conf template for a more rigid one.

I prefer present approach, that preserve pre-existent API variable sympa_web_authentication. I consider very elegant how present version of auth.conf template solves all possible backends, withe their specific parameters. I think we should take this approach for other templates, such as robots.conf: some parameters deserve specific consideration (such as listmaster), but after we can loop all the rest of key: value parameters as {{ key }} {{ value }}. The devops configuring her sympa server should ensure all are legitimate parameters for the .conf file.

It deserves to notice that building rigid templates doesn't really ensure we will detect configuration errors. If a parameter is misspelled, it will silently be ignored, eventually leading to a misconfiguration.

If we want to check configuration, we should explicitely do it with a loop of asserts on all possible values. For templates, considering sympa configuration file structure, appropriate loops on key: value parameters templated as {{ key }} {{ value }} are ok.

Comentario original en español:

¿no deberíamos permitir una authenticación en ldap sin encriptado, con un segundo booleano o con algunos valores a esta variable, en vez de enable/disable booleano?

Sea dicho de paso ya hay otra PR para la auth LDAP. Toma un enfoque diferente: define una estructura sympa_config_ldap, y cambia el template auth.conf por uno más rígido.

Prefiero el enfoque tomado acá, que respeta la variable de API pre-existente sympa_web_authentication. Y me parece muy elegante como la versión actual el template auth.conf resuelve todos los backend possibles, con sus diversos parámetros. Creo que podríamos tomarla para otras plantillas, como robots.conf: algunos parámetros merecen tratamiento especial (como listmaster), pero luego toda la lista restante de key: value, a carga do quién use el rol de poner ahí parámetros legitimos para un robot sympa.

ulvida · 2025-08-08T00:24:34Z

+{% if item.config_colors is defined %}
+## Colors
+{% for color in item.config_colors %}
+color_{{ loop.index - 1 }} {{ color }}


Entiendo que el parámetro color_x fue abandonado en la 6.2.70.

ulvida · 2025-08-08T00:32:52Z

 cookie_domain {{ item.web_domain | default(item.domain) }}
 {% endif %}
+{% if item.config_title is defined %}
+title {{ item.config_title }}


Why adding prefix config_ to all the following parameters?

We are proposing the definition of sympa_robots role's API. I think we should stick as close as possible of sympa configuration vocabulary.

¿porqué config_ para todos los siguientes parámetros que agregan de un robot?

acá están proponiendo la API de la sympa_robots. Creo que el nombre debe ser igual al parámetro sympa. Idealmente, la plantilla debe considerar todos los parámetros de config que la doc indica se pueden usar en un robots.conf.

idem, my own fault.

ulvida · 2025-08-08T14:27:21Z

+sympa_dkim_feature: false
+sympa_dkim_add_signature_to:
+  - robot
+  - list


Candide question: Shouldn't we allow to sign each robot with its specific DKIM key?

I guess it would be better if each robot is from an entirely different fqdn

Now, in cielito.sympa.install, we hard coded dkim_same_key: false when calling dkim role .

But: are we proposing a complete and consistent DKIM features API for this upstream role?

Was dkim_same_key from our fork or from upstream? Why is it broken in our fork? Anyways, it doesn't seem like a great idea to have the same key for every (sub)domain.

ulvida · 2025-08-08T14:29:22Z

@@ -1,8 +1,31 @@
 # Robot configuration for {{ item.domain }}


For code clarity, shouldn't we give a name (for instance sympa_robot) to this item loop variable?

Oups... my own fault.

Now I remember I preferred "not to change too much". I'll grap this in a PR specific to robots configuration.

ulvida · 2025-08-08T14:57:49Z

+  set_fact:
+    sympa_cpanm_features: "{{ sympa_cpanm_features + ['ldap-secure'] }}"
+  when: sympa_ldap_auth_enabled | bool
+  tags: sympaldap


role tags are sympa-conf, sympa-auth, ... should therefore be sympa-ldap here.

What about sympasoap, though?

right... anyway, all tagging is still to review. not very relevant, so.

I think these tags are not meant to be run until everything prior has been ran at least once, because the rest of the file has no tags at all

fzadikian · 2025-08-11T19:34:58Z

I added the LDAP variables to default/main.yml as well as README.md. and I also added the option for non-SSL LDAP authentication.

ulvida · 2025-08-13T15:46:19Z

+
+#### *sympa_ldap_auth_ssl*
+
+Whether authentication through LDAP uses SSL. Defaults to `true`.


Is it SSL or TLS? And shouldn't we allow any transport protocol, for sysadmin to be able to fit their installation with their LDAP requirements?

By the way, to complete LDAP understanding we should also document somewhere how to build sympa_web_authentication. A link to ldap auth.conf configuration would be worthy.

Is it SSL or TLS?

ldap-secure depends on IO::Socket::SSL, so I guess it is SSL.

And shouldn't we allow any transport protocol, for sysadmin to be able to fit their installation with their LDAP requirements?

What do you mean? We are currently covering both ldap and ldap-secure, the only ldap related features on sympa's cpanfile.

ldap-secure depends on IO::Socket::SSL, so I guess it is SSL.

SSL is often used for secure layer, but strictly SSL is deprecated and being replaced by TLS. Seeing our LDAP parameters, I think we are using TLS 1.2, on port 389. ldaps (i.e. LDAP on SSL listen on port 636).

What do you mean?
I mean the LDAP may pre-exists and Sympa should bind to itl as provided: basic ldap on port 389, ldaps on 636 and/or ldap+starttls on 389 (our LDAP, for instance, took a while to offer this last protocol)

We are currently covering both ldap and ldap-secure, the only ldap related features on sympa's cpanfile.

There are also TLS parameters of the authentication.

There are also TLS parameters of the authentication.

I see, then shall I rename variables to reference TLS instead of SSL, or directly reference "secure" instead?

I mean the LDAP may pre-exists and Sympa should bind to itl as provided: basic ldap on port 389, ldaps on 636 and/or ldap+starttls on 389 (our LDAP, for instance, took a while to offer this last protocol)

So then, does that need any special configurations from this role? I thought the {{ key }} {{ value }} approach would eliminate all those quirks.

ulvida

Non blocking details in API and templating, for a better PR to upstream.

ulvida · 2025-09-02T13:47:30Z

+{% if sympa_dkim_signature_apply_on | length>0 %}
+dkim_signature_apply_on {{ sympa_dkim_signature_apply_on | join(',') }}
+{% else %}
+dkim_signature_apply_on none


dkim_signature_apply_on is not mandatory, and default behaviour of sympa when its not defined - or empty? - is not none. I think it's good to define sympa_dkim_signature_apply_on as a list, but not to "over-template" it.

So, just keep the if statement and remove the else statement entirely?

ulvida · 2025-09-02T13:55:32Z

+sympa_dkim_feature: false
+sympa_dkim_add_signature_to:
+  - robot
+  - list


Now, in cielito.sympa.install, we hard coded dkim_same_key: false when calling dkim role .

But: are we proposing a complete and consistent DKIM features API for this upstream role?

ulvida · 2025-09-02T14:02:18Z

+  - md5_authenticated_messages
+  - smime_authenticated_messages
+  - dkim_authenticated_messages
+  - editor_validated_messages


I understand you set these values because of these sympa defaults.

However, this is the default behaviour of sympa when dkim_signature_apply_on is not defined. Here, we must allow such setting, not set values that reproduce this behaviour. As we need to allow "dkim_signature_apply_on not defined" in sympa, we shouldn't set a default value here.

So, we shouldn't set any of these four values, and if the user decides to set dkim_signature_apply_on, they should handle it themselves? Just in case, this code was snipped from our legacy sympa role and adapted by @paulo-ruiz.

So, we shouldn't set any of these four values, and if the user decides to set dkim_signature_apply_on, they should handle it themselves?

Yes. Eventually the role could assert permitted values, but I think re-coding in the role the default behaviour of sympa conflicts with DRY principle. don't you think so?

Just in case, this code was snipped from our legacy sympa role and adapted by @paulo-ruiz.

legacy or upstream sympa role? There is nothing about DKIM in upstream version of this template. We are building our PR.

legacy or upstream sympa role? There is nothing about DKIM in upstream version of this template. We are building our PR.

I am referring to this.

Yes. Eventually the role could assert permitted values, but I think re-coding in the role the default behaviour of sympa conflicts with DRY principle. don't you think so?

Yes, I agree that it's better to simplify this.

… template

ulvida · 2025-09-09T16:07:22Z

+# adapted from stuvusIT/sympa repository on Github
+
+## dkim_feature
+dkim_feature {{ sympa_dkim_feature | bool | ternary('on', 'off') }}


All the previous parameters of this file are prefixed with sympa_config_* and for this one and following we switch to sympa_*. Should we harmonise naming? or sympa_config_dkim_* is too much?

both variables exist, though. sympa_dkim_feature actually configures dkim, and sympa_config_dkim_feature just installs the dependencies. i would change the last one, but that would break upstream. merging them into one doesn't seem ideal either.

ulvida · 2025-09-12T18:12:21Z

+    path: "{{ sympa_installation_directory }}/arc/{{ sympa_robot_domain }}"
+    owner: "{{ sympa_unix_user }}"
+    group: "{{ sympa_unix_group }}"
+    mode: 0755


Apparently, these folders per robot are not needed. Sympa create archive folder with the full name of the list list@robot_domain.

…ll folder

Daniel Viñar Ulriksen and others added 10 commits November 13, 2024 21:02

WIP: running the role

3425f77

Additionnal aliases_db_type in sympa.conf

08265c2

superflous changes

f76d0de

adding LDAP perl module

153012a

adding IO::Socket::SSL module, also for LDAP auth

3c535f7

fixing list of modules, robots.conf configuration

58e92a6

Merge branch 'main' into cielito

f7fdcaf

fix if clause, perl modules

9df15d0

add task to install LDAP libraries needed

0e47990

Update sympa.conf template to use DKIM

123afa6

ulvida commented Aug 8, 2025

View reviewed changes

Daniel Viñar Ulriksen and others added 9 commits August 8, 2025 20:43

WIP: configuring robots

55d051b

considering compounding parameters

735140f

fixing: compounding parameters

bc172c6

default value for sympa_robots, documentation

fee0a63

Merge branch 'robots-config' into cielito

55c990c

sympa_robots as a dict

67a06e6

Merge branch 'robots-config' into cielito

245491a

fixing robot.conf template merge

f61f71a

add no ssl ldap option

b00cb5a

add LDAP to README.md

5498477

ulvida commented Aug 13, 2025

View reviewed changes

Daniel Viñar Ulriksen added 4 commits August 13, 2025 15:31

wwsympa_url instead of web_domain

f095e88

Merge branch 'robots-config' into cielito

3b26398

sympa_robots default as empty dict, not list

a9fb16f

if dkim_feature, config dkim requirements

8340e19

ulvida commented Sep 2, 2025

View reviewed changes

fzadikian and others added 8 commits September 2, 2025 15:28

rename ldaps variable

b63c87b

follow DRY principle for dkim_signature_apply_on

f2f7e00

Update sympa.conf template to use DKIM

6f70d93

if dkim_feature, config dkim requirements

a286266

respect sympa defaults, add missing dkim_add_signature_to variable to…

17846bc

… template

merge branch 'dkim-upstream' into cielito

99e78d8

sympa_config_extra_parameters as a dict

7214997

accept string instead of list parameter

a089342

ulvida commented Sep 9, 2025

View reviewed changes

Daniel Viñar Ulriksen and others added 2 commits September 9, 2025 16:53

templates and vars: list or strings as jinja macro

29a605e

workaround dbtype for postgresql

f163afd

ulvida commented Sep 12, 2025

View reviewed changes

Daniel Viñar Ulriksen and others added 4 commits September 12, 2025 17:39

no arc folder per robot, set attributes to existing elementi in insta…

2bff66c

…ll folder

accepitng sympa_robots elements only with domain key

9f3af37

fix bug from when expression

592b602

fix more conditionals containing 'count'

af2f987

fzadikian force-pushed the cielito branch from 35aa20f to af2f987 Compare October 9, 2025 20:26


		#### sympa_ldap_auth_ssl

		Whether authentication through LDAP uses SSL. Defaults to `true`.

Uh oh!

Conversation

ulvida commented Aug 8, 2025

Uh oh!

ulvida left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fzadikian Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fzadikian commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulvida left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulvida Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulvida Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulvida Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fzadikian Sep 2, 2025 •

edited

Loading

fzadikian commented Aug 11, 2025 •

edited

Loading

ulvida Sep 2, 2025 •

edited

Loading

ulvida Sep 5, 2025 •

edited

Loading

ulvida Sep 9, 2025 •

edited

Loading