While *technically* breaking as it changes the method signature for
the `create_ticket` call to use a struct for its parameters, this is
only (supposed to be) used via its `CREATE_TICKET_API_METHOD` handler
to be passed to a router. Direct use of this does not make sense.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
adds a new endpoint that is useful when dealing with HttpOnly cookies
that cannot be removed by client-side javascript (and by extension
wasm) code. the logout handle simply removes the cookie that is used
for storing the current ticket. this works the same way as it does in
the front-end: by setting an expired cookie with the same name.
as cookies are now prefixed with `__Host-` by default, the cookie here
also needs to be `Secure` and have the same `Path` to not be rejected
by the browser before it can remove the old cookie.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
this adds a new endpoint for requesting tickets. instead of returning
the ticket in the responses body, the ticket is set as a HttpOnly
cookie. this has a couple of advantages:
- the cookie cannot be stolen if an attacker downgrades the connection
to http and injects malicious javascript (`HttpOnly`)
- we don't need to rely on the client to make sure that the cookie is
only send in the appropriate context and only over https
connections (`Secure`, `SameSite`).
- the cookie cannot be overwritten by other subdomains, insecure
connections etc. (the default is to prefix them with `__Host-`)
this follows the best practice guide for secure cookies from MDN
[1]. we also set the cookies to expire when the ticket would so that
the browser removes the cookie once the ticket isn't valid anymore.
the endpoint still returns a ticket that only contains the
informational portions of the ticket but not a valid signature. this
is helpful to let clients know when to refresh the ticket by querying
this endpoint again. it still protects the cookie, though, as it
isn't a valid ticket by itself.
[1]: https://developer.mozilla.org/en-US/docs/Web/Security/Practical_implementation_guides/Cookies
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
this makes sure that newly generated cookies that are prefixed with,
for example, `__Host-`, for security purposes, are correctly picked
up on. otherwise, the new cookies would not be able to yield proper
authentication.
currently this still falls back to insecure non-prefixed cookies. we
should deprecate them in the long-term and remove this fallback.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
this adds the function `prefixed_auth_cookie_name` to the
`AuthContext` trait. said function can be used by users of this crate
to modify the expected prefix of the auth cookie. most products
should be able to use the default of `__Host-` though, so this also
adds a default implementation.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
Fixes the clippy warning:
warning: doc list item missing indentation
--> proxmox-subscription/src/subscription_info.rs:179:9
|
179 | /// (this mode is used to decide whether to refresh the subscription information)
| ^
|
= help: if this is supposed to be its own paragraph, add a blank line
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
= note: `#[warn(clippy::doc_lazy_continuation)]` on by default
help: indent this line
|
179 | /// (this mode is used to decide whether to refresh the subscription information)
| +
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Fixes the following clippy warnings:
warning: the borrowed expression implements the required traits
--> proxmox-tfa/src/api/recovery.rs:86:24
|
86 | Ok(hex::encode(&hmac))
| ^^^^^ help: change this to: `hmac`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
and
warning: this expression creates a reference which is immediately dereferenced by the compiler
--> proxmox-network-api/src/api_impl.rs:108:47
|
108 | interface.set_bond_slave_list(&slaves)?;
| ^^^^^^^ help: change this to: `slaves`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
= note: `#[warn(clippy::needless_borrow)]` on by default
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
due to missing `use` statements they failed, as they should, but for
the wrong reasons. also adapt a test case that presumably was meant
to test whether `TokennameRef` can be compared, but instead
duplicated the `UsernameRef` test case.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
previously we used our own hmac-like implementation for csrf token
signing that simply appended the key to the message (csrf token).
however, this is possibly insecure as an attacker that finds a
collision in the hash function can easily forge a signature. after all,
two messages would then produce the same start conditions before
hashing the key. while this is probably a theoretic attack on our csrf
implementation, it does not hurt to move to the safer standard hmac
implementation that avoids such pitfalls.
this commit re-uses the hmac key wrapper used for the keyring. it also
keeps the old construction around so we can use it for a transition
period between old and new csrf token implementations.
this is a breaking change as it changes the signature of the
`csrf_secret` method of the `AuthContext` trait to return an hmac
key.
also exposes `assemble_csrf_prevention_toke` so we can re-use this
code here instead of duplicating it in e.g. proxmox-backup's
auth_helpers.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
by using openssl's `memcmp::eq()` we can avoid potential side-channel
attack on the csrf token comparison. this comparison's runtime only
depends on the length of the two byte vectors, not their contents.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
previously we only used asymmetric cryptographic schemes to
authenticate tickets. this is fairly costly and not necessary in every
instance. imagine a service that runs as a single daemon. this daemon
is then the only party that needs to sign and verify tickets. this
makes hmac perfectly suitable for such usecases. hmac has some
advantages over asymmetric schemes:
- much simpler and well reviewed construction
- much faster and better optimized crypto primitives (hash functions)
this commit first introduces a new hmac key wrapper that uses openssl's
hmac implementation and can easily be reused by other parts of the
code. it also refactors the keyring code to make it easier to rotate
new hmac keys into place so switching to hmac keys is easier.
hmac keys are symmetric, so the verification key is the same key as the
signing key. this breaks the previous assumption by the keyring that
these correspond to public and private keys. thus, this commit
introduces two wrapper enums to distinguish between hmac and asymmetric
signature schemes.
the verification of hmac keys is also done via `openssl::memcmp::eq()`
to avoid potential timing side-channel attacks.
below are some simple benchmarks done with criterion.rs to show how much
faster hmac is, no matter the actual hash function:
rsa 4096 + sha256 time: [2.7825 ms 2.7907 ms 2.7995 ms]
ed25519 time: [94.411 µs 94.840 µs 95.324 µs]
hmac sha256 time: [5.7202 µs 5.7412 µs 5.7645 µs]
hmac sha384 time: [6.6577 µs 6.6780 µs 6.7006 µs]
hmac sha3_256 time: [5.6930 µs 5.7114 µs 5.7322 µs]
rsa with 4096 bit keys and a sha256 digest is our current default. the
test itself consists of a single sign + verification cycle. criterion
repeats this test as it sees fit to arrive at the above numbers.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
previously we used P-256 as the curve of our choice for ec signatures.
however, in the meantime Ed25519 has become a lot more wide-spread.
this simplifies our ec generation code significantly while keeping the
same security level. Ed25519 was also specifically designed and
reviewed to avoid implementation errors likely making it a more secure
choice
note that Ed25519 as a signature scheme always uses sha512, so signing
or verifying with a chosen digest is not supported.
as this mostly affects newly generated keys, this should not break any
existing setups.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
this commit moves the current ticket signing code into the private key
implementation. the upside is that the caller does not need to deal
with openssl's `Signer` directly. it also simplifies and unifies the
code by using the same helper for verifying a signature and creating it.
also derive `Clone` on `PrivateKey` and `PublicKey`. as they are
essentially thin wrappers around `openssl::pkey::PKey<Private>` and
`openssl::pkey::PKey<Public>`, which can be cloned, deriving `Clone`
just makes them easier to use.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
These were missing the new client-ip parameter in the auth
function calls which was introduced to support `PAM_RHOST`.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
