Browse Source

few bugfixes, fido/u2f still WIP

Roman 3 weeks ago
parent
commit
6c551b08d8

+ 4 - 1
Core/API/Request.class.php

@@ -255,8 +255,11 @@ abstract class Request {
       $this->success = $req->execute(["method" => self::getEndpoint()]);
       $this->lastError = $req->getLastError();
       if (!$this->success) {
+        $res = $req->getResult();
         if (!$this->context->getUser()) {
           $this->result["loggedIn"] = false;
+        } else if (isset($res["twoFactorToken"])) {
+          $this->result["twoFactorToken"] = $res["twoFactorToken"];
         }
         return false;
       }
@@ -284,7 +287,7 @@ abstract class Request {
         // this should actually not occur, how to handle this case?
         $this->success = $success;
       }
-    } catch (\Error $err) {
+    } catch (\Throwable $err) {
       http_response_code(500);
       $this->createError($err->getMessage());
       $this->logger->error($err->getMessage());

+ 2 - 5
Core/API/TfaAPI.class.php

@@ -61,7 +61,6 @@ namespace Core\API\TFA {
 
   use Core\API\Parameter\StringType;
   use Core\API\TfaAPI;
-  use Core\Driver\SQL\Condition\Compare;
   use Core\Driver\SQL\Query\Insert;
   use Core\Objects\Context;
   use Core\Objects\TwoFactor\AttestationObject;
@@ -265,10 +264,7 @@ namespace Core\API\TFA {
       $settings = $this->context->getSettings();
       $relyingParty = $settings->getSiteName();
       $sql = $this->context->getSQL();
-
-      // TODO: for react development, localhost / HTTP_HOST is required, otherwise a DOMException is thrown
       $domain = parse_url($settings->getBaseUrl(),  PHP_URL_HOST);
-      // $domain = "localhost";
 
       if (!$clientDataJSON || !$attestationObjectRaw) {
         $challenge = null;
@@ -329,12 +325,13 @@ namespace Core\API\TFA {
           return $this->createError("Unsupported key type. Expected: -7");
         }
 
+        $twoFactorToken->authenticate();
         $this->success = $twoFactorToken->confirmKeyBased($sql, base64_encode($authData->getCredentialID()), $publicKey) !== false;
         $this->lastError = $sql->getLastError();
 
         if ($this->success) {
           $this->result["twoFactorToken"] = $twoFactorToken->jsonSerialize();
-          $this->context->invalidateSessions();
+          $this->context->invalidateSessions(true);
         }
       }
 

+ 11 - 5
Core/API/UserAPI.class.php

@@ -146,6 +146,7 @@ namespace Core\API\User {
   use Core\Driver\SQL\Condition\Compare;
   use Core\Driver\SQL\Condition\CondIn;
   use Core\Driver\SQL\Expression\JsonArrayAgg;
+  use Core\Objects\TwoFactor\KeyBasedTwoFactorToken;
   use ImagickException;
   use Core\Objects\Context;
   use Core\Objects\DatabaseEntity\GpgKey;
@@ -374,6 +375,12 @@ namespace Core\API\User {
         $this->result["loggedIn"] = false;
         $userGroups = [];
       } else {
+
+        $twoFactorToken = $currentUser->getTwoFactorToken();
+        if ($twoFactorToken instanceof KeyBasedTwoFactorToken && !$twoFactorToken->hasChallenge()) {
+          $twoFactorToken->generateChallenge();
+        }
+
         $this->result["loggedIn"] = true;
         $userGroups = array_keys($currentUser->getGroups());
         $this->result["user"] = $currentUser->jsonSerialize();
@@ -629,7 +636,7 @@ namespace Core\API\User {
 
             $this->result["loggedIn"] = true;
             $this->result["user"] = $user->jsonSerialize();
-            $this->result["session"] = $session->jsonSerialize();
+            $this->result["session"] = $session->jsonSerialize(["expires", "csrfToken"]);
             $this->result["logoutIn"] = $session->getExpiresSeconds();
             $this->check2FA($tfaToken);
             $this->success = true;
@@ -1310,6 +1317,7 @@ namespace Core\API\User {
       }
 
       $settings = $this->context->getSettings();
+      $siteName = htmlspecialchars($settings->getSiteName());
       $baseUrl = htmlspecialchars($settings->getBaseUrl());
       $token = htmlspecialchars(urlencode($token));
       $url = "$baseUrl/confirmGPG?token=$token";
@@ -1317,14 +1325,12 @@ namespace Core\API\User {
         "you imported a GPG public key for end-to-end encrypted mail communication. " .
         "To confirm the key and verify, you own the corresponding private key, please click on the following link. " .
         "The link is active for one hour.<br><br>" .
-        "<a href='$url'>$url</a><br>
-        Best Regards<br>" .
-      $settings->getSiteName() . " Administration";
+        "<a href='$url'>$url</a><br>Best Regards<br>$siteName Administration";
 
       $sendMail = new \Core\API\Mail\Send($this->context);
       $this->success = $sendMail->execute(array(
         "to" => $currentUser->getEmail(),
-        "subject" => $settings->getSiteName() . " - Confirm GPG-Key",
+        "subject" => "[$siteName] Confirm GPG-Key",
         "body" => $mailBody,
         "gpgFingerprint" => $gpgKey->getFingerprint()
       ));

+ 1 - 1
Core/API/VisitorsAPI.class.php

@@ -29,7 +29,7 @@ namespace Core\API\Visitors {
   class ProcessVisit extends VisitorsAPI {
     public function __construct(Context $context, bool $externalCall = false) {
       parent::__construct($context, $externalCall, array(
-        "cookie" => new StringType("cookie")
+        "cookie" => new StringType("cookie", 26)
       ));
       $this->isPublic = false;
     }

+ 1 - 0
Core/Objects/Context.class.php

@@ -91,6 +91,7 @@ class Context {
   }
 
   public function sendCookies(): void {
+    // TODO: what will we do, when there is a domain mismatch? forbid access or just send cookies for the current domain? or should we send a redirect?
     $domain = $this->getSettings()->getDomain();
     $this->language->sendCookie($domain);
     $this->session?->sendCookie($domain);

+ 4 - 3
Core/Objects/TwoFactor/AttestationObject.class.php

@@ -2,6 +2,7 @@
 
 namespace Core\Objects\TwoFactor;
 
+use CBOR\MapObject;
 use Core\Objects\ApiObject;
 
 class AttestationObject extends ApiObject {
@@ -9,14 +10,14 @@ class AttestationObject extends ApiObject {
   use CBORDecoder;
 
   private string $format;
-  private array $statement;
+  private MapObject $statement;
   private AuthenticationData $authData;
 
   public function __construct(string $buffer) {
-    $data = $this->decode($buffer)->getNormalizedData();
+    $data = $this->decode($buffer);
     $this->format = $data["fmt"];
     $this->statement = $data["attStmt"];
-    $this->authData = new AuthenticationData($data["authData"]);
+    $this->authData = new AuthenticationData($data["authData"]->getValue());
   }
 
   public function jsonSerialize(): array {

+ 30 - 14
Core/Objects/TwoFactor/AuthenticationData.class.php

@@ -6,31 +6,47 @@ use Core\Objects\ApiObject;
 
 class AuthenticationData extends ApiObject {
 
+  use CBORDecoder;
+
+  const FLAG_USER_PRESENT = 1;
+  const FLAG_USER_VERIFIED = 4;
+  const FLAG_ATTESTED_DATA_INCLUDED = 64;
+  const FLAG_EXTENSION_DATA_INCLUDED = 128;
+
   private string $rpIDHash;
   private int $flags;
-  private int $counter;
+  private int $signCount;
   private string $aaguid;
   private string $credentialID;
+  private array $extensions;
   private PublicKey $publicKey;
 
   public function __construct(string $buffer) {
 
-    if (strlen($buffer) < 32 + 1 + 4) {
+    $bufferLength = strlen($buffer);
+    if ($bufferLength < 32 + 1 + 4) {
       throw new \Exception("Invalid authentication data buffer size");
     }
 
     $offset = 0;
     $this->rpIDHash = substr($buffer, $offset, 32); $offset += 32;
-    $this->flags = ord($buffer[$offset]); $offset += 1;
-    $this->counter = unpack("N", $buffer, $offset)[1]; $offset += 4;
+    $this->flags = ord(substr($buffer, $offset, 1)); $offset += 1;
+    $this->signCount = unpack("N", substr($buffer, $offset, 4))[1]; $offset += 4;
 
-    if (strlen($buffer) >= $offset + 4 + 2) {
+    if ($this->attestedCredentialData()) {
       $this->aaguid = substr($buffer, $offset, 16); $offset += 16;
-      $credentialIdLength = unpack("n", $buffer, $offset)[1]; $offset += 2;
+      $credentialIdLength = unpack("n",  substr($buffer, $offset, 4))[1]; $offset += 2;
       $this->credentialID = substr($buffer, $offset, $credentialIdLength); $offset += $credentialIdLength;
 
-      $credentialData = substr($buffer, $offset);
-      $this->publicKey = new PublicKey($credentialData);
+      if ($offset < $bufferLength) {
+        $publicKeyData = $this->decode(substr($buffer, $offset));
+        $this->publicKey = new PublicKey($publicKeyData);
+        // TODO: we should add $publicKeyData->length to $offset, but it's not implemented yet?;
+      }
+    }
+
+    if ($this->hasExtensionData()) {
+      // not supported yet
     }
   }
 
@@ -38,7 +54,7 @@ class AuthenticationData extends ApiObject {
     return [
       "rpIDHash" => base64_encode($this->rpIDHash),
       "flags" => $this->flags,
-      "counter" => $this->counter,
+      "signCount" => $this->signCount,
       "aaguid" => base64_encode($this->aaguid),
       "credentialID" => base64_encode($this->credentialID),
       "publicKey" => $this->publicKey->jsonSerialize()
@@ -54,26 +70,26 @@ class AuthenticationData extends ApiObject {
   }
 
   public function isUserPresent(): bool {
-    return boolval($this->flags & (1 << 0));
+    return boolval($this->flags & self::FLAG_USER_PRESENT);
   }
 
   public function isUserVerified(): bool {
-		return boolval($this->flags & (1 << 2));
+		return boolval($this->flags & self::FLAG_USER_VERIFIED);
 	}
 
   public function attestedCredentialData(): bool {
-		return boolval($this->flags & (1 << 6));
+		return boolval($this->flags & self::FLAG_ATTESTED_DATA_INCLUDED);
 	}
 
   public function hasExtensionData(): bool {
-		return boolval($this->flags & (1 << 7));
+		return boolval($this->flags & self::FLAG_EXTENSION_DATA_INCLUDED);
 	}
 
   public function getPublicKey(): PublicKey {
     return $this->publicKey;
   }
 
-  public function getCredentialID() {
+  public function getCredentialID(): string {
     return $this->credentialID;
   }
 }

+ 2 - 3
Core/Objects/TwoFactor/CBORDecoder.trait.php

@@ -2,14 +2,13 @@
 
 namespace Core\Objects\TwoFactor;
 
+use CBOR\Decoder;
 use CBOR\StringStream;
 
 trait CBORDecoder {
 
   protected function decode(string $buffer): \CBOR\CBORObject {
-    $objectManager = new \CBOR\OtherObject\OtherObjectManager();
-    $tagManager = new \CBOR\Tag\TagObjectManager();
-    $decoder = new \CBOR\Decoder($tagManager, $objectManager);
+    $decoder = Decoder::create();
     return $decoder->decode(new StringStream($buffer));
   }
 

+ 1 - 1
Core/Objects/TwoFactor/KeyBasedTwoFactorToken.class.php

@@ -37,7 +37,7 @@ class KeyBasedTwoFactorToken extends TwoFactorToken {
   }
 
   public function hasChallenge(): bool {
-    return isset($this->challenge);
+    return isset($this->challenge) && !empty($this->challenge);
   }
 
   public function getChallenge(): string {

+ 10 - 12
Core/Objects/TwoFactor/PublicKey.class.php

@@ -2,26 +2,24 @@
 
 namespace Core\Objects\TwoFactor;
 
+use CBOR\MapObject;
 use Core\Objects\ApiObject;
 
 class PublicKey extends ApiObject {
 
-  use CBORDecoder;
-
   private int $keyType;
   private int $usedAlgorithm;
   private int $curveType;
   private string $xCoordinate;
   private string $yCoordinate;
 
-  public function __construct(?string $cborData = null) {
-    if ($cborData) {
-      $data = $this->decode($cborData)->getNormalizedData();
-      $this->keyType = $data["1"];
-      $this->usedAlgorithm = $data["3"];
-      $this->curveType = $data["-1"];
-      $this->xCoordinate = $data["-2"];
-      $this->yCoordinate = $data["-3"];
+  public function __construct(?MapObject $publicKeyData = null) {
+    if ($publicKeyData) {
+      $this->keyType = $publicKeyData["1"]->getValue();
+      $this->usedAlgorithm = $publicKeyData["3"]->getValue();
+      $this->curveType = $publicKeyData["-1"]->getValue();
+      $this->xCoordinate = $publicKeyData["-2"]->getValue();
+      $this->yCoordinate = $publicKeyData["-3"]->getValue();
     }
   }
 
@@ -53,8 +51,8 @@ class PublicKey extends ApiObject {
 
   public function getNormalizedData(): array {
     return [
-      "1" => $this->keyType,
-      "3" => $this->usedAlgorithm,
+      "1"  => $this->keyType,
+      "3"  => $this->usedAlgorithm,
       "-1" => $this->curveType,
       "-2" => $this->xCoordinate,
       "-3" => $this->yCoordinate,

+ 1 - 1
index.php

@@ -67,7 +67,7 @@ if ($installation) {
     } else {
       try {
         $response = $router->run($requestedUri);
-      } catch (\Error $e) {
+      } catch (\Throwable $e) {
         http_response_code(500);
         $router->getLogger()->error($e->getMessage());
         $response = $router->returnStatusCode(500);

+ 1 - 1
js/account.js

@@ -51,7 +51,7 @@ $(document).ready(function () {
                 btn.prop("disabled", false);
                 $("#password").val("");
                 createdDiv.hide();
-                if (res.emailConfirmed === false) {
+                if (res.user.confirmed === false) {
                     showAlert("danger", res.msg + ' <a href="/resendConfirmEmail">Click here</a> to resend the confirmation mail.', true);
                 } else {
                     showAlert("danger", res.msg);

+ 1 - 1
react/admin-panel/package.json

@@ -6,7 +6,7 @@
     "react": "^18.2.0"
   },
   "scripts": {
-    "dev": "react-app-rewired start"
+    "dev": "HTTPS=true react-app-rewired start"
   },
   "author": "",
   "license": "ISC",

+ 10 - 2
react/admin-panel/src/App.jsx

@@ -28,6 +28,14 @@ export default function App() {
         });
     }, [api]);
 
+    const onLogout = useCallback(() => {
+        api.logout().then(data => {
+            if (!data.success) {
+                setError("Error logging out: " + data.msg);
+            }
+        });
+    }, [api]);
+
     const onInit = useCallback((force = false) => {
         if (loaded && !force) {
             return;
@@ -97,8 +105,8 @@ export default function App() {
         } else {
             return <b>{L("general.loading")}… <Icon icon={"spinner"}/></b>
         }
-    } else if (!user || !api.loggedIn) {
-        return <LoginForm api={api} info={info} onLogin={fetchUser} />
+    } else if (!user || !api.loggedIn || (api.user.twoFactorToken?.confirmed && !api.user.twoFactorToken.authenticated)) {
+        return <LoginForm api={api} info={info} onLogin={fetchUser} onLogout={onLogout} />
     } else {
         return <AdminDashboard api={api} info={info} />
     }

+ 63 - 4
react/admin-panel/src/views/profile/mfa-fido.js

@@ -1,17 +1,76 @@
-import {Box, Paper} from "@mui/material";
+import {Box, CircularProgress, Paper} from "@mui/material";
 import {LocaleContext} from "shared/locale";
 import {useCallback, useContext} from "react";
+import {decodeText, encodeText} from "shared/util";
 
 export default function MfaFido(props) {
 
-    const {api, showDialog, setDialogData, ...other} = props;
+    const {api, showDialog, setDialogData, set2FA, ...other} = props;
     const {translate: L} = useContext(LocaleContext);
 
     const openDialog = useCallback(() => {
-        if (api.hasPermission("tfa/registerKey")) {
+        if (!api.hasPermission("tfa/registerKey")) {
+            return;
+        }
 
+        if (typeof navigator.credentials !== 'object' || typeof navigator.credentials.create !== 'function') {
+            showDialog(L("Key-based Two-Factor-Authentication (2FA) is not supported on this device."), L("Not supported"));
         }
-    }, [api, showDialog]);
+
+        api.register2FA().then(res => {
+            if (!res.success) {
+                showDialog(res.msg, L("Error registering 2FA-Device"));
+                return;
+            }
+
+            setDialogData({
+                show: true,
+                title: L("Register a 2FA-Device"),
+                message: L("You may need to interact with your Device, e.g. typing in your PIN or touching to confirm the registration."),
+                inputs: [
+                    { type: "custom", key: "progress", element: CircularProgress }
+                ],
+                options: [L("general.cancel")],
+            })
+
+            navigator.credentials.create({
+                publicKey: {
+                    challenge: encodeText(window.atob(res.data.challenge)),
+                    rp: res.data.relyingParty,
+                    user: {
+                        id: encodeText(res.data.id),
+                        name: api.user.name,
+                        displayName: api.user.fullName
+                    },
+                    userVerification: "discouraged",
+                    attestation: "direct",
+                    pubKeyCredParams: [{
+                        type: "public-key",
+                        alg: -7, // "ES256" IANA COSE Algorithms registry
+                    }]
+                }
+            }).then(res => {
+                if (res.response) {
+                    let clientDataJSON = decodeText(res.response.clientDataJSON);
+                    let attestationObject = window.btoa(String.fromCharCode.apply(null, new Uint8Array(res.response.attestationObject)));
+                    api.register2FA(clientDataJSON, attestationObject).then((res) => {
+                        setDialogData({show: false});
+                        if (res.success) {
+                            showDialog(L("account.confirm_fido_success"), L("general.success"));
+                            set2FA({ confirmed: true, type: "fido", authenticated: true });
+                        } else {
+                            showDialog(res.msg, L("Error registering 2FA-Device"));
+                        }
+                    });
+                } else {
+                    showDialog(JSON.stringify(res), L("Error registering 2FA-Device"));
+                }
+            }).catch(ex => {
+                setDialogData({show: false});
+                showDialog(ex.toString(), L("Error registering 2FA-Device"));
+            });
+        });
+    }, [api, showDialog, setDialogData, set2FA]);
 
     const disabledStyle = {
         background: "gray",

+ 6 - 4
react/admin-panel/src/views/profile/mfa-totp.js

@@ -4,7 +4,7 @@ import {LocaleContext} from "shared/locale";
 
 export default function MfaTotp(props) {
 
-    const {setDialogData, api, showDialog, ...other} = props;
+    const {setDialogData, api, showDialog, set2FA, ...other} = props;
     const {translate: L} = useContext(LocaleContext);
 
     const onConfirmTOTP = useCallback((code) => {
@@ -14,10 +14,11 @@ export default function MfaTotp(props) {
             } else {
                 setDialogData({show: false});
                 showDialog(L("account.confirm_totp_success"), L("general.success"));
+                set2FA({ confirmed: true, type: "totp", authenticated: true });
             }
         });
         return false;
-    }, [api, showDialog]);
+    }, [api, showDialog, set2FA, setDialogData]);
 
     const openDialog = useCallback(() => {
         if (api.hasPermission("tfa/generateQR")) {
@@ -28,8 +29,8 @@ export default function MfaTotp(props) {
                     "On Android, you can use the Google Authenticator."),
                 inputs: [
                     {
-                        type: "custom", element: Box, textAlign: "center", children:
-                            <img src={"/api/tfa/generateQR?nocache=" + Math.random()} alt={"[QR-Code]"}/>
+                        type: "custom", element: Box, textAlign: "center", key: "qr-code",
+                        children: <img src={"/api/tfa/generateQR?nocache=" + Math.random()} alt={"[QR-Code]"} />
                     },
                     {
                         type: "number", placeholder: L("account.6_digit_code"),
@@ -37,6 +38,7 @@ export default function MfaTotp(props) {
                         sx: { "& input": { textAlign: "center", fontFamily: "monospace" } },
                     }
                 ],
+                options: [L("general.ok"), L("general.cancel")],
                 onOption: (option, data) => option === 0 ? onConfirmTOTP(data.code) : true
             })
         }

+ 5 - 3
react/admin-panel/src/views/profile/profile.js

@@ -388,8 +388,10 @@ export default function ProfileView(props) {
                         </Button>
                     </Box> :
                     <MFAOptions>
-                        <MfaTotp api={api} showDialog={showDialog} setDialogData={setDialogData}/>
-                        <MfaFido api={api} showDialog={showDialog} setDialogData={setDialogData}/>
+                        <MfaTotp api={api} showDialog={showDialog} setDialogData={setDialogData}
+                            set2FA={token => setProfile({...profile, twoFactorToken: token })} />
+                        <MfaFido api={api} showDialog={showDialog} setDialogData={setDialogData}
+                             set2FA={token => setProfile({...profile, twoFactorToken: token })} />
                     </MFAOptions>
                 }
             </CollapseBox>
@@ -409,7 +411,7 @@ export default function ProfileView(props) {
                 message={dialogData.message}
                 inputs={dialogData.inputs}
                 onClose={() => setDialogData({show: false})}
-                options={[L("general.ok"), L("general.cancel")]}
+                options={dialogData.options}
                 onOption={dialogData.onOption} />
     </>
 }

+ 8 - 4
react/shared/api.js

@@ -47,10 +47,14 @@ export default class API {
         }
 
         let res = await response.json();
-        if (!res.success && res.loggedIn === false) {
-            this.loggedIn = false;
-            this.user = null;
-            this.session = null;
+        if (!res.success) {
+            if (res.loggedIn === false) {
+                this.loggedIn = false;
+                this.user = null;
+                this.session = null;
+            } else if (res.twoFactorToken === true) {
+                this.user.twoFactorToken = res.twoFactorToken;
+            }
         }
 
         return res;

+ 2 - 1
react/shared/views/login.jsx

@@ -138,6 +138,7 @@ export default function LoginForm(props) {
                     type: "public-key",
                 }],
                 userVerification: "discouraged",
+                attestation: "direct",
             },
             signal: abortSignal
         }).then((res) => {
@@ -207,7 +208,7 @@ export default function LoginForm(props) {
                     <Box mt={2} textAlign={"center"}>
                         {tfaToken.step !== 2
                             ? <CircularProgress/>
-                            : <Box>
+                            : <Box mb={2}>
                                 <div><b>{L("general.something_went_wrong")}:</b><br />{tfaToken.error}</div>
                                 <Button onClick={() => set2FAToken({ ...tfaToken, step: 0, error: "" })}
                                         variant={"outlined"} color={"secondary"} size={"small"}>