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

Validations update #199

Merged
merged 10 commits into from Jan 27, 2022
Merged

Conversation

DenisRybas
Copy link
Collaborator

No description provided.

@@ -21,10 +21,10 @@ message MsgCertifyModel {
int32 vid = 2 [(gogoproto.moretags) = "validate:\"gte=1,lte=65535\""];
int32 pid = 3 [(gogoproto.moretags) = "validate:\"gte=1,lte=65535\""];
uint32 software_version = 4;
string software_version_string = 5 [(gogoproto.moretags) = "validate:\"required\""];
string software_version_string = 5 [(gogoproto.moretags) = "validate:\"required,max=10\""];
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe software_version_string should be 64 as in ModelVersion and the spec (please fix for all occurrences)

uint32 c_d_version_number = 6 [(gogoproto.moretags) = "validate:\"gte=0,lte=65535\""];
string certification_date = 7 [(gogoproto.moretags) = "validate:\"required\""];
string certification_type = 8 [(gogoproto.moretags) = "validate:\"required\""];
string certification_type = 8 [(gogoproto.moretags) = "validate:\"required,max=100\""];
string reason = 9;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add max restriction for the reason field.

string software_version_string = 5 [(gogoproto.moretags) = "validate:\"required\""];
string test_result = 6 [(gogoproto.moretags) = "validate:\"required\""];
string software_version_string = 5 [(gogoproto.moretags) = "validate:\"required,max=10\""];
string test_result = 6 [(gogoproto.moretags) = "validate:\"required,max=256\""];
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe test result may contain quite a lot of info, so 256 is too small.
Let's constrain it to 10 MB, so max=10*1024*1024

@@ -21,51 +21,51 @@ service Msg {

message MsgProposeAddX509RootCert {
string signer = 1 [(cosmos_proto.scalar) = "cosmos.AddressString", (gogoproto.moretags) = "validate:\"required\""];
string cert = 2 [(gogoproto.moretags) = "validate:\"required\""];
string cert = 2 [(gogoproto.moretags) = "validate:\"required,max=3500000\""];
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's constraint to 10MB (10 * 1024 * 1024)

string subject = 2 [(gogoproto.moretags) = "validate:\"required\""];
string subject_key_id = 3 [(gogoproto.moretags) = "validate:\"required\""];
string subject = 2 [(gogoproto.moretags) = "validate:\"required,max=50\""];
string subject_key_id = 3 [(gogoproto.moretags) = "validate:\"required,max=150\""];
Copy link
Contributor

Choose a reason for hiding this comment

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

According to https://www.rfc-editor.org/rfc/rfc3280#section-4.2.1.2
subject key ID should be either 160 bit or 60 bit. So, 150 may be too small.
It looks like other ways of generation of subject_key_id are also possible.
So, let's assign max=256.

}

message MsgProposeAddX509RootCertResponse {
}

message MsgApproveAddX509RootCert {
string signer = 1 [(cosmos_proto.scalar) = "cosmos.AddressString", (gogoproto.moretags) = "validate:\"required\""];
string subject = 2 [(gogoproto.moretags) = "validate:\"required\""];
string subject_key_id = 3 [(gogoproto.moretags) = "validate:\"required\""];
string subject = 2 [(gogoproto.moretags) = "validate:\"required,max=50\""];
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

None yet

2 participants