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

ASN.1 contraints get checked and throw errors if broken, but invalid values still get assigned #21

Open
TheActualOtterOne opened this issue Nov 20, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@TheActualOtterOne
Copy link

TheActualOtterOne commented Nov 20, 2024

First of, thank you for maintaining this repository so good.

Working with it was nice, but also brought up some questions for me.

In the pycrate_asn1rt.asnobj.py the set_val() function sets the value and only then checks it's validity. This behaviour is questionable for me, as invalid values can be set, even though an exception is caught. To avoid invalid values from being set, one would have to save the current value, use the set_val() funtion, catch the exception, and if it was an invalid value, set the value again back to it's original value.

Specificly I wonder why it's possible to set an integer defined as INTEGER (0..127) to negative values.

ASN.1 file:

Error DEFINTIONS AUTOMATIC TAGS ::= BEGIN
    Integer ::= INTEGER (0..127)
END

Python file:

integer: int =-128
ea = error_asn.Error()
try:
    i = ea.Integer.to_aper(integer)    #i never gets assigned a value but Integer now has a invalid value
    ea.Integer.from_aper(i)
except err.ASN1ObjErr as e:
    print(e)

Is this intended bahaviour or am I missing a setting, which changes the behaviour to only set valid values?
The fix would be as easy as to change the snippet in the set_val()

def set_val(self, val):
    """sets the given value ˋval' into self
    """
    self._val = val
    if self._SAFE_VAL:
        self._safechk_val(self._val)
    if self._SAFE_BND:
        self._safechk_bnd(self._val)

to

def set_val(self, val):
    """sets the given value ˋval' into self
    """
    if self._SAFE_VAL:
        self._safechk_val(self._val)
    if self._SAFE_BND:
        self._safechk_bnd(self._val)
    self._val = val

There must be a way to set values by hand without serialization or not? And this seemed to be the intended way to me.

Thank you very much in advance.
Best regards.

@mitshell mitshell self-assigned this Nov 23, 2024
@mitshell
Copy link
Member

set_val() is used both for the encoding and decoding processes in the ASN.1 runtime. During decoding, there are cases where we want to keep decoded values despite them being out of constraint and an exception to be raised. For instance with BER/CER/DER, constraints may not always be honored by some applications. If I recollect correctly, the pycrate runtime is done this way to still being able to interwork with such applications.

I need to further think about this, and if you have any further thought on the topic, do not hesitate to share.

@mitshell mitshell added the enhancement New feature or request label Nov 23, 2024
@TheActualOtterOne
Copy link
Author

TheActualOtterOne commented Nov 25, 2024

Thank you very much for answering my question.
I was wondering why it was implemented that way and now I get the picture.

I must admit though I strongly believe, constraints should be implemented the way they are indetended to, from developement. To address the issue, a boolean could be defined. It could allow assigning invalid values as default, to prevent breaking existing systems. The boolean can be set to it's other non-default value to acommodate for people, who want to filter constraints through this library. An update to the wiki-page to explain the behaviour and where to find the switch, would certainly help.
This would be important to mark in the wiki, in my opinion, because I'd expect a set_val() function, which throws an exception, because of an invalid value, to do so BEFORE assigning it. I only found out, because of a lucky accicent of my side.

_ALLOW_ASSIGNING_INVALID_VALUES = True #set to false, if you do not want to assign invalid values through contraints. Set to True for compatability issues. Add this to line 141.

def set_val(self, val):
    """sets the given value ˋval' into self
    """
    if _ALLOW_ASSIGNING_INVALID_VALUES:
        self._val = val
        if self._SAFE_VAL:
            self._safechk_val(self._val)
        if self._SAFE_BND:
            self._safechk_bnd(self._val)
    else:
        if self._SAFE_VAL:
            self._safechk_val(self._val)
        if self._SAFE_BND:
            self._safechk_bnd(self._val)
        self._val = val

This would be my suggestion for a code change. It would not break compatability, but allow the other behaviour as well. An addition to the wiki in the runtime section would also help a lot.

Again, thank you very much. Especially for maintaining this repository, so thouroughly..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants