|
| 1 | +# Pre-1.3 |
| 2 | + |
| 3 | +## 2025-01-03 – Multiple IP addresses |
| 4 | + |
| 5 | +WireGuard network interface can be assigned multiple IP addresses (both IPv4 and IPv6). The first address (leftmost on the interface configuration) is the primary address, and this one will be used for IP address assignment for devices. The other IP addresses are auxiliary and are not managed by Defguard. |
| 6 | + |
| 7 | +## 2024-11-14 – Use Authorization Code flow for external OpenID |
| 8 | + |
| 9 | +Abandon Implicit flow, which is not recommended. |
| 10 | + |
| 11 | +## 2024-11-11 – Require User-Agent HTTP header for login |
| 12 | + |
| 13 | +User-Agent header is set by all HTTP clients, including [Curl](https://curl.se), so there is no point to make it optional. That also simplifies the code. |
| 14 | + |
| 15 | +## 2024-09-07 - Typestate pattern to make working with optional ids easier |
| 16 | + |
| 17 | +* There is a recurring issue with optional ids for database objects. Until such object is saved to database, it’s id is none. Because of this it is necessary to check for id presence whenever such objects are used, even if we know that those objects come from database and must have ids. This leads to unnecessary `.expect() / .unwrap()`s. |
| 18 | +* Typical object with optional Id looks like this: |
| 19 | + |
| 20 | +``` |
| 21 | +#[derive(Debug)] |
| 22 | +pub struct WireguardKeys { |
| 23 | + pub id: Option<i64>, |
| 24 | + pub instance_id: i64, |
| 25 | + pub pubkey: String, |
| 26 | + pub prvkey: String, |
| 27 | +} |
| 28 | +``` |
| 29 | + |
| 30 | +* To fix this we introduce a generic type \<I> that defines type of id field |
| 31 | +* The generic type can take one of two variants |
| 32 | +* We split impl blocks into \<Id> and \<NoId> appropriately - the idea is that objects coming from database are \<Id>, methods like `new()` return \<NoId> |
| 33 | + |
| 34 | +``` |
| 35 | +// Id variants |
| 36 | +pub type Id = i64; |
| 37 | +pub struct NoId; |
| 38 | +
|
| 39 | +// Generic id type used in the structure |
| 40 | +#[derive(Debug)] |
| 41 | +pub struct WireguardKeys<I = NoId> { |
| 42 | + pub id: I, |
| 43 | + pub instance_id: i64, |
| 44 | + pub pubkey: String, |
| 45 | + pub prvkey: String, |
| 46 | +} |
| 47 | +
|
| 48 | +// Impl blocks split between the variants |
| 49 | +impl WireguardKeys<Id> { |
| 50 | + pub async fn find_by_instance_id<'e, E>( |
| 51 | + executor: E, |
| 52 | + instance_id: i64, |
| 53 | + ) -> Result<Option<Self>, SqlxError> |
| 54 | + where |
| 55 | + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, |
| 56 | + { |
| 57 | + query_as!( |
| 58 | + Self, |
| 59 | + "SELECT id \"id: _\", instance_id, pubkey, prvkey \ |
| 60 | + FROM wireguard_keys WHERE instance_id = $1;", |
| 61 | + instance_id |
| 62 | + ) |
| 63 | + .fetch_optional(executor) |
| 64 | + .await |
| 65 | + } |
| 66 | +} |
| 67 | +
|
| 68 | +impl WireguardKeys<NoId> { |
| 69 | + #[must_use] |
| 70 | + pub fn new(instance_id: i64, pubkey: String, prvkey: String) -> Self { |
| 71 | + WireguardKeys { |
| 72 | + id: NoId, |
| 73 | + instance_id, |
| 74 | + pubkey, |
| 75 | + prvkey, |
| 76 | + } |
| 77 | + } |
| 78 | +} |
| 79 | +``` |
| 80 | + |
| 81 | +## 2024-09-06 – External OpenID login |
| 82 | + |
| 83 | +* Currently our OpenID login implementation matches the user by email. This is not a standard practice, as most services use the “sub” field (a guaranteed unique identifier) to identify the user (e.g. matrix/element). Our approach may be problematic when the email changes on the provider’s side (it may be unlikely in the case of google or microsoft but may happen in the case of keycloak). |
| 84 | +* To prevent such scenario and to standardize our approach we could add a “sub” field to the user and perform openid login on its basis. Additionally, if we’d like to link existing defguard accounts with the external provider, we could try to also match by email on first user login and then use the sub field only on subsequent login attempts. This doesn’t seem to require a massive rework of already existing code. |
| 85 | + |
| 86 | +## 2024-09-02 – Client configuration updates |
| 87 | + |
| 88 | +* Since client <→ proxy communication is REST, easiest way to implement client config updates (“make it work, make it right, make it fast”, “good enough”) is with HTTP polling requests |
| 89 | +* As a consequence, updates are not immediate, worst case scenario update arrives at client after full polling interval |
| 90 | +* Objects that DON’T need to be updated in the client database: |
| 91 | + * WireGuard keys - since client does not implement key management features, updating public key in client db guarantees that the client won’t be able to connect to the gateway (public-private keys don’t match) |
| 92 | + * only way to update keys at this point is using standard enrollment procedure |
| 93 | + * Instances - effectively only the `name` field may change, we can deal with it later maybe |
| 94 | +* Objects that need to be updated in client db: |
| 95 | + * Instance locations - those contain all the necessary configuration that make connecting to the gateway possible and define parameters like `allowed_ips`, `dns` etc. |
| 96 | +* We can’t simply update the locations whenever they change, client may be connected to one of the locations that get removed in the update and we end up in invalid state. |
| 97 | +* Solution: |
| 98 | + * After detecting changes in the configuration, polling mechanism checks if there are any active connections for given instance |
| 99 | + * If there are no active connections, update the database |
| 100 | + * If there are active connections, display the message “Configuration updated for instance \<name>, disconnect all locations to apply changes” |
| 101 | + * On each disconnect trigger the polling mechanism |
| 102 | + |
0 commit comments