Skip to content

Commit

Permalink
feat: Client secrets now are protected with Argon2id. (#124)
Browse files Browse the repository at this point in the history
  • Loading branch information
@antoniotarricone
antoniotarricone authored Jul 8, 2024
1 parent b3958ba commit 76b269f
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 52 deletions.
21 changes: 21 additions & 0 deletions dep-sha256.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,27 @@
"version": "9.40",
"sha256": "dxKO1TdWQhv1nS_H8xVU2inqgcrYpTRZdyda23xSVMg="
},
{
"id": "org.bouncycastle:bcpkix-lts8on:jar:2.73.6",
"artifactId": "bcpkix-lts8on",
"groupId": "org.bouncycastle",
"version": "2.73.6",
"sha256": "FMrvWW0y7TivX17Z6GwQQRvxGxhgrJVo439OiN2eKwo="
},
{
"id": "org.bouncycastle:bcutil-lts8on:jar:2.73.6",
"artifactId": "bcutil-lts8on",
"groupId": "org.bouncycastle",
"version": "2.73.6",
"sha256": "W0jYbYbqL7GLMMSLy1G0ujHgB9b81Fmu53MMi8mAx4M="
},
{
"id": "org.bouncycastle:bcprov-lts8on:jar:2.73.6",
"artifactId": "bcprov-lts8on",
"groupId": "org.bouncycastle",
"version": "2.73.6",
"sha256": "OstCqMHFSFZ6PpX9iM6jPMFNqG0sofdHbuuOe-Eixds="
},
{
"id": "io.quarkus:quarkus-junit5:jar:3.12.1",
"artifactId": "quarkus-junit5",
Expand Down
34 changes: 22 additions & 12 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
<groupId>it.pagopa.swclient.mil</groupId>
<artifactId>auth</artifactId>
<version>2.6.0</version>
<description>Authorization Microservice for Multi-channel Integration Layer of SW Client Project.</description>
<description>Authorization Microservice for Multi-channel Integration Layer
of SW Client Project.</description>

<developers>
<developer>
Expand All @@ -18,7 +19,7 @@
<organization>PagoPA S.p.A.</organization>
</developer>
</developers>

<properties>
<!-- Java version -->
<java.version>21</java.version>
Expand Down Expand Up @@ -51,10 +52,10 @@
<sonar.qualitygate.wait>true</sonar.qualitygate.wait>
<sonar.qualitygate.timeout>300</sonar.qualitygate.timeout>
<sonar.coverage.jacoco.xmlReportPaths>target/jacoco-report/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>

<skipITs>true</skipITs>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
Expand All @@ -66,7 +67,7 @@
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<dependency>
<groupId>io.quarkus</groupId>
Expand Down Expand Up @@ -110,6 +111,11 @@
<groupId>com.nimbusds</groupId>
<artifactId>nimbus-jose-jwt</artifactId>
</dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcpkix-lts8on</artifactId>
<version>2.73.6</version>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5</artifactId>
Expand Down Expand Up @@ -172,7 +178,7 @@
<version>${otel-exporter-azure.version}</version>
</dependency>
</dependencies>

<repositories>
<repository>
<id>github</id>
Expand All @@ -183,14 +189,14 @@
<url>https://maven.pkg.github.com/pagopa/mil-azure-services</url>
</repository>
</repositories>

<pluginRepositories>
<pluginRepository>
<id>github</id>
<url>https://maven.pkg.github.com/pagopa/depcheck</url>
</pluginRepository>
</pluginRepositories>

<build>
<plugins>
<plugin>
Expand Down Expand Up @@ -222,7 +228,8 @@
<version>${surefire-plugin.version}</version>
<configuration>
<systemPropertyVariables>
<java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
<java.util.logging.manager>
org.jboss.logmanager.LogManager</java.util.logging.manager>
<maven.home>${maven.home}</maven.home>
</systemPropertyVariables>
</configuration>
Expand All @@ -238,8 +245,10 @@
</goals>
<configuration>
<systemPropertyVariables>
<native.image.path>${project.build.directory}/${project.build.finalName}-runner</native.image.path>
<java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
<native.image.path>
${project.build.directory}/${project.build.finalName}-runner</native.image.path>
<java.util.logging.manager>
org.jboss.logmanager.LogManager</java.util.logging.manager>
<maven.home>${maven.home}</maven.home>
</systemPropertyVariables>
</configuration>
Expand Down Expand Up @@ -277,7 +286,8 @@
</goals>
<phase>test</phase>
<configuration>
<dataFile>${project.build.directory}/jacoco-quarkus.exec</dataFile>
<dataFile>
${project.build.directory}/jacoco-quarkus.exec</dataFile>
<rules>
<rule>
<element>CLASS</element>
Expand Down
29 changes: 11 additions & 18 deletions src/main/java/it/pagopa/swclient/mil/auth/service/ClientVerifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/
package it.pagopa.swclient.mil.auth.service;

import java.security.NoSuchAlgorithmException;
import java.util.Objects;

import io.quarkus.logging.Log;
Expand All @@ -14,7 +13,7 @@
import it.pagopa.swclient.mil.auth.bean.Client;
import it.pagopa.swclient.mil.auth.util.AuthError;
import it.pagopa.swclient.mil.auth.util.AuthException;
import it.pagopa.swclient.mil.auth.util.PasswordVerifier;
import it.pagopa.swclient.mil.auth.util.SecretVerifier;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import jakarta.ws.rs.WebApplicationException;
Expand Down Expand Up @@ -94,22 +93,16 @@ private Client verifyChannel(Client clientEntity, String expectedChannel) {
private Client verifySecret(Client clientEntity, String expectedSecret) {
Log.trace("Secret verification");
String foundSecret = clientEntity.getSecretHash();
try {
if (foundSecret == null && expectedSecret == null) {
Log.debug("Secret is not used");
return clientEntity;
} else if (foundSecret != null && expectedSecret != null && PasswordVerifier.verify(expectedSecret, clientEntity.getSalt(), foundSecret)) {
Log.debug("Secret is ok");
return clientEntity;
} else {
String message = String.format("[%s] Wrong secret", AuthErrorCode.WRONG_SECRET);
Log.warn(message);
throw new AuthException(AuthErrorCode.WRONG_SECRET, message);
}
} catch (NoSuchAlgorithmException e) {
String message = String.format("[%s] Error verifing secret", AuthErrorCode.ERROR_VERIFING_SECRET);
Log.error(message);
throw new AuthError(AuthErrorCode.ERROR_VERIFING_SECRET, message);
if (foundSecret == null && expectedSecret == null) {
Log.debug("Secret is not used");
return clientEntity;
} else if (foundSecret != null && expectedSecret != null && SecretVerifier.verify(expectedSecret, clientEntity.getSalt(), foundSecret)) {
Log.debug("Secret is ok");
return clientEntity;
} else {
String message = String.format("[%s] Wrong secret", AuthErrorCode.WRONG_SECRET);
Log.warn(message);
throw new AuthException(AuthErrorCode.WRONG_SECRET, message);
}
}

Expand Down
62 changes: 62 additions & 0 deletions src/main/java/it/pagopa/swclient/mil/auth/util/SecretVerifier.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* PasswordVerifier.java
*
* 20 mar 2023
*/
package it.pagopa.swclient.mil.auth.util;

import java.nio.charset.StandardCharsets;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.Base64;

import org.bouncycastle.crypto.generators.Argon2BytesGenerator;
import org.bouncycastle.crypto.params.Argon2Parameters;

/**
*
* @author Antonio Tarricone
*/
public class SecretVerifier {
/*
* The following parameters are suggested by OWASP.
* https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id
*/
private static final int ITERATIONS = 2;
private static final int MEM_LIMIT = 19 * 1024; // 19MB
private static final int PARALLELISM = 1;

/**
*
*/
private SecretVerifier() {
}

/**
* @param password
* @param salt
* @param hash
* @return
* @throws NoSuchAlgorithmException
*/
public static boolean verify(String password, String salt, String hash) {
byte[] passwordBytes = password.getBytes(StandardCharsets.UTF_8);
byte[] saltBytes = Base64.getDecoder().decode(salt);
byte[] hashBytes = Base64.getDecoder().decode(hash);

Argon2Parameters.Builder builder = new Argon2Parameters.Builder(Argon2Parameters.ARGON2_id)
.withVersion(Argon2Parameters.ARGON2_VERSION_13)
.withIterations(ITERATIONS)
.withMemoryAsKB(MEM_LIMIT)
.withParallelism(PARALLELISM)
.withSalt(saltBytes);

Argon2BytesGenerator verifier = new Argon2BytesGenerator();
verifier.init(builder.build());

byte[] testHash = new byte[hashBytes.length];
verifier.generateBytes(passwordBytes, testHash, 0, hashBytes.length);

return Arrays.equals(hashBytes, testHash);
}
}
44 changes: 22 additions & 22 deletions src/test/java/it/pagopa/swclient/mil/auth/service/ClientVerifierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.when;

import java.security.NoSuchAlgorithmException;
//import java.security.NoSuchAlgorithmException;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
//import org.mockito.MockedStatic;
//import org.mockito.Mockito;

import io.quarkus.test.InjectMock;
import io.quarkus.test.junit.QuarkusTest;
Expand All @@ -23,7 +23,7 @@
import it.pagopa.swclient.mil.auth.bean.Client;
import it.pagopa.swclient.mil.auth.util.AuthError;
import it.pagopa.swclient.mil.auth.util.AuthException;
import it.pagopa.swclient.mil.auth.util.PasswordVerifier;
//import it.pagopa.swclient.mil.auth.util.PasswordVerifier;
import it.pagopa.swclient.mil.auth.util.UniGenerator;
import jakarta.inject.Inject;
import jakarta.ws.rs.WebApplicationException;
Expand All @@ -41,7 +41,7 @@ class ClientVerifierTest {
private static final String ID = "3965df56-ca9a-49e5-97e8-061433d4a25b";
private static final String CHANNEL = "POS";
private static final String SALT = "aGw/h/8Fm9S2aNvlvIaxJyhKP67ZU4FEm6mDVhL3aEVrahXFif9x2BkQ4OY87Z9tWVyWbSB/JeztYVmTshrFWQ==";
private static final String HASH = "G3oYMwnLVR9+m7WB4/pvoVeHxzsTdeyhndpVoruHzog=";
private static final String HASH = "EOPjxZXy7YbxLubGSs7EhavNqbjVF0ywYQPFE0WYbSw=";
private static final String SECRET = "5ceef788-4115-43a7-a704-b1bcc9a47c86";
private static final String DESCRIPTION = "VAS Layer";
private static final String WRONG_CHANNEL = "ATM";
Expand Down Expand Up @@ -214,23 +214,23 @@ void given_clientDataWithUnexpectedNullSecret_when_invokeVerify_then_getFailure(
/**
*
*/
@Test
void given_errorVerifingSecret_when_invokeVerify_then_getFailure() {
Client client = new Client(ID, CHANNEL, SALT, HASH, DESCRIPTION, null, null);

when(repository.getClient(ID))
.thenReturn(UniGenerator.item(client));

try (MockedStatic<PasswordVerifier> digest = Mockito.mockStatic(PasswordVerifier.class)) {
digest.when(() -> PasswordVerifier.verify(anyString(), anyString(), anyString()))
.thenThrow(NoSuchAlgorithmException.class);

verifier.verify(ID, CHANNEL, SECRET)
.subscribe()
.withSubscriber(UniAssertSubscriber.create())
.assertFailedWith(AuthError.class);
}
}
// @Test
// void given_errorVerifingSecret_when_invokeVerify_then_getFailure() {
// Client client = new Client(ID, CHANNEL, SALT, HASH, DESCRIPTION, null, null);
//
// when(repository.getClient(ID))
// .thenReturn(UniGenerator.item(client));
//
// try (MockedStatic<PasswordVerifier> digest = Mockito.mockStatic(PasswordVerifier.class)) {
// digest.when(() -> PasswordVerifier.verify(anyString(), anyString(), anyString()))
// .thenThrow(NoSuchAlgorithmException.class);
//
// verifier.verify(ID, CHANNEL, SECRET)
// .subscribe()
// .withSubscriber(UniAssertSubscriber.create())
// .assertFailedWith(AuthError.class);
// }
// }

/**
*
Expand Down

0 comments on commit 76b269f

Please sign in to comment.