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

CryptoPAN #861

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mikemcdermottredjack
Copy link

@mikemcdermottredjack mikemcdermottredjack commented Jan 25, 2023

Move from RustCrypto/utils 831

CryptoPAN is a prefix-preserving IP address anonymization algorithm.

use std::net::{Ipv4Addr, Ipv6Addr};

/// The CryptoPAN anonymizer
pub struct CryptoPAn {
Copy link
Member

Choose a reason for hiding this comment

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

Per RFC 430 I think this should just be CryptoPan:

Suggested change
pub struct CryptoPAn {
pub struct CryptoPan {

Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

As suggested in the previous PR, I think it's worth to make CryptoPan generic over the cipher traits. The original paper (unsurprisingly) defines the algorithm in a generic fashion and even mentions AES ciphers with other key lengths. In addition to supporting other ciphers, such change would also allow to use reference to cipher (i.e. &Aes128) instead of owned instance.

I would also remove dependency on bitvec, but we can do it in a separate PR.

readme = "README.md"

[dependencies]
aes="0.8.1"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
aes="0.8.1"
aes = "0.8.1"

Comment on lines +36 to +37


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

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.

3 participants