From 140aeb1bf14655f369e1143f83aa78832a577de4 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Tue, 15 Dec 2020 22:09:45 +0100
Subject: [PATCH] Configurable symmetric encryption algorithm (#1377)
---
docs/simplesamlphp-reference-idp-remote.md | 11 ++++++++-
docs/simplesamlphp-reference-sp-remote.md | 15 ++++++++++---
modules/saml/lib/IdP/SAML2.php | 8 ++++++-
modules/saml/lib/Message.php | 26 ++++++++++++++++++----
4 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/docs/simplesamlphp-reference-idp-remote.md b/docs/simplesamlphp-reference-idp-remote.md
index 4a12dc74c..35e1624d8 100644
--- a/docs/simplesamlphp-reference-idp-remote.md
+++ b/docs/simplesamlphp-reference-idp-remote.md
@@ -219,8 +219,17 @@ There are two modes of encryption supported by SimpleSAMLphp. One is symmetric e
: Note that this option overrides the option with the same name in the SP configuration.
`sharedkey`
-: Symmetric key which should be used for decryption. This should be a 128-bit key. If this option is not specified, public key encryption will be used instead.
+: Symmetric key which should be used for decryption. This should be a 128-bit, 192-bit or 256-bit key based on the algorithm used. If this option is not specified, public key encryption will be used instead.
+`sharedkey_algorithm`
+: Algorithm which should be used for decryption. Possible values are:
+
+ * http://www.w3.org/2001/04/xmlenc#aes128-cbc
+ * http://www.w3.org/2001/04/xmlenc#aes192-cbc
+ * http://www.w3.org/2001/04/xmlenc#aes256-cbc
+ * http://www.w3.org/2009/xmlenc11#aes128-gcm
+ * http://www.w3.org/2009/xmlenc11#aes192-gcm
+ * http://www.w3.org/2009/xmlenc11#aes256-gcm
### Fields for signing and validating messages
diff --git a/docs/simplesamlphp-reference-sp-remote.md b/docs/simplesamlphp-reference-sp-remote.md
index 63e01f860..2acc6e8e4 100644
--- a/docs/simplesamlphp-reference-sp-remote.md
+++ b/docs/simplesamlphp-reference-sp-remote.md
@@ -328,9 +328,18 @@ of the SP.
`sharedkey`
: Symmetric key which should be used for encryption. This should be a
- 128-bit key. If this option is not specified, public key encryption
- will be used instead.
-
+ 128-bit, 192-bit or 256-bit key based on the algorithm used.
+ If this option is not specified, public key encryption will be used instead.
+
+`sharedkey_algorithm`
+: Algorithm which should be used for encryption. Possible values are:
+
+ * http://www.w3.org/2001/04/xmlenc#aes128-cbc
+ * http://www.w3.org/2001/04/xmlenc#aes192-cbc
+ * http://www.w3.org/2001/04/xmlenc#aes256-cbc
+ * http://www.w3.org/2009/xmlenc11#aes128-gcm
+ * http://www.w3.org/2009/xmlenc11#aes192-gcm
+ * http://www.w3.org/2009/xmlenc11#aes256-gcm
### Fields for signing and validating messages
diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php
index ac692da2b..972341044 100644
--- a/modules/saml/lib/IdP/SAML2.php
+++ b/modules/saml/lib/IdP/SAML2.php
@@ -1294,7 +1294,13 @@ class SAML2
$sharedKey = $spMetadata->getString('sharedkey', null);
if ($sharedKey !== null) {
- $key = new XMLSecurityKey(XMLSecurityKey::AES128_CBC);
+ $algo = $spMetadata->getString('sharedkey_algorithm', null);
+ if ($algo === null) {
+ // If no algorithm is configured, use a sane default
+ $algo = $idpMetadata->getString('sharedkey_algorithm', XMLSecurityKey::AES128_CBC);
+ }
+
+ $key = new XMLSecurityKey($algo);
$key->loadKey($sharedKey);
} else {
$keys = $spMetadata->getPublicKeys('encryption', true);
diff --git a/modules/saml/lib/Message.php b/modules/saml/lib/Message.php
index 4955c2037..3cd86b252 100644
--- a/modules/saml/lib/Message.php
+++ b/modules/saml/lib/Message.php
@@ -236,16 +236,29 @@ class Message
*
* @param \SimpleSAML\Configuration $srcMetadata The metadata of the sender (IdP).
* @param \SimpleSAML\Configuration $dstMetadata The metadata of the recipient (SP).
+ * @psalm-suppress UndefinedDocblockClass This can be removed after upgrading to saml2v5
+ * @param \SimpleSAML\SAML2\XML\xenc\EncryptionMethod|null $encryptionMethod The EncryptionMethod from the assertion.
*
* @return array Array of decryption keys.
*/
public static function getDecryptionKeys(
Configuration $srcMetadata,
- Configuration $dstMetadata
+ Configuration $dstMetadata,
+ $encryptionMethod = null
): array {
$sharedKey = $srcMetadata->getString('sharedkey', null);
if ($sharedKey !== null) {
- $key = new XMLSecurityKey(XMLSecurityKey::AES128_CBC);
+ if ($encryptionMethod !== null) {
+ $algo = $encryptionMethod->getAlgorithm();
+ } else {
+ $algo = $srcMetadata->getString('sharedkey_algorithm', null);
+ if ($algo === null) {
+ // If no algorithm is supplied or configured, use a sane default as a last resort
+ $algo = $dstMetadata->getString('sharedkey_algorithm', XMLSecurityKey::AES128_CBC);
+ }
+ }
+
+ $key = new XMLSecurityKey($algo);
$key->loadKey($sharedKey);
return [$key];
}
@@ -324,7 +337,7 @@ class Message
Configuration $dstMetadata,
$assertion
): Assertion {
- Assert::isInstanceOfAny($assertion, [\SAML2\Assertion::class, \SAML2\EncryptedAssertion::class]);
+ Assert::isInstanceOfAny($assertion, [Assertion::class, EncryptedAssertion::class]);
if ($assertion instanceof Assertion) {
$encryptAssertion = $srcMetadata->getBoolean('assertion.encryption', null);
@@ -340,7 +353,12 @@ class Message
}
try {
- $keys = self::getDecryptionKeys($srcMetadata, $dstMetadata);
+ // @todo Enable this code for saml2v5 to automatically determine encryption algorithm
+ //$encryptionMethod = $assertion->getEncryptedData()->getEncryptionMethod();
+ //$keys = self::getDecryptionKeys($srcMetadata, $dstMetadata, $encryptionMethod);
+
+ $encryptionMethod = null;
+ $keys = self::getDecryptionKeys($srcMetadata, $dstMetadata, $encryptionMethod);
} catch (\Exception $e) {
throw new SSP_Error\Exception('Error decrypting assertion: ' . $e->getMessage());
}
--
GitLab