Security fixes (#292)

Co-authored-by: tchapi <regbasket@gmail.com>

Author: tchapi

.env

@@ -49,7 +49,7 @@ AUTH_REALM=SabreDAV
 AUTH_METHOD=Basic
 
 # In case of IMAP Auth, you must specify the url of the mailbox in the following format host[:port].
-IMAP_AUTH_URL=null
+IMAP_AUTH_URL=
 IMAP_ENCRYPTION_METHOD=ssl
 IMAP_CERTIFICATE_VALIDATION=true
 IMAP_AUTH_USER_AUTOCREATE=false

composer.json

@@ -31,6 +31,7 @@
         "symfony/process": "^7.4",
         "symfony/property-access": "^7.4",
         "symfony/property-info": "^7.4",
+        "symfony/rate-limiter": "7.4.*",
         "symfony/runtime": "^7.4",
         "symfony/security-bundle": "^7.4",
         "symfony/serializer": "^7.4",

composer.lock

@@ -18,19 +18,24 @@ security:
             custom_authenticators:
                 - App\Security\LoginFormAuthenticator
             provider: admin_user_provider
+            login_throttling:
+                max_attempts: 5
             logout:
                 path: app_logout
                 target: dashboard
                 # Without this, any page the admin visits can log them out with a plain GET
                 enable_csrf: true
 
 
+    # Only the first matching rule applies, so the public endpoints are listed first and the
+    # admin interface is whatever is left. A route added later is protected unless it is added
+    # to this list on purpose.
     access_control:
         - { path: ^/$, roles: PUBLIC_ACCESS }
         - { path: ^/dav, roles: PUBLIC_ACCESS }
-        - { path: ^/dashboard, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" }
-        - { path: ^/users, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" }
-        - { path: ^/calendars, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" }
-        - { path: ^/addressbooks, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" }
+        - { path: ^/\.well-known/, roles: PUBLIC_ACCESS }
+        - { path: ^/login$, roles: PUBLIC_ACCESS }
+        - { path: ^/logout$, roles: PUBLIC_ACCESS }
         - { path: ^/api/v1/health$, roles: PUBLIC_ACCESS }
         - { path: ^/api, roles: IS_AUTHENTICATED }
+        - { path: ^/, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" }

config/packages/security.yaml

@@ -629,7 +629,7 @@
  *         }>,
  *     },
  *     rate_limiter?: bool|array{ // Rate limiter configuration
- *         enabled?: bool|Param, // Default: false
+ *         enabled?: bool|Param, // Default: true
  *         limiters?: array<string, array{ // Default: []
  *             lock_factory?: scalar|Param|null, // The service ID of the lock factory used by this limiter (or null to disable locking). // Default: "auto"
  *             cache_pool?: scalar|Param|null, // The cache pool to use for storing the current limiter state. // Default: "cache.rate_limiter"

config/reference.php

@@ -1,5 +1,6 @@
 # General settings
-APP_ENV=prod # or dev
+# Either prod or dev
+APP_ENV=prod
 
 CALDAV_ENABLED=true
 CARDDAV_ENABLED=true
@@ -26,7 +27,8 @@ ADMIN_PASSWORD=admin
 ADMIN_AUTH_BYPASS=false
 
 # DAV auth settings
-AUTH_METHOD=Basic   # Basic or IMAP or LDAP
+# Either Basic, IMAP or LDAP
+AUTH_METHOD=Basic
 
 # Basic HTTP auth settings
 AUTH_REALM=SabreDAV
@@ -42,7 +44,8 @@ LDAP_AUTH_URL=ldap://127.0.0.1:3890
 LDAP_DN_PATTERN=uid=%u,ou=users,dc=domain,dc=com
 LDAP_MAIL_ATTRIBUTE=mail
 LDAP_AUTH_USER_AUTOCREATE=false
-LDAP_CERTIFICATE_CHECKING_STRATEGY=try # never, hard, demand, try, or allow
+# Either never, hard, demand, try or allow
+LDAP_CERTIFICATE_CHECKING_STRATEGY=try
 
 # WebDAV settings
 WEBDAV_TMP_DIR=/webdav/tmp

docker/.env

@@ -23,10 +23,12 @@ class AddressBook
     private $principalUri;
 
     #[ORM\Column(name: 'displayname', type: 'string', length: 255, nullable: true)]
+    #[Assert\Length(max: 255)]
     private $displayName;
 
     #[ORM\Column(type: 'string', length: 255)]
     #[Assert\Regex("/[0-9a-z\-]+/")]
+    #[Assert\Length(max: 255)]
     private $uri;
 
     #[ORM\Column(type: 'text', nullable: true)]

src/Entity/AddressBook.php

@@ -37,10 +37,12 @@ public static function getOwnerAccesses(): array
     private $access;
 
     #[ORM\Column(name: 'displayname', type: 'string', length: 255, nullable: true)]
+    #[Assert\Length(max: 255)]
     private $displayName;
 
     #[ORM\Column(type: 'string', length: 255, nullable: true)]
     #[Assert\Regex("/[0-9a-z\-]+/")]
+    #[Assert\Length(max: 255)]
     private $uri;
 
     #[ORM\Column(type: 'text', nullable: true)]
@@ -51,6 +53,7 @@ public static function getOwnerAccesses(): array
 
     #[ORM\Column(name: 'calendarcolor', type: 'string', length: 10, nullable: true)]
     #[Assert\Regex("/\#[0-9A-F]{6}/")]
+    #[Assert\Length(max: 10)]
     private $calendarColor;
 
     #[ORM\Column(type: 'text', nullable: true)]

src/Entity/CalendarInstance.php

@@ -88,18 +88,6 @@ public function setCalendar(?Calendar $calendar): self
         return $this;
     }
 
-    public function getLastModifier(): ?int
-    {
-        return $this->lastModifier;
-    }
-
-    public function setLastModifier(?int $lastModifier): self
-    {
-        $this->lastModifier = $lastModifier;
-
-        return $this;
-    }
-
     public function getEtag(): ?string
     {
         return $this->etag;

src/Entity/CalendarObject.php

@@ -24,24 +24,24 @@ class Principal
     private $id;
 
     #[ORM\Column(type: 'string', length: 255, unique: true)]
-    #[Assert\Unique]
     #[Assert\NotBlank]
+    #[Assert\Length(max: 255)]
     private $uri;
 
     #[ORM\Column(type: 'string', length: 255, nullable: true)]
     #[Assert\NotBlank]
     #[Assert\Email(message: "The email '{{ value }}' is not a valid email.")]
+    #[Assert\Length(max: 255)]
     private $email;
 
     #[ORM\Column(name: 'displayname', type: 'string', length: 255, nullable: true)]
+    #[Assert\Length(max: 255)]
     private $displayName;
 
     #[ORM\Column(type: 'boolean')]
-    #[Assert\NotBlank]
     private $isMain;
 
     #[ORM\Column(type: 'boolean')]
-    #[Assert\NotBlank]
     private $isAdmin;
 
     #[ORM\ManyToMany(targetEntity: 'Principal')]