Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Client secrets now are protected with Argon2id. #124

Merged
merged 1 commit into from
Jul 8, 2024
Merged

feat: Client secrets now are protected with Argon2id. #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions dep-sha256.json
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading